Skip to content

Commit b2de813

Browse files
srawlinssrawlins
authored
Remove support for deprecated leading new in comment references (#3529)
1 parent b350c68 commit b2de813

File tree

8 files changed

+76
-197
lines changed

8 files changed

+76
-197
lines changed

‎lib/src/comment_references/model_comment_reference.dart

Lines changed: 1 addition & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,7 @@ import 'package:analyzer/file_system/file_system.dart';
99
import 'package:dartdoc/src/comment_references/parser.dart';
1010

1111
abstract class ModelCommentReference {
12-
/// Does the structure of the reference itself imply a possible unnamed
13-
/// constructor?
14-
bool get allowUnnamedConstructor;
15-
bool get allowUnnamedConstructorParameter;
1612
String get codeRef;
17-
bool get hasConstructorHint;
1813
bool get hasCallableHint;
1914
List<String> get referenceBy;
2015
Element? get staticElement;
@@ -33,63 +28,15 @@ abstract class ModelCommentReference {
3328
/// information needed for Dartdoc. Drops link to the [CommentReference]
3429
/// and [ResourceProvider] after construction.
3530
class _ModelCommentReferenceImpl implements ModelCommentReference {
36-
void _initAllowCache() {
37-
final referencePieces =
38-
parsed.whereType<IdentifierNode>().toList(growable: false);
39-
_allowUnnamedConstructor = false;
40-
_allowUnnamedConstructorParameter = false;
41-
if (referencePieces.length >= 2) {
42-
for (var i = 0; i <= referencePieces.length - 2; i++) {
43-
if (referencePieces[i].text == referencePieces[i + 1].text) {
44-
if (i + 2 == referencePieces.length) {
45-
// This looks like an old-style reference to an unnamed
46-
// constructor, e.g. [lib_name.C.C].
47-
_allowUnnamedConstructor = true;
48-
} else {
49-
// This could be a reference to a parameter or type parameter of
50-
// an unnamed/new-declared constructor.
51-
_allowUnnamedConstructorParameter = true;
52-
}
53-
}
54-
}
55-
// e.g. [C.new], which may be the unnamed constructor.
56-
if (referencePieces.isNotEmpty && referencePieces.last.text == 'new') {
57-
_allowUnnamedConstructor = true;
58-
}
59-
}
60-
}
61-
62-
bool? _allowUnnamedConstructor;
63-
@override
64-
bool get allowUnnamedConstructor {
65-
if (_allowUnnamedConstructor == null) {
66-
_initAllowCache();
67-
}
68-
return _allowUnnamedConstructor!;
69-
}
70-
71-
bool? _allowUnnamedConstructorParameter;
72-
@override
73-
bool get allowUnnamedConstructorParameter {
74-
if (_allowUnnamedConstructorParameter == null) {
75-
_initAllowCache();
76-
}
77-
return _allowUnnamedConstructorParameter!;
78-
}
79-
8031
@override
8132
final String codeRef;
8233

8334
@override
8435
bool get hasCallableHint =>
8536
parsed.isNotEmpty &&
86-
(parsed.first is ConstructorHintStartNode ||
37+
((parsed.length > 1 && parsed.last.text == 'new') ||
8738
parsed.last is CallableHintEndNode);
8839

89-
@override
90-
bool get hasConstructorHint =>
91-
parsed.isNotEmpty && parsed.first is ConstructorHintStartNode;
92-
9340
@override
9441
List<String> get referenceBy => parsed
9542
.whereType<IdentifierNode>()

‎lib/src/comment_references/parser.dart

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -157,29 +157,20 @@ class CommentReferenceParser {
157157
return children;
158158
}
159159

160-
static const _constructorHintPrefix = 'new';
161160
static const _ignorePrefixes = ['const', 'final', 'var'];
162161

163162
/// Implement parsing a prefix to a comment reference.
164163
///
165164
/// ```text
166-
/// <prefix> ::= <constructorPrefixHint>
167-
/// | <leadingJunk>
165+
/// <prefix> ::= <leadingModifiers>
168166
///
169-
/// <constructorPrefixHint> ::= 'new '
170-
///
171-
/// <leadingJunk> ::= ('const' | 'final' | 'var')(' '+)
167+
/// <leadingModifiers> ::= ('const' | 'final' | 'var')(' '+)
172168
/// ```
173169
_PrefixParseResult _parsePrefix() {
174170
if (_atEnd) {
175171
return _PrefixParseResult.endOfFile;
176172
}
177173
_walkPastWhitespace();
178-
if (_tryMatchLiteral(_constructorHintPrefix,
179-
requireTrailingNonidentifier: true)) {
180-
return _PrefixParseResult.ok(
181-
ConstructorHintStartNode(_constructorHintPrefix));
182-
}
183174
if (_ignorePrefixes
184175
.any((p) => _tryMatchLiteral(p, requireTrailingNonidentifier: true))) {
185176
return _PrefixParseResult.junk;
@@ -387,9 +378,6 @@ class _PrefixParseResult {
387378

388379
const _PrefixParseResult._(this.type, this.node);
389380

390-
factory _PrefixParseResult.ok(ConstructorHintStartNode node) =>
391-
_PrefixParseResult._(_PrefixResultType.parsedConstructorHint, node);
392-
393381
static const _PrefixParseResult endOfFile =
394382
_PrefixParseResult._(_PrefixResultType.endOfFile, null);
395383

@@ -484,16 +472,6 @@ sealed class CommentReferenceNode {
484472
String get text;
485473
}
486474

487-
class ConstructorHintStartNode extends CommentReferenceNode {
488-
@override
489-
final String text;
490-
491-
ConstructorHintStartNode(this.text);
492-
493-
@override
494-
String toString() => 'ConstructorHintStartNode["$text"]';
495-
}
496-
497475
class CallableHintEndNode extends CommentReferenceNode {
498476
@override
499477
final String text;

‎lib/src/markdown_processor.dart

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,6 @@ bool _rejectUnnamedAndShadowingConstructors(CommentReferable? referable) {
160160
bool _requireCallable(CommentReferable? referable) =>
161161
referable is ModelElement && referable.isCallable;
162162

163-
/// Returns false unless the passed [referable] represents a constructor.
164-
bool _requireConstructor(CommentReferable? referable) =>
165-
referable is Constructor;
166-
167163
MatchingLinkResult _getMatchingLinkElement(
168164
String referenceText, Warnable element) {
169165
var commentReference = ModelCommentReference.synthetic(referenceText);
@@ -174,38 +170,21 @@ MatchingLinkResult _getMatchingLinkElement(
174170
// An "allow tree" filter to be used by [CommentReferable.referenceBy].
175171
bool Function(CommentReferable?) allowTree;
176172

177-
// Constructor references are pretty ambiguous by nature since they can be
178-
// declared with the same name as the class they are constructing, and even
179-
// if they don't use field-formal parameters, sometimes have parameters
180-
// named the same as members.
181-
// Maybe clean this up with inspiration from constructor tear-off syntax?
182-
if (commentReference.allowUnnamedConstructor) {
183-
allowTree = (_) => true;
184-
// Neither reject, nor require, a unnamed constructor in the event the
185-
// comment reference structure implies one. (We can not require it in case
186-
// a library name is the same as a member class name and the class is the
187-
// intended lookup). For example, '[FooClass.FooClass]' structurally
188-
// "looks like" an unnamed constructor, so we should allow it here.
189-
filter = commentReference.hasCallableHint ? _requireCallable : (_) => true;
190-
} else if (commentReference.hasConstructorHint &&
191-
commentReference.hasCallableHint) {
192-
allowTree = (_) => true;
193-
// This takes precedence over the callable hint if both are present --
194-
// pick a constructor if and only constructor if we see `new`.
195-
filter = _requireConstructor;
196-
} else if (commentReference.hasCallableHint) {
173+
if (commentReference.hasCallableHint) {
197174
allowTree = (_) => true;
198175
// Trailing parens indicate we are looking for a callable.
199176
filter = _requireCallable;
200177
} else {
201-
if (!commentReference.allowUnnamedConstructorParameter) {
202-
allowTree = _rejectUnnamedAndShadowingConstructors;
203-
} else {
204-
allowTree = (_) => true;
205-
}
206-
// Without hints, reject unnamed constructors and their parameters to force
207-
// resolution to the class.
208-
filter = _rejectUnnamedAndShadowingConstructors;
178+
allowTree = (_) => true;
179+
// Neither reject, nor require, an unnamed constructor in the event the
180+
// comment reference structure implies one. (We cannot require it in case a
181+
// library name is the same as a member class name and the class is the
182+
// intended lookup).
183+
filter = commentReference.hasCallableHint
184+
? _requireCallable
185+
// Without hints, reject unnamed constructors and their parameters to
186+
// force resolution to the class.
187+
: filter = _rejectUnnamedAndShadowingConstructors;
209188
}
210189
var lookupResult = element.referenceBy(commentReference.referenceBy,
211190
allowTree: allowTree, filter: filter);

‎lib/src/matching_link_result.dart

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
// BSD-style license that can be found in the LICENSE file.
44

55
import 'package:dartdoc/src/model/comment_referable.dart';
6-
import 'package:dartdoc/src/model/model.dart';
76

87
class MatchingLinkResult {
98
final CommentReferable? commentReferable;
@@ -19,8 +18,6 @@ class MatchingLinkResult {
1918

2019
@override
2120
String toString() {
22-
// TODO(srawlins): Scrap the 'new' keyword?
23-
final newKeyword = commentReferable is Constructor ? 'new ' : '';
24-
return 'element: [$newKeyword${commentReferable?.fullyQualifiedName}]';
21+
return 'element: [${commentReferable?.fullyQualifiedName}]';
2522
}
2623
}

‎test/comment_referable/parser_test.dart

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,10 @@ void main() {
99
void expectParseEquivalent(String codeRef, List<String> parts,
1010
{bool constructorHint = false, bool callableHint = false}) {
1111
var result = CommentReferenceParser(codeRef).parse();
12-
var hasConstructorHint =
13-
result.isNotEmpty && result.first is ConstructorHintStartNode;
1412
var hasCallableHint =
1513
result.isNotEmpty && result.last is CallableHintEndNode;
1614
var stringParts = result.whereType<IdentifierNode>().map((i) => i.text);
1715
expect(stringParts, equals(parts));
18-
expect(hasConstructorHint, equals(constructorHint));
1916
expect(hasCallableHint, equals(callableHint));
2017
}
2118

@@ -52,15 +49,12 @@ void main() {
5249
test('Check that basic references parse', () {
5350
expectParseEquivalent('Funvas.u', ['Funvas', 'u']);
5451
expectParseEquivalent('valid', ['valid']);
55-
expectParseEquivalent('new valid', ['valid'], constructorHint: true);
5652
expectParseEquivalent('valid()', ['valid'], callableHint: true);
5753
expectParseEquivalent('const valid()', ['valid'], callableHint: true);
5854
expectParseEquivalent('final valid', ['valid']);
5955
expectParseEquivalent('this.is.valid', ['this', 'is', 'valid']);
6056
expectParseEquivalent('this.is.valid()', ['this', 'is', 'valid'],
6157
callableHint: true);
62-
expectParseEquivalent('new this.is.valid', ['this', 'is', 'valid'],
63-
constructorHint: true);
6458
expectParseEquivalent('const this.is.valid', ['this', 'is', 'valid']);
6559
expectParseEquivalent('final this.is.valid', ['this', 'is', 'valid']);
6660
expectParseEquivalent('var this.is.valid', ['this', 'is', 'valid']);
@@ -79,10 +73,8 @@ void main() {
7973
expectParsePassthrough('operatorThingy');
8074
expectParseEquivalent('operator+', ['+']);
8175
expectParseError('const()');
82-
// TODO(jcollins-g): might need to revisit these two with constructor
83-
// tearoffs?
8476
expectParsePassthrough('new');
85-
expectParseError('new()');
77+
expectParseEquivalent('new()', ['new'], callableHint: true);
8678
});
8779

8880
test('Check that operator references parse', () {

‎test/end2end/model_special_cases_test.dart

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -112,40 +112,16 @@ void main() {
112112
});
113113

114114
test('reference regression', () {
115-
expect(referenceLookup(constructorTearoffs, 'A.A'),
116-
equals(MatchingLinkResult(Anew)));
117-
expect(referenceLookup(constructorTearoffs, 'new A()'),
118-
equals(MatchingLinkResult(Anew)));
119115
expect(referenceLookup(constructorTearoffs, 'A()'),
120116
equals(MatchingLinkResult(Anew)));
121-
expect(referenceLookup(constructorTearoffs, 'B.B'),
122-
equals(MatchingLinkResult(Bnew)));
123-
expect(referenceLookup(constructorTearoffs, 'new B()'),
124-
equals(MatchingLinkResult(Bnew)));
125117
expect(referenceLookup(constructorTearoffs, 'B()'),
126118
equals(MatchingLinkResult(Bnew)));
127-
expect(referenceLookup(constructorTearoffs, 'C.C'),
128-
equals(MatchingLinkResult(Cnew)));
129-
expect(referenceLookup(constructorTearoffs, 'new C()'),
130-
equals(MatchingLinkResult(Cnew)));
131119
expect(referenceLookup(constructorTearoffs, 'C()'),
132120
equals(MatchingLinkResult(Cnew)));
133-
expect(referenceLookup(constructorTearoffs, 'D.D'),
134-
equals(MatchingLinkResult(Dnew)));
135-
expect(referenceLookup(constructorTearoffs, 'new D()'),
136-
equals(MatchingLinkResult(Dnew)));
137121
expect(referenceLookup(constructorTearoffs, 'D()'),
138122
equals(MatchingLinkResult(Dnew)));
139-
expect(referenceLookup(constructorTearoffs, 'E.E'),
140-
equals(MatchingLinkResult(Enew)));
141-
expect(referenceLookup(constructorTearoffs, 'new E()'),
142-
equals(MatchingLinkResult(Enew)));
143123
expect(referenceLookup(constructorTearoffs, 'E()'),
144124
equals(MatchingLinkResult(Enew)));
145-
expect(referenceLookup(constructorTearoffs, 'F.F'),
146-
equals(MatchingLinkResult(Fnew)));
147-
expect(referenceLookup(constructorTearoffs, 'new F()'),
148-
equals(MatchingLinkResult(Fnew)));
149125
expect(referenceLookup(constructorTearoffs, 'F()'),
150126
equals(MatchingLinkResult(Fnew)));
151127
});

0 commit comments

Comments
 (0)