Add warnings for "free-hanging" generics in docs [#1250] #1313
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@Hixie asked for warnings when the generics are "free-hanging", and not wrapped into [] block (like `Apple<Cat>`, but not `[Apple<Cat>]`). This commit adds those warnings. We try to find the tags, which are not wrapped into square brackets, which are not from the whitelist of possible HTML tags. Also, now output all the warnings to the console: * When there's a missing reference in the comments * When there're ambiguous reference in the comments * When there're "free-hanging" generics But that generates TONS AND TONS of warnings when you e.g. generate Dart SDK docs or Flutter docs. So, I've added the flag `--show-warnings`, without it we don't show the warnings.
Hixie
reviewed
Dec 29, 2016
| import 'reporting.dart'; | ||
|
|
||
| const validHtmlTags = const [ | ||
| "a", "abbr", "address", "area", "article", "aside", "audio", "b", "base", |
There was a problem hiding this comment.
we should probably comment out the following tags:
- base
- body
- embed
- head
- html
- rb
- rtc
Where did you get the list?
There was a problem hiding this comment.
From there: http://www.htmldog.com/references/html/tags/
There was a problem hiding this comment.
https://html.spec.whatwg.org/#elements-3 would be a more authoritative source
devoncarew
reviewed
Dec 30, 2016
| // TODO: Could handle fatal warnings here, or print to stderr, or remember | ||
| // that we had at least one warning, and exit with non-null exit code in this case. | ||
| if (config != null && config.showWarnings) { | ||
| print("warning: ${message}"); |
There was a problem hiding this comment.
We should probably print to stderr here (stderr.writeln()).
devoncarew
reviewed
Dec 30, 2016
| @@ -0,0 +1,11 @@ | |||
| library reporting; | |||
There was a problem hiding this comment.
Please add a copyright header (available in the other files).
devoncarew
reviewed
Dec 30, 2016
| if (config != null && config.showWarnings) { | ||
| print("warning: ${message}"); | ||
| } | ||
| } |
devoncarew
reviewed
Dec 30, 2016
| @@ -0,0 +1,27 @@ | |||
| // Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file | |||
devoncarew
reviewed
Dec 30, 2016
| // TODO: add --fatal-warning, which would make the app crash in case of ambiguous references | ||
| print( | ||
| "Ambiguous reference to [${codeRef}] in '${element.fullyQualifiedName}' (${element.sourceFileName}:${element.lineNumber}). " + | ||
| warning( |
There was a problem hiding this comment.
👍 to adding a warning() method
devoncarew
reviewed
Dec 30, 2016
| return '<code>${HTML_ESCAPE.convert(label)}</code>'; | ||
| } | ||
| } | ||
|
|
||
| String _elementSource(ModelElement element) { |
devoncarew
reviewed
Dec 30, 2016
| List<int> tagPositions = findFreeHangingGenericsPositions(text); | ||
| if (tagPositions.isNotEmpty) { | ||
| tagPositions.forEach((int position) { | ||
| String errorMessage = "There's a generic type handled as HTML"; |
There was a problem hiding this comment.
Perhaps: Generic type handled as HTML .
devoncarew
reviewed
Dec 30, 2016
| int squareBracketsDepth = 0; | ||
| List<int> results = []; | ||
| while (true) { | ||
| final nextOpenBracket = string.indexOf("[", currentPosition); |
There was a problem hiding this comment.
please add a type annotation here (and below)
|
Awesome! Some suggested changes (in addition to the one that @Hixie pointed out). Can you also include some sample output (just in a reply to the PR) of it finding warnings? |
|
Warnings: |
|
@devoncarew PTAL |
keertip
approved these changes
Jan 3, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@Hixie asked for warnings when the generics are "free-hanging", and not
wrapped into [] block (like
Apple<Cat>, but not[Apple<Cat>]). Thiscommit adds those warnings. We try to find the tags, which are not
wrapped into square brackets, which are not from the whitelist of
possible HTML tags.
Also, now output all the warnings to the console:
But that generates TONS AND TONS of warnings when you e.g. generate Dart
SDK docs or Flutter docs. So, I've added the flag
--show-warnings,without it we don't show the warnings.
Fixes #1250