Remove support for deprecated leading new in comment references

Author: srawlins

lib/src/comment_references/model_comment_reference.dart

@@ -9,12 +9,7 @@ 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 unnamed
-  /// constructor?
-  bool get allowUnnamedConstructor;
-  bool get allowUnnamedConstructorParameter;
   String get codeRef;
-  bool get hasConstructorHint;
   bool get hasCallableHint;
   List<String> get referenceBy;
   Element? get staticElement;
@@ -33,63 +28,15 @@ abstract class ModelCommentReference {
 /// information needed for Dartdoc.  Drops link to the [CommentReference]
 /// and [ResourceProvider] after construction.
 class _ModelCommentReferenceImpl implements ModelCommentReference {
-  void _initAllowCache() {
-    final referencePieces =
-        parsed.whereType<IdentifierNode>().toList(growable: false);
-    _allowUnnamedConstructor = false;
-    _allowUnnamedConstructorParameter = false;
-    if (referencePieces.length >= 2) {
-      for (var i = 0; i <= referencePieces.length - 2; i++) {
-        if (referencePieces[i].text == referencePieces[i + 1].text) {
-          if (i + 2 == referencePieces.length) {
-            // This looks like an old-style reference to an unnamed
-            // constructor, e.g. [lib_name.C.C].
-            _allowUnnamedConstructor = true;
-          } else {
-            // This could be a reference to a parameter or type parameter of
-            // an unnamed/new-declared constructor.
-            _allowUnnamedConstructorParameter = true;
-          }
-        }
-      }
-      // e.g. [C.new], which may be the unnamed constructor.
-      if (referencePieces.isNotEmpty && referencePieces.last.text == 'new') {
-        _allowUnnamedConstructor = true;
-      }
-    }
-  }
-
-  bool? _allowUnnamedConstructor;
-  @override
-  bool get allowUnnamedConstructor {
-    if (_allowUnnamedConstructor == null) {
-      _initAllowCache();
-    }
-    return _allowUnnamedConstructor!;
-  }
-
-  bool? _allowUnnamedConstructorParameter;
-  @override
-  bool get allowUnnamedConstructorParameter {
-    if (_allowUnnamedConstructorParameter == null) {
-      _initAllowCache();
-    }
-    return _allowUnnamedConstructorParameter!;
-  }
-
   @override
   final String codeRef;
 
   @override
   bool get hasCallableHint =>
       parsed.isNotEmpty &&
-      (parsed.first is ConstructorHintStartNode ||
+      ((parsed.length > 1 && parsed.last.text == 'new') ||
           parsed.last is CallableHintEndNode);
 
-  @override
-  bool get hasConstructorHint =>
-      parsed.isNotEmpty && parsed.first is ConstructorHintStartNode;
-
   @override
   List<String> get referenceBy => parsed
       .whereType<IdentifierNode>()

lib/src/comment_references/parser.dart

@@ -157,29 +157,20 @@ class CommentReferenceParser {
     return children;
   }
 
-  static const _constructorHintPrefix = 'new';
   static const _ignorePrefixes = ['const', 'final', 'var'];
 
   /// Implement parsing a prefix to a comment reference.
   ///
   /// ```text
-  /// <prefix> ::= <constructorPrefixHint>
-  ///    | <leadingJunk>
+  /// <prefix> ::= <leadingModifiers>
   ///
-  /// <constructorPrefixHint> ::= 'new '
-  ///
-  /// <leadingJunk> ::= ('const' | 'final' | 'var')(' '+)
+  /// <leadingModifiers> ::= ('const' | 'final' | 'var')(' '+)
   /// ```
   _PrefixParseResult _parsePrefix() {
     if (_atEnd) {
       return _PrefixParseResult.endOfFile;
     }
     _walkPastWhitespace();
-    if (_tryMatchLiteral(_constructorHintPrefix,
-        requireTrailingNonidentifier: true)) {
-      return _PrefixParseResult.ok(
-          ConstructorHintStartNode(_constructorHintPrefix));
-    }
     if (_ignorePrefixes
         .any((p) => _tryMatchLiteral(p, requireTrailingNonidentifier: true))) {
       return _PrefixParseResult.junk;
@@ -387,9 +378,6 @@ class _PrefixParseResult {
 
   const _PrefixParseResult._(this.type, this.node);
 
-  factory _PrefixParseResult.ok(ConstructorHintStartNode node) =>
-      _PrefixParseResult._(_PrefixResultType.parsedConstructorHint, node);
-
   static const _PrefixParseResult endOfFile =
       _PrefixParseResult._(_PrefixResultType.endOfFile, null);
 
@@ -484,16 +472,6 @@ sealed class CommentReferenceNode {
   String get text;
 }
 
-class ConstructorHintStartNode extends CommentReferenceNode {
-  @override
-  final String text;
-
-  ConstructorHintStartNode(this.text);
-
-  @override
-  String toString() => 'ConstructorHintStartNode["$text"]';
-}
-
 class CallableHintEndNode extends CommentReferenceNode {
   @override
   final String text;

lib/src/markdown_processor.dart

@@ -160,10 +160,6 @@ bool _rejectUnnamedAndShadowingConstructors(CommentReferable? referable) {
 bool _requireCallable(CommentReferable? referable) =>
     referable is ModelElement && referable.isCallable;
 
-/// Returns false unless the passed [referable] represents a constructor.
-bool _requireConstructor(CommentReferable? referable) =>
-    referable is Constructor;
-
 MatchingLinkResult _getMatchingLinkElement(
     String referenceText, Warnable element) {
   var commentReference = ModelCommentReference.synthetic(referenceText);
@@ -174,38 +170,21 @@ MatchingLinkResult _getMatchingLinkElement(
   // An "allow tree" filter to be used by [CommentReferable.referenceBy].
   bool Function(CommentReferable?) allowTree;
 
-  // 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.allowUnnamedConstructor) {
-    allowTree = (_) => true;
-    // Neither reject, nor require, a unnamed 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 is the
-    // intended lookup).  For example, '[FooClass.FooClass]' structurally
-    // "looks like" an unnamed constructor, so we should allow it here.
-    filter = commentReference.hasCallableHint ? _requireCallable : (_) => true;
-  } else if (commentReference.hasConstructorHint &&
-      commentReference.hasCallableHint) {
-    allowTree = (_) => true;
-    // This takes precedence over the callable hint if both are present --
-    // pick a constructor if and only constructor if we see `new`.
-    filter = _requireConstructor;
-  } else if (commentReference.hasCallableHint) {
+  if (commentReference.hasCallableHint) {
     allowTree = (_) => true;
     // Trailing parens indicate we are looking for a callable.
     filter = _requireCallable;
   } else {
-    if (!commentReference.allowUnnamedConstructorParameter) {
-      allowTree = _rejectUnnamedAndShadowingConstructors;
-    } else {
-      allowTree = (_) => true;
-    }
-    // Without hints, reject unnamed constructors and their parameters to force
-    // resolution to the class.
-    filter = _rejectUnnamedAndShadowingConstructors;
+    allowTree = (_) => true;
+    // Neither reject, nor require, an unnamed constructor in the event the
+    // comment reference structure implies one. (We cannot require it in case a
+    // library name is the same as a member class name and the class is the
+    // intended lookup).
+    filter = commentReference.hasCallableHint
+        ? _requireCallable
+        // Without hints, reject unnamed constructors and their parameters to
+        // force resolution to the class.
+        : filter = _rejectUnnamedAndShadowingConstructors;
   }
   var lookupResult = element.referenceBy(commentReference.referenceBy,
       allowTree: allowTree, filter: filter);

lib/src/matching_link_result.dart

@@ -3,7 +3,6 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:dartdoc/src/model/comment_referable.dart';
-import 'package:dartdoc/src/model/model.dart';
 
 class MatchingLinkResult {
   final CommentReferable? commentReferable;
@@ -19,8 +18,6 @@ class MatchingLinkResult {
 
   @override
   String toString() {
-    // TODO(srawlins): Scrap the 'new' keyword?
-    final newKeyword = commentReferable is Constructor ? 'new ' : '';
-    return 'element: [$newKeyword${commentReferable?.fullyQualifiedName}]';
+    return 'element: [${commentReferable?.fullyQualifiedName}]';
   }
 }

test/comment_referable/parser_test.dart

@@ -9,13 +9,10 @@ void main() {
   void expectParseEquivalent(String codeRef, List<String> parts,
       {bool constructorHint = false, bool callableHint = false}) {
     var result = CommentReferenceParser(codeRef).parse();
-    var hasConstructorHint =
-        result.isNotEmpty && result.first is ConstructorHintStartNode;
     var hasCallableHint =
         result.isNotEmpty && result.last is CallableHintEndNode;
     var stringParts = result.whereType<IdentifierNode>().map((i) => i.text);
     expect(stringParts, equals(parts));
-    expect(hasConstructorHint, equals(constructorHint));
     expect(hasCallableHint, equals(callableHint));
   }
 
@@ -52,15 +49,12 @@ void main() {
     test('Check that basic references parse', () {
       expectParseEquivalent('Funvas.u', ['Funvas', 'u']);
       expectParseEquivalent('valid', ['valid']);
-      expectParseEquivalent('new valid', ['valid'], constructorHint: true);
       expectParseEquivalent('valid()', ['valid'], callableHint: true);
       expectParseEquivalent('const valid()', ['valid'], callableHint: true);
       expectParseEquivalent('final valid', ['valid']);
       expectParseEquivalent('this.is.valid', ['this', 'is', 'valid']);
       expectParseEquivalent('this.is.valid()', ['this', 'is', 'valid'],
           callableHint: true);
-      expectParseEquivalent('new this.is.valid', ['this', 'is', 'valid'],
-          constructorHint: true);
       expectParseEquivalent('const this.is.valid', ['this', 'is', 'valid']);
       expectParseEquivalent('final this.is.valid', ['this', 'is', 'valid']);
       expectParseEquivalent('var this.is.valid', ['this', 'is', 'valid']);
@@ -79,10 +73,8 @@ void main() {
       expectParsePassthrough('operatorThingy');
       expectParseEquivalent('operator+', ['+']);
       expectParseError('const()');
-      // TODO(jcollins-g): might need to revisit these two with constructor
-      // tearoffs?
       expectParsePassthrough('new');
-      expectParseError('new()');
+      expectParseEquivalent('new()', ['new'], callableHint: true);
     });
 
     test('Check that operator references parse', () {

test/end2end/model_special_cases_test.dart

@@ -112,40 +112,16 @@ void main() {
     });
 
     test('reference regression', () {
-      expect(referenceLookup(constructorTearoffs, 'A.A'),
-          equals(MatchingLinkResult(Anew)));
-      expect(referenceLookup(constructorTearoffs, 'new A()'),
-          equals(MatchingLinkResult(Anew)));
       expect(referenceLookup(constructorTearoffs, 'A()'),
           equals(MatchingLinkResult(Anew)));
-      expect(referenceLookup(constructorTearoffs, 'B.B'),
-          equals(MatchingLinkResult(Bnew)));
-      expect(referenceLookup(constructorTearoffs, 'new B()'),
-          equals(MatchingLinkResult(Bnew)));
       expect(referenceLookup(constructorTearoffs, 'B()'),
           equals(MatchingLinkResult(Bnew)));
-      expect(referenceLookup(constructorTearoffs, 'C.C'),
-          equals(MatchingLinkResult(Cnew)));
-      expect(referenceLookup(constructorTearoffs, 'new C()'),
-          equals(MatchingLinkResult(Cnew)));
       expect(referenceLookup(constructorTearoffs, 'C()'),
           equals(MatchingLinkResult(Cnew)));
-      expect(referenceLookup(constructorTearoffs, 'D.D'),
-          equals(MatchingLinkResult(Dnew)));
-      expect(referenceLookup(constructorTearoffs, 'new D()'),
-          equals(MatchingLinkResult(Dnew)));
       expect(referenceLookup(constructorTearoffs, 'D()'),
           equals(MatchingLinkResult(Dnew)));
-      expect(referenceLookup(constructorTearoffs, 'E.E'),
-          equals(MatchingLinkResult(Enew)));
-      expect(referenceLookup(constructorTearoffs, 'new E()'),
-          equals(MatchingLinkResult(Enew)));
       expect(referenceLookup(constructorTearoffs, 'E()'),
           equals(MatchingLinkResult(Enew)));
-      expect(referenceLookup(constructorTearoffs, 'F.F'),
-          equals(MatchingLinkResult(Fnew)));
-      expect(referenceLookup(constructorTearoffs, 'new F()'),
-          equals(MatchingLinkResult(Fnew)));
       expect(referenceLookup(constructorTearoffs, 'F()'),
           equals(MatchingLinkResult(Fnew)));
     });