Skip to content

Commit 0b94228

Browse files
stereotype441commit-bot@chromium.org
stereotype441
authored and
[email protected]
committed
Enforce that all ErrorReporter arguments come from a limited set of types.
This change ensures that the only types ever passed to ErrorReporter arguments are String, DartType, int, Uri, or Element. A few call sites had to be changed so that instead of passing in AST nodes they passed in the result of calling `toString` on those AST nodes. In future CLs, I plan to move toward a system where instead of passing the message arguments to ErrorReporter as a list of `Object`, we call a `withArguments` method that's specific to each error; that method will have a specific list of argument types that it supports, so that if we accidentally pass the wrong things into an error message, the problem will be detected at compile time (instead of the user just seeing a confusing error message). The `withArguments` methods will be code generated from annotations in the analyzer's `messages.yaml` file. Keeping the set of types supported by those `withArguments` messages small should help simplify the code generation process, and provide flexibility for how errors are represented in the future. Change-Id: I4a793db2fe5b1344199b770898634b61be41ad11 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/216301 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Daco Harkes <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
1 parent b335f28 commit 0b94228

23 files changed

+241
-52
lines changed

‎pkg/analyzer/lib/error/listener.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,12 @@ class ErrorReporter {
165165
arguments[i] = argument.getDisplayString(
166166
withNullability: isNonNullableByDefault,
167167
);
168+
} else if (!(argument is String ||
169+
argument is DartType ||
170+
argument is int ||
171+
argument is Uri)) {
172+
throw ArgumentError(
173+
'Tried to format an error using ${argument.runtimeType}');
168174
}
169175
}
170176
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,10 @@ class FunctionReferenceResolver {
6464
_errorReporter.reportErrorForNode(
6565
CompileTimeErrorCode.WRONG_NUMBER_OF_TYPE_ARGUMENTS_CONSTRUCTOR,
6666
typeArguments,
67-
[function.constructorName.type2.name, function.constructorName.name!],
67+
[
68+
function.constructorName.type2.name.toSource(),
69+
function.constructorName.name!.name
70+
],
6871
);
6972
_resolve(node: node, rawType: function.staticType);
7073
}

‎pkg/analyzer/lib/src/error/best_practices_verifier.dart

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -482,9 +482,10 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
482482
// always named, so we can safely assume
483483
// `overriddenElement.enclosingElement.name` is non-`null`.
484484
_errorReporter.reportErrorForNode(
485-
HintCode.INVALID_OVERRIDE_OF_NON_VIRTUAL_MEMBER,
486-
field.name,
487-
[field.name, overriddenElement.enclosingElement.name!]);
485+
HintCode.INVALID_OVERRIDE_OF_NON_VIRTUAL_MEMBER, field.name, [
486+
field.name.name,
487+
overriddenElement.enclosingElement.displayName
488+
]);
488489
}
489490
if (!_invalidAccessVerifier._inTestDirectory) {
490491
_checkForAssignmentOfDoNotStore(field.initializer);
@@ -671,7 +672,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<void> {
671672
_errorReporter.reportErrorForNode(
672673
HintCode.INVALID_OVERRIDE_OF_NON_VIRTUAL_MEMBER,
673674
node.name,
674-
[node.name, overriddenElement.enclosingElement.name!]);
675+
[node.name.name, overriddenElement.enclosingElement.displayName]);
675676
}
676677

677678
super.visitMethodDeclaration(node);

‎pkg/analyzer/lib/src/error/type_arguments_verifier.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,8 @@ class TypeArgumentsVerifier {
499499
NodeList<TypeAnnotation> arguments, ErrorCode errorCode) {
500500
for (TypeAnnotation type in arguments) {
501501
if (type is NamedType && type.type is TypeParameterType) {
502-
_errorReporter.reportErrorForNode(errorCode, type, [type.name]);
502+
_errorReporter
503+
.reportErrorForNode(errorCode, type, [type.name.toSource()]);
503504
}
504505
}
505506
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ class ElementResolver extends SimpleAstVisitor<void> {
438438
_errorReporter.reportErrorForNode(
439439
CompileTimeErrorCode.UNDEFINED_CONSTRUCTOR_IN_INITIALIZER,
440440
node,
441-
[superType, name]);
441+
[superType, name.name]);
442442
} else {
443443
_errorReporter.reportErrorForNode(
444444
CompileTimeErrorCode.UNDEFINED_CONSTRUCTOR_IN_INITIALIZER_DEFAULT,

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

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
795795
if (parameterType is FunctionType &&
796796
parameterType.returnType.isDynamic) {
797797
errorReporter.reportErrorForNode(LanguageCode.IMPLICIT_DYNAMIC_RETURN,
798-
node.identifier, [node.identifier]);
798+
node.identifier, [node.identifier.name]);
799799
}
800800
}
801801

@@ -2089,12 +2089,12 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
20892089
errorReporter.reportErrorForNode(
20902090
CompileTimeErrorCode.CONST_WITH_UNDEFINED_CONSTRUCTOR,
20912091
name,
2092-
[className, name]);
2092+
[className.toSource(), name.name]);
20932093
} else {
20942094
errorReporter.reportErrorForNode(
20952095
CompileTimeErrorCode.CONST_WITH_UNDEFINED_CONSTRUCTOR_DEFAULT,
20962096
constructorName,
2097-
[className]);
2097+
[className.toSource()]);
20982098
}
20992099
}
21002100

@@ -2593,7 +2593,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
25932593
}
25942594
// Parameters associated with a variable always have a name, so we can
25952595
// safely rely on [id] being non-`null`.
2596-
errorReporter.reportErrorForNode(errorCode, node, [id!]);
2596+
errorReporter.reportErrorForNode(errorCode, node, [id!.toSource()]);
25972597
}
25982598
}
25992599

@@ -2739,18 +2739,18 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
27392739
errorReporter.reportErrorForNode(
27402740
CompileTimeErrorCode.INITIALIZER_FOR_NON_EXISTENT_FIELD,
27412741
initializer,
2742-
[fieldName]);
2742+
[fieldName.name]);
27432743
} else if (staticElement.isStatic) {
27442744
errorReporter.reportErrorForNode(
27452745
CompileTimeErrorCode.INITIALIZER_FOR_STATIC_FIELD,
27462746
initializer,
2747-
[fieldName]);
2747+
[fieldName.name]);
27482748
}
27492749
} else {
27502750
errorReporter.reportErrorForNode(
27512751
CompileTimeErrorCode.INITIALIZER_FOR_NON_EXISTENT_FIELD,
27522752
initializer,
2753-
[fieldName]);
2753+
[fieldName.name]);
27542754
return;
27552755
}
27562756
}
@@ -3328,12 +3328,12 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
33283328
errorReporter.reportErrorForNode(
33293329
CompileTimeErrorCode.NEW_WITH_UNDEFINED_CONSTRUCTOR,
33303330
name,
3331-
[className, name]);
3331+
[className.toSource(), name.name]);
33323332
} else {
33333333
errorReporter.reportErrorForNode(
33343334
CompileTimeErrorCode.NEW_WITH_UNDEFINED_CONSTRUCTOR_DEFAULT,
33353335
constructorName,
3336-
[className]);
3336+
[className.toSource()]);
33373337
}
33383338
}
33393339

@@ -3651,8 +3651,9 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
36513651

36523652
if (treatedAsDouble) {
36533653
// Suggest the nearest valid double (as a BigInt for printing reasons).
3654-
extraErrorArgs
3655-
.add(BigInt.from(IntegerLiteralImpl.nearestValidDouble(lexeme)));
3654+
extraErrorArgs.add(
3655+
BigInt.from(IntegerLiteralImpl.nearestValidDouble(lexeme))
3656+
.toString());
36563657
}
36573658

36583659
errorReporter.reportErrorForNode(
@@ -4063,7 +4064,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
40634064
errorReporter.reportErrorForNode(
40644065
CompileTimeErrorCode.TYPE_ANNOTATION_DEFERRED_CLASS,
40654066
type,
4066-
[type.name]);
4067+
[type.name.toSource()]);
40674068
}
40684069
}
40694070

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,11 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
108108
return;
109109
}
110110
if (typename.ffiClass != null) {
111-
_errorReporter.reportErrorForNode(
112-
subtypeOfFfiCode, typename, [node.name, typename.name]);
111+
_errorReporter.reportErrorForNode(subtypeOfFfiCode, typename,
112+
[node.name.name, typename.name.toSource()]);
113113
} else if (typename.isCompoundSubtype) {
114-
_errorReporter.reportErrorForNode(
115-
subtypeOfStructCode, typename, [node.name, typename.name]);
114+
_errorReporter.reportErrorForNode(subtypeOfStructCode, typename,
115+
[node.name.name, typename.name.toSource()]);
116116
}
117117
}
118118

@@ -133,7 +133,7 @@ class FfiVerifier extends RecursiveAstVisitor<void> {
133133

134134
if (inCompound && node.declaredElement!.typeParameters.isNotEmpty) {
135135
_errorReporter.reportErrorForNode(
136-
FfiCode.GENERIC_STRUCT_SUBCLASS, node.name, [node.name]);
136+
FfiCode.GENERIC_STRUCT_SUBCLASS, node.name, [node.name.name]);
137137
}
138138
super.visitClassDeclaration(node);
139139
}

‎pkg/analyzer/test/src/dart/resolution/function_reference_test.dart

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,13 +311,33 @@ class A<T> {
311311
var x = A.foo<int>;
312312
''', [
313313
error(CompileTimeErrorCode.WRONG_NUMBER_OF_TYPE_ARGUMENTS_CONSTRUCTOR, 42,
314-
5),
314+
5,
315+
messageContains: ["'A.foo'"]),
315316
]);
316317

317318
assertFunctionReference(findNode.functionReference('A.foo<int>;'),
318319
findElement.constructor('foo'), 'dynamic');
319320
}
320321

322+
test_constructorReference_prefixed() async {
323+
await assertErrorsInCode('''
324+
import 'dart:async' as a;
325+
var x = a.Future.delayed<int>;
326+
''', [
327+
error(CompileTimeErrorCode.WRONG_NUMBER_OF_TYPE_ARGUMENTS_CONSTRUCTOR, 50,
328+
5,
329+
messageContains: ["'a.Future.delayed'"]),
330+
]);
331+
assertFunctionReference(
332+
findNode.functionReference('a.Future.delayed<int>;'),
333+
findElement
334+
.import('dart:async')
335+
.importedLibrary!
336+
.getType('Future')!
337+
.getNamedConstructor('delayed'),
338+
'dynamic');
339+
}
340+
321341
test_dynamicTyped() async {
322342
await assertErrorsInCode('''
323343
dynamic i = 1;

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

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,23 @@ f() {
2424
return const A.noSuchConstructor();
2525
}
2626
''', [
27-
error(CompileTimeErrorCode.CONST_WITH_UNDEFINED_CONSTRUCTOR, 48, 17),
27+
error(CompileTimeErrorCode.CONST_WITH_UNDEFINED_CONSTRUCTOR, 48, 17,
28+
messageContains: ["class 'A'", "constructor 'noSuchConstructor'"]),
29+
]);
30+
}
31+
32+
test_named_prefixed() async {
33+
await assertErrorsInCode(r'''
34+
import 'dart:async' as a;
35+
f() {
36+
return const a.Future.noSuchConstructor();
37+
}
38+
''', [
39+
error(CompileTimeErrorCode.CONST_WITH_UNDEFINED_CONSTRUCTOR, 56, 17,
40+
messageContains: [
41+
"class 'a.Future'",
42+
"constructor 'noSuchConstructor'"
43+
]),
2844
]);
2945
}
3046

@@ -53,7 +69,26 @@ f() {
5369
}
5470
''', [
5571
error(
56-
CompileTimeErrorCode.CONST_WITH_UNDEFINED_CONSTRUCTOR_DEFAULT, 51, 1),
72+
CompileTimeErrorCode.CONST_WITH_UNDEFINED_CONSTRUCTOR_DEFAULT, 51, 1,
73+
messageContains: ["'A'"]),
74+
]);
75+
}
76+
77+
test_unnamed_prefixed() async {
78+
newFile('$testPackageLibPath/lib1.dart', content: '''
79+
class A {
80+
const A.name();
81+
}
82+
''');
83+
await assertErrorsInCode(r'''
84+
import 'lib1.dart' as lib1;
85+
f() {
86+
return const lib1.A();
87+
}
88+
''', [
89+
error(
90+
CompileTimeErrorCode.CONST_WITH_UNDEFINED_CONSTRUCTOR_DEFAULT, 49, 6,
91+
messageContains: ["'lib1.A'"]),
5792
]);
5893
}
5994
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class S<T> extends Struct {
2222
external Pointer notEmpty;
2323
}
2424
''', [
25-
error(FfiCode.GENERIC_STRUCT_SUBCLASS, 25, 1),
25+
error(FfiCode.GENERIC_STRUCT_SUBCLASS, 25, 1, messageContains: ["'S'"]),
2626
]);
2727
}
2828

@@ -33,7 +33,7 @@ class S<T> extends Union {
3333
external Pointer notEmpty;
3434
}
3535
''', [
36-
error(FfiCode.GENERIC_STRUCT_SUBCLASS, 25, 1),
36+
error(FfiCode.GENERIC_STRUCT_SUBCLASS, 25, 1, messageContains: ["'S'"]),
3737
]);
3838
}
3939

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ class C {
4949
final f = (<dynamic>[])[0];
5050
}
5151
''', [
52-
error(LanguageCode.IMPLICIT_DYNAMIC_FIELD, 18, 20),
52+
error(LanguageCode.IMPLICIT_DYNAMIC_FIELD, 18, 20,
53+
messageContains: ["'f'"]),
5354
]);
5455
}
5556

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,20 @@ class A {
2424
}
2525
A a = const A();
2626
''', [
27-
error(CompileTimeErrorCode.INITIALIZER_FOR_NON_EXISTENT_FIELD, 24, 9),
27+
error(CompileTimeErrorCode.INITIALIZER_FOR_NON_EXISTENT_FIELD, 24, 9,
28+
messageContains: ["'x'"]),
29+
]);
30+
}
31+
32+
test_getter() async {
33+
await assertErrorsInCode('''
34+
class A {
35+
int get x => 0;
36+
A() : x = 0;
37+
}
38+
''', [
39+
error(CompileTimeErrorCode.INITIALIZER_FOR_NON_EXISTENT_FIELD, 36, 5,
40+
messageContains: ["'x'"]),
2841
]);
2942
}
3043

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ class A {
3333
A() : x = 0 {}
3434
}
3535
''', [
36-
error(CompileTimeErrorCode.INITIALIZER_FOR_STATIC_FIELD, 38, 5),
36+
error(CompileTimeErrorCode.INITIALIZER_FOR_STATIC_FIELD, 38, 5,
37+
messageContains: ["'x'"]),
3738
]);
3839
}
3940
}

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ class B extends C {
3535
int g = 0;
3636
}
3737
''', [
38-
error(HintCode.INVALID_OVERRIDE_OF_NON_VIRTUAL_MEMBER, 113, 1),
38+
error(HintCode.INVALID_OVERRIDE_OF_NON_VIRTUAL_MEMBER, 113, 1,
39+
messageContains: ["member 'g'", "in 'C'"]),
3940
]);
4041
}
4142

@@ -70,7 +71,8 @@ class B extends C {
7071
int get g => 0;
7172
}
7273
''', [
73-
error(HintCode.INVALID_OVERRIDE_OF_NON_VIRTUAL_MEMBER, 117, 1),
74+
error(HintCode.INVALID_OVERRIDE_OF_NON_VIRTUAL_MEMBER, 117, 1,
75+
messageContains: ["member 'g'", "in 'C'"]),
7476
]);
7577
}
7678

@@ -228,7 +230,8 @@ class B with M {
228230
void f() {}
229231
}
230232
''', [
231-
error(HintCode.INVALID_OVERRIDE_OF_NON_VIRTUAL_MEMBER, 111, 1),
233+
error(HintCode.INVALID_OVERRIDE_OF_NON_VIRTUAL_MEMBER, 111, 1,
234+
messageContains: ["member 'f'", "in 'M'"]),
232235
]);
233236
}
234237

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ class A<E> {
2323
}
2424
}
2525
''', [
26-
error(CompileTimeErrorCode.INVALID_TYPE_ARGUMENT_IN_CONST_LIST, 39, 1),
26+
error(CompileTimeErrorCode.INVALID_TYPE_ARGUMENT_IN_CONST_LIST, 39, 1,
27+
messageContains: ["'E'"]),
2728
]);
2829
}
2930

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ class A<E> {
2323
}
2424
}
2525
''', [
26-
error(CompileTimeErrorCode.INVALID_TYPE_ARGUMENT_IN_CONST_MAP, 39, 1),
26+
error(CompileTimeErrorCode.INVALID_TYPE_ARGUMENT_IN_CONST_MAP, 39, 1,
27+
messageContains: ["'E'"]),
2728
]);
2829
}
2930

@@ -45,7 +46,8 @@ class A<E> {
4546
}
4647
}
4748
''', [
48-
error(CompileTimeErrorCode.INVALID_TYPE_ARGUMENT_IN_CONST_MAP, 47, 1),
49+
error(CompileTimeErrorCode.INVALID_TYPE_ARGUMENT_IN_CONST_MAP, 47, 1,
50+
messageContains: ["'E'"]),
4951
]);
5052
}
5153
}

0 commit comments

Comments
 (0)