Skip to content

Commit b7d139a

Browse files
stereotype441commit-bot@chromium.org
authored andcommitted
External variable support: add a notion of external top level variables to the AST and element model.
In follow-up CLs I will update error reporting logic. Change-Id: Iee42088beffee3915a2ef6a4c8cb5c1031535d4d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/158149 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 9ccc7f8 commit b7d139a

26 files changed

+158
-11
lines changed

pkg/analysis_server/lib/src/computer/computer_highlights.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,13 @@ class _DartUnitHighlightsComputerVisitor extends RecursiveAstVisitor<void> {
755755
super.visitThisExpression(node);
756756
}
757757

758+
@override
759+
void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
760+
computer._addRegion_token(
761+
node.externalKeyword, HighlightRegionType.BUILT_IN);
762+
super.visitTopLevelVariableDeclaration(node);
763+
}
764+
758765
@override
759766
void visitTryStatement(TryStatement node) {
760767
computer._addRegion_token(node.tryKeyword, HighlightRegionType.KEYWORD);

pkg/analysis_server/lib/src/computer/computer_highlights2.dart

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,13 @@ class _DartUnitHighlightsComputerVisitor2 extends RecursiveAstVisitor<void> {
864864
super.visitThrowExpression(node);
865865
}
866866

867+
@override
868+
void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
869+
computer._addRegion_token(
870+
node.externalKeyword, HighlightRegionType.BUILT_IN);
871+
super.visitTopLevelVariableDeclaration(node);
872+
}
873+
867874
@override
868875
void visitTryStatement(TryStatement node) {
869876
computer._addRegion_token(node.tryKeyword, HighlightRegionType.KEYWORD);

pkg/analysis_server/lib/src/services/completion/dart/keyword_contributor.dart

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,29 @@ class _KeywordVisitor extends GeneralizingAstVisitor<void> {
676676
}
677677
}
678678

679+
@override
680+
void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
681+
var variableDeclarationList = node.variables;
682+
if (entity != variableDeclarationList) return;
683+
var variables = variableDeclarationList.variables;
684+
if (variables.isEmpty || request.offset > variables.first.beginToken.end) {
685+
return;
686+
}
687+
if (node.externalKeyword == null) {
688+
_addSuggestion(Keyword.EXTERNAL);
689+
}
690+
if (variableDeclarationList.lateKeyword == null &&
691+
request.featureSet.isEnabled(Feature.non_nullable)) {
692+
_addSuggestion(Keyword.LATE);
693+
}
694+
if (!variables.first.isConst) {
695+
_addSuggestion(Keyword.CONST);
696+
}
697+
if (!variables.first.isFinal) {
698+
_addSuggestion(Keyword.FINAL);
699+
}
700+
}
701+
679702
@override
680703
void visitTryStatement(TryStatement node) {
681704
var obj = entity;

pkg/analysis_server/tool/completion_metrics/code_metrics.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,6 +1196,7 @@ class CodeShapeDataCollector extends RecursiveAstVisitor<void> {
11961196
_visitChildren(node, {
11971197
'documentationComment': node.documentationComment,
11981198
'metadata': node.metadata,
1199+
'externalKeyword': node.externalKeyword,
11991200
'variables': node.variables,
12001201
});
12011202
super.visitTopLevelVariableDeclaration(node);

pkg/analysis_server/tool/completion_metrics/visitors.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,12 @@ class ExpectedCompletionsVisitor extends RecursiveAstVisitor<void> {
665665
return super.visitThrowExpression(node);
666666
}
667667

668+
@override
669+
void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
670+
safelyRecordKeywordCompletion(node.externalKeyword);
671+
return super.visitTopLevelVariableDeclaration(node);
672+
}
673+
668674
@override
669675
void visitTryStatement(TryStatement node) {
670676
safelyRecordKeywordCompletion(node.tryKeyword);

pkg/analyzer/lib/dart/ast/ast.dart

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4959,11 +4959,21 @@ abstract class ThrowExpression implements Expression {
49594959
/// The declaration of one or more top-level variables of the same type.
49604960
///
49614961
/// topLevelVariableDeclaration ::=
4962-
/// ('final' | 'const') type? staticFinalDeclarationList ';'
4963-
/// | variableDeclaration ';'
4962+
/// ('final' | 'const') <type>? <staticFinalDeclarationList> ';'
4963+
/// | 'late' 'final' <type>? <initializedIdentifierList> ';'
4964+
/// | 'late'? <varOrType> <initializedIdentifierList> ';'
4965+
/// | 'external' <finalVarOrType> <identifierList> ';'
4966+
///
4967+
/// (Note: there is no <topLevelVariableDeclaration> production in the grammar;
4968+
/// this is a subset of the grammar production <topLevelDeclaration>, which
4969+
/// encompasses everything that can appear inside a Dart file after part
4970+
/// directives).
49644971
///
49654972
/// Clients may not extend, implement or mix-in this class.
49664973
abstract class TopLevelVariableDeclaration implements CompilationUnitMember {
4974+
/// The `external` keyword, or `null` if the keyword was not used.
4975+
Token get externalKeyword;
4976+
49674977
/// Return the semicolon terminating the declaration.
49684978
Token get semicolon;
49694979

pkg/analyzer/lib/dart/ast/ast_factory.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -909,7 +909,8 @@ abstract class AstFactory {
909909
Comment comment,
910910
List<Annotation> metadata,
911911
VariableDeclarationList variableList,
912-
Token semicolon);
912+
Token semicolon,
913+
{Token externalKeyword});
913914

914915
/// Returns a newly created try statement. The list of [catchClauses] can be
915916
/// `null` if there are no catch clauses. The [finallyKeyword] and

pkg/analyzer/lib/dart/element/element.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1694,6 +1694,9 @@ abstract class ShowElementCombinator implements NamespaceCombinator {
16941694
abstract class TopLevelVariableElement implements PropertyInducingElement {
16951695
@override
16961696
TopLevelVariableElement get declaration;
1697+
1698+
/// Return `true` if this field was explicitly marked as being external.
1699+
bool get isExternal;
16971700
}
16981701

16991702
/// An element that defines a type.

pkg/analyzer/lib/src/dart/ast/ast.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9796,6 +9796,9 @@ class TopLevelVariableDeclarationImpl extends CompilationUnitMemberImpl
97969796
/// The top-level variables being declared.
97979797
VariableDeclarationListImpl _variableList;
97989798

9799+
@override
9800+
Token externalKeyword;
9801+
97999802
/// The semicolon terminating the declaration.
98009803
@override
98019804
Token semicolon;
@@ -9806,6 +9809,7 @@ class TopLevelVariableDeclarationImpl extends CompilationUnitMemberImpl
98069809
TopLevelVariableDeclarationImpl(
98079810
CommentImpl comment,
98089811
List<Annotation> metadata,
9812+
this.externalKeyword,
98099813
VariableDeclarationListImpl variableList,
98109814
this.semicolon)
98119815
: super(comment, metadata) {
@@ -9823,7 +9827,8 @@ class TopLevelVariableDeclarationImpl extends CompilationUnitMemberImpl
98239827
Token get endToken => semicolon;
98249828

98259829
@override
9826-
Token get firstTokenAfterCommentAndMetadata => _variableList.beginToken;
9830+
Token get firstTokenAfterCommentAndMetadata =>
9831+
externalKeyword ?? _variableList.beginToken;
98279832

98289833
@override
98299834
VariableDeclarationList get variables => _variableList;

pkg/analyzer/lib/src/dart/ast/ast_factory.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,9 +1020,10 @@ class AstFactoryImpl extends AstFactory {
10201020
Comment comment,
10211021
List<Annotation> metadata,
10221022
VariableDeclarationList variableList,
1023-
Token semicolon) =>
1023+
Token semicolon,
1024+
{Token externalKeyword}) =>
10241025
TopLevelVariableDeclarationImpl(
1025-
comment, metadata, variableList, semicolon);
1026+
comment, metadata, externalKeyword, variableList, semicolon);
10261027

10271028
@override
10281029
TryStatement tryStatement(

pkg/analyzer/lib/src/dart/ast/to_source_visitor.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,6 +1075,7 @@ class ToSourceVisitor implements AstVisitor<void> {
10751075

10761076
@override
10771077
void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
1078+
safelyVisitTokenWithSuffix(node.externalKeyword, " ");
10781079
safelyVisitNodeWithSuffix(node.variables, ";");
10791080
}
10801081

pkg/analyzer/lib/src/dart/ast/utilities.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -975,7 +975,8 @@ class AstCloner implements AstVisitor<AstNode> {
975975
cloneNode(node.documentationComment),
976976
cloneNodeList(node.metadata),
977977
cloneNode(node.variables),
978-
cloneToken(node.semicolon));
978+
cloneToken(node.semicolon),
979+
externalKeyword: cloneToken(node.externalKeyword));
979980

980981
@override
981982
TryStatement visitTryStatement(TryStatement node) => astFactory.tryStatement(
@@ -2193,6 +2194,7 @@ class AstComparator implements AstVisitor<bool> {
21932194
return isEqualNodes(
21942195
node.documentationComment, other.documentationComment) &&
21952196
_isEqualNodeLists(node.metadata, other.metadata) &&
2197+
isEqualTokens(node.externalKeyword, other.externalKeyword) &&
21962198
isEqualNodes(node.variables, other.variables) &&
21972199
isEqualTokens(node.semicolon, other.semicolon);
21982200
}
@@ -5266,6 +5268,7 @@ class ResolutionCopier implements AstVisitor<bool> {
52665268
return _and(
52675269
_isEqualNodes(node.documentationComment, toNode.documentationComment),
52685270
_isEqualNodeLists(node.metadata, toNode.metadata),
5271+
_isEqualTokens(node.externalKeyword, toNode.externalKeyword),
52695272
_isEqualNodes(node.variables, toNode.variables),
52705273
_isEqualTokens(node.semicolon, toNode.semicolon));
52715274
}

pkg/analyzer/lib/src/dart/element/element.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7472,6 +7472,14 @@ class TopLevelVariableElementImpl extends PropertyInducingElementImpl
74727472
@override
74737473
TopLevelVariableElement get declaration => this;
74747474

7475+
@override
7476+
bool get isExternal {
7477+
if (linkedNode != null) {
7478+
return enclosingUnit.linkedContext.isExternal(linkedNode);
7479+
}
7480+
return hasModifier(Modifier.EXTERNAL);
7481+
}
7482+
74757483
@override
74767484
bool get isStatic => true;
74777485

pkg/analyzer/lib/src/dart/element/member.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -954,6 +954,9 @@ class TopLevelVariableMember extends VariableMember
954954
return PropertyAccessorMember(baseGetter, _substitution, isLegacy);
955955
}
956956

957+
@override
958+
bool get isExternal => declaration.isExternal;
959+
957960
@override
958961
PropertyAccessorElement get setter {
959962
var baseSetter = declaration.setter;

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2153,7 +2153,7 @@ class AstBuilder extends StackListener {
21532153
assert(optional(';', semicolon));
21542154
debugEvent("TopLevelFields");
21552155

2156-
if (externalToken != null) {
2156+
if (externalToken != null && !enableNonNullable) {
21572157
handleRecoverableError(
21582158
messageExternalField, externalToken, externalToken);
21592159
}
@@ -2169,7 +2169,8 @@ class AstBuilder extends StackListener {
21692169
List<Annotation> metadata = pop();
21702170
Comment comment = _findComment(metadata, beginToken);
21712171
declarations.add(ast.topLevelVariableDeclaration(
2172-
comment, metadata, variableList, semicolon));
2172+
comment, metadata, variableList, semicolon,
2173+
externalKeyword: externalToken));
21732174
}
21742175

21752176
@override

pkg/analyzer/lib/src/generated/testing/ast_test_factory.dart

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,12 +1296,16 @@ class AstTestFactory {
12961296
TokenFactory.tokenFromType(TokenType.SEMICOLON));
12971297

12981298
static TopLevelVariableDeclaration topLevelVariableDeclaration2(
1299-
Keyword keyword, List<VariableDeclaration> variables) =>
1299+
Keyword keyword, List<VariableDeclaration> variables,
1300+
{bool isExternal = false}) =>
13001301
astFactory.topLevelVariableDeclaration(
13011302
null,
13021303
null,
13031304
variableDeclarationList(keyword, null, variables),
1304-
TokenFactory.tokenFromType(TokenType.SEMICOLON));
1305+
TokenFactory.tokenFromType(TokenType.SEMICOLON),
1306+
externalKeyword: isExternal
1307+
? TokenFactory.tokenFromKeyword(Keyword.EXTERNAL)
1308+
: null);
13051309

13061310
static TryStatement tryStatement(Block body, Block finallyClause) =>
13071311
tryStatement3(body, <CatchClause>[], finallyClause);

pkg/analyzer/lib/src/summary2/ast_binary_reader.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1548,6 +1548,8 @@ class AstBinaryReader {
15481548
_readNodeListLazy(data.annotatedNode_metadata),
15491549
_readNode(data.topLevelVariableDeclaration_variableList),
15501550
_Tokens.SEMICOLON,
1551+
externalKeyword:
1552+
AstBinaryFlags.isExternal(data.flags) ? _Tokens.EXTERNAL : null,
15511553
);
15521554
LazyTopLevelVariableDeclaration.setData(node, data);
15531555
return node;

pkg/analyzer/lib/src/summary2/ast_binary_writer.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1328,6 +1328,9 @@ class AstBinaryWriter extends ThrowingAstVisitor<LinkedNodeBuilder> {
13281328
informativeId: getInformativeId(node),
13291329
topLevelVariableDeclaration_variableList: node.variables?.accept(this),
13301330
);
1331+
builder.flags = AstBinaryFlags.encode(
1332+
isExternal: node.externalKeyword != null,
1333+
);
13311334
_storeCompilationUnitMember(builder, node);
13321335

13331336
return builder;

pkg/analyzer/lib/src/summary2/ast_text_printer.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -906,6 +906,7 @@ class AstTextPrinter extends ThrowingAstVisitor<void> {
906906
@override
907907
void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
908908
_compilationUnitMember(node);
909+
_token(node.externalKeyword);
909910
node.variables.accept(this);
910911
_token(node.semicolon);
911912
}

pkg/analyzer/lib/src/summary2/linked_unit_context.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,8 @@ class LinkedUnitContext {
841841
var grandParent = parent.parent;
842842
if (grandParent is FieldDeclaration) {
843843
return grandParent.externalKeyword != null;
844+
} else if (grandParent is TopLevelVariableDeclaration) {
845+
return grandParent.externalKeyword != null;
844846
} else {
845847
throw UnimplementedError('${grandParent.runtimeType}');
846848
}

pkg/analyzer/test/generated/parser_fasta_test.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4621,6 +4621,12 @@ mixin A {
46214621
expectCommentText(declaration.documentationComment, '/// Doc');
46224622
}
46234623

4624+
void test_parseTopLevelVariable_external() {
4625+
var unit = parseCompilationUnit('external int i;', featureSet: nonNullable);
4626+
var declaration = unit.declarations[0] as TopLevelVariableDeclaration;
4627+
expect(declaration.externalKeyword, isNotNull);
4628+
}
4629+
46244630
void test_parseTopLevelVariable_final_late() {
46254631
var unit = parseCompilationUnit('final late a;',
46264632
featureSet: nonNullable,
@@ -4676,6 +4682,12 @@ mixin A {
46764682
expect(declarationList.type, isNotNull);
46774683
expect(declarationList.variables, hasLength(1));
46784684
}
4685+
4686+
void test_parseTopLevelVariable_non_external() {
4687+
var unit = parseCompilationUnit('int i;', featureSet: nonNullable);
4688+
var declaration = unit.declarations[0] as TopLevelVariableDeclaration;
4689+
expect(declaration.externalKeyword, isNull);
4690+
}
46794691
}
46804692

46814693
@reflectiveTest

pkg/analyzer/test/src/dart/ast/to_source_visitor_test.dart

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2616,6 +2616,14 @@ class ToSourceVisitor2Test {
26162616
AstTestFactory.throwExpression2(AstTestFactory.identifier3("e")));
26172617
}
26182618

2619+
void test_visitTopLevelVariableDeclaration_external() {
2620+
_assertSource(
2621+
"external var a;",
2622+
AstTestFactory.topLevelVariableDeclaration2(
2623+
Keyword.VAR, [AstTestFactory.variableDeclaration("a")],
2624+
isExternal: true));
2625+
}
2626+
26192627
void test_visitTopLevelVariableDeclaration_multiple() {
26202628
_assertSource(
26212629
"var a;",

pkg/analyzer/test/src/dart/ast/utilities_test.dart

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,29 @@ class ResolutionCopierTest with ElementsTypesMixin {
132132
@override
133133
final TypeProvider typeProvider = TestTypeProvider();
134134

135+
void test_topLevelVariableDeclaration_external() {
136+
TopLevelVariableDeclaration fromNode =
137+
AstTestFactory.topLevelVariableDeclaration2(
138+
Keyword.VAR, [AstTestFactory.variableDeclaration('x')],
139+
isExternal: false);
140+
TopLevelVariableElement element = TopLevelVariableElementImpl('x', -1);
141+
fromNode.variables.variables[0].name.staticElement = element;
142+
TopLevelVariableDeclaration toNode1 =
143+
AstTestFactory.topLevelVariableDeclaration2(
144+
Keyword.VAR, [AstTestFactory.variableDeclaration('x')],
145+
isExternal: false);
146+
ResolutionCopier.copyResolutionData(fromNode, toNode1);
147+
// Nodes matched so resolution data should have been copied.
148+
expect(toNode1.variables.variables[0].declaredElement, same(element));
149+
TopLevelVariableDeclaration toNode2 =
150+
AstTestFactory.topLevelVariableDeclaration2(
151+
Keyword.VAR, [AstTestFactory.variableDeclaration('x')],
152+
isExternal: true);
153+
ResolutionCopier.copyResolutionData(fromNode, toNode1);
154+
// Nodes didn't match so resolution data should not have been copied.
155+
expect(toNode2.variables.variables[0].declaredElement, isNull);
156+
}
157+
135158
void test_visitAdjacentStrings() {
136159
AdjacentStrings createNode() => astFactory.adjacentStrings([
137160
astFactory.simpleStringLiteral(null, 'hello'),

pkg/analyzer/test/src/summary/element_text.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -971,6 +971,7 @@ class _ElementWriter {
971971
} else {
972972
writeDocumentation(e);
973973
writeMetadata(e, '', '\n');
974+
writeIf(e is TopLevelVariableElementImpl && e.isExternal, 'external ');
974975
}
975976

976977
writeIf(e.isLate, 'late ');

pkg/analyzer/test/src/summary/resolved_ast_printer.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,6 +1269,7 @@ class ResolvedAstPrinter extends ThrowingAstVisitor<void> {
12691269
_writeln('TopLevelVariableDeclaration');
12701270
_withIndent(() {
12711271
var properties = _Properties();
1272+
properties.addToken('externalKeyword', node.externalKeyword);
12721273
properties.addToken('semicolon', node.semicolon);
12731274
properties.addNode('variables', node.variables);
12741275
_addCompilationUnitMember(properties, node);

0 commit comments

Comments
 (0)