Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit f5a3bce

Browse files
stereotype441commit-bot@chromium.org
stereotype441
authored and
commit-bot@chromium.org
committed
Fix scope resolution of metadata on type parameters
Previously, when encountering identifiers in metadata on a class's type parameter, the analyzer would resolve them using the type parameter scope, but then fall back on using implicit `this`. The CFE would resolve them using the class body scope. In both cases, the end result was that the annotation could refer to static class members. This change brings the behavior of both the analyzer and the CFE in line with the spec, by preventing the use of implicit `this` in these annotations, and resolving them in the type parameter scope. This is not expected to break any code in practice, because annotations on type parameters are rare, as are annotations referring to static class members, and the overlap between these two should be negligibly small. Fixes dart-lang/language#1790. Bug: dart-lang/language#1790 Change-Id: Ibe5a421e04a53d29074a8b1509e1390658ed72e5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/210040 Commit-Queue: Paul Berry <paulberry@google.com> Reviewed-by: Johnni Winther <johnniwinther@google.com> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
1 parent c9d954e commit f5a3bce

17 files changed

+388
-112
lines changed

‎CHANGELOG.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,26 @@
1+
## 2.15.0
2+
3+
### Language
4+
5+
- Annotations on type parameters of classes can no longer refer to class members
6+
without a prefix. For example, this used to be permitted:
7+
8+
```dart
9+
class C<@Annotation(foo) T> {
10+
static void foo() {}
11+
}
12+
```
13+
14+
Now, the reference must be qualified with the class name, i.e.:
15+
16+
```dart
17+
class C<@Annotation(C.foo) T> {
18+
static void foo() {}
19+
}
20+
```
21+
22+
This brings the implementation behavior in line with the spec.
23+
124
## 2.14.0
225

326
### Language

‎pkg/analyzer/lib/src/generated/resolver.dart

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2007,17 +2007,6 @@ class ResolverVisitor extends ResolverBase with ErrorDetectionHelpers {
20072007
@override
20082008
void visitTypeName(TypeName node) {}
20092009

2010-
@override
2011-
void visitTypeParameter(TypeParameter node) {
2012-
var previousThisType = _thisType;
2013-
try {
2014-
_setupThisType();
2015-
super.visitTypeParameter(node);
2016-
} finally {
2017-
_thisType = previousThisType;
2018-
}
2019-
}
2020-
20212010
@override
20222011
void visitVariableDeclaration(VariableDeclaration node) {
20232012
_variableDeclarationResolver.resolve(node as VariableDeclarationImpl);

‎pkg/analyzer/test/src/diagnostics/undefined_identifier_test.dart

Lines changed: 82 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -43,61 +43,6 @@ void f(int value) {
4343
}
4444

4545
mixin UndefinedIdentifierTestCases on PubPackageResolutionTest {
46-
test_annotation_favors_scope_resolution_over_this_resolution_class() async {
47-
// If an annotation on a class type parameter cannot be resolved using the
48-
// normal scope resolution mechanism, it is resolved via implicit `this`.
49-
// Note: this behavior doesn't match the spec so we may change it - see
50-
// https://github.com/dart-lang/language/issues/1790
51-
await assertNoErrorsInCode('''
52-
class C<@Annotation.function(foo) @Annotation.type(B) T> {
53-
static void foo() {}
54-
static void B() {}
55-
}
56-
class B {}
57-
class Annotation {
58-
const Annotation.function(void Function() f);
59-
const Annotation.type(Type t);
60-
}
61-
''');
62-
}
63-
64-
test_annotation_favors_scope_resolution_over_this_resolution_extension() async {
65-
// If an annotation on an extension type parameter cannot be resolved using
66-
// the normal scope resolution mechanism, it is resolved via implicit
67-
// `this`. Note: this behavior doesn't match the spec so we may change it -
68-
// see https://github.com/dart-lang/language/issues/1790
69-
await assertNoErrorsInCode('''
70-
extension E<@Annotation.function(foo) @Annotation.type(B) T> on C {}
71-
class C {
72-
static void foo() {}
73-
static void B() {}
74-
}
75-
class B {}
76-
class Annotation {
77-
const Annotation.function(void Function() f);
78-
const Annotation.type(Type t);
79-
}
80-
''');
81-
}
82-
83-
test_annotation_favors_scope_resolution_over_this_resolution_mixin() async {
84-
// If an annotation on a mixin type parameter cannot be resolved using the
85-
// normal scope resolution mechanism, it is resolved via implicit `this`.
86-
// Note: this behavior doesn't match the spec so we may change it - see
87-
// https://github.com/dart-lang/language/issues/1790
88-
await assertNoErrorsInCode('''
89-
mixin M<@Annotation.function(foo) @Annotation.type(B) T> {
90-
static void foo() {}
91-
static void B() {}
92-
}
93-
class B {}
94-
class Annotation {
95-
const Annotation.function(void Function() f);
96-
const Annotation.type(Type t);
97-
}
98-
''');
99-
}
100-
10146
test_annotation_references_static_method_in_class() async {
10247
await assertErrorsInCode('''
10348
@Annotation(foo)
@@ -114,18 +59,19 @@ class Annotation {
11459
}
11560

11661
test_annotation_references_static_method_in_class_from_type_parameter() async {
117-
// It is allowed for an annotation of a class type parameter to refer to
118-
// a method in a class (note: this doesn't match the spec but we currently
119-
// test it to make sure we match CFE behavior - see
120-
// https://github.com/dart-lang/language/issues/1790)
121-
await assertNoErrorsInCode('''
62+
// It not is allowed for an annotation of a class type parameter to refer to
63+
// a method in a class.
64+
await assertErrorsInCode('''
12265
class C<@Annotation(foo) T> {
12366
static void foo() {}
12467
}
12568
class Annotation {
12669
const Annotation(dynamic d);
12770
}
128-
''');
71+
''', [
72+
error(CompileTimeErrorCode.UNDEFINED_IDENTIFIER, 20, 3),
73+
error(CompileTimeErrorCode.CONST_WITH_NON_CONSTANT_ARGUMENT, 20, 3),
74+
]);
12975
}
13076

13177
test_annotation_references_static_method_in_extension() async {
@@ -144,18 +90,19 @@ class Annotation {
14490
}
14591

14692
test_annotation_references_static_method_in_extension_from_type_parameter() async {
147-
// It is allowed for an annotation of a mixin type parameter to refer to
148-
// a method in a class (note: this doesn't match the spec but we currently
149-
// test it to make sure we match CFE behavior - see
150-
// https://github.com/dart-lang/language/issues/1790)
151-
await assertNoErrorsInCode('''
93+
// It is not allowed for an annotation of an extension type parameter to
94+
// refer to a method in a class.
95+
await assertErrorsInCode('''
15296
extension E<@Annotation(foo) T> on T {
15397
static void foo() {}
15498
}
15599
class Annotation {
156100
const Annotation(dynamic d);
157101
}
158-
''');
102+
''', [
103+
error(CompileTimeErrorCode.CONST_WITH_NON_CONSTANT_ARGUMENT, 24, 3),
104+
error(CompileTimeErrorCode.UNDEFINED_IDENTIFIER, 24, 3),
105+
]);
159106
}
160107

161108
test_annotation_references_static_method_in_mixin() async {
@@ -174,18 +121,80 @@ class Annotation {
174121
}
175122

176123
test_annotation_references_static_method_in_mixin_from_type_parameter() async {
177-
// It is allowed for an annotation of a mixin type parameter to refer to
178-
// a method in a class (note: this doesn't match the spec but we currently
179-
// test it to make sure we match CFE behavior - see
180-
// https://github.com/dart-lang/language/issues/1790)
181-
await assertNoErrorsInCode('''
124+
// It is not allowed for an annotation of a mixin type parameter to refer to
125+
// a method in a class.
126+
await assertErrorsInCode('''
182127
mixin M<@Annotation(foo) T> {
183128
static void foo() {}
184129
}
185130
class Annotation {
186131
const Annotation(dynamic d);
187132
}
188-
''');
133+
''', [
134+
error(CompileTimeErrorCode.UNDEFINED_IDENTIFIER, 20, 3),
135+
error(CompileTimeErrorCode.CONST_WITH_NON_CONSTANT_ARGUMENT, 20, 3),
136+
]);
137+
}
138+
139+
test_annotation_uses_scope_resolution_class() async {
140+
// If an annotation on a class type parameter cannot be resolved using the
141+
// normal scope resolution mechanism, it is not resolved via implicit
142+
// `this`.
143+
await assertErrorsInCode('''
144+
class C<@Annotation.function(foo) @Annotation.type(B) T> {
145+
static void foo() {}
146+
static void B() {}
147+
}
148+
class B {}
149+
class Annotation {
150+
const Annotation.function(void Function() f);
151+
const Annotation.type(Type t);
152+
}
153+
''', [
154+
error(CompileTimeErrorCode.UNDEFINED_IDENTIFIER, 29, 3),
155+
error(CompileTimeErrorCode.CONST_WITH_NON_CONSTANT_ARGUMENT, 29, 3),
156+
]);
157+
}
158+
159+
test_annotation_uses_scope_resolution_extension() async {
160+
// If an annotation on an extension type parameter cannot be resolved using
161+
// the normal scope resolution mechanism, it is not resolved via implicit
162+
// `this`.
163+
await assertErrorsInCode('''
164+
extension E<@Annotation.function(foo) @Annotation.type(B) T> on C {}
165+
class C {
166+
static void foo() {}
167+
static void B() {}
168+
}
169+
class B {}
170+
class Annotation {
171+
const Annotation.function(void Function() f);
172+
const Annotation.type(Type t);
173+
}
174+
''', [
175+
error(CompileTimeErrorCode.CONST_WITH_NON_CONSTANT_ARGUMENT, 33, 3),
176+
error(CompileTimeErrorCode.UNDEFINED_IDENTIFIER, 33, 3),
177+
]);
178+
}
179+
180+
test_annotation_uses_scope_resolution_mixin() async {
181+
// If an annotation on a mixin type parameter cannot be resolved using the
182+
// normal scope resolution mechanism, it is not resolved via implicit
183+
// `this`.
184+
await assertErrorsInCode('''
185+
mixin M<@Annotation.function(foo) @Annotation.type(B) T> {
186+
static void foo() {}
187+
static void B() {}
188+
}
189+
class B {}
190+
class Annotation {
191+
const Annotation.function(void Function() f);
192+
const Annotation.type(Type t);
193+
}
194+
''', [
195+
error(CompileTimeErrorCode.UNDEFINED_IDENTIFIER, 29, 3),
196+
error(CompileTimeErrorCode.CONST_WITH_NON_CONSTANT_ARGUMENT, 29, 3),
197+
]);
189198
}
190199

191200
@failingTest

‎pkg/front_end/lib/src/fasta/builder/class_builder.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -354,12 +354,12 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
354354
delayedActionPerformers, synthesizedFunctionNodes);
355355
}
356356

357-
MetadataBuilder.buildAnnotations(
358-
isPatch ? origin.cls : cls, metadata, library, this, null, fileUri);
357+
MetadataBuilder.buildAnnotations(isPatch ? origin.cls : cls, metadata,
358+
library, this, null, fileUri, library.scope);
359359
if (typeVariables != null) {
360360
for (int i = 0; i < typeVariables!.length; i++) {
361-
typeVariables![i].buildOutlineExpressions(
362-
library, this, null, coreTypes, delayedActionPerformers);
361+
typeVariables![i].buildOutlineExpressions(library, this, null,
362+
coreTypes, delayedActionPerformers, scope.parent!);
363363
}
364364
}
365365

‎pkg/front_end/lib/src/fasta/builder/field_builder.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,8 +400,8 @@ class SourceFieldBuilder extends MemberBuilderImpl implements FieldBuilder {
400400
_fieldEncoding.completeSignature(coreTypes);
401401

402402
for (Annotatable annotatable in _fieldEncoding.annotatables) {
403-
MetadataBuilder.buildAnnotations(
404-
annotatable, metadata, library, classBuilder, this, fileUri);
403+
MetadataBuilder.buildAnnotations(annotatable, metadata, library,
404+
classBuilder, this, fileUri, classBuilder?.scope ?? library.scope);
405405
}
406406

407407
// For modular compilation we need to include initializers of all const

‎pkg/front_end/lib/src/fasta/builder/function_builder.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -486,16 +486,18 @@ abstract class FunctionBuilderImpl extends MemberBuilderImpl
486486
isClassMember || isExtensionMember
487487
? parent as DeclarationBuilder
488488
: null;
489-
MetadataBuilder.buildAnnotations(
490-
member, metadata, library, classOrExtensionBuilder, this, fileUri);
489+
Scope parentScope = classOrExtensionBuilder?.scope ?? library.scope;
490+
MetadataBuilder.buildAnnotations(member, metadata, library,
491+
classOrExtensionBuilder, this, fileUri, parentScope);
491492
if (typeVariables != null) {
492493
for (int i = 0; i < typeVariables!.length; i++) {
493494
typeVariables![i].buildOutlineExpressions(
494495
library,
495496
classOrExtensionBuilder,
496497
this,
497498
coreTypes,
498-
delayedActionPerformers);
499+
delayedActionPerformers,
500+
computeTypeParameterScope(parentScope));
499501
}
500502
}
501503

‎pkg/front_end/lib/src/fasta/builder/metadata_builder.dart

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,8 @@ class MetadataBuilder {
3030
SourceLibraryBuilder library,
3131
DeclarationBuilder? classOrExtensionBuilder,
3232
MemberBuilder? member,
33-
Uri fileUri) {
33+
Uri fileUri, Scope scope) {
3434
if (metadata == null) return;
35-
Scope scope = parent is Library ||
36-
parent is Class ||
37-
parent is Extension ||
38-
classOrExtensionBuilder == null
39-
? library.scope
40-
: classOrExtensionBuilder.scope;
4135
BodyBuilder bodyBuilder = library.loader
4236
.createBodyBuilderForOutlineExpression(library, classOrExtensionBuilder,
4337
member ?? classOrExtensionBuilder ?? library, scope, fileUri);

‎pkg/front_end/lib/src/fasta/builder/type_variable_builder.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import '../fasta_codes.dart'
1313
templateInternalProblemUnfinishedTypeVariable,
1414
templateTypeArgumentsOnTypeVariable;
1515

16+
import '../scope.dart';
1617
import '../source/source_library_builder.dart';
1718
import '../util/helpers.dart';
1819

@@ -200,9 +201,9 @@ class TypeVariableBuilder extends TypeDeclarationBuilderImpl {
200201
DeclarationBuilder? classOrExtensionBuilder,
201202
MemberBuilder? memberBuilder,
202203
CoreTypes coreTypes,
203-
List<DelayedActionPerformer> delayedActionPerformers) {
204+
List<DelayedActionPerformer> delayedActionPerformers, Scope scope) {
204205
MetadataBuilder.buildAnnotations(parameter, metadata, libraryBuilder,
205-
classOrExtensionBuilder, memberBuilder, fileUri!);
206+
classOrExtensionBuilder, memberBuilder, fileUri!, scope);
206207
}
207208

208209
@override

‎pkg/front_end/lib/src/fasta/source/source_extension_builder.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -278,11 +278,11 @@ class SourceExtensionBuilder extends ExtensionBuilderImpl {
278278
List<DelayedActionPerformer> delayedActionPerformers,
279279
List<SynthesizedFunctionNode> synthesizedFunctionNodes) {
280280
MetadataBuilder.buildAnnotations(isPatch ? origin.extension : extension,
281-
metadata, library, this, null, fileUri);
281+
metadata, library, this, null, fileUri, library.scope);
282282
if (typeParameters != null) {
283283
for (int i = 0; i < typeParameters!.length; i++) {
284-
typeParameters![i].buildOutlineExpressions(
285-
library, this, null, coreTypes, delayedActionPerformers);
284+
typeParameters![i].buildOutlineExpressions(library, this, null,
285+
coreTypes, delayedActionPerformers, scope.parent!);
286286
}
287287
}
288288

‎pkg/front_end/lib/src/fasta/source/source_library_builder.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2648,7 +2648,7 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
26482648
@override
26492649
void buildOutlineExpressions() {
26502650
MetadataBuilder.buildAnnotations(
2651-
library, metadata, this, null, null, fileUri);
2651+
library, metadata, this, null, null, fileUri, scope);
26522652
}
26532653

26542654
/// Builds the core AST structures for [declaration] needed for the outline.

0 commit comments

Comments
 (0)