[analyzer] Fix flutter#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>

Author: MichaelRFairhurst

pkg/analyzer/lib/error/error.dart

@@ -293,6 +293,7 @@ const List<ErrorCode> errorCodeValues = [
   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,

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

@@ -5904,6 +5904,21 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
           correction: "Try declaring a zero argument constructor in '{0}', or "
               "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

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

@@ -787,7 +787,10 @@ class ElementResolver extends SimpleAstVisitor<void> {
       }
       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]);
       }

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

@@ -401,7 +401,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
       _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 ErrorVerifier extends RecursiveAstVisitor<void> {
     // class.
     if (!_checkForExtendsDisallowedClass(superclass) &&
         !_checkForImplementsClauseErrorCodes(implementsClause) &&
-        !_checkForAllMixinErrorCodes(withClause)) {
+        !_checkForAllMixinErrorCodes(withClause) &&
+        !_checkForNoGenerativeConstructorsInSuperclass(superclass)) {
       _checkForImplicitDynamicType(superclass);
       _checkForExtendsDeferredClass(superclass);
       _checkForRepeatedType(implementsClause?.interfaces,
@@ -1260,6 +1260,9 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
       _checkMixinInference(node, withClause);
       _checkForMixinWithConflictingPrivateMember(withClause, superclass);
       _checkForConflictingGenerics(node);
+      if (node is ClassDeclaration) {
+        _checkForNoDefaultSuperConstructorImplicit(node);
+      }
     }
   }
 
@@ -3421,6 +3424,35 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
     }
   }
 
+  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 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
       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) {

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),
+    ]);
+  }
+}

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

@@ -19,12 +19,13 @@ class NonGenerativeConstructorTest extends DriverResolutionTest {
     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,24 +57,26 @@ class B extends A {
     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),
     ]);
   }
 
   test_implicit2() async {
     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),
     ]);
   }
 }

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

@@ -380,6 +380,8 @@ import 'no_annotation_constructor_arguments_test.dart'
     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 @@ main() {
     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();