Basic .new support for constructors (#2766)

* partial

* Delete the lookup code itself

* obliterate all the 'everything' tables that supported the old lookup code

* dartfmt

* dartfmt

* revert erroneous/non-working const stuff for a later PR

* Remove some broken tests and tweak the grinder slightly

Author: jcollins-g

lib/src/comment_references/model_comment_reference.dart

@@ -8,12 +8,10 @@ import 'package:analyzer/file_system/file_system.dart';
 import 'package:dartdoc/src/comment_references/parser.dart';
 
 abstract class ModelCommentReference {
-  /// Does the structure of the reference itself imply a possible default
+  /// Does the structure of the reference itself imply a possible unnamed
   /// constructor?
-  // TODO(jcollins-g): rewrite/discard this once default constructor tear-off
-  // design process is complete.
-  bool get allowDefaultConstructor;
-  bool get allowDefaultConstructorParameter;
+  bool get allowUnnamedConstructor;
+  bool get allowUnnamedConstructorParameter;
   String get codeRef;
   bool get hasConstructorHint;
   bool get hasCallableHint;
@@ -34,42 +32,45 @@ abstract class ModelCommentReference {
 /// information needed for Dartdoc.  Drops link to the [CommentReference]
 /// and [ResourceProvider] after construction.
 class _ModelCommentReferenceImpl implements ModelCommentReference {
-  bool _allowDefaultConstructor;
+  bool _allowUnnamedConstructor;
 
   void _initAllowCache() {
     var referencePieces = parsed.whereType<IdentifierNode>().toList();
-    _allowDefaultConstructor = false;
-    _allowDefaultConstructorParameter = false;
+    _allowUnnamedConstructor = false;
+    _allowUnnamedConstructorParameter = false;
     if (referencePieces.length >= 2) {
       IdentifierNode nodeLast;
       for (var f in referencePieces) {
         if (f.text == nodeLast?.text) {
           if (identical(referencePieces.last, f)) {
-            _allowDefaultConstructor = true;
+            _allowUnnamedConstructor = true;
           } else {
-            _allowDefaultConstructorParameter = true;
+            _allowUnnamedConstructorParameter = true;
           }
         }
         nodeLast = f;
       }
+      if (referencePieces.last.text == 'new') {
+        _allowUnnamedConstructor = true;
+      }
     }
   }
 
   @override
-  bool get allowDefaultConstructor {
-    if (_allowDefaultConstructor == null) {
+  bool get allowUnnamedConstructor {
+    if (_allowUnnamedConstructor == null) {
       _initAllowCache();
     }
-    return _allowDefaultConstructor;
+    return _allowUnnamedConstructor;
   }
 
-  bool _allowDefaultConstructorParameter;
+  bool _allowUnnamedConstructorParameter;
   @override
-  bool get allowDefaultConstructorParameter {
-    if (_allowDefaultConstructorParameter == null) {
+  bool get allowUnnamedConstructorParameter {
+    if (_allowUnnamedConstructorParameter == null) {
       _initAllowCache();
     }
-    return _allowDefaultConstructorParameter;
+    return _allowUnnamedConstructorParameter;
   }
 
   @override

lib/src/markdown_processor.dart

@@ -157,14 +157,12 @@ class _IterableBlockParser extends md.BlockParser {
   }
 }
 
-/// Return false if the passed [referable] is a default [Constructor],
+/// Return false if the passed [referable] is an unnamed [Constructor],
 /// or if it is shadowing another type of element, or is a parameter of
 /// one of the above.
-bool _rejectDefaultAndShadowingConstructors(CommentReferable referable) {
+bool _rejectUnnamedAndShadowingConstructors(CommentReferable referable) {
   if (referable is Constructor) {
-    if (referable.name == referable.enclosingElement.name) {
-      return false;
-    }
+    if (referable.isUnnamedConstructor) return false;
     if (referable.enclosingElement
         .referenceChildren[referable.name.split('.').last] is! Constructor) {
       return false;
@@ -191,12 +189,12 @@ MatchingLinkResult _getMatchingLinkElementCommentReferable(
   bool Function(CommentReferable) filter;
   bool Function(CommentReferable) allowTree;
 
-  // Constructor references are pretty ambiguous by nature since they are
+  // Constructor references are pretty ambiguous by nature since they can be
   // declared with the same name as the class they are constructing, and even
   // if they don't use field-formal parameters, sometimes have parameters
   // named the same as members.
   // Maybe clean this up with inspiration from constructor tear-off syntax?
-  if (commentReference.allowDefaultConstructor) {
+  if (commentReference.allowUnnamedConstructor) {
     // Neither reject, nor require, a default constructor in the event
     // the comment reference structure implies one.  (We can not require it
     // in case a library name is the same as a member class name and the class
@@ -212,12 +210,12 @@ MatchingLinkResult _getMatchingLinkElementCommentReferable(
     // Trailing parens indicate we are looking for a callable.
     filter = _requireCallable;
   } else {
-    // Without hints, reject default constructors and their parameters to force
+    // Without hints, reject unnamed constructors and their parameters to force
     // resolution to the class.
-    filter = _rejectDefaultAndShadowingConstructors;
+    filter = _rejectUnnamedAndShadowingConstructors;
 
-    if (!commentReference.allowDefaultConstructorParameter) {
-      allowTree = _rejectDefaultAndShadowingConstructors;
+    if (!commentReference.allowUnnamedConstructorParameter) {
+      allowTree = _rejectUnnamedAndShadowingConstructors;
     }
   }
 

lib/src/model/class.dart

@@ -70,10 +70,15 @@ class Class extends Container
     return _unnamedConstructor;
   }
 
-  @Deprecated(
-      'Renamed to `unnamedConstructor`; this getter with the old name will be '
-      'removed as early as Dartdoc 1.0.0')
-  Constructor get defaultConstructor => unnamedConstructor;
+  Constructor _defaultConstructor;
+
+  /// With constructor tearoffs, this is no longer equivalent to the unnamed
+  /// constructor and assumptions based on that are incorrect.
+  Constructor get defaultConstructor {
+    _defaultConstructor ??= unnamedConstructor ??
+        constructors.firstWhere((c) => c.isDefaultConstructor);
+    return _defaultConstructor;
+  }
 
   @override
   Iterable<Method> get instanceMethods =>
@@ -613,8 +618,11 @@ class Class extends Container
     for (var constructor in source) {
       yield MapEntry(constructor.referenceName, constructor);
       yield MapEntry(
-          '${constructor.enclosingElement.name}.${constructor.referenceName}',
+          '${constructor.enclosingElement.referenceName}.${constructor.referenceName}',
           constructor);
+      if (constructor.isDefaultConstructor) {
+        yield MapEntry('new', constructor);
+      }
     }
   }
 

lib/src/model/constructor.dart

@@ -67,16 +67,10 @@ class Constructor extends ModelElement
   @override
   bool get isConst => element.isConst;
 
-  bool get isUnnamedConstructor =>
-      name == enclosingElement.name || name == '${enclosingElement.name}.new';
-
-  @Deprecated(
-      // TODO(jcollins-g): This, in retrospect, seems like a bad idea.
-      // We should disambiguate the two concepts (default and unnamed) and
-      // allow both if useful.
-      'Renamed to `isUnnamedConstructor`; this getter with the old name will '
-      'be removed as early as Dartdoc 1.0.0')
-  bool get isDefaultConstructor => isUnnamedConstructor;
+  bool get isUnnamedConstructor => name == enclosingElement.name;
+
+  bool get isDefaultConstructor =>
+      name == '${enclosingElement.name}.new' || isUnnamedConstructor;
 
   bool get isFactory => element.isFactory;
 

test/end2end/model_special_cases_test.dart

@@ -171,6 +171,21 @@ void main() {
         expect(referenceLookup(constructorTearoffs, 'F()'),
             equals(MatchingLinkResult(Fnew)));
       });
+
+      test('.new works on classes', () {
+        expect(referenceLookup(constructorTearoffs, 'A.new'),
+            equals(MatchingLinkResult(Anew)));
+        expect(referenceLookup(constructorTearoffs, 'B.new'),
+            equals(MatchingLinkResult(Bnew)));
+        expect(referenceLookup(constructorTearoffs, 'C.new'),
+            equals(MatchingLinkResult(Cnew)));
+        expect(referenceLookup(constructorTearoffs, 'D.new'),
+            equals(MatchingLinkResult(Dnew)));
+        expect(referenceLookup(constructorTearoffs, 'E.new'),
+            equals(MatchingLinkResult(Enew)));
+        expect(referenceLookup(constructorTearoffs, 'F.new'),
+            equals(MatchingLinkResult(Fnew)));
+      });
     }, skip: !_constructorTearoffsAllowed.allows(utils.platformVersion));
   });
 

tool/grind.dart

@@ -232,11 +232,16 @@ void updateThirdParty() async {
 }
 
 @Task('Analyze dartdoc to ensure there are no errors and warnings')
+@Depends(analyzeTestPackages)
 void analyze() async {
   await SubprocessLauncher('analyze').runStreamed(
     sdkBin('dartanalyzer'),
     ['--fatal-infos', '--options', 'analysis_options_presubmit.yaml', '.'],
   );
+}
+
+@Task('Analyze the test packages')
+void analyzeTestPackages() async {
   var testPackagePaths = [testPackage.path];
   if (Platform.version.contains('dev')) {
     testPackagePaths.add(testPackageExperiments.path);