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

bin/dartdoc.dart

@@ -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);
@@ -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',

lib/dartdoc.dart

@@ -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);

lib/src/config.dart

@@ -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);
 }

lib/src/markdown_processor.dart

@@ -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'
@@ -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", "base",
+  "bdi", "bdo", "blockquote", "body", "br", "button", "canvas", "caption",
+  "cite", "code", "col", "colgroup", "data", "datalist", "dd", "del", "dfn",
+  "div", "dl", "dt", "em", "embed", "fieldset", "figcaption", "figure",
+  "footer", "form", "h1", "h2", "h3", "h4", "h5", "h6", "head", "header", "hr",
+  "html", "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", "rb", "rp", "rt", "rtc", "ruby", "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"];
@@ -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(
+        "Ambiguous reference to [${codeRef}] in ${_elementSource(element)}. " +
             "We found matches to the following elements: ${result.keys.map((k) => "'${k}'").join(", ")}");
     return new MatchingLinkResult(null, null);
   }
@@ -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 ${_elementSource(element)}");
     return '<code>${HTML_ESCAPE.convert(label)}</code>';
   }
 }
 
+String _elementSource(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 = "There's a generic type handled as HTML";
+      if (element != null) {
+        errorMessage += " in ${_elementSource(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 nextOpenBracket = string.indexOf("[", currentPosition);
+    final nextCloseBracket = string.indexOf("]", currentPosition);
+    final nextNonHTMLTag = string.indexOf(nonHTMLRegexp, currentPosition);
+    final 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;

lib/src/reporting.dart

@@ -0,0 +1,11 @@
+library reporting;
+
+import 'config.dart';
+
+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) {
+    print("warning: ${message}");
+  }
+}

test/all.dart

@@ -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();
@@ -24,4 +25,5 @@ void main() {
   package_meta_tests.main();
   resource_loader_tests.main();
   template_tests.main();
+  markdown_processor_tests.main();
 }

test/markdown_processor_test.dart

@@ -0,0 +1,27 @@
+// Copyright (c) 2015, 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]));
+    });
+  });
+}