[analyzer] Refactor visitListLiteral to handle Constants.

Add CONST_FOR_LOOP error message to be clearer that for loops are not
allowed in const contexts.

visitListLiteral handles Constants.

Change-Id: I84465ecf4e1f044c256db45c723fac081b50a40c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/306917
Commit-Queue: Kallen Tu <kallentu@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -380,6 +380,8 @@ CompileTimeErrorCode.CONST_DEFERRED_CLASS:
   status: needsFix
   notes: |-
     Remove the `deferred` keyword from the import.
+CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT:
+  status: needsEvaluation
 CompileTimeErrorCode.CONST_EVAL_THROWS_EXCEPTION:
   status: noFix
 CompileTimeErrorCode.CONST_EVAL_THROWS_IDBZE:

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

@@ -570,6 +570,7 @@ class ConstantVerifier extends RecursiveAstVisitor<void> {
               CompileTimeErrorCode.CONST_CONSTRUCTOR_PARAM_TYPE_MISMATCH) ||
           identical(
               dataErrorCode, CompileTimeErrorCode.VARIABLE_TYPE_MISMATCH) ||
+          identical(dataErrorCode, CompileTimeErrorCode.NON_BOOL_CONDITION) ||
           identical(
               dataErrorCode,
               CompileTimeErrorCode
@@ -1043,7 +1044,8 @@ class _ConstLiteralVerifier {
 
       return true;
     } else if (element is ForElement) {
-      verifier._errorReporter.reportErrorForNode(errorCode, element);
+      verifier._errorReporter.reportErrorForNode(
+          CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT, element);
       return false;
     } else if (element is IfElement) {
       var conditionValue = verifier._validate(element.expression, errorCode);
@@ -1151,6 +1153,9 @@ class _ConstLiteralVerifier {
       if (value.isNull && element.isNullAware) {
         return true;
       }
+      // TODO(kallentu): Consolidate this with
+      // [ConstantVisitor._addElementsToList] and the other similar
+      // _addElementsTo methods..
       verifier._errorReporter.reportErrorForNode(
         CompileTimeErrorCode.CONST_SPREAD_EXPECTED_LIST_OR_SET,
         element.expression,

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

@@ -925,19 +925,20 @@ class ConstantVisitor extends UnifyingAstVisitor<Constant> {
   }
 
   @override
-  Constant? visitListLiteral(ListLiteral node) {
+  Constant visitListLiteral(ListLiteral node) {
     if (!node.isConst) {
+      // TODO(kallentu): Don't report the error here.
       _errorReporter.reportErrorForNode(
           CompileTimeErrorCode.MISSING_CONST_IN_LIST_LITERAL, node);
-      return null;
+      return InvalidConstant(
+          node, CompileTimeErrorCode.MISSING_CONST_IN_LIST_LITERAL);
     }
-    bool errorOccurred = false;
     List<DartObjectImpl> list = [];
     for (CollectionElement element in node.elements) {
-      errorOccurred = errorOccurred | _addElementsToList(list, element);
-    }
-    if (errorOccurred) {
-      return null;
+      var result = _addElementsToList(list, element);
+      if (result is InvalidConstant) {
+        return result;
+      }
     }
     var nodeType = node.staticType;
     DartType elementType =
@@ -1260,42 +1261,64 @@ class ConstantVisitor extends UnifyingAstVisitor<Constant> {
   }
 
   /// Add the entries produced by evaluating the given collection [element] to
-  /// the given [list]. Return `true` if the evaluation of one or more of the
-  /// elements failed.
-  bool _addElementsToList(List<DartObject> list, CollectionElement element) {
-    if (element is ForElement) {
-      _error(element, null);
-    } else if (element is IfElement) {
-      var conditionValue = _evaluateCondition(element.expression);
-      if (conditionValue == null) {
-        return true;
-      } else if (conditionValue) {
-        return _addElementsToList(list, element.thenElement);
-      } else if (element.elseElement != null) {
-        return _addElementsToList(list, element.elseElement!);
-      }
-      return false;
-    } else if (element is Expression) {
-      var value = element.accept(this);
-      if (value == null || value is! DartObjectImpl) {
-        return true;
-      }
-      list.add(value);
-      return false;
-    } else if (element is SpreadElement) {
-      // TODO(kallentu): Remove constant unwrapping.
-      var elementConstant = element.expression.accept(this);
-      var elementResult =
-          elementConstant is DartObjectImpl ? elementConstant : null;
-      var value = elementResult?.toListValue();
-      if (value == null) {
-        return true;
-      }
-      list.addAll(value);
-      return false;
+  /// the given [list]. Return an [InvalidConstant] if the evaluation of one or
+  /// more of the elements failed.
+  InvalidConstant? _addElementsToList(
+      List<DartObject> list, CollectionElement element) {
+    switch (element) {
+      case Expression():
+        var expression = _getConstant(element);
+        switch (expression) {
+          case InvalidConstant():
+            return expression;
+          case DartObjectImpl():
+            list.add(expression);
+            return null;
+        }
+      case ForElement():
+        // TODO(kallentu): Don't report error here.
+        _errorReporter.reportErrorForNode(
+            CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT, element);
+        return InvalidConstant(
+            element, CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT);
+      case IfElement():
+        var condition = _getConstant(element.expression);
+        switch (condition) {
+          case InvalidConstant():
+            return condition;
+          case DartObjectImpl():
+            var conditionValue = condition.toBoolValue();
+            if (conditionValue == null) {
+              // TODO(kallentu): Don't report error here.
+              _errorReporter.reportErrorForNode(
+                  CompileTimeErrorCode.NON_BOOL_CONDITION, element.expression);
+              return InvalidConstant(
+                  element.expression, CompileTimeErrorCode.NON_BOOL_CONDITION);
+            } else if (conditionValue) {
+              return _addElementsToList(list, element.thenElement);
+            } else if (element.elseElement != null) {
+              return _addElementsToList(list, element.elseElement!);
+            }
+            // There's no else element, but the condition value is false.
+            return null;
+        }
+      case MapLiteralEntry():
+        return InvalidConstant(element, CompileTimeErrorCode.INVALID_CONSTANT);
+      case SpreadElement():
+        var spread = _getConstant(element.expression);
+        switch (spread) {
+          case InvalidConstant():
+            return spread;
+          case DartObjectImpl():
+            var listValue = spread.toListValue();
+            if (listValue == null) {
+              return InvalidConstant(element.expression,
+                  CompileTimeErrorCode.CONST_SPREAD_EXPECTED_LIST_OR_SET);
+            }
+            list.addAll(listValue);
+            return null;
+        }
     }
-    // This error should have been reported elsewhere.
-    return true;
   }
 
   /// Add the entries produced by evaluating the given map [element] to the
@@ -1435,6 +1458,9 @@ class ConstantVisitor extends UnifyingAstVisitor<Constant> {
 
   /// Evaluate the given [condition] with the assumption that it must be a
   /// `bool`.
+  ///
+  /// TODO(kallentu): Remove once all methods that call [_evaluateCondition] now
+  /// uses [_getInvalidConstantForConditionTypeMismatch]
   bool? _evaluateCondition(Expression condition) {
     // TODO(kallentu): Remove constant unwrapping.
     var conditionConstant = condition.accept(this);

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

@@ -795,6 +795,14 @@ class CompileTimeErrorCode extends AnalyzerErrorCode {
     hasPublishedDocs: true,
   );
 
+  static const CompileTimeErrorCode CONST_EVAL_FOR_ELEMENT =
+      CompileTimeErrorCode(
+    'CONST_EVAL_FOR_ELEMENT',
+    "Constant expressions don't support 'for' elements.",
+    correctionMessage:
+        "Try replacing the 'for' element with a spread, or removing 'const'.",
+  );
+
   ///  16.12.2 Const: It is a compile-time error if evaluation of a constant
   ///  object results in an uncaught exception being thrown.
   static const CompileTimeErrorCode CONST_EVAL_THROWS_EXCEPTION =

pkg/analyzer/lib/src/error/error_code_values.g.dart

@@ -109,6 +109,7 @@ const List<ErrorCode> errorCodeValues = [
   CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_CONST_SUPER,
   CompileTimeErrorCode.CONST_CONSTRUCTOR_WITH_NON_FINAL_FIELD,
   CompileTimeErrorCode.CONST_DEFERRED_CLASS,
+  CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT,
   CompileTimeErrorCode.CONST_EVAL_THROWS_EXCEPTION,
   CompileTimeErrorCode.CONST_EVAL_THROWS_IDBZE,
   CompileTimeErrorCode.CONST_EVAL_TYPE_BOOL,

pkg/analyzer/lib/src/lint/linter.dart

@@ -907,6 +907,7 @@ class _ConstantAnalysisErrorListener extends AnalysisErrorListener {
         case CompileTimeErrorCode.CONST_EVAL_TYPE_NUM:
         case CompileTimeErrorCode.CONST_EVAL_THROWS_EXCEPTION:
         case CompileTimeErrorCode.CONST_EVAL_THROWS_IDBZE:
+        case CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT:
         case CompileTimeErrorCode.CONST_MAP_KEY_NOT_PRIMITIVE_EQUALITY:
         case CompileTimeErrorCode.CONST_SET_ELEMENT_NOT_PRIMITIVE_EQUALITY:
         case CompileTimeErrorCode.CONST_WITH_NON_CONST:

pkg/analyzer/messages.yaml

@@ -2568,6 +2568,9 @@ CompileTimeErrorCode:
 
       const json2 = convert.JsonCodec();
       ```
+  CONST_EVAL_FOR_ELEMENT:
+    problemMessage: "Constant expressions don't support 'for' elements."
+    correctionMessage: "Try replacing the 'for' element with a spread, or removing 'const'."
   CONST_EVAL_THROWS_EXCEPTION:
     problemMessage: Evaluation of this constant expression throws an exception.
     comment: |-

pkg/analyzer/test/src/dart/constant/evaluation_test.dart

@@ -1304,6 +1304,46 @@ class A {}
     expect(result.toBoolValue(), true);
   }
 
+  test_visitListLiteral_forElement() async {
+    await assertErrorsInCode(r'''
+const x = [for (int i = 0; i < 3; i++) i];
+''', [
+      error(CompileTimeErrorCode.CONST_INITIALIZED_WITH_NON_CONSTANT_VALUE, 10,
+          31),
+      error(CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT, 11, 29),
+    ]);
+    _assertNull('x');
+  }
+
+  test_visitListLiteral_ifElement_nonBoolCondition() async {
+    await assertErrorsInCode(r'''
+const dynamic c = 2;
+const x = [1, if (c) 2 else 3, 4];
+''', [
+      error(CompileTimeErrorCode.NON_BOOL_CONDITION, 39, 1),
+    ]);
+    _assertNull('x');
+  }
+
+  test_visitListLiteral_ifElement_nonBoolCondition_static() async {
+    await assertErrorsInCode(r'''
+const x = [1, if (1) 2 else 3, 4];
+''', [
+      error(CompileTimeErrorCode.NON_BOOL_CONDITION, 18, 1),
+    ]);
+    _assertNull('x');
+  }
+
+  test_visitListLiteral_spreadElement() async {
+    await assertErrorsInCode(r'''
+const dynamic a = 5;
+const x = <int>[...a];
+''', [
+      error(CompileTimeErrorCode.CONST_SPREAD_EXPECTED_LIST_OR_SET, 40, 1),
+    ]);
+    _assertNull('x');
+  }
+
   test_visitPrefixedIdentifier_genericFunction_instantiated() async {
     await resolveTestCode('''
 import '' as self;
@@ -1391,6 +1431,40 @@ Record(int, String, {bool c})
 ''');
   }
 
+  test_visitSetOrMapLiteral_map_forElement() async {
+    await assertErrorsInCode(r'''
+const x = {1: null, for (final i in const []) i: null};
+''', [
+      error(CompileTimeErrorCode.CONST_INITIALIZED_WITH_NON_CONSTANT_VALUE, 10,
+          44),
+      error(CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT, 20, 33),
+    ]);
+    _assertNull('x');
+  }
+
+  test_visitSetOrMapLiteral_map_forElement_nested() async {
+    await assertErrorsInCode(r'''
+const x = {1: null, if (true) for (final i in const []) i: null};
+''', [
+      error(CompileTimeErrorCode.CONST_INITIALIZED_WITH_NON_CONSTANT_VALUE, 10,
+          54),
+      error(CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT, 30, 33),
+    ]);
+    _assertNull('x');
+  }
+
+  test_visitSetOrMapLiteral_set_forElement() async {
+    await assertErrorsInCode(r'''
+const Set set = {};
+const x = {for (final i in set) i};
+''', [
+      error(CompileTimeErrorCode.CONST_INITIALIZED_WITH_NON_CONSTANT_VALUE, 30,
+          24),
+      error(CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT, 31, 22),
+    ]);
+    _assertNull('x');
+  }
+
   test_visitSimpleIdentifier_className() async {
     await resolveTestCode('''
 const a = C;

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

@@ -0,0 +1,69 @@
+// Copyright (c) 2023, 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/context_collection_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(ConstEvalForElementTest);
+  });
+}
+
+@reflectiveTest
+class ConstEvalForElementTest extends PubPackageResolutionTest {
+  test_listLiteral() async {
+    await assertErrorsInCode(r'''
+const x = [for (int i = 0; i < 3; i++) i];
+''', [
+      error(CompileTimeErrorCode.CONST_INITIALIZED_WITH_NON_CONSTANT_VALUE, 10,
+          31),
+      error(CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT, 11, 29),
+    ]);
+  }
+
+  test_listLiteral_forIn() async {
+    await assertErrorsInCode(r'''
+const Set set = {};
+const x = [for(final i in set) i];
+''', [
+      error(CompileTimeErrorCode.CONST_INITIALIZED_WITH_NON_CONSTANT_VALUE, 30,
+          23),
+      error(CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT, 31, 21),
+    ]);
+  }
+
+  test_mapLiteral_forIn() async {
+    await assertErrorsInCode(r'''
+const x = {for (final i in const []) i: null};
+''', [
+      error(CompileTimeErrorCode.CONST_INITIALIZED_WITH_NON_CONSTANT_VALUE, 10,
+          35),
+      error(CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT, 11, 33),
+    ]);
+  }
+
+  test_mapLiteral_forIn_nested() async {
+    await assertErrorsInCode(r'''
+const x = {if (true) for (final i in const []) i: null};
+''', [
+      error(CompileTimeErrorCode.CONST_INITIALIZED_WITH_NON_CONSTANT_VALUE, 10,
+          45),
+      error(CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT, 21, 33),
+    ]);
+  }
+
+  test_setLiteral_forIn() async {
+    await assertErrorsInCode(r'''
+const Set set = {};
+const x = {for (final i in set) i};
+''', [
+      error(CompileTimeErrorCode.CONST_INITIALIZED_WITH_NON_CONSTANT_VALUE, 30,
+          24),
+      error(CompileTimeErrorCode.CONST_EVAL_FOR_ELEMENT, 31, 22),
+    ]);
+  }
+}

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

@@ -255,15 +255,6 @@ const c = [if (1 < 0) nil + 1];
 ''');
   }
 
-  test_ifElement_nonBoolCondition_list() async {
-    await assertErrorsInCode('''
-const dynamic nonBool = 3;
-const c = const [if (nonBool) 'a'];
-''', [
-      error(CompileTimeErrorCode.CONST_EVAL_THROWS_EXCEPTION, 48, 7),
-    ]);
-  }
-
   test_ifElement_nonBoolCondition_map() async {
     await assertErrorsInCode('''
 const dynamic nonBool = null;

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

@@ -27,6 +27,15 @@ var b = const <int>[...a];
     ]);
   }
 
+  test_const_listInt_constVariable() async {
+    await assertErrorsInCode('''
+const dynamic a = 5;
+const x = <int>[...a];
+''', [
+      error(CompileTimeErrorCode.CONST_SPREAD_EXPECTED_LIST_OR_SET, 40, 1),
+    ]);
+  }
+
   test_const_listList() async {
     await assertNoErrorsInCode('''
 const dynamic a = [5];

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

@@ -19,6 +19,23 @@ main() {
 
 @reflectiveTest
 class NonBoolConditionTest extends PubPackageResolutionTest {
+  test_const_list_ifElement() async {
+    await assertErrorsInCode(r'''
+const dynamic c = 2;
+const x = [1, if (c) 2 else 3, 4];
+''', [
+      error(CompileTimeErrorCode.NON_BOOL_CONDITION, 39, 1),
+    ]);
+  }
+
+  test_const_list_ifElement_static() async {
+    await assertErrorsInCode(r'''
+const x = [1, if (1) 2 else 3, 4];
+''', [
+      error(CompileTimeErrorCode.NON_BOOL_CONDITION, 18, 1),
+    ]);
+  }
+
   test_guardedPattern_whenClause() async {
     await assertErrorsInCode(r'''
 void f() {

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

@@ -18,15 +18,6 @@ class NonConstantListElementTest extends PubPackageResolutionTest
     with NonConstantListElementTestCases {}
 
 mixin NonConstantListElementTestCases on PubPackageResolutionTest {
-  test_const_forElement() async {
-    await assertErrorsInCode(r'''
-const Set set = {};
-var v = const [for(final x in set) x];
-''', [
-      error(CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT, 35, 21),
-    ]);
-  }
-
   test_const_ifElement_thenElseFalse_finalElse() async {
     await assertErrorsInCode('''
 final dynamic a = 0;

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

@@ -20,28 +20,6 @@ class NonConstantMapElementTest extends PubPackageResolutionTest
     with NonConstantMapElementTestCases {}
 
 mixin NonConstantMapElementTestCases on PubPackageResolutionTest {
-  test_forElement_cannotBeConst() async {
-    await assertErrorsInCode('''
-void main() {
-  const {1: null, for (final x in const []) null: null};
-}
-''', [
-      error(CompileTimeErrorCode.NON_CONSTANT_MAP_ELEMENT, 32, 36),
-      error(HintCode.UNUSED_LOCAL_VARIABLE, 43, 1),
-    ]);
-  }
-
-  test_forElement_nested_cannotBeConst() async {
-    await assertErrorsInCode('''
-void main() {
-  const {1: null, if (true) for (final x in const []) null: null};
-}
-''', [
-      error(CompileTimeErrorCode.NON_CONSTANT_MAP_ELEMENT, 42, 36),
-      error(HintCode.UNUSED_LOCAL_VARIABLE, 53, 1),
-    ]);
-  }
-
   test_forElement_notConst_noError() async {
     await assertNoErrorsInCode('''
 void main() {

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

@@ -18,15 +18,6 @@ class NonConstantSetElementTest extends PubPackageResolutionTest
     with NonConstantSetElementTestCases {}
 
 mixin NonConstantSetElementTestCases on PubPackageResolutionTest {
-  test_const_forElement() async {
-    await assertErrorsInCode(r'''
-const Set set = {};
-var v = const {for (final x in set) x};
-''', [
-      error(CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT, 35, 22),
-    ]);
-  }
-
   test_const_ifElement_thenElseFalse_finalElse() async {
     await assertErrorsInCode('''
 final dynamic a = 0;

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

@@ -103,6 +103,7 @@ import 'const_constructor_with_non_const_super_test.dart'
 import 'const_constructor_with_non_final_field_test.dart'
     as const_constructor_with_non_final_field;
 import 'const_deferred_class_test.dart' as const_deferred_class;
+import 'const_eval_for_element_test.dart' as const_eval_for_element;
 import 'const_eval_throws_exception_test.dart' as const_eval_throws_exception;
 import 'const_eval_throws_idbze_test.dart' as const_eval_throws_idbze;
 import 'const_eval_type_bool_int_test.dart' as const_eval_type_bool_int;
@@ -950,6 +951,7 @@ main() {
     const_constructor_with_non_const_super.main();
     const_constructor_with_non_final_field.main();
     const_deferred_class.main();
+    const_eval_for_element.main();
     const_eval_throws_exception.main();
     const_eval_throws_idbze.main();
     const_eval_type_bool_int.main();