Skip to content

Add warnings for "free-hanging" generics in docs [#1250] #1313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions bin/dartdoc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ main(List<String> arguments) async {
initializeConfig(
addCrossdart: addCrossdart,
examplePathPrefix: args['example-path-prefix'],
showWarnings: args['show-warnings'],
includeSource: includeSource,
inputDir: inputDir,
sdkVersion: sdk.sdkVersion);
Expand Down Expand Up @@ -176,6 +177,8 @@ ArgParser _createArgsParser() {
defaultsTo: false);
parser.addFlag('sdk-docs',
help: 'Generate ONLY the docs for the Dart SDK.', negatable: false);
parser.addFlag('show-warnings',
help: 'Display warnings.', negatable: false);
parser.addFlag('show-progress',
help: 'Display progress indications to console stdout', negatable: false);
parser.addOption('sdk-readme',
Expand Down
2 changes: 2 additions & 0 deletions lib/dartdoc.dart
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,14 @@ Future<List<Generator>> initGenerators(String url, List<String> headerFilePaths,
void initializeConfig(
{Directory inputDir,
String sdkVersion,
bool showWarnings: false,
bool addCrossdart: false,
String examplePathPrefix,
bool includeSource: true}) {
setConfig(
inputDir: inputDir,
sdkVersion: sdkVersion,
showWarnings: showWarnings,
addCrossdart: addCrossdart,
examplePathPrefix: examplePathPrefix,
includeSource: includeSource);
Expand Down
6 changes: 4 additions & 2 deletions lib/src/config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,23 @@ import 'dart:io';
class Config {
final Directory inputDir;
final bool addCrossdart;
final bool showWarnings;
final String examplePathPrefix;
final bool includeSource;
final String sdkVersion;
Config._(
this.inputDir, this.addCrossdart, this.examplePathPrefix, this.includeSource, this.sdkVersion);
this.inputDir, this.showWarnings, this.addCrossdart, this.examplePathPrefix, this.includeSource, this.sdkVersion);
}

Config _config;
Config get config => _config;

void setConfig(
{Directory inputDir,
bool showWarnings: false,
String sdkVersion,
bool addCrossdart: false,
String examplePathPrefix,
bool includeSource: true}) {
_config = new Config._(inputDir, addCrossdart, examplePathPrefix, includeSource, sdkVersion);
_config = new Config._(inputDir, showWarnings, addCrossdart, examplePathPrefix, includeSource, sdkVersion);
}
85 changes: 76 additions & 9 deletions lib/src/markdown_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
library dartdoc.markdown_processor;

import 'dart:convert';
import 'dart:math';

import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/element/element.dart'
Expand All @@ -14,8 +15,23 @@ import 'package:html/parser.dart' show parse;
import 'package:markdown/markdown.dart' as md;

import 'model.dart';

const bool _emitWarning = false;
import 'reporting.dart';

const validHtmlTags = const [
"a", "abbr", "address", "area", "article", "aside", "audio", "b",
"bdi", "bdo", "blockquote", "br", "button", "canvas", "caption",
"cite", "code", "col", "colgroup", "data", "datalist", "dd", "del", "dfn",
"div", "dl", "dt", "em", "fieldset", "figcaption", "figure",
"footer", "form", "h1", "h2", "h3", "h4", "h5", "h6", "header", "hr",
"i", "iframe", "img", "input", "ins", "kbd", "keygen", "label",
"legend", "li", "link", "main", "map", "mark", "meta", "meter", "nav",
"noscript", "object", "ol", "optgroup", "option", "output", "p", "param",
"pre", "progress", "q", "s", "samp",
"script", "section", "select", "small", "source", "span", "strong", "style",
"sub", "sup", "table", "tbody", "td", "template", "textarea", "tfoot", "th",
"thead", "time", "title", "tr", "track", "u", "ul", "var", "video", "wbr"
];
final nonHTMLRegexp = new RegExp("</?(?!(${validHtmlTags.join("|")})[> ])\\w+[> ]");

// We don't emit warnings currently: #572.
const List<String> _oneLinerSkipTags = const ["code", "pre"];
Expand Down Expand Up @@ -137,9 +153,8 @@ MatchingLinkResult _findRefElementInLibrary(String codeRef, ModelElement element
} else if (result.length == 1) {
return new MatchingLinkResult(result.values.first, result.values.first.name);
} else {
// 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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 to adding a warning() method

"Ambiguous reference to [${codeRef}] in ${_elementLocation(element)}. " +
"We found matches to the following elements: ${result.keys.map((k) => "'${k}'").join(", ")}");
return new MatchingLinkResult(null, null);
}
Expand All @@ -165,23 +180,75 @@ String _linkDocReference(String reference, ModelElement element, NodeList<Commen
// different for doc references. sigh.
return '<a ${classContent}href="${linkedElement.href}">$label</a>';
} else {
if (_emitWarning) {
// TODO: add --fatal-warning, which would make the app crash in case of ambiguous references
print(" warning: unresolved doc reference '$reference' (in $element)");
}
warning("unresolved doc reference '$reference' (in ${_elementLocation(element)}");
return '<code>${HTML_ESCAPE.convert(label)}</code>';
}
}

String _elementLocation(ModelElement element) {
while ((element.element.documentationComment == null || element.element.documentationComment == "")
&& element.overriddenElement != null) {
element = element.overriddenElement;
}
return "'${element.fullyQualifiedName}' (${element.sourceFileName}:${element.lineNumber})";
}

String _renderMarkdownToHtml(String text, [ModelElement element]) {
md.Node _linkResolver(String name) {
NodeList<CommentReference> commentRefs = _getCommentRefs(element);
return new md.Text(_linkDocReference(name, element, commentRefs));
}

_showWarningsForGenericsOutsideSquareBracketsBlocks(text, element);
return md.markdownToHtml(text, inlineSyntaxes: _markdown_syntaxes, linkResolver: _linkResolver);
}

// Generics should be wrapped into `[]` blocks, to avoid handling them as HTML tags
// (like, [Apple<int>]). @Hixie asked for a warning when there's something, that looks
// like a non HTML tag (a generic?) outside of a `[]` block.
// https://github.com/dart-lang/dartdoc/issues/1250#issuecomment-269257942
void _showWarningsForGenericsOutsideSquareBracketsBlocks(String text, [ModelElement element]) {
List<int> tagPositions = findFreeHangingGenericsPositions(text);
if (tagPositions.isNotEmpty) {
tagPositions.forEach((int position) {
String errorMessage = "Generic type handled as HTML";
if (element != null) {
errorMessage += " in ${_elementLocation(element)}";
}
errorMessage += " - '${text.substring(max(position - 20, 0), min(position + 20, text.length))}'";
warning(errorMessage);
});
}
}

List<int> findFreeHangingGenericsPositions(String string) {
int currentPosition = 0;
int squareBracketsDepth = 0;
List<int> results = [];
while (true) {
final int nextOpenBracket = string.indexOf("[", currentPosition);
final int nextCloseBracket = string.indexOf("]", currentPosition);
final int nextNonHTMLTag = string.indexOf(nonHTMLRegexp, currentPosition);
final Iterable<int> nextPositions = [nextOpenBracket, nextCloseBracket, nextNonHTMLTag].where((p) => p != -1);
if (nextPositions.isNotEmpty) {
final minPos = nextPositions.reduce(min);
if (nextOpenBracket == minPos) {
squareBracketsDepth += 1;
} else if (nextCloseBracket == minPos) {
squareBracketsDepth = max(squareBracketsDepth - 1, 0);
} else if (nextNonHTMLTag == minPos) {
if (squareBracketsDepth == 0) {
results.add(minPos);
}
}
currentPosition = minPos + 1;
} else {
break;
}
}
return results;
}

class Documentation {
final String raw;
final String asHtml;
Expand Down
16 changes: 16 additions & 0 deletions lib/src/reporting.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

library reporting;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a copyright header (available in the other files).


import 'config.dart';
import 'dart:io';

void warning(String message) {
// 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) {
stderr.writeln("warning: ${message}");
}
}
2 changes: 2 additions & 0 deletions test/all.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import 'model_utils_test.dart' as model_utils_tests;
import 'package_meta_test.dart' as package_meta_tests;
import 'resource_loader_test.dart' as resource_loader_tests;
import 'template_test.dart' as template_tests;
import 'markdown_processor_test.dart' as markdown_processor_tests;

void main() {
compare_output_tests.main();
Expand All @@ -24,4 +25,5 @@ void main() {
package_meta_tests.main();
resource_loader_tests.main();
template_tests.main();
markdown_processor_tests.main();
}
27 changes: 27 additions & 0 deletions test/markdown_processor_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

library dartdoc.markdown_processor_test;

import 'package:dartdoc/src/markdown_processor.dart';
import 'package:test/test.dart';

void main() {
group('findFreeHangingGenericsPositions()', () {
test('returns empty array if all the generics are in []', () {
final string = "One two [three<four>] [[five<six>] seven eight";
expect(findFreeHangingGenericsPositions(string), equals([]));
});

test('returns positions of generics outside of []', () {
final string = "One two<int> [[three<four>] five<six>] seven<eight>";
expect(findFreeHangingGenericsPositions(string), equals([7, 44]));
});

test('ignores HTML tags', () {
final string = "One two<int> foo<pre> [[three<four>] five<six>] bar</pre> seven<eight>";
expect(findFreeHangingGenericsPositions(string), equals([7, 63]));
});
});
}