Skip to content

Commit 56dbe29

Browse files
srawlinscommit-bot@chromium.org
authored andcommitted
Analyzer: Do not report const constructors with mixed in synthetic fields
Fixes #37810 Also report the offending fields in the error. Change-Id: I8328bf5f16f56f1b70bbe4bdb7623ade6f11943c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151027 Commit-Queue: Samuel Rawlins <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent fa68d27 commit 56dbe29

File tree

4 files changed

+102
-27
lines changed

4 files changed

+102
-27
lines changed

pkg/analyzer/lib/error/error.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ const List<ErrorCode> errorCodeValues = [
100100
CompileTimeErrorCode.CONST_CONSTRUCTOR_THROWS_EXCEPTION,
101101
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_FIELD_INITIALIZED_BY_NON_CONST,
102102
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD,
103+
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELDS,
103104
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_CONST_SUPER,
104105
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_FINAL_FIELD,
105106
CompileTimeErrorCode.CONST_DEFERRED_CLASS,

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

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -960,15 +960,43 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
960960
* implicitly declared constructor named ... is declared. If Sq is a
961961
* generative const constructor, and M does not declare any fields, Cq is
962962
* also a const constructor.
963+
*
964+
* Parameters:
965+
* 0: the name of the instance field.
963966
*/
964967
static const CompileTimeErrorCode CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD =
965-
CompileTimeErrorCode(
968+
CompileTimeErrorCodeWithUniqueName(
969+
'CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD',
970+
'CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD',
971+
"This constructor can't be declared 'const' because a mixin adds the "
972+
"instance field: {0}.",
973+
correction: "Try removing the 'const' keyword or removing the 'with' "
974+
"clause from the class declaration, or removing the field from "
975+
"the mixin class.");
976+
977+
/**
978+
* 7.6.3 Constant Constructors: The superinitializer that appears, explicitly
979+
* or implicitly, in the initializer list of a constant constructor must
980+
* specify a constant constructor of the superclass of the immediately
981+
* enclosing class or a compile-time error occurs.
982+
*
983+
* 12.1 Mixin Application: For each generative constructor named ... an
984+
* implicitly declared constructor named ... is declared. If Sq is a
985+
* generative const constructor, and M does not declare any fields, Cq is
986+
* also a const constructor.
987+
*
988+
* Parameters:
989+
* 0: the names of the instance fields.
990+
*/
991+
static const CompileTimeErrorCode CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELDS =
992+
CompileTimeErrorCodeWithUniqueName(
966993
'CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD',
967-
"Const constructor can't be declared for a class with a mixin "
968-
"that declares an instance field.",
969-
correction: "Try removing the 'const' keyword or "
970-
"removing the 'with' clause from the class declaration, "
971-
"or removing fields from the mixin class.");
994+
'CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELDS',
995+
"This constructor can't be declared 'const' because the mixins add "
996+
"the instance fields: {0}.",
997+
correction: "Try removing the 'const' keyword or removing the 'with' "
998+
"clause from the class declaration, or removing the fields from "
999+
"the mixin classes.");
9721000

9731001
/**
9741002
* 7.6.3 Constant Constructors: The superinitializer that appears, explicitly

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

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1903,12 +1903,12 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
19031903
}
19041904
}
19051905

1906-
/**
1907-
* Verify that if the given [constructor] declaration is 'const' then there
1908-
* are no invocations of non-'const' super constructors.
1909-
*
1910-
* See [CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_CONST_SUPER].
1911-
*/
1906+
/// Verify that if the given [constructor] declaration is 'const' then there
1907+
/// are no invocations of non-'const' super constructors, and that there are
1908+
/// no instance variables mixed in.
1909+
///
1910+
/// See [CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_CONST_SUPER], and
1911+
/// [CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD].
19121912
void _checkForConstConstructorWithNonConstSuper(
19131913
ConstructorDeclaration constructor) {
19141914
if (!_enclosingExecutable.isConstConstructor) {
@@ -1920,21 +1920,26 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
19201920
}
19211921

19221922
// check for mixins
1923-
var hasInstanceField = false;
1923+
var instanceFields = <FieldElement>[];
19241924
for (var mixin in _enclosingClass.mixins) {
1925-
var fields = mixin.element.fields;
1926-
for (var i = 0; i < fields.length; ++i) {
1927-
if (!fields[i].isStatic) {
1928-
hasInstanceField = true;
1929-
break;
1930-
}
1931-
}
1925+
instanceFields.addAll(mixin.element.fields
1926+
.where((field) => !field.isStatic && !field.isSynthetic));
19321927
}
1933-
if (hasInstanceField) {
1934-
// TODO(scheglov) Provide the list of fields.
1928+
if (instanceFields.length == 1) {
1929+
var field = instanceFields.single;
19351930
_errorReporter.reportErrorForNode(
19361931
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD,
1937-
constructor.returnType);
1932+
constructor.returnType,
1933+
["'${field.enclosingElement.name}.${field.name}'"]);
1934+
return;
1935+
} else if (instanceFields.length > 1) {
1936+
var fieldNames = instanceFields
1937+
.map((field) => "'${field.enclosingElement.name}.${field.name}'")
1938+
.join(', ');
1939+
_errorReporter.reportErrorForNode(
1940+
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELDS,
1941+
constructor.returnType,
1942+
[fieldNames]);
19381943
return;
19391944
}
19401945

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

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,46 @@ main() {
1515

1616
@reflectiveTest
1717
class ConstConstructorWithMixinWithFieldTest extends DriverResolutionTest {
18-
test_class_instance() async {
18+
test_class_instance_final() async {
19+
await assertErrorsInCode('''
20+
class A {
21+
final a = 0;
22+
}
23+
24+
class B extends Object with A {
25+
const B();
26+
}
27+
''', [
28+
error(
29+
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD, 68, 1),
30+
]);
31+
}
32+
33+
test_class_instance_getter() async {
34+
await assertNoErrorsInCode('''
35+
class A {
36+
int get a => 7;
37+
}
38+
39+
class B extends Object with A {
40+
const B();
41+
}
42+
''');
43+
}
44+
45+
test_class_instance_setter() async {
46+
await assertNoErrorsInCode('''
47+
class A {
48+
set a(int x) {}
49+
}
50+
51+
class B extends Object with A {
52+
const B();
53+
}
54+
''');
55+
}
56+
57+
test_class_instanceField() async {
1958
await assertErrorsInCode('''
2059
class A {
2160
var a;
@@ -31,18 +70,20 @@ class B extends Object with A {
3170
]);
3271
}
3372

34-
test_class_instance_final() async {
73+
test_class_multipleInstanceFields() async {
3574
await assertErrorsInCode('''
3675
class A {
37-
final a = 0;
76+
var a;
77+
var b;
3878
}
3979
4080
class B extends Object with A {
4181
const B();
4282
}
4383
''', [
84+
error(CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_FINAL_FIELD, 71, 1),
4485
error(
45-
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD, 68, 1),
86+
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELDS, 71, 1),
4687
]);
4788
}
4889

0 commit comments

Comments
 (0)