Warn "not yet implemented" for comment reference resolution for extension methods (dart-lang#2040)

* Add some tests to trigger crash bug

* Add warning for not yet implemented and seal off comment resolution to avoid type errors

* Extend a timeout

Author: jcollins-g

lib/src/markdown_processor.dart

@@ -205,9 +205,13 @@ MatchingLinkResult _getMatchingLinkElement(
   // Try expensive not-scoped lookup.
   if (refModelElement == null && element is ModelElement) {
     Container preferredClass = _getPreferredClass(element);
-    refModelElement =
-        _MarkdownCommentReference(codeRef, element, commentRefs, preferredClass)
-            .computeReferredElement();
+    if (preferredClass is Extension) {
+      element.warn(PackageWarning.notImplemented, message: 'Comment reference resolution inside extension methods is not yet implemented');
+    } else {
+      refModelElement =
+          _MarkdownCommentReference(codeRef, element, commentRefs, preferredClass)
+              .computeReferredElement();
+    }
   }
 
   // Did not find it anywhere.

lib/src/model.dart

@@ -5271,6 +5271,9 @@ class PackageGraph {
         warningMessage =
             "private API of ${message} is reexported by libraries in other packages: ";
         break;
+      case PackageWarning.notImplemented:
+        warningMessage = message;
+        break;
       case PackageWarning.unresolvedDocReference:
         warningMessage = "unresolved doc reference [${message}]";
         referredFromPrefix = 'in documentation inherited from';

lib/src/warnings.dart

@@ -135,6 +135,10 @@ final Map<PackageWarning, PackageWarningDefinition> packageWarningDefinitions =
       PackageWarning.noCanonicalFound,
       "no-canonical-found",
       "A symbol is part of the public interface for this package, but no library documented with this package documents it so dartdoc can not link to it"),
+  PackageWarning.notImplemented: PackageWarningDefinition(
+      PackageWarning.notImplemented,
+      "not-implemented",
+      "The code makes use of a feature that is not yet implemented in dartdoc"),
   PackageWarning.noLibraryLevelDocs: PackageWarningDefinition(
       PackageWarning.noLibraryLevelDocs,
       "no-library-level-docs",
@@ -226,6 +230,7 @@ enum PackageWarning {
   ambiguousReexport,
   ignoredCanonicalFor,
   noCanonicalFound,
+  notImplemented,
   noLibraryLevelDocs,
   packageOrderGivesMissingPackageName,
   reexportedPrivateApiAcrossPackages,

test/dartdoc_integration_test.dart

@@ -69,7 +69,7 @@ void main() {
       expect(outputLines, contains(matches('^  warning:')));
       expect(outputLines.last, matches(r'^found \d+ warnings and \d+ errors'));
       expect(outputDir.listSync(), isNotEmpty);
-    }, timeout: new Timeout.factor(2));
+    }, timeout: Timeout.factor(2));
 
     test('invalid parameters return non-zero and print a fatal-error',
         () async {
@@ -229,7 +229,7 @@ void main() {
 
       File outFile = File(path.join(tempDir.path, 'index.html'));
       expect(outFile.readAsStringSync(), contains('footer text include'));
-    });
+    }, timeout: Timeout.factor(2));
 
     test('--footer-text excludes version', () async {
       String _testPackagePath =

test/model_test.dart

@@ -1904,7 +1904,7 @@ void main() {
     });
 
     test('correctly finds all the classes', () {
-      expect(classes, hasLength(31));
+      expect(classes, hasLength(33));
     });
 
     test('abstract', () {
@@ -2114,15 +2114,37 @@ void main() {
 
   group('Extension', () {
     Extension ext, fancyList;
-    Method s;
+    Class extensionReferencer;
+    Method doSomeStuff, doStuff, s;
     List<Extension> extensions;
 
     setUpAll(() {
       ext = exLibrary.extensions.firstWhere((e) => e.name == 'AppleExtension');
+      extensionReferencer = exLibrary.classes.firstWhere((c) => c.name == 'ExtensionReferencer');
       fancyList = exLibrary.extensions.firstWhere((e) => e.name == 'FancyList');
+      doSomeStuff = exLibrary.classes.firstWhere((c) => c.name == 'ExtensionUser')
+          .allInstanceMethods.firstWhere((m) => m.name == 'doSomeStuff');
+      doStuff = exLibrary.extensions.firstWhere((e) => e.name == 'SimpleStringExtension')
+          .instanceMethods.firstWhere((m) => m.name == 'doStuff');
       extensions = exLibrary.publicExtensions.toList();
     });
 
+    // TODO(jcollins-g): implement feature and update tests
+    test('documentation links do not crash in base cases', () {
+      packageGraph.packageWarningCounter.hasWarning(doStuff, PackageWarning.notImplemented,
+          'Comment reference resolution inside extension methods is not yet implemented');
+      packageGraph.packageWarningCounter.hasWarning(doSomeStuff, PackageWarning.notImplemented,
+          'Comment reference resolution inside extension methods is not yet implemented');
+      expect(doStuff.documentationAsHtml, contains('<code>another</code>'));
+      expect(doSomeStuff.documentationAsHtml, contains('<code>String.extensionNumber</code>'));
+    });
+
+    test('references from outside an extension refer correctly to the extension', () {
+      expect(extensionReferencer.documentationAsHtml, contains('<code>_Shhh</code>'));
+      expect(extensionReferencer.documentationAsHtml, contains('<a href="ex/FancyList.html">FancyList</a>'));
+      expect(extensionReferencer.documentationAsHtml, contains('<a href="ex/AnExtension/call.html">AnExtension.call</a>'));
+    });
+
     test('has a fully qualified name', () {
       expect(ext.fullyQualifiedName, 'ex.AppleExtension');
     });
@@ -2179,11 +2201,11 @@ void main() {
     });
 
     test('correctly finds all the extensions', () {
-      expect(exLibrary.extensions, hasLength(7));
+      expect(exLibrary.extensions, hasLength(8));
     });
 
     test('correctly finds all the public extensions', () {
-      expect(extensions, hasLength(5));
+      expect(extensions, hasLength(6));
     });
   });
 

testing/test_package/lib/example.dart

@@ -627,6 +627,24 @@ extension AnExtension<Q> on WithGeneric<Q> {
   int call(String s) => 0;
 }
 
+extension SimpleStringExtension on String {
+  /// Print this and [another].
+  /// Refer to [indexOf], from [String].
+  /// Also refer to [extensionNumber].
+  void doStuff(String another) {
+    print(this + another);
+  }
+
+  int get extensionNumber => 3;
+}
+
+class ExtensionUser {
+  /// Refer to [String.extensionNumber], which we use here.
+  void doSomeStuff(String things) {
+    print(things.extensionNumber + 1);
+  }
+}
+
 /// Extension on List
 extension FancyList<Z> on List<Z> {
   int get doubleLength => this.length * 2;
@@ -654,4 +672,9 @@ extension _Shhh on Object {
 // Extension with no name
 extension on Object {
   void bar() { }
-}
+}
+
+
+/// This class has nothing to do with [_Shhh], [FancyList], or [AnExtension.call],
+/// but should not crash because we referenced them.
+class ExtensionReferencer {}