analyzer: Improved checks of type variables in const function references

In particular, check the two cases where a constructor reference or
function reference may appear in which they must be constant
expressions, but are not in constant contexts:

* default parameter value
* field initializer in a class with at least one generative const
  constructor

We also update NodeReplacer to also replace a DefaultParameterElement's
reference to the AST of the default value.

Bug: https://github.com/dart-lang/sdk/issues/46020
Change-Id: Iea92265d41459b9f79317128e96d80de5069abfc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214340
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>

pkg/analyzer/lib/src/dart/ast/utilities.dart

@@ -10,6 +10,7 @@ import 'package:analyzer/dart/ast/visitor.dart';
 import 'package:analyzer/exception/exception.dart';
 import 'package:analyzer/src/dart/ast/ast.dart';
 import 'package:analyzer/src/dart/ast/to_source_visitor.dart';
+import 'package:analyzer/src/dart/element/element.dart';
 import 'package:analyzer/src/generated/engine.dart' show AnalysisEngine;
 
 export 'package:analyzer/src/dart/ast/constant_evaluator.dart';
@@ -1923,6 +1924,12 @@ class NodeReplacer implements AstVisitor<bool> {
       return true;
     } else if (identical(node.defaultValue, _oldNode)) {
       node.defaultValue = _newNode as Expression;
+      var parameterElement = node.declaredElement;
+      if (parameterElement is DefaultParameterElementImpl) {
+        parameterElement.constantInitializer = _newNode as Expression;
+      } else if (parameterElement is DefaultFieldFormalParameterElementImpl) {
+        parameterElement.constantInitializer = _newNode as Expression;
+      }
       return true;
     }
     return visitNode(node);
@@ -2904,6 +2911,9 @@ class NodeReplacer implements AstVisitor<bool> {
     } else if (identical(node.initializer, _oldNode)) {
       node.initializer = _newNode as Expression;
       return true;
+      // TODO(srawlins) also replace node's declared element's
+      // `constantInitializer`, if the element is [ConstFieldElementImpl],
+      // [ConstLocalVariableElementImpl], or [ConstTopLevelVariableElementImpl].
     }
     return visitAnnotatedNode(node);
   }

pkg/analyzer/lib/src/dart/constant/constant_verifier.dart

@@ -116,7 +116,7 @@ class ConstantVerifier extends RecursiveAstVisitor<void> {
   @override
   void visitConstructorReference(ConstructorReference node) {
     super.visitConstructorReference(node);
-    if (node.inConstantContext) {
+    if (node.inConstantContext || node.inConstantExpression) {
       _checkForConstWithTypeParameters(node.constructorName.type2,
           CompileTimeErrorCode.CONST_WITH_TYPE_PARAMETERS_CONSTRUCTOR_TEAROFF);
     }
@@ -131,7 +131,7 @@ class ConstantVerifier extends RecursiveAstVisitor<void> {
   @override
   void visitFunctionReference(FunctionReference node) {
     super.visitFunctionReference(node);
-    if (node.inConstantContext) {
+    if (node.inConstantContext || node.inConstantExpression) {
       var typeArguments = node.typeArguments;
       if (typeArguments == null) {
         return;
@@ -545,13 +545,10 @@ class ConstantVerifier extends RecursiveAstVisitor<void> {
     }
   }
 
-  /// Validates that the expressions of any field initializers in the class
-  /// declaration are all compile time constants. Since this is only required if
-  /// the class has a constant constructor, the error is reported at the
-  /// constructor site.
-  ///
-  /// @param classDeclaration the class which should be validated
-  /// @param errorSite the site at which errors should be reported.
+  /// Validates that the expressions of any field initializers in
+  /// [classDeclaration] are all compile-time constants. Since this is only
+  /// required if the class has a constant constructor, the error is reported at
+  /// [constKeyword], the const keyword on such a constant constructor.
   void _validateFieldInitializers(
       ClassOrMixinDeclaration classDeclaration, Token constKeyword) {
     NodeList<ClassMember> members = classDeclaration.members;
@@ -1048,3 +1045,44 @@ class _SetVerifierConfig {
     required this.elementType,
   });
 }
+
+extension on Expression {
+  /// Returns whether `this` is found in a constant expression.
+  ///
+  /// This does not check whether `this` is found in a constant context.
+  bool get inConstantExpression {
+    AstNode child = this;
+    var parent = child.parent;
+    while (parent != null) {
+      if (parent is DefaultFormalParameter && child == parent.defaultValue) {
+        // A parameter default value does not constitute a constant context, but
+        // must be a constant expression.
+        return true;
+      } else if (parent is VariableDeclaration && child == parent.initializer) {
+        var declarationList = parent.parent;
+        if (declarationList is VariableDeclarationList) {
+          var declarationListParent = declarationList.parent;
+          if (declarationListParent is FieldDeclaration &&
+              !declarationListParent.isStatic) {
+            var container = declarationListParent.parent;
+            if (container is ClassOrMixinDeclaration) {
+              var enclosingClass = container.declaredElement;
+              if (enclosingClass != null) {
+                // A field initializer of a class with at least one generative
+                // const constructor does not constitute a constant context, but
+                // must be a constant expression.
+                return enclosingClass.constructors
+                    .any((c) => c.isConst && !c.isFactory);
+              }
+            }
+          }
+        }
+        return false;
+      } else {
+        child = parent;
+        parent = child.parent;
+      }
+    }
+    return false;
+  }
+}

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

@@ -2,8 +2,8 @@
 // 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/dart/error/syntactic_errors.dart';
 import 'package:analyzer/src/error/codes.dart';
-import 'package:test/test.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
 import '../dart/resolution/context_collection_resolution.dart';
@@ -34,6 +34,41 @@ void g() {
     ]);
   }
 
+  test_defaultValue() async {
+    await assertErrorsInCode('''
+class A<T> {
+  void m([var fn = A<T>.new]) {}
+}
+''', [
+      error(CompileTimeErrorCode.CONST_WITH_TYPE_PARAMETERS_CONSTRUCTOR_TEAROFF,
+          34, 1),
+    ]);
+  }
+
+  test_defaultValue_fieldFormalParameter() async {
+    await assertErrorsInCode('''
+class A<T> {
+  A<T> Function() fn;
+  A([this.fn = A<T>.new]);
+}
+''', [
+      error(CompileTimeErrorCode.CONST_WITH_TYPE_PARAMETERS_CONSTRUCTOR_TEAROFF,
+          52, 1),
+    ]);
+  }
+
+  test_defaultValue_noTypeVariableInferredFromParameter() async {
+    await assertErrorsInCode('''
+class A<T> {
+  void m([A<T> Function() fn = A.new]) {}
+}
+''', [
+      // `A<dynamic> Function()` cannot be assigned to `A<T> Function()`, but
+      // there should not be any other error reported here.
+      error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 44, 5),
+    ]);
+  }
+
   test_direct() async {
     await assertErrorsInCode('''
 class A<T> {
@@ -48,6 +83,18 @@ class A<T> {
     ]);
   }
 
+  test_fieldValue_constClass() async {
+    await assertErrorsInCode('''
+class A<T> {
+  const A();
+  final x = A<T>.new;
+}
+''', [
+      error(CompileTimeErrorCode.CONST_WITH_TYPE_PARAMETERS_CONSTRUCTOR_TEAROFF,
+          40, 1),
+    ]);
+  }
+
   test_indirect() async {
     await assertErrorsInCode('''
 class A<T> {
@@ -88,24 +135,16 @@ class A<T> {
 @reflectiveTest
 class ConstWithTypeParametersFunctionTearoffTest
     extends PubPackageResolutionTest {
-  @FailingTest(
-    reason: 'The default value of an optional parameter is not considered a '
-        '"constant context". Currently only ConstantVerifier checks '
-        'CONST_WITH_TYPE_PARAMETERS (and related) errors, and only for '
-        'constant contexts. These checks should probably be moved to '
-        'ConstantVisitor (evaluation.dart), so as to check all expressions '
-        'expected to be constant expressions. Another example of a missing '
-        'error is a field initializer in a class with a constant constructor.',
-  )
   test_defaultValue() async {
-    addTestFile('''
+    await assertErrorsInCode('''
 void f<T>(T a) {}
 class A<U> {
   void m([void Function(U) fn = f<U>]) {}
 }
-''');
-    await resolveTestFile();
-    expect(result.errors, isNotEmpty);
+''', [
+      error(CompileTimeErrorCode.CONST_WITH_TYPE_PARAMETERS_FUNCTION_TEAROFF,
+          65, 1),
+    ]);
   }
 
   test_direct() async {
@@ -123,6 +162,42 @@ class A<U> {
     ]);
   }
 
+  test_fieldValue_constClass() async {
+    await assertErrorsInCode('''
+void f<T>(T a) {}
+class A<U> {
+  const A();
+  final x = f<U>;
+}
+''', [
+      error(CompileTimeErrorCode.CONST_WITH_TYPE_PARAMETERS_FUNCTION_TEAROFF,
+          58, 1),
+    ]);
+  }
+
+  test_fieldValue_extension() async {
+    await assertErrorsInCode('''
+void f<T>(T a) {}
+class A<U> {}
+extension<U> on A<U> {
+  final x = f<U>;
+}
+''', [
+      // An instance field is illegal, but we should not also report an
+      // additional error for the type variable.
+      error(ParserErrorCode.EXTENSION_DECLARES_INSTANCE_FIELD, 63, 1),
+    ]);
+  }
+
+  test_fieldValue_nonConstClass() async {
+    await assertNoErrorsInCode('''
+void f<T>(T a) {}
+class A<U> {
+  final x = f<U>;
+}
+''');
+  }
+
   test_indirect() async {
     await assertErrorsInCode('''
 void f<T>(T a) {}