Skip to content

Commit b18ae2e

Browse files
mosuemCommit Queue
authored andcommitted
Improve @mustBeConst verifier
Remove use of the constant evaluator, in favor of simpler and faster type checks. Also fixes #55573. Change-Id: I803144c0e3bb1c5e0dcbf86933901f0ae4e2311b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/387721 Commit-Queue: Moritz Sümmermann <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]>
1 parent 681e16a commit b18ae2e

File tree

4 files changed

+82
-14
lines changed

4 files changed

+82
-14
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ import 'package:analyzer/dart/element/element.dart';
9292
/// In addition, this class defines several values that can be returned to
9393
/// indicate various conditions encountered during evaluation. These are
9494
/// documented with the static fields that define those values.
95+
@Deprecated('This has no uses in package:analyzer and not exhaustive.')
9596
class ConstantEvaluator extends GeneralizingAstVisitor<Object> {
9697
/// The value returned for expressions (or non-expression nodes) that are not
9798
/// compile-time constant expressions.

pkg/analyzer/lib/src/error/const_argument_verifier.dart

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@ import 'package:analyzer/dart/ast/ast.dart';
66
import 'package:analyzer/dart/ast/syntactic_entity.dart';
77
import 'package:analyzer/dart/ast/token.dart';
88
import 'package:analyzer/dart/ast/visitor.dart';
9+
import 'package:analyzer/dart/element/element.dart';
910
import 'package:analyzer/dart/element/type.dart';
1011
import 'package:analyzer/error/listener.dart';
11-
import 'package:analyzer/src/dart/ast/utilities.dart';
12+
import 'package:analyzer/src/dart/ast/ast.dart';
1213
import 'package:analyzer/src/dart/element/element.dart';
1314
import 'package:analyzer/src/error/codes.dart';
1415

@@ -17,10 +18,7 @@ import 'package:analyzer/src/error/codes.dart';
1718
class ConstArgumentsVerifier extends SimpleAstVisitor<void> {
1819
final ErrorReporter _errorReporter;
1920

20-
final ConstantEvaluator _constantEvaluator;
21-
22-
ConstArgumentsVerifier(this._errorReporter)
23-
: _constantEvaluator = ConstantEvaluator();
21+
ConstArgumentsVerifier(this._errorReporter);
2422

2523
@override
2624
void visitAssignmentExpression(AssignmentExpression node) {
@@ -114,15 +112,7 @@ class ConstArgumentsVerifier extends SimpleAstVisitor<void> {
114112
} else {
115113
resolvedArgument = argument;
116114
}
117-
if (resolvedArgument is Identifier) {
118-
var staticElement = resolvedArgument.staticElement;
119-
if (staticElement != null &&
120-
staticElement.nonSynthetic is ConstVariableElement) {
121-
return;
122-
}
123-
}
124-
if (resolvedArgument.accept(_constantEvaluator) ==
125-
ConstantEvaluator.NOT_A_CONSTANT) {
115+
if (!_isConst(resolvedArgument)) {
126116
_errorReporter.atNode(
127117
argument,
128118
WarningCode.NON_CONST_ARGUMENT_FOR_CONST_PARAMETER,
@@ -132,4 +122,35 @@ class ConstArgumentsVerifier extends SimpleAstVisitor<void> {
132122
}
133123
}
134124
}
125+
126+
bool _isConst(Expression expression) {
127+
if (expression.inConstantContext) {
128+
return true;
129+
} else if (expression is InstanceCreationExpression && expression.isConst) {
130+
return true;
131+
} else if (expression is Literal) {
132+
return switch (expression) {
133+
BooleanLiteral() => true,
134+
DoubleLiteral() => true,
135+
IntegerLiteral() => true,
136+
NullLiteral() => true,
137+
SimpleStringLiteral() => true,
138+
AdjacentStrings() => true,
139+
SymbolLiteral() => true,
140+
RecordLiteral() => expression.isConst,
141+
TypedLiteral() => expression.isConst,
142+
// TODO(mosum): Expand the logic to check if the individual interpolation elements are const.
143+
StringInterpolation() => false,
144+
};
145+
} else if (expression is Identifier) {
146+
var staticElement = expression.staticElement;
147+
if (staticElement != null) {
148+
if ((staticElement is VariableElement && staticElement.isConst) ||
149+
staticElement.nonSynthetic is ConstVariableElement) {
150+
return true;
151+
}
152+
}
153+
}
154+
return false;
155+
}
135156
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
// ignore_for_file: deprecated_member_use_from_same_package
6+
57
import 'package:analyzer/src/dart/ast/constant_evaluator.dart';
68
import 'package:analyzer/src/test_utilities/find_node.dart';
79
import 'package:test/test.dart';

pkg/analyzer/test/src/diagnostics/non_const_argument_for_const_parameter_test.dart

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,18 @@ class ConstAnnotationTest extends PubPackageResolutionTest {
2121
writeTestPackageConfigWithMeta();
2222
}
2323

24+
test_adjacentLiteral_succeeds() async {
25+
await assertNoErrorsInCode(r'''
26+
import 'package:meta/meta.dart' show mustBeConst;
27+
28+
final c = C('H' 'ello');
29+
30+
class C {
31+
C(@mustBeConst String s);
32+
}
33+
''');
34+
}
35+
2436
test_binaryOperator_fails() async {
2537
await assertErrorsInCode(r'''
2638
import 'package:meta/meta.dart' show mustBeConst;
@@ -55,6 +67,22 @@ class A {
5567
''');
5668
}
5769

70+
test_constExpression_succeeds() async {
71+
await assertNoErrorsInCode(r'''
72+
import 'package:meta/meta.dart' show mustBeConst;
73+
74+
void f() {
75+
g(const C());
76+
}
77+
78+
void g(@mustBeConst C c) {}
79+
80+
class C {
81+
const C();
82+
}
83+
''');
84+
}
85+
5886
test_constructor_constantLiteral_succeeds() async {
5987
await assertNoErrorsInCode(r'''
6088
import 'package:meta/meta.dart' show mustBeConst;
@@ -164,6 +192,22 @@ class A {
164192
]);
165193
}
166194

195+
test_interpolationLiteral_fails() async {
196+
await assertErrorsInCode(r'''
197+
import 'package:meta/meta.dart' show mustBeConst;
198+
199+
const a = 'ello';
200+
201+
final c = C('H$a');
202+
203+
class C {
204+
C(@mustBeConst String s);
205+
}
206+
''', [
207+
error(WarningCode.NON_CONST_ARGUMENT_FOR_CONST_PARAMETER, 82, 5),
208+
]);
209+
}
210+
167211
test_localFunction_constantLiteral_succeeds() async {
168212
await assertNoErrorsInCode(r'''
169213
import 'package:meta/meta.dart' show mustBeConst;

0 commit comments

Comments
 (0)