Fix redirecting constructor const check

Fixes #27617

Change-Id: Id8629354e844d3f265302eda6348afccc2d63be1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/122120
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>

Author: srawlins

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

@@ -3994,7 +3994,7 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
   static const CompileTimeErrorCode REDIRECT_TO_NON_CONST_CONSTRUCTOR =
       const CompileTimeErrorCode(
           'REDIRECT_TO_NON_CONST_CONSTRUCTOR',
-          "Constant factory constructor can't delegate to a non-constant "
+          "Constant redirecting constructor can't redirect to a non-constant "
               "constructor.",
           correction: "Try redirecting to a different constructor.");
 
@@ -6407,8 +6407,8 @@ class StaticWarningCode extends AnalyzerErrorCode {
    * abstract class and <i>q</i> is not a factory constructor.
    */
   static const StaticWarningCode NEW_WITH_ABSTRACT_CLASS =
-      const StaticWarningCode('NEW_WITH_ABSTRACT_CLASS',
-          "Abstract classes can't be instantiated.",
+      const StaticWarningCode(
+          'NEW_WITH_ABSTRACT_CLASS', "Abstract classes can't be instantiated.",
           correction: "Try creating an instance of a subtype.");
 
   /**

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

@@ -607,7 +607,6 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
         _checkForAllRedirectConstructorErrorCodes(node);
       }
       _checkForUndefinedConstructorInInitializerImplicit(node);
-      _checkForRedirectToNonConstConstructor(node, constructorElement);
       _checkForReturnInGenerativeConstructor(node);
       super.visitConstructorDeclaration(node);
     } finally {
@@ -4547,6 +4546,8 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
               parameter.identifier);
         }
       }
+      _checkForRedirectToNonConstConstructor(declaration.declaredElement,
+          redirectedConstructor.staticElement, redirectedConstructor);
     }
     // check if there are redirected invocations
     int numRedirections = 0;
@@ -4579,6 +4580,10 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
             }
           }
         }
+        // [declaration] is a redirecting constructor via a redirecting
+        // initializer.
+        _checkForRedirectToNonConstConstructor(declaration.declaredElement,
+            initializer.staticElement, initializer.constructorName);
         numRedirections++;
       }
     }
@@ -4605,41 +4610,21 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
   }
 
   /**
-   * Check whether the given constructor [declaration] has redirected
-   * constructor and references itself directly or indirectly. The
-   * constructor [element] is the element introduced by the declaration.
+   * Check whether the redirecting constructor, [element], is const, and
+   * [redirectedElement], its redirectee, is not const.
    *
    * See [CompileTimeErrorCode.REDIRECT_TO_NON_CONST_CONSTRUCTOR].
    */
-  void _checkForRedirectToNonConstConstructor(
-      ConstructorDeclaration declaration, ConstructorElement element) {
-    // prepare redirected constructor
-    ConstructorName redirectedConstructorNode =
-        declaration.redirectedConstructor;
-    if (redirectedConstructorNode == null) {
-      return;
-    }
-    // prepare element
-    if (element == null) {
-      return;
-    }
-    // OK, it is not 'const'
-    if (!element.isConst) {
-      return;
-    }
-    // prepare redirected constructor
-    ConstructorElement redirectedConstructor = element.redirectedConstructor;
-    if (redirectedConstructor == null) {
-      return;
-    }
-    // OK, it is also 'const'
-    if (redirectedConstructor.isConst) {
-      return;
+  void _checkForRedirectToNonConstConstructor(ConstructorElement element,
+      ConstructorElement redirectedElement, AstNode reportingNode) {
+    // This constructor is const, but it redirects to a non-const constructor.
+    if (redirectedElement != null &&
+        element.isConst &&
+        !redirectedElement.isConst) {
+      _errorReporter.reportErrorForNode(
+          CompileTimeErrorCode.REDIRECT_TO_NON_CONST_CONSTRUCTOR,
+          reportingNode);
     }
-
-    _errorReporter.reportErrorForNode(
-        CompileTimeErrorCode.REDIRECT_TO_NON_CONST_CONSTRUCTOR,
-        redirectedConstructorNode);
   }
 
   void _checkForReferenceBeforeDeclaration(SimpleIdentifier node) {

pkg/analyzer/test/generated/compile_time_error_code.dart

@@ -4418,17 +4418,6 @@ class B {
     ]);
   }
 
-  test_redirectToNonConstConstructor() async {
-    await assertErrorsInCode(r'''
-class A {
-  A.a() {}
-  const factory A.b() = A.a;
-}
-''', [
-      error(CompileTimeErrorCode.REDIRECT_TO_NON_CONST_CONSTRUCTOR, 45, 3),
-    ]);
-  }
-
   test_referencedBeforeDeclaration_hideInBlock_comment() async {
     await assertNoErrorsInCode(r'''
 main() {

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

@@ -0,0 +1,80 @@
+// Copyright (c) 2019, 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(RedirectToNonConstConstructorTest);
+  });
+}
+
+@reflectiveTest
+class RedirectToNonConstConstructorTest extends DriverResolutionTest {
+  test_constRedirector_cannotResolveRedirectee() async {
+    // No crash when redirectee cannot be resolved.
+    await assertErrorsInCode(r'''
+class A {
+  const factory A.b() = A.a;
+}
+''', [
+      error(CompileTimeErrorCode.REDIRECT_TO_MISSING_CONSTRUCTOR, 34, 3),
+    ]);
+  }
+
+  test_constRedirector_constRedirectee() async {
+    await assertNoErrorsInCode(r'''
+class A {
+  const A.a();
+  const factory A.b() = A.a;
+}
+''');
+  }
+
+  test_constRedirector_constRedirectee_viaInitializer() async {
+    await assertNoErrorsInCode(r'''
+class A {
+  const A.a();
+  const A.b() : this.a();
+}
+''');
+  }
+
+  test_constRedirector_nonConstRedirectee() async {
+    await assertErrorsInCode(r'''
+class A {
+  A.a();
+  const factory A.b() = A.a;
+}
+''', [
+      error(CompileTimeErrorCode.REDIRECT_TO_NON_CONST_CONSTRUCTOR, 43, 3),
+    ]);
+  }
+
+  test_constRedirector_nonConstRedirectee_viaInitializer() async {
+    await assertErrorsInCode(r'''
+class A {
+  A.a();
+  const A.b() : this.a();
+}
+''', [
+      error(CompileTimeErrorCode.REDIRECT_TO_NON_CONST_CONSTRUCTOR, 40, 1),
+    ]);
+  }
+
+  test_constRedirector_viaInitializer_cannotResolveRedirectee() async {
+    // No crash when redirectee cannot be resolved.
+    await assertErrorsInCode(r'''
+class A {
+  const A.b() : this.a();
+}
+''', [
+      error(CompileTimeErrorCode.REDIRECT_GENERATIVE_TO_MISSING_CONSTRUCTOR, 26,
+          8),
+    ]);
+  }
+}

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

@@ -267,6 +267,8 @@ import 'redirect_to_invalid_return_type_test.dart'
 import 'redirect_to_missing_constructor_test.dart'
     as redirect_to_missing_constructor;
 import 'redirect_to_non_class_test.dart' as redirect_to_non_class;
+import 'redirect_to_non_const_constructor_test.dart'
+    as redirect_to_non_const_constructor;
 import 'return_without_value_test.dart' as return_without_value;
 import 'sdk_version_as_expression_in_const_context_test.dart'
     as sdk_version_as_expression_in_const_context;
@@ -538,6 +540,7 @@ main() {
     redirect_to_invalid_return_type.main();
     redirect_to_missing_constructor.main();
     redirect_to_non_class.main();
+    redirect_to_non_const_constructor.main();
     return_without_value.main();
     set_element_from_deferred_library.main();
     sdk_version_as_expression_in_const_context.main();