[analyzer] Fix #25874, confusing error for extending Exception.
In the case of extending Exception, the raised error is around the
configuration of the constructors. However, there is no valid
configuration of constructors to be found, and therefore, the better
user facing message is to say that the class cannot be extended as
written.
Change-Id: I5950d438877c450c44cdc914f9f20d7779a64768
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/154126
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analyzer/lib/error/error.dart b/pkg/analyzer/lib/error/error.dart
index 04bcae0..b439810 100644
--- a/pkg/analyzer/lib/error/error.dart
+++ b/pkg/analyzer/lib/error/error.dart
@@ -293,6 +293,7 @@
CompileTimeErrorCode.NO_COMBINED_SUPER_SIGNATURE,
CompileTimeErrorCode.NO_DEFAULT_SUPER_CONSTRUCTOR_EXPLICIT,
CompileTimeErrorCode.NO_DEFAULT_SUPER_CONSTRUCTOR_IMPLICIT,
+ CompileTimeErrorCode.NO_GENERATIVE_CONSTRUCTORS_IN_SUPERCLASS,
CompileTimeErrorCode.NON_ABSTRACT_CLASS_INHERITS_ABSTRACT_MEMBER_FIVE_PLUS,
CompileTimeErrorCode.NON_ABSTRACT_CLASS_INHERITS_ABSTRACT_MEMBER_FOUR,
CompileTimeErrorCode.NON_ABSTRACT_CLASS_INHERITS_ABSTRACT_MEMBER_ONE,
diff --git a/pkg/analyzer/lib/src/error/codes.dart b/pkg/analyzer/lib/src/error/codes.dart
index 75b626a..a44d2434 100644
--- a/pkg/analyzer/lib/src/error/codes.dart
+++ b/pkg/analyzer/lib/src/error/codes.dart
@@ -5905,6 +5905,21 @@
"explicitly invoking a different constructor in '{0}'.");
/**
+ * User friendly specialized error for [NON_GENERATIVE_CONSTRUCTOR]. This
+ * handles the case of `class E extends Exception` which will never work
+ * because [Exception] has no generative constructors.
+ */
+ static const CompileTimeErrorCode NO_GENERATIVE_CONSTRUCTORS_IN_SUPERCLASS =
+ CompileTimeErrorCode(
+ 'NO_GENERATIVE_CONSTRUCTOR_IN_SUPERCLASS',
+ "The class '{0}' cannot extend '{1}' because '{1}' only has factory"
+ " constructors (no generative constructors), and '{0}' has at"
+ ' least one generative constructor.',
+ correction: 'Try implementing the class instead, adding a generative'
+ " (not factory) constructor to the superclass {0}, or a factory"
+ ' constructor to the subclass.');
+
+ /**
* Parameters:
* 0: the name of the superclass that does not define an implicitly invoked
* constructor
diff --git a/pkg/analyzer/lib/src/generated/element_resolver.dart b/pkg/analyzer/lib/src/generated/element_resolver.dart
index e36b41b..ac374e1 100644
--- a/pkg/analyzer/lib/src/generated/element_resolver.dart
+++ b/pkg/analyzer/lib/src/generated/element_resolver.dart
@@ -787,7 +787,10 @@
}
return;
} else {
- if (element.isFactory) {
+ if (element.isFactory &&
+ // Check if we've reported [NO_GENERATIVE_CONSTRUCTORS_IN_SUPERCLASS].
+ !element.enclosingElement.constructors
+ .every((constructor) => constructor.isFactory)) {
_errorReporter.reportErrorForNode(
CompileTimeErrorCode.NON_GENERATIVE_CONSTRUCTOR, node, [element]);
}
diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
index ee5953b..4bad678 100644
--- a/pkg/analyzer/lib/src/generated/error_verifier.dart
+++ b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -401,7 +401,6 @@
_duplicateDefinitionVerifier.checkClass(node);
_checkForBuiltInIdentifierAsName(
node.name, CompileTimeErrorCode.BUILT_IN_IDENTIFIER_AS_TYPE_NAME);
- _checkForNoDefaultSuperConstructorImplicit(node);
_checkForConflictingClassTypeVariableErrorCodes();
TypeName superclass = node.extendsClause?.superclass;
ImplementsClause implementsClause = node.implementsClause;
@@ -1250,7 +1249,8 @@
// class.
if (!_checkForExtendsDisallowedClass(superclass) &&
!_checkForImplementsClauseErrorCodes(implementsClause) &&
- !_checkForAllMixinErrorCodes(withClause)) {
+ !_checkForAllMixinErrorCodes(withClause) &&
+ !_checkForNoGenerativeConstructorsInSuperclass(superclass)) {
_checkForImplicitDynamicType(superclass);
_checkForExtendsDeferredClass(superclass);
_checkForRepeatedType(implementsClause?.interfaces,
@@ -1260,6 +1260,9 @@
_checkMixinInference(node, withClause);
_checkForMixinWithConflictingPrivateMember(withClause, superclass);
_checkForConflictingGenerics(node);
+ if (node is ClassDeclaration) {
+ _checkForNoDefaultSuperConstructorImplicit(node);
+ }
}
}
@@ -3421,6 +3424,35 @@
}
}
+ bool _checkForNoGenerativeConstructorsInSuperclass(TypeName superclass) {
+ InterfaceType superType = _enclosingClass.supertype;
+ if (superType == null) {
+ return false;
+ }
+ if (_enclosingClass.constructors
+ .every((constructor) => constructor.isFactory)) {
+ // A class with no generative constructors *can* be extended if the
+ // subclass has only factory constructors.
+ return false;
+ }
+ ClassElement superElement = superType.element;
+ if (superElement.constructors.isEmpty) {
+ // Exclude empty constructor set, which indicates other errors occurred.
+ return false;
+ }
+ if (superElement.constructors
+ .every((constructor) => constructor.isFactory)) {
+ // For `E extends Exception`, etc., this will never work, because it has
+ // no generative constructors. State this clearly to users.
+ _errorReporter.reportErrorForNode(
+ CompileTimeErrorCode.NO_GENERATIVE_CONSTRUCTORS_IN_SUPERCLASS,
+ superclass,
+ [_enclosingClass.name, superElement.name]);
+ return true;
+ }
+ return false;
+ }
+
/// Verify the given map [literal] either:
/// * has `const modifier`
/// * has explicit type arguments
@@ -4155,6 +4187,11 @@
return;
}
ClassElement superElement = superType.element;
+ if (superElement.constructors
+ .every((constructor) => constructor.isFactory)) {
+ // Already reported [NO_GENERATIVE_CONSTRUCTORS_IN_SUPERCLASS].
+ return;
+ }
ConstructorElement superUnnamedConstructor =
superElement.unnamedConstructor;
if (superUnnamedConstructor != null) {
diff --git a/pkg/analyzer/test/src/diagnostics/no_generative_constructors_in_superclass_test.dart b/pkg/analyzer/test/src/diagnostics/no_generative_constructors_in_superclass_test.dart
new file mode 100644
index 0000000..0ace42c
--- /dev/null
+++ b/pkg/analyzer/test/src/diagnostics/no_generative_constructors_in_superclass_test.dart
@@ -0,0 +1,96 @@
+// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:analyzer/src/error/codes.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/driver_resolution.dart';
+
+main() {
+ defineReflectiveSuite(() {
+ defineReflectiveTests(NoGenerativeConstructorsInSuperclassTest);
+ });
+}
+
+@reflectiveTest
+class NoGenerativeConstructorsInSuperclassTest extends DriverResolutionTest {
+ test_explicit() async {
+ await assertErrorsInCode(r'''
+class A {
+ factory A() => throw '';
+}
+class B extends A {
+ B() : super();
+}
+''', [
+ error(
+ CompileTimeErrorCode.NO_GENERATIVE_CONSTRUCTORS_IN_SUPERCLASS, 55, 1),
+ ]);
+ }
+
+ test_explicit_oneFactory() async {
+ await assertErrorsInCode(r'''
+class A {
+ factory A() => throw '';
+}
+class B extends A {
+ B() : super();
+ factory B.second() => throw '';
+}
+''', [
+ error(
+ CompileTimeErrorCode.NO_GENERATIVE_CONSTRUCTORS_IN_SUPERCLASS, 55, 1),
+ ]);
+ }
+
+ test_hasFactories() async {
+ await assertNoErrorsInCode(r'''
+class A {
+ factory A() => throw '';
+}
+class B extends A {
+ factory B() => throw '';
+ factory B.second() => throw '';
+}
+''');
+ }
+
+ test_hasFactory() async {
+ await assertNoErrorsInCode(r'''
+class A {
+ factory A() => throw '';
+}
+class B extends A {
+ factory B() => throw '';
+}
+''');
+ }
+
+ test_implicit() async {
+ await assertErrorsInCode(r'''
+class A {
+ factory A() => throw '';
+}
+class B extends A {
+ B();
+}
+''', [
+ error(
+ CompileTimeErrorCode.NO_GENERATIVE_CONSTRUCTORS_IN_SUPERCLASS, 55, 1),
+ ]);
+ }
+
+ test_implicit2() async {
+ await assertErrorsInCode(r'''
+class A {
+ factory A() => throw '';
+}
+class B extends A {
+}
+''', [
+ error(
+ CompileTimeErrorCode.NO_GENERATIVE_CONSTRUCTORS_IN_SUPERCLASS, 55, 1),
+ ]);
+ }
+}
diff --git a/pkg/analyzer/test/src/diagnostics/non_generative_constructor_test.dart b/pkg/analyzer/test/src/diagnostics/non_generative_constructor_test.dart
index fd89b22..8fbda30 100644
--- a/pkg/analyzer/test/src/diagnostics/non_generative_constructor_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/non_generative_constructor_test.dart
@@ -19,12 +19,13 @@
await assertErrorsInCode(r'''
class A {
factory A.named() => throw 0;
+ A.generative();
}
class B extends A {
B() : super.named();
}
''', [
- error(CompileTimeErrorCode.NON_GENERATIVE_CONSTRUCTOR, 72, 13),
+ error(CompileTimeErrorCode.NON_GENERATIVE_CONSTRUCTOR, 90, 13),
]);
}
@@ -56,12 +57,13 @@
await assertErrorsInCode(r'''
class A {
factory A() => throw 0;
+ A.named();
}
class B extends A {
B();
}
''', [
- error(CompileTimeErrorCode.NON_GENERATIVE_CONSTRUCTOR, 60, 1),
+ error(CompileTimeErrorCode.NON_GENERATIVE_CONSTRUCTOR, 73, 1),
]);
}
@@ -69,11 +71,12 @@
await assertErrorsInCode(r'''
class A {
factory A() => throw 0;
+ A.named();
}
class B extends A {
}
''', [
- error(CompileTimeErrorCode.NON_GENERATIVE_CONSTRUCTOR, 44, 1),
+ error(CompileTimeErrorCode.NON_GENERATIVE_CONSTRUCTOR, 57, 1),
]);
}
}
diff --git a/pkg/analyzer/test/src/diagnostics/test_all.dart b/pkg/analyzer/test/src/diagnostics/test_all.dart
index c623a01..1eb0ad3 100644
--- a/pkg/analyzer/test/src/diagnostics/test_all.dart
+++ b/pkg/analyzer/test/src/diagnostics/test_all.dart
@@ -380,6 +380,8 @@
as no_annotation_constructor_arguments;
import 'no_combined_super_signature_test.dart' as no_combined_super_signature;
import 'no_default_super_constructor_test.dart' as no_default_super_constructor;
+import 'no_generative_constructors_in_superclass_test.dart'
+ as no_generative_constructors_in_superclass;
import 'non_abstract_class_inherits_abstract_member_test.dart'
as non_abstract_class_inherits_abstract_member;
import 'non_bool_condition_test.dart' as non_bool_condition;
@@ -872,6 +874,7 @@
no_annotation_constructor_arguments.main();
no_combined_super_signature.main();
no_default_super_constructor.main();
+ no_generative_constructors_in_superclass.main();
non_abstract_class_inherits_abstract_member.main();
non_bool_condition.main();
non_bool_expression.main();
