Report MIXIN_DECLARES_CONSTRUCTOR and FINAL_NOT_INITIALIZED for mixins.
I renamed the previous error const to MIXIN_CLASS_DECLARES_CONSTRUCTOR.
R=brianwilkerson@google.com
Change-Id: If06f264e61b7d3e7c254eb457f4b2dc45ef76174
Reviewed-on: https://dart-review.googlesource.com/72860
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
diff --git a/pkg/analyzer/lib/error/error.dart b/pkg/analyzer/lib/error/error.dart
index 17387f6..d97f582 100644
--- a/pkg/analyzer/lib/error/error.dart
+++ b/pkg/analyzer/lib/error/error.dart
@@ -192,6 +192,7 @@
CompileTimeErrorCode.MISSING_CONST_IN_LIST_LITERAL,
CompileTimeErrorCode.MISSING_CONST_IN_MAP_LITERAL,
CompileTimeErrorCode.MISSING_DART_LIBRARY,
+ CompileTimeErrorCode.MIXIN_CLASS_DECLARES_CONSTRUCTOR,
CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR,
CompileTimeErrorCode.MIXIN_DEFERRED_CLASS,
CompileTimeErrorCode.MIXIN_HAS_NO_CONSTRUCTORS,
diff --git a/pkg/analyzer/lib/src/dart/element/builder.dart b/pkg/analyzer/lib/src/dart/element/builder.dart
index fa3788d..567dd7c 100644
--- a/pkg/analyzer/lib/src/dart/element/builder.dart
+++ b/pkg/analyzer/lib/src/dart/element/builder.dart
@@ -82,25 +82,8 @@
}
}
- ElementHolder holder = new ElementHolder();
- //
- // Process field declarations before constructors and methods so that field
- // formal parameters can be correctly resolved to their fields.
- //
- ElementHolder previousHolder = _currentHolder;
- _currentHolder = holder;
- try {
- List<ClassMember> nonFields = new List<ClassMember>();
- node.visitChildren(
- new _ElementBuilder_visitClassDeclaration(this, nonFields));
- _buildFieldMap(holder.fieldsWithoutFlushing);
- int count = nonFields.length;
- for (int i = 0; i < count; i++) {
- nonFields[i].accept(this);
- }
- } finally {
- _currentHolder = previousHolder;
- }
+ ElementHolder holder = _buildClassMembers(node);
+
SimpleIdentifier className = node.name;
ClassElementImpl element = new ClassElementImpl.forNode(className);
_setCodeRange(element, node);
@@ -604,14 +587,7 @@
@override
Object visitMixinDeclaration(MixinDeclaration node) {
- ElementHolder holder = new ElementHolder();
- ElementHolder previousHolder = _currentHolder;
- _currentHolder = holder;
- try {
- node.visitChildren(this);
- } finally {
- _currentHolder = previousHolder;
- }
+ ElementHolder holder = _buildClassMembers(node);
SimpleIdentifier nameNode = node.name;
MixinElementImpl element = new MixinElementImpl.forNode(nameNode);
@@ -721,6 +697,29 @@
return null;
}
+ ElementHolder _buildClassMembers(AstNode classNode) {
+ ElementHolder holder = new ElementHolder();
+ //
+ // Process field declarations before constructors and methods so that field
+ // formal parameters can be correctly resolved to their fields.
+ //
+ ElementHolder previousHolder = _currentHolder;
+ _currentHolder = holder;
+ try {
+ List<ClassMember> nonFields = new List<ClassMember>();
+ classNode.visitChildren(
+ new _ClassNotExecutableElementsBuilder(this, nonFields));
+ _buildFieldMap(holder.fieldsWithoutFlushing);
+ int count = nonFields.length;
+ for (int i = 0; i < count; i++) {
+ nonFields[i].accept(this);
+ }
+ } finally {
+ _currentHolder = previousHolder;
+ }
+ return holder;
+ }
+
/**
* Build the table mapping field names to field elements for the [fields]
* defined in the current class.
@@ -1695,12 +1694,14 @@
}
}
-class _ElementBuilder_visitClassDeclaration extends UnifyingAstVisitor<Object> {
+/**
+ * Builds elements for all node that are not constructors or methods.
+ */
+class _ClassNotExecutableElementsBuilder extends UnifyingAstVisitor<Object> {
final ApiElementBuilder builder;
+ final List<ClassMember> nonFields;
- List<ClassMember> nonFields;
-
- _ElementBuilder_visitClassDeclaration(this.builder, this.nonFields) : super();
+ _ClassNotExecutableElementsBuilder(this.builder, this.nonFields);
@override
Object visitConstructorDeclaration(ConstructorDeclaration node) {
diff --git a/pkg/analyzer/lib/src/error/codes.dart b/pkg/analyzer/lib/src/error/codes.dart
index 7663cc3..3bbd44a 100644
--- a/pkg/analyzer/lib/src/error/codes.dart
+++ b/pkg/analyzer/lib/src/error/codes.dart
@@ -1670,13 +1670,21 @@
* Parameters:
* 0: the name of the mixin that is invalid
*/
- static const CompileTimeErrorCode MIXIN_DECLARES_CONSTRUCTOR =
+ static const CompileTimeErrorCode MIXIN_CLASS_DECLARES_CONSTRUCTOR =
const CompileTimeErrorCode(
- 'MIXIN_DECLARES_CONSTRUCTOR',
+ 'MIXIN_CLASS_DECLARES_CONSTRUCTOR',
"The class '{0}' can't be used as a mixin because it declares a "
"constructor.");
/**
+ * The <i>mixinMember</i> production allows the same instance or static
+ * members that a class would allow, but no constructors (for now).
+ */
+ static const CompileTimeErrorCode MIXIN_DECLARES_CONSTRUCTOR =
+ const CompileTimeErrorCode(
+ 'MIXIN_DECLARES_CONSTRUCTOR', "Mixins can't declare constructors.");
+
+ /**
* 9.1 Mixin Application: It is a compile-time error if the with clause of a
* mixin application <i>C</i> includes a deferred type expression.
*
diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
index f18c8d2..70d9148 100644
--- a/pkg/analyzer/lib/src/generated/error_verifier.dart
+++ b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -501,8 +501,9 @@
withClause != null) {
_checkClassInheritance(node, superclass, withClause, implementsClause);
}
- visitClassDeclarationIncrementally(node);
- _checkForFinalNotInitializedInClass(node);
+
+ _initializeInitialFieldElementsMap(_enclosingClass.fields);
+ _checkForFinalNotInitializedInClass(node.members);
_checkForDuplicateDefinitionInheritance();
_checkForConflictingInstanceMethodSetter(node);
_checkForBadFunctionUse(node);
@@ -514,28 +515,6 @@
}
}
- /**
- * Implementation of this method should be synchronized with
- * [visitClassDeclaration].
- */
- void visitClassDeclarationIncrementally(ClassDeclaration node) {
- _isInNativeClass = node.nativeClause != null;
- _enclosingClass = AbstractClassElementImpl.getImpl(node.declaredElement);
- // initialize initialFieldElementsMap
- if (_enclosingClass != null) {
- List<FieldElement> fieldElements = _enclosingClass.fields;
- _initialFieldElementsMap = new HashMap<FieldElement, INIT_STATE>();
- for (FieldElement fieldElement in fieldElements) {
- if (!fieldElement.isSynthetic) {
- _initialFieldElementsMap[fieldElement] =
- fieldElement.initializer == null
- ? INIT_STATE.NOT_INIT
- : INIT_STATE.INIT_IN_DECLARATION;
- }
- }
- }
- }
-
@override
Object visitClassTypeAlias(ClassTypeAlias node) {
_checkForBuiltInIdentifierAsName(
@@ -592,6 +571,7 @@
_checkForConflictingConstructorNameAndMember(node, constructorElement);
_checkForAllFinalInitializedErrorCodes(node);
_checkForRedirectingConstructorErrorCodes(node);
+ _checkForMixinDeclaresConstructor(node);
_checkForMultipleSuperInitializers(node);
_checkForRecursiveConstructorRedirect(node, constructorElement);
if (!_checkForRecursiveFactoryRedirect(node, constructorElement)) {
@@ -1053,7 +1033,7 @@
// ClassElementImpl outerClass = _enclosingClass;
try {
// _isInNativeClass = node.nativeClause != null;
-// _enclosingClass = AbstractClassElementImpl.getImpl(node.declaredElement);
+ _enclosingClass = AbstractClassElementImpl.getImpl(node.declaredElement);
// _checkDuplicateClassMembers(node);
// _checkForBuiltInIdentifierAsName(
// node.name, CompileTimeErrorCode.BUILT_IN_IDENTIFIER_AS_TYPE_NAME);
@@ -1068,15 +1048,16 @@
if (onClause != null || implementsClause != null) {
_checkMixinInheritance(node, onClause, implementsClause);
}
-// visitClassDeclarationIncrementally(node);
-// _checkForFinalNotInitializedInClass(node);
+
+ _initializeInitialFieldElementsMap(_enclosingClass.fields);
+ _checkForFinalNotInitializedInClass(node.members);
// _checkForDuplicateDefinitionInheritance();
// _checkForConflictingInstanceMethodSetter(node);
// _checkForBadFunctionUse(node);
return super.visitMixinDeclaration(node);
} finally {
// _isInNativeClass = false;
-// _initialFieldElementsMap = null;
+ _initialFieldElementsMap = null;
// _enclosingClass = outerClass;
}
}
@@ -1816,7 +1797,7 @@
/**
* Verify that all classes of the given [withClause] are valid.
*
- * See [CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR],
+ * See [CompileTimeErrorCode.MIXIN_CLASS_DECLARES_CONSTRUCTOR],
* [CompileTimeErrorCode.MIXIN_INHERITS_FROM_NOT_OBJECT], and
* [CompileTimeErrorCode.MIXIN_REFERENCES_SUPER].
*/
@@ -1837,7 +1818,7 @@
mixinName, CompileTimeErrorCode.MIXIN_DEFERRED_CLASS)) {
problemReported = true;
}
- if (_checkForMixinDeclaresConstructor(mixinName, mixinElement)) {
+ if (_checkForMixinClassDeclaresConstructor(mixinName, mixinElement)) {
problemReported = true;
}
if (!enableSuperMixins &&
@@ -3510,23 +3491,20 @@
}
/**
- * Verify that final fields in the given class [declaration] that are
- * declared, without any constructors in the enclosing class, are
- * initialized. Cases in which there is at least one constructor are handled
- * at the end of
- * [_checkForAllFinalInitializedErrorCodes].
+ * If there are no constructors in the given [members], verify that all
+ * final fields are initialized. Cases in which there is at least one
+ * constructor are handled in [_checkForAllFinalInitializedErrorCodes].
*
* See [CompileTimeErrorCode.CONST_NOT_INITIALIZED], and
* [StaticWarningCode.FINAL_NOT_INITIALIZED].
*/
- void _checkForFinalNotInitializedInClass(ClassDeclaration declaration) {
- NodeList<ClassMember> classMembers = declaration.members;
- for (ClassMember classMember in classMembers) {
+ void _checkForFinalNotInitializedInClass(List<ClassMember> members) {
+ for (ClassMember classMember in members) {
if (classMember is ConstructorDeclaration) {
return;
}
}
- for (ClassMember classMember in classMembers) {
+ for (ClassMember classMember in members) {
if (classMember is FieldDeclaration) {
_checkForFinalNotInitialized(classMember.fields);
}
@@ -4459,14 +4437,14 @@
* constructor. The [mixinName] is the node to report problem on. The
* [mixinElement] is the mixing to evaluate.
*
- * See [CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR].
+ * See [CompileTimeErrorCode.MIXIN_CLASS_DECLARES_CONSTRUCTOR].
*/
- bool _checkForMixinDeclaresConstructor(
+ bool _checkForMixinClassDeclaresConstructor(
TypeName mixinName, ClassElement mixinElement) {
for (ConstructorElement constructor in mixinElement.constructors) {
if (!constructor.isSynthetic && !constructor.isFactory) {
_errorReporter.reportErrorForNode(
- CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR,
+ CompileTimeErrorCode.MIXIN_CLASS_DECLARES_CONSTRUCTOR,
mixinName,
[mixinElement.name]);
return true;
@@ -4475,6 +4453,13 @@
return false;
}
+ void _checkForMixinDeclaresConstructor(ConstructorDeclaration node) {
+ if (_enclosingClass.isMixin) {
+ _errorReporter.reportErrorForNode(
+ CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR, node.returnType);
+ }
+ }
+
/**
* Report the error [CompileTimeErrorCode.MIXIN_HAS_NO_CONSTRUCTORS] if
* appropriate.
@@ -4954,7 +4939,7 @@
/**
* Verify that all classes of the given [onClause] are valid.
*
- * See [CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR],
+ * See [CompileTimeErrorCode.MIXIN_CLASS_DECLARES_CONSTRUCTOR],
* [CompileTimeErrorCode.MIXIN_INHERITS_FROM_NOT_OBJECT], and
* [CompileTimeErrorCode.MIXIN_REFERENCES_SUPER].
*/
@@ -6323,6 +6308,18 @@
return visitor.hasSelfReference;
}
+ void _initializeInitialFieldElementsMap(List<FieldElement> fields) {
+ _initialFieldElementsMap = new HashMap<FieldElement, INIT_STATE>();
+ for (FieldElement fieldElement in fields) {
+ if (!fieldElement.isSynthetic) {
+ _initialFieldElementsMap[fieldElement] =
+ fieldElement.initializer == null
+ ? INIT_STATE.NOT_INIT
+ : INIT_STATE.INIT_IN_DECLARATION;
+ }
+ }
+ }
+
bool _isFunctionType(DartType type) {
if (type.isDynamic || type.isDartCoreNull) {
return true;
diff --git a/pkg/analyzer/test/generated/compile_time_error_code_test.dart b/pkg/analyzer/test/generated/compile_time_error_code_test.dart
index 4d6b0b7..0463112 100644
--- a/pkg/analyzer/test/generated/compile_time_error_code_test.dart
+++ b/pkg/analyzer/test/generated/compile_time_error_code_test.dart
@@ -4055,37 +4055,31 @@
verify([source]);
}
- @failingTest
- test_mixinDeclaresConstructor() async {
- Source source = addSource(r'''
-class A {
- A() {}
-}
-class B extends Object mixin A {}''');
- await computeAnalysisResult(source);
- assertErrors(source, [CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR]);
- verify([source]);
- }
-
- test_mixinDeclaresConstructor_classDeclaration() async {
+ test_mixinClassDeclaresConstructor_classDeclaration() async {
Source source = addSource(r'''
class A {
A() {}
}
class B extends Object with A {}''');
await computeAnalysisResult(source);
- assertErrors(source, [CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR]);
+ assertErrors(
+ source,
+ [CompileTimeErrorCode.MIXIN_CLASS_DECLARES_CONSTRUCTOR],
+ );
verify([source]);
}
- test_mixinDeclaresConstructor_typeAlias() async {
+ test_mixinClassDeclaresConstructor_typeAlias() async {
Source source = addSource(r'''
class A {
A() {}
}
class B = Object with A;''');
await computeAnalysisResult(source);
- assertErrors(source, [CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR]);
+ assertErrors(
+ source,
+ [CompileTimeErrorCode.MIXIN_CLASS_DECLARES_CONSTRUCTOR],
+ );
verify([source]);
}
diff --git a/pkg/analyzer/test/generated/simple_resolver_test.dart b/pkg/analyzer/test/generated/simple_resolver_test.dart
index 732ed20..f38ced1 100644
--- a/pkg/analyzer/test/generated/simple_resolver_test.dart
+++ b/pkg/analyzer/test/generated/simple_resolver_test.dart
@@ -1070,7 +1070,10 @@
ClassElement a = unit.getType('A');
expect(a.isValidMixin, isFalse);
await computeAnalysisResult(source);
- assertErrors(source, [CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR]);
+ assertErrors(
+ source,
+ [CompileTimeErrorCode.MIXIN_CLASS_DECLARES_CONSTRUCTOR],
+ );
verify([source]);
}
@@ -1088,7 +1091,10 @@
ClassElement a = unit.getType('A');
expect(a.isValidMixin, isFalse);
await computeAnalysisResult(source);
- assertErrors(source, [CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR]);
+ assertErrors(
+ source,
+ [CompileTimeErrorCode.MIXIN_CLASS_DECLARES_CONSTRUCTOR],
+ );
verify([source]);
}
diff --git a/pkg/analyzer/test/src/dart/resolution/find_element.dart b/pkg/analyzer/test/src/dart/resolution/find_element.dart
index df7c82b..0b1ed90 100644
--- a/pkg/analyzer/test/src/dart/resolution/find_element.dart
+++ b/pkg/analyzer/test/src/dart/resolution/find_element.dart
@@ -48,6 +48,13 @@
}
FieldElement field(String name) {
+ for (var type in unitElement.mixins) {
+ for (var field in type.fields) {
+ if (field.name == name) {
+ return field;
+ }
+ }
+ }
for (var type in unitElement.types) {
for (var field in type.fields) {
if (field.name == name) {
diff --git a/pkg/analyzer/test/src/dart/resolution/find_node.dart b/pkg/analyzer/test/src/dart/resolution/find_node.dart
index 54b907a..55998cc 100644
--- a/pkg/analyzer/test/src/dart/resolution/find_node.dart
+++ b/pkg/analyzer/test/src/dart/resolution/find_node.dart
@@ -28,10 +28,18 @@
return _node(search, (n) => n is CommentReference);
}
+ ConstructorDeclaration constructor(String search) {
+ return _node(search, (n) => n is ConstructorDeclaration);
+ }
+
ExportDirective export(String search) {
return _node(search, (n) => n is ExportDirective);
}
+ FieldFormalParameter fieldFormalParameter(String search) {
+ return _node(search, (n) => n is FieldFormalParameter);
+ }
+
FunctionExpression functionExpression(String search) {
return _node(search, (n) => n is FunctionExpression);
}
diff --git a/pkg/analyzer/test/src/dart/resolution/mixin_test.dart b/pkg/analyzer/test/src/dart/resolution/mixin_test.dart
index a39a2c3..d1bb294 100644
--- a/pkg/analyzer/test/src/dart/resolution/mixin_test.dart
+++ b/pkg/analyzer/test/src/dart/resolution/mixin_test.dart
@@ -1,3 +1,4 @@
+import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -153,6 +154,60 @@
assertElementTypes(element.superclassConstraints, [objectType]);
}
+ test_error_finalNotInitialized() async {
+ addTestFile(r'''
+mixin M {
+ final int f;
+}
+''');
+ await resolveTestFile();
+ assertTestErrors([StaticWarningCode.FINAL_NOT_INITIALIZED]);
+ }
+
+ test_error_finalNotInitialized_OK() async {
+ addTestFile(r'''
+mixin M {
+ final int f = 0;
+}
+''');
+ await resolveTestFile();
+ assertNoTestErrors();
+ }
+
+ test_error_finalNotInitializedConstructor() async {
+ addTestFile(r'''
+mixin M {
+ final int f;
+ M();
+}
+''');
+ await resolveTestFile();
+ assertTestErrors([
+ CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR,
+ StaticWarningCode.FINAL_NOT_INITIALIZED_CONSTRUCTOR_1,
+ ]);
+ }
+
+ test_error_finalNotInitializedConstructor_OK() async {
+ addTestFile(r'''
+mixin M {
+ final int f;
+ M(this.f);
+}
+''');
+ await resolveTestFile();
+ assertTestErrors([CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR]);
+
+ var element = findElement.mixin('M');
+ var constructorElement = element.constructors.single;
+
+ var fpNode = findNode.fieldFormalParameter('f);');
+ assertElement(fpNode.identifier, constructorElement.parameters[0]);
+
+ FieldFormalParameterElement fpElement = fpNode.declaredElement;
+ expect(fpElement.field, same(findElement.field('f')));
+ }
+
test_error_implementsClause_deferredClass() async {
addTestFile(r'''
import 'dart:math' deferred as math;
@@ -208,6 +263,37 @@
assertTypeName(typeRef, null, 'void');
}
+ test_error_mixinDeclaresConstructor() async {
+ addTestFile(r'''
+mixin M {
+ M(int a) {
+ a; // read
+ }
+}
+''');
+ await resolveTestFile();
+ assertTestErrors([CompileTimeErrorCode.MIXIN_DECLARES_CONSTRUCTOR]);
+
+ // Even though it is an error for a mixin to declare a constructor,
+ // we still build elements for constructors, and resolve them.
+
+ var element = findElement.mixin('M');
+ var constructors = element.constructors;
+ expect(constructors, hasLength(1));
+ var constructorElement = constructors[0];
+
+ var constructorNode = findNode.constructor('M(int a)');
+ assertElement(constructorNode, constructorElement);
+
+ var aElement = constructorElement.parameters[0];
+ var aNode = constructorNode.parameters.parameters[0];
+ assertElement(aNode, aElement);
+
+ var aRef = findNode.simple('a; // read');
+ assertElement(aRef, aElement);
+ assertType(aRef, 'int');
+ }
+
test_error_onClause_deferredClass() async {
addTestFile(r'''
import 'dart:math' deferred as math;
diff --git a/pkg/analyzer/test/src/dart/resolution/resolution.dart b/pkg/analyzer/test/src/dart/resolution/resolution.dart
index a7751c1..eacf271 100644
--- a/pkg/analyzer/test/src/dart/resolution/resolution.dart
+++ b/pkg/analyzer/test/src/dart/resolution/resolution.dart
@@ -199,6 +199,8 @@
return node.staticElement;
} else if (node is Declaration) {
return node.declaredElement;
+ } else if (node is FormalParameter) {
+ return node.declaredElement;
} else if (node is Identifier) {
return node.staticElement;
} else if (node is IndexExpression) {
