Fix constructor resolution regression and add test (#1868)

Author: jcollins-g

lib/src/markdown_processor.dart

@@ -690,13 +690,13 @@ class _MarkdownCommentReference {
         .where((m) => _ConsiderIfConstructor(m));
     if (codeRefChompedParts.first == c.name) {
       // [Foo...thing], a member of this class (possibly a parameter).
-      membersToCheck.map((m) => _addCanonicalResult(m, tryClass));
+      membersToCheck.forEach((m) => _addCanonicalResult(m, tryClass));
     } else if (codeRefChompedParts.length > 1 &&
         codeRefChompedParts[codeRefChompedParts.length - 2] == c.name) {
       // [....Foo.thing], a member of this class partially specified.
       membersToCheck
           .whereType<Constructor>()
-          .map((m) => _addCanonicalResult(m, tryClass));
+          .forEach((m) => _addCanonicalResult(m, tryClass));
     }
     results.remove(null);
     if (results.isNotEmpty) return;

lib/src/model.dart

@@ -4134,8 +4134,10 @@ abstract class ModelElement extends Canonicalization
         // Count the number of invocations of tools in this dartdoc block,
         // so that tools can differentiate different blocks from each other.
         invocationIndex++;
-        return await config.tools.runner.run(args,
-            (String message) async => warn(PackageWarning.toolError, message: message),
+        return await config.tools.runner.run(
+            args,
+            (String message) async =>
+                warn(PackageWarning.toolError, message: message),
             content: basicMatch[2],
             environment: {
               'SOURCE_LINE': lineAndColumn?.item1?.toString(),

lib/src/tool_runner.dart

@@ -25,17 +25,19 @@ final MultiFutureTracker _toolTracker = new MultiFutureTracker(4);
 class ToolTempFileTracker {
   final Directory temporaryDirectory;
 
-  ToolTempFileTracker._() : temporaryDirectory = Directory.systemTemp.createTempSync('dartdoc_tools_');
+  ToolTempFileTracker._()
+      : temporaryDirectory =
+            Directory.systemTemp.createTempSync('dartdoc_tools_');
 
   static ToolTempFileTracker _instance;
-  static ToolTempFileTracker get instance => _instance ??= ToolTempFileTracker._();
+  static ToolTempFileTracker get instance =>
+      _instance ??= ToolTempFileTracker._();
 
   int _temporaryFileCount = 0;
   Future<File> createTemporaryFile() async {
     _temporaryFileCount++;
     File tempFile = new File(pathLib.join(
-        temporaryDirectory.absolute.path,
-        'input_$_temporaryFileCount'));
+        temporaryDirectory.absolute.path, 'input_$_temporaryFileCount'));
     await tempFile.create(recursive: true);
     return tempFile;
   }
@@ -60,7 +62,9 @@ class ToolRunner {
   final ToolConfiguration toolConfiguration;
 
   void _runSetup(
-      String name, ToolDefinition tool, Map<String, String> environment,
+      String name,
+      ToolDefinition tool,
+      Map<String, String> environment,
       ToolErrorCallback toolErrorCallback) async {
     bool isDartSetup = ToolDefinition.isDartExecutable(tool.setupCommand[0]);
     var args = tool.setupCommand.toList();
@@ -71,12 +75,17 @@ class ToolRunner {
     } else {
       commandPath = args.removeAt(0);
     }
-    await _runProcess(name, '', commandPath, args, environment, toolErrorCallback);
+    await _runProcess(
+        name, '', commandPath, args, environment, toolErrorCallback);
     tool.setupComplete = true;
   }
 
-  Future<String> _runProcess(String name, String content, String commandPath,
-      List<String> args, Map<String, String> environment,
+  Future<String> _runProcess(
+      String name,
+      String content,
+      String commandPath,
+      List<String> args,
+      Map<String, String> environment,
       ToolErrorCallback toolErrorCallback) async {
     String commandString() => ([commandPath] + args).join(' ');
     try {
@@ -113,7 +122,8 @@ class ToolRunner {
     Future runner;
     // Prevent too many tools from running simultaneously.
     await _toolTracker.addFutureFromClosure(() {
-      runner = _run(args, toolErrorCallback, content: content, environment: environment);
+      runner = _run(args, toolErrorCallback,
+          content: content, environment: environment);
       return runner;
     });
     return runner;
@@ -127,7 +137,8 @@ class ToolRunner {
     environment ??= <String, String>{};
     var tool = args.removeAt(0);
     if (!toolConfiguration.tools.containsKey(tool)) {
-      toolErrorCallback('Unable to find definition for tool "$tool" in tool map. '
+      toolErrorCallback(
+          'Unable to find definition for tool "$tool" in tool map. '
           'Did you add it to dartdoc_options.yaml?');
       return '';
     }
@@ -193,12 +204,12 @@ class ToolRunner {
     }
 
     if (callCompleter != null) {
-      return _runProcess(
-              tool, content, commandPath, argsWithInput, envWithInput, toolErrorCallback)
+      return _runProcess(tool, content, commandPath, argsWithInput,
+              envWithInput, toolErrorCallback)
           .whenComplete(callCompleter);
     } else {
-      return _runProcess(
-          tool, content, commandPath, argsWithInput, envWithInput, toolErrorCallback);
+      return _runProcess(tool, content, commandPath, argsWithInput,
+          envWithInput, toolErrorCallback);
     }
   }
 }

test/model_test.dart

@@ -2919,7 +2919,7 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
     Constructor appleDefaultConstructor, constCatConstructor;
     Constructor appleConstructorFromString;
     Constructor constructorTesterDefault, constructorTesterFromSomething;
-    Class apple, constCat, constructorTester;
+    Class apple, constCat, constructorTester, referToADefaultConstructor;
     setUpAll(() {
       apple = exLibrary.classes.firstWhere((c) => c.name == 'Apple');
       constCat = exLibrary.classes.firstWhere((c) => c.name == 'ConstantCat');
@@ -2934,6 +2934,20 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
           .firstWhere((c) => c.name == 'ConstructorTester');
       constructorTesterFromSomething = constructorTester.constructors
           .firstWhere((c) => c.name == 'ConstructorTester.fromSomething');
+      referToADefaultConstructor = fakeLibrary.classes
+          .firstWhere((c) => c.name == 'ReferToADefaultConstructor');
+    });
+
+    test('calculates comment references to classes vs. constructors correctly',
+        () {
+      expect(
+          referToADefaultConstructor.documentationAsHtml,
+          contains(
+              '<a href="fake/ReferToADefaultConstructor-class.html">ReferToADefaultConstructor</a>'));
+      expect(
+          referToADefaultConstructor.documentationAsHtml,
+          contains(
+              '<a href="fake/ReferToADefaultConstructor/ReferToADefaultConstructor.html">ReferToADefaultConstructor.ReferToADefaultConstructor</a>'));
     });
 
     test('displays generic parameters correctly', () {

testing/test_package/lib/fake.dart

@@ -926,6 +926,15 @@ void paintImage2(String fooParam,
   // nothing to do here -
 }
 
+/// This is to test referring to a constructor.
+///
+/// This should refer to a class: [ReferToADefaultConstructor].
+/// This should refer to the constructor: [ReferToADefaultConstructor.ReferToADefaultConstructor].
+class ReferToADefaultConstructor {
+  /// A default constructor.
+  ReferToADefaultConstructor();
+}
+
 /// Test operator references: [OperatorReferenceClass.==].
 class OperatorReferenceClass {
   OperatorReferenceClass();