Do not report const ctor with a mixed in abstract final field

Additionally, reduce duplicated errors regarding invalid const constructors.

Fixes https://github.com/dart-lang/sdk/issues/46641
Change-Id: Ica397ad6e28a05bc5340d5ceca236fbedf1e95f6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/207621
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Sam Rawlins
2021-07-21 17:56:08 +00:00
committed by commit-bot@chromium.org
parent c88193538d
commit ea5333a23f
5 changed files with 64 additions and 54 deletions
@@ -518,8 +518,9 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
_withEnclosingExecutable(element, () {
_checkForInvalidModifierOnBody(
node.body, CompileTimeErrorCode.INVALID_MODIFIER_ON_CONSTRUCTOR);
_checkForConstConstructorWithNonFinalField(node, element);
_checkForConstConstructorWithNonConstSuper(node);
if (!_checkForConstConstructorWithNonConstSuper(node)) {
_checkForConstConstructorWithNonFinalField(node, element);
}
_constructorFieldsVerifier.verify(node);
_checkForRedirectingConstructorErrorCodes(node);
_checkForMultipleSuperInitializers(node);
@@ -1851,23 +1852,40 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
/// are no invocations of non-'const' super constructors, and that there are
/// no instance variables mixed in.
///
/// Return `true` if an error is reported here, and the caller should stop
/// checking the constructor for constant-related errors.
///
/// See [CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_CONST_SUPER], and
/// [CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD].
void _checkForConstConstructorWithNonConstSuper(
bool _checkForConstConstructorWithNonConstSuper(
ConstructorDeclaration constructor) {
if (!_enclosingExecutable.isConstConstructor) {
return;
return false;
}
// OK, const factory, checked elsewhere
if (constructor.factoryKeyword != null) {
return;
return false;
}
// check for mixins
var instanceFields = <FieldElement>[];
for (var mixin in _enclosingClass!.mixins) {
instanceFields.addAll(mixin.element.fields
.where((field) => !field.isStatic && !field.isSynthetic));
instanceFields.addAll(mixin.element.fields.where((field) {
if (field.isStatic) {
return false;
}
if (field.isSynthetic) {
return false;
}
// From the abstract and external fields specification:
// > An abstract instance variable declaration D is treated as an
// > abstract getter declaration and possibly an abstract setter
// > declaration. The setter is included if and only if D is non-final.
if (field.isAbstract && field.isFinal) {
return false;
}
return true;
}));
}
if (instanceFields.length == 1) {
var field = instanceFields.single;
@@ -1875,7 +1893,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD,
constructor.returnType,
["'${field.enclosingElement.name}.${field.name}'"]);
return;
return true;
} else if (instanceFields.length > 1) {
var fieldNames = instanceFields
.map((field) => "'${field.enclosingElement.name}.${field.name}'")
@@ -1884,7 +1902,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELDS,
constructor.returnType,
[fieldNames]);
return;
return true;
}
// try to find and check super constructor invocation
@@ -1892,26 +1910,26 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
if (initializer is SuperConstructorInvocation) {
var element = initializer.staticElement;
if (element == null || element.isConst) {
return;
return false;
}
errorReporter.reportErrorForNode(
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_CONST_SUPER,
initializer,
[element.enclosingElement.displayName]);
return;
return true;
}
}
// no explicit super constructor invocation, check default constructor
var supertype = _enclosingClass!.supertype;
if (supertype == null) {
return;
return false;
}
if (supertype.isDartCoreObject) {
return;
return false;
}
var unnamedConstructor = supertype.element.unnamedConstructor;
if (unnamedConstructor == null || unnamedConstructor.isConst) {
return;
return false;
}
// default constructor is not 'const', report problem
@@ -1919,6 +1937,7 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_CONST_SUPER,
constructor.returnType,
[supertype]);
return true;
}
/// Verify that if the given [constructor] declaration is 'const' then there
@@ -1935,10 +1954,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
if (!classElement.hasNonFinalField) {
return;
}
// TODO(brianwilkerson) Stop generating
// CONST_CONSTRUCTOR_WITH_NON_FINAL_FIELD when either
// CONST_CONSTRUCTOR_WITH_NON_CONST_SUPER or
// CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD is also generated.
errorReporter.reportErrorForName(
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_FINAL_FIELD,
constructor);
@@ -15,6 +15,37 @@ main() {
@reflectiveTest
class ConstConstructorWithMixinWithFieldTest extends PubPackageResolutionTest {
test_class_instance_abstract() async {
await assertErrorsInCode('''
mixin A {
abstract int a;
}
class B with A {
@override
int a;
const B(this.a);
}
''', [
error(
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD, 77, 1),
]);
}
test_class_instance_abstract_final() async {
await assertNoErrorsInCode('''
mixin A {
abstract final int a;
}
class B with A {
@override
final int a;
const B(this.a);
}
''');
}
test_class_instance_final() async {
await assertErrorsInCode('''
class A {
@@ -64,7 +95,6 @@ class B extends Object with A {
const B();
}
''', [
error(CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_FINAL_FIELD, 62, 1),
error(
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD, 62, 1),
]);
@@ -81,7 +111,6 @@ class B extends Object with A {
const B();
}
''', [
error(CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_FINAL_FIELD, 71, 1),
error(
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELDS, 71, 1),
]);
@@ -119,7 +148,6 @@ class X extends Object with M {
const X();
}
''', [
error(CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_FINAL_FIELD, 62, 1),
error(
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD, 62, 1),
]);
@@ -15,35 +15,6 @@ main() {
@reflectiveTest
class ConstConstructorWithNonFinalFieldTest extends PubPackageResolutionTest {
test_mixin() async {
await assertErrorsInCode(r'''
class A {
var a;
}
class B extends Object with A {
const B();
}
''', [
error(CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_FINAL_FIELD, 61, 1),
error(
CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD, 61, 1),
]);
}
test_super() async {
await assertErrorsInCode(r'''
class A {
var a;
}
class B extends A {
const B();
}
''', [
error(CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_FINAL_FIELD, 49, 1),
error(CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_CONST_SUPER, 49, 1),
]);
}
test_this_named() async {
await assertErrorsInCode(r'''
class A {
@@ -15,8 +15,6 @@ class B extends A
{
const B(foo) : super(foo);
// ^
// [analyzer] COMPILE_TIME_ERROR.CONST_CONSTRUCTOR_WITH_NON_FINAL_FIELD
// ^
// [analyzer] COMPILE_TIME_ERROR.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD
// ^
// [cfe] A constant constructor can't call a non-constant super constructor.
@@ -17,8 +17,6 @@ class B extends A
{
const B(foo) : super(foo);
// ^
// [analyzer] COMPILE_TIME_ERROR.CONST_CONSTRUCTOR_WITH_NON_FINAL_FIELD
// ^
// [analyzer] COMPILE_TIME_ERROR.CONST_CONSTRUCTOR_WITH_MIXIN_WITH_FIELD
// ^
// [cfe] A constant constructor can't call a non-constant super constructor.