[analyzer] Improve existing non-generative constructor errors

As opposed to the parent CL which creates a new error for cases where no
non-generative constructor is available at all, these improvements
include a bugfix to the previous error message.

The previous message was incorrect in saying 'expected {0} but factory found',
because the argument passed to '{0}' was the factory constructor that
was found, rather than any kind of expected constructor signature.
Fixed.

Also, for implicit constructors, break out the error message to
deliberately explicitly describe that this affects the implicit
constructor.

I considered making an error code for implicit super initializers as
well, (ie `class Foo extends Bar { Foo(); }`), but don't think it is
necessary/common/etc.

Change-Id: I0643ad00abbfb2848cbd2f8f1f83e7c33d4fb846
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/155831
Commit-Queue: Mike Fairhurst <mfairhurst@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

Author: MichaelRFairhurst

pkg/analyzer/lib/error/error.dart

@@ -318,6 +318,7 @@ const List<ErrorCode> errorCodeValues = [
   CompileTimeErrorCode.NON_CONSTANT_MAP_VALUE_FROM_DEFERRED_LIBRARY,
   CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT,
   CompileTimeErrorCode.NON_GENERATIVE_CONSTRUCTOR,
+  CompileTimeErrorCode.NON_GENERATIVE_IMPLICIT_CONSTRUCTOR,
   CompileTimeErrorCode.NON_SYNC_FACTORY,
   CompileTimeErrorCode.NON_TYPE_AS_TYPE_ARGUMENT,
   CompileTimeErrorCode.NON_TYPE_IN_CATCH_CLAUSE,

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

@@ -6651,14 +6651,39 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
    * Let <i>k</i> be a generative constructor. It is a compile-time error if
    * class <i>S</i> does not declare a generative constructor named <i>S</i>
    * (respectively <i>S.id</i>)
+   *
+   * Parameters:
+   * 0: the non-generative constructor
    */
   static const CompileTimeErrorCode NON_GENERATIVE_CONSTRUCTOR =
-      CompileTimeErrorCode('NON_GENERATIVE_CONSTRUCTOR',
-          "The generative constructor '{0}' expected, but factory found.",
+      CompileTimeErrorCode(
+          'NON_GENERATIVE_CONSTRUCTOR',
+          "The constructor '{0}' is a factory constructor, but must be a"
+              " generative constructor to be a valid superinitializer.",
           correction:
-              "Try calling a different constructor in the superclass, or "
+              "Try calling a different constructor of the superclass, or "
               "making the called constructor not be a factory constructor.");
 
+  /**
+   * An error code for when a class has no explicit constructor, and therefore
+   * a constructor is implicitly defined which uses a factory as a
+   * superinitializer. See [NON_GENERATIVE_CONSTRUCTOR].
+   *
+   * Parameters:
+   * 0: the name of the superclass
+   * 1: the name of the current class
+   * 2: the implicitly called factory constructor of the superclass
+   */
+  static const CompileTimeErrorCode NON_GENERATIVE_IMPLICIT_CONSTRUCTOR =
+      CompileTimeErrorCode(
+          'NON_GENERATIVE_IMPLICIT_CONSTRUCTOR',
+          "The default constructor of superclass '{0}' (called by the implicit"
+              " default constructor of '{1}') must be a generative constructor,"
+              " but factory found.",
+          correction: 'Try adding an explicit constructor that has a different'
+              " superinitializer or changing the superclass constructor '{2}'"
+              " to not be a factory constructor.");
+
   static const CompileTimeErrorCode NON_SYNC_FACTORY = CompileTimeErrorCode(
       'NON_SYNC_FACTORY',
       "Factory bodies can't use 'async', 'async*', or 'sync*'.");

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

@@ -3404,9 +3404,9 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
     if (superUnnamedConstructor != null) {
       if (superUnnamedConstructor.isFactory) {
         _errorReporter.reportErrorForNode(
-            CompileTimeErrorCode.NON_GENERATIVE_CONSTRUCTOR,
+            CompileTimeErrorCode.NON_GENERATIVE_IMPLICIT_CONSTRUCTOR,
             declaration.name,
-            [superUnnamedConstructor]);
+            [superElement.name, _enclosingClass.name, superUnnamedConstructor]);
         return;
       }
       if (superUnnamedConstructor.isDefaultConstructor) {

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

@@ -66,17 +66,4 @@ class B extends A {
       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, 57, 1),
-    ]);
-  }
 }

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

@@ -0,0 +1,30 @@
+// 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(NonGenerativeImplicitConstructorTest);
+  });
+}
+
+@reflectiveTest
+class NonGenerativeImplicitConstructorTest extends DriverResolutionTest {
+  test_implicit() async {
+    await assertErrorsInCode(r'''
+class A {
+  factory A() => throw 0;
+  A.named();
+}
+class B extends A {
+}
+''', [
+      error(CompileTimeErrorCode.NON_GENERATIVE_IMPLICIT_CONSTRUCTOR, 57, 1),
+    ]);
+  }
+}

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

@@ -413,6 +413,8 @@ import 'non_constant_map_value_test.dart' as non_constant_map_value;
 import 'non_constant_set_element_test.dart' as non_constant_set_element;
 import 'non_constant_type_argument_test.dart' as non_constant_type_argument;
 import 'non_generative_constructor_test.dart' as non_generative_constructor;
+import 'non_generative_implicit_constructor_test.dart'
+    as non_generative_implicit_constructor;
 import 'non_native_function_type_argument_to_pointer_test.dart'
     as non_native_function_type_argument_to_pointer;
 import 'non_null_opt_out_test.dart' as non_null_opt_out;
@@ -895,6 +897,7 @@ main() {
     non_constant_set_element.main();
     non_constant_type_argument.main();
     non_generative_constructor.main();
+    non_generative_implicit_constructor.main();
     non_native_function_type_argument_to_pointer.main();
     non_null_opt_out.main();
     non_type_as_type_argument.main();