Skip to content

Commit cb34251

Browse files
srawlinsCommit Bot
authored and
Commit Bot
committed
Support for three-word references in Doc Comments
This expands the front_end's and analyzer's support for doc comment references to include `/// [a.b.c]`. Such a reference must be to a getter, setter, method, or constructor of a class or extension which is referenced with an import prefix, such as `[async.Future.value]`. In the existing support for two words, the two words are named "prefix" and "token". In a three-word reference, the _first_ word must be an import prefix, but I've chosen the new names, "leadingPrefix", "prefix", and "token", to minimize the change. If an improved change is desirable, considering the names from scratch, I might choose something like "prefix", "container", and "identifier". But these names will be applied to one-word and two-word references as well, so `[async.Future]`, a reference to the Future class provided by an import prefixed as 'async', would have a 'prefix' of null, a 'container' of 'async', and an identifier of 'Future'. Bug: #47444 Change-Id: I7758ec16872677221c456dd1dd6dc2a4df0f6102 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/223661 Reviewed-by: Johnni Winther <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent ac7bca3 commit cb34251

File tree

12 files changed

+605
-203
lines changed

12 files changed

+605
-203
lines changed

pkg/_fe_analyzer_shared/lib/src/parser/forwarding_listener.dart

+8-2
Original file line numberDiff line numberDiff line change
@@ -1259,8 +1259,14 @@ class ForwardingListener implements Listener {
12591259

12601260
@override
12611261
void handleCommentReference(
1262-
Token? newKeyword, Token? prefix, Token? period, Token token) {
1263-
listener?.handleCommentReference(newKeyword, prefix, period, token);
1262+
Token? newKeyword,
1263+
Token? firstToken,
1264+
Token? firstPeriod,
1265+
Token? secondToken,
1266+
Token? secondPeriod,
1267+
Token thirdToken) {
1268+
listener?.handleCommentReference(newKeyword, firstToken, firstPeriod,
1269+
secondToken, secondPeriod, thirdToken);
12641270
}
12651271

12661272
@override

pkg/_fe_analyzer_shared/lib/src/parser/listener.dart

+10-3
Original file line numberDiff line numberDiff line change
@@ -1840,13 +1840,20 @@ class Listener implements UnescapeErrorListener {
18401840

18411841
/// A single comment reference has been parsed.
18421842
/// * [newKeyword] may be null.
1843-
/// * [prefix] and [period] are either both tokens or both `null`.
1844-
/// * [token] can be an identifier or an operator.
1843+
/// * [firstToken] and [firstPeriod] are either both tokens or both
1844+
/// `null`.
1845+
/// * [secondToken] and [secondPeriod] are either both tokens or both `null`.
1846+
/// * [thirdToken] can be an identifier or an operator.
18451847
///
18461848
/// This event is generated by the parser when the parser's
18471849
/// `parseOneCommentReference` method is called.
18481850
void handleCommentReference(
1849-
Token? newKeyword, Token? prefix, Token? period, Token token) {}
1851+
Token? newKeyword,
1852+
Token? firstToken,
1853+
Token? firstPeriod,
1854+
Token? secondToken,
1855+
Token? secondPeriod,
1856+
Token thirdToken) {}
18501857

18511858
/// This event is generated by the parser when the parser's
18521859
/// `parseOneCommentReference` method is called.

pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart

+25-16
Original file line numberDiff line numberDiff line change
@@ -8355,26 +8355,33 @@ class Parser {
83558355
newKeyword = token;
83568356
token = token.next!;
83578357
}
8358-
Token? prefix, period;
8358+
Token? firstToken, firstPeriod, secondToken, secondPeriod;
83598359
if (token.isIdentifier && optional('.', token.next!)) {
8360-
prefix = token;
8361-
period = token.next!;
8362-
Token identifier = period.next!;
8360+
secondToken = token;
8361+
secondPeriod = token.next!;
8362+
if (secondPeriod.next!.isIdentifier &&
8363+
optional('.', secondPeriod.next!.next!)) {
8364+
firstToken = secondToken;
8365+
firstPeriod = secondPeriod;
8366+
secondToken = secondPeriod.next!;
8367+
secondPeriod = secondToken.next!;
8368+
}
8369+
Token identifier = secondPeriod.next!;
83638370
if (identifier.kind == KEYWORD_TOKEN && optional('new', identifier)) {
83648371
// Treat `new` after `.` is as an identifier so that it can represent an
83658372
// unnamed constructor. This support is separate from the
83668373
// constructor-tearoffs feature.
83678374
rewriter.replaceTokenFollowing(
8368-
period,
8375+
secondPeriod,
83698376
new StringToken(TokenType.IDENTIFIER, identifier.lexeme,
83708377
identifier.charOffset));
83718378
}
8372-
token = period.next!;
8379+
token = secondPeriod.next!;
83738380
}
83748381
if (token.isEof) {
83758382
// Recovery: Insert a synthetic identifier for code completion
83768383
token = rewriter.insertSyntheticIdentifier(
8377-
period ?? newKeyword ?? syntheticPreviousToken(token));
8384+
secondPeriod ?? newKeyword ?? syntheticPreviousToken(token));
83788385
if (begin == token.next!) {
83798386
begin = token;
83808387
}
@@ -8386,21 +8393,21 @@ class Parser {
83868393
}
83878394
if (token.isUserDefinableOperator) {
83888395
if (token.next!.isEof) {
8389-
parseOneCommentReferenceRest(
8390-
begin, referenceOffset, newKeyword, prefix, period, token);
8396+
parseOneCommentReferenceRest(begin, referenceOffset, newKeyword,
8397+
firstToken, firstPeriod, secondToken, secondPeriod, token);
83918398
return true;
83928399
}
83938400
} else {
83948401
token = operatorKeyword ?? token;
83958402
if (token.next!.isEof) {
83968403
if (token.isIdentifier) {
8397-
parseOneCommentReferenceRest(
8398-
begin, referenceOffset, newKeyword, prefix, period, token);
8404+
parseOneCommentReferenceRest(begin, referenceOffset, newKeyword,
8405+
firstToken, firstPeriod, secondToken, secondPeriod, token);
83998406
return true;
84008407
}
84018408
Keyword? keyword = token.keyword;
84028409
if (newKeyword == null &&
8403-
prefix == null &&
8410+
secondToken == null &&
84048411
(keyword == Keyword.THIS ||
84058412
keyword == Keyword.NULL ||
84068413
keyword == Keyword.TRUE ||
@@ -8421,8 +8428,10 @@ class Parser {
84218428
Token begin,
84228429
int referenceOffset,
84238430
Token? newKeyword,
8424-
Token? prefix,
8425-
Token? period,
8431+
Token? firstToken,
8432+
Token? firstPeriod,
8433+
Token? secondToken,
8434+
Token? secondPeriod,
84268435
Token identifierOrOperator) {
84278436
// Adjust the token offsets to match the enclosing comment token.
84288437
Token token = begin;
@@ -8431,8 +8440,8 @@ class Parser {
84318440
token = token.next!;
84328441
} while (!token.isEof);
84338442

8434-
listener.handleCommentReference(
8435-
newKeyword, prefix, period, identifierOrOperator);
8443+
listener.handleCommentReference(newKeyword, firstToken, firstPeriod,
8444+
secondToken, secondPeriod, identifierOrOperator);
84368445
}
84378446

84388447
/// Given that we have just found bracketed text within the given [comment],

pkg/analyzer/lib/src/dart/analysis/index.dart

+2
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,8 @@ class _IndexContributor extends GeneralizingAstVisitor {
596596
return;
597597
}
598598
}
599+
} else if (expression is PropertyAccess) {
600+
// Nothing to do?
599601
} else {
600602
throw UnimplementedError('Unhandled CommentReference expression type: '
601603
'${expression.runtimeType}');
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:analyzer/dart/ast/ast.dart';
6+
import 'package:analyzer/dart/element/element.dart';
7+
import 'package:analyzer/dart/element/type.dart';
8+
import 'package:analyzer/src/dart/ast/ast.dart';
9+
import 'package:analyzer/src/dart/element/type_provider.dart';
10+
import 'package:analyzer/src/dart/resolver/type_property_resolver.dart';
11+
import 'package:analyzer/src/generated/resolver.dart';
12+
13+
class CommentReferenceResolver {
14+
final TypeProviderImpl _typeProvider;
15+
16+
final ResolverVisitor _resolver;
17+
18+
/// Helper for resolving properties on types.
19+
final TypePropertyResolver _typePropertyResolver;
20+
21+
CommentReferenceResolver(this._typeProvider, this._resolver)
22+
: _typePropertyResolver = _resolver.typePropertyResolver;
23+
24+
/// Resolves [commentReference].
25+
void resolve(CommentReference commentReference) {
26+
var expression = commentReference.expression;
27+
if (expression is SimpleIdentifierImpl) {
28+
_resolveSimpleIdentifierReference(expression,
29+
hasNewKeyword: commentReference.newKeyword != null);
30+
} else if (expression is PrefixedIdentifierImpl) {
31+
_resolvePrefixedIdentifierReference(expression,
32+
hasNewKeyword: commentReference.newKeyword != null);
33+
} else if (expression is PropertyAccessImpl) {
34+
_resolvePropertyAccessReference(expression,
35+
hasNewKeyword: commentReference.newKeyword != null);
36+
}
37+
}
38+
39+
void _resolvePrefixedIdentifierReference(
40+
PrefixedIdentifierImpl expression, {
41+
required bool hasNewKeyword,
42+
}) {
43+
var prefix = expression.prefix;
44+
var prefixElement = _resolveSimpleIdentifier(prefix);
45+
prefix.staticElement = prefixElement;
46+
47+
if (prefixElement == null) {
48+
return;
49+
}
50+
51+
var name = expression.identifier;
52+
53+
if (prefixElement is PrefixElement) {
54+
var prefixScope = prefixElement.scope;
55+
var lookupResult = prefixScope.lookup(name.name);
56+
var element = lookupResult.getter ?? lookupResult.setter;
57+
element = _resolver.toLegacyElement(element);
58+
name.staticElement = element;
59+
return;
60+
}
61+
62+
if (!hasNewKeyword) {
63+
if (prefixElement is ClassElement) {
64+
name.staticElement = prefixElement.getMethod(name.name) ??
65+
prefixElement.getGetter(name.name) ??
66+
prefixElement.getSetter(name.name) ??
67+
prefixElement.getNamedConstructor(name.name);
68+
} else if (prefixElement is ExtensionElement) {
69+
name.staticElement = prefixElement.getMethod(name.name) ??
70+
prefixElement.getGetter(name.name) ??
71+
prefixElement.getSetter(name.name);
72+
} else {
73+
// TODO(brianwilkerson) Report this error.
74+
}
75+
} else if (prefixElement is ClassElement) {
76+
var constructor = prefixElement.getNamedConstructor(name.name);
77+
if (constructor == null) {
78+
// TODO(brianwilkerson) Report this error.
79+
} else {
80+
name.staticElement = constructor;
81+
}
82+
} else {
83+
// TODO(brianwilkerson) Report this error.
84+
}
85+
}
86+
87+
void _resolvePropertyAccessReference(
88+
PropertyAccessImpl expression, {
89+
required bool hasNewKeyword,
90+
}) {
91+
var target = expression.target;
92+
if (target is! PrefixedIdentifierImpl) {
93+
// A PropertyAccess with a target more complex than a
94+
// [PrefixedIdentifier] is not a valid comment reference.
95+
return;
96+
}
97+
98+
var prefix = target.prefix;
99+
var prefixElement = _resolveSimpleIdentifier(prefix);
100+
prefix.staticElement = prefixElement;
101+
102+
if (prefixElement is! PrefixElement) {
103+
// The only valid prefixElement is a PrefixElement; otherwise, this is
104+
// not a comment reference.
105+
return;
106+
}
107+
108+
var name = target.identifier;
109+
var prefixScope = prefixElement.scope;
110+
var lookupResult = prefixScope.lookup(name.name);
111+
var element = lookupResult.getter ?? lookupResult.setter;
112+
element = _resolver.toLegacyElement(element);
113+
name.staticElement = element;
114+
115+
var propertyName = expression.propertyName;
116+
if (element is ClassElement) {
117+
propertyName.staticElement = element.getMethod(propertyName.name) ??
118+
element.getGetter(propertyName.name) ??
119+
element.getSetter(propertyName.name) ??
120+
element.getNamedConstructor(propertyName.name);
121+
} else if (element is ExtensionElement) {
122+
propertyName.staticElement = element.getMethod(propertyName.name) ??
123+
element.getGetter(propertyName.name) ??
124+
element.getSetter(propertyName.name);
125+
}
126+
}
127+
128+
/// Resolves the given simple [identifier] if possible.
129+
///
130+
/// Returns the resolved element, or `null` if the identifier could not be
131+
/// resolved. This does not record the results of the resolution.
132+
Element? _resolveSimpleIdentifier(SimpleIdentifierImpl identifier) {
133+
var lookupResult = identifier.scopeLookupResult!;
134+
135+
var element = _resolver.toLegacyElement(lookupResult.getter) ??
136+
_resolver.toLegacyElement(lookupResult.setter);
137+
138+
if (element == null) {
139+
InterfaceType enclosingType;
140+
var enclosingClass = _resolver.enclosingClass;
141+
if (enclosingClass != null) {
142+
enclosingType = enclosingClass.thisType;
143+
} else {
144+
var enclosingExtension = _resolver.enclosingExtension;
145+
if (enclosingExtension == null) {
146+
return null;
147+
}
148+
var extendedType = enclosingExtension.extendedType
149+
.resolveToBound(_typeProvider.objectType);
150+
if (extendedType is InterfaceType) {
151+
enclosingType = extendedType;
152+
} else if (extendedType is FunctionType) {
153+
enclosingType = _typeProvider.functionType;
154+
} else {
155+
return null;
156+
}
157+
}
158+
var result = _typePropertyResolver.resolve(
159+
receiver: null,
160+
receiverType: enclosingType,
161+
name: identifier.name,
162+
propertyErrorEntity: identifier,
163+
nameErrorEntity: identifier,
164+
);
165+
if (identifier.parent is CommentReference) {
166+
// TODO(srawlins): Why is the setter preferred? This seems very flawed
167+
// as it will only use the setter for a [SimpleIdentifier] comment
168+
// reference, and not a [PrefixedIdentifier] or a [PropertyAccess].
169+
element = result.setter;
170+
}
171+
element ??= result.getter;
172+
}
173+
return element;
174+
}
175+
176+
void _resolveSimpleIdentifierReference(
177+
SimpleIdentifierImpl expression, {
178+
required bool hasNewKeyword,
179+
}) {
180+
var element = _resolveSimpleIdentifier(expression);
181+
if (element == null) {
182+
return;
183+
}
184+
expression.staticElement = element;
185+
if (hasNewKeyword) {
186+
if (element is ClassElement) {
187+
var constructor = element.unnamedConstructor;
188+
if (constructor == null) {
189+
// TODO(brianwilkerson) Report this error.
190+
} else {
191+
expression.staticElement = constructor;
192+
}
193+
} else {
194+
// TODO(brianwilkerson) Report this error.
195+
}
196+
}
197+
}
198+
}

pkg/analyzer/lib/src/fasta/ast_builder.dart

+19-6
Original file line numberDiff line numberDiff line change
@@ -2668,13 +2668,26 @@ class AstBuilder extends StackListener {
26682668

26692669
@override
26702670
void handleCommentReference(
2671-
Token? newKeyword, Token? prefix, Token? period, Token token) {
2672-
Identifier identifier = ast.simpleIdentifier(token);
2673-
if (prefix != null) {
2674-
identifier = ast.prefixedIdentifier(ast.simpleIdentifier(prefix), period!,
2675-
identifier as SimpleIdentifier);
2671+
Token? newKeyword,
2672+
Token? firstToken,
2673+
Token? firstPeriod,
2674+
Token? secondToken,
2675+
Token? secondPeriod,
2676+
Token thirdToken,
2677+
) {
2678+
var identifier = ast.simpleIdentifier(thirdToken);
2679+
if (firstToken != null) {
2680+
var target = ast.prefixedIdentifier(ast.simpleIdentifier(firstToken),
2681+
firstPeriod!, ast.simpleIdentifier(secondToken!));
2682+
var expression = ast.propertyAccess(target, secondPeriod!, identifier);
2683+
push(ast.commentReference(newKeyword, expression));
2684+
} else if (secondToken != null) {
2685+
var expression = ast.prefixedIdentifier(
2686+
ast.simpleIdentifier(secondToken), secondPeriod!, identifier);
2687+
push(ast.commentReference(newKeyword, expression));
2688+
} else {
2689+
push(ast.commentReference(newKeyword, identifier));
26762690
}
2677-
push(ast.commentReference(newKeyword, identifier));
26782691
}
26792692

26802693
@override

0 commit comments

Comments
 (0)