Fix a bug in the ConstantVerifier so that conditional elements are lazily verified
Change-Id: Ib8b8d5af3dc3a27280116d275616bf26a482bb90
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/96360
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
diff --git a/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart b/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart
index 3373f64..ddffed2 100644
--- a/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart
+++ b/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart
@@ -467,21 +467,32 @@
}
return true;
} else if (element is IfElement) {
- bool validCondition =
- !isConst || _validate(element.condition, errorCode) != null;
- return validCondition &&
- _validateCollectionElement(
+ if (isConst) {
+ DartObject conditionResult = _validate(element.condition, errorCode);
+ bool conditionValue = conditionResult?.toBoolValue();
+ if (conditionValue == null) {
+ // The errors have already been reported.
+ return false;
+ } else if (conditionValue) {
+ return _validateCollectionElement(
element.thenElement, isConst, keys, invalidKeys, errorCode) !=
- null &&
- _validateCollectionElement(
+ null;
+ } else if (element.elseElement != null) {
+ return _validateCollectionElement(
element.elseElement, isConst, keys, invalidKeys, errorCode) !=
null;
+ } else {
+ return true;
+ }
+ }
+ return true;
} else if (element is MapLiteralEntry) {
return _validateMapLiteralEntry(element, isConst, keys, invalidKeys);
} else if (element is SpreadElement) {
return !isConst || _validate(element.expression, errorCode) != null;
}
- return null;
+ throw new UnsupportedError(
+ 'Unhandled type of collection element: ${element.runtimeType}');
}
/// Validate that if the passed arguments are constant expressions.
diff --git a/pkg/analyzer/test/src/dart/constant/evaluation_test.dart b/pkg/analyzer/test/src/dart/constant/evaluation_test.dart
index f0e2140..7b9330f 100644
--- a/pkg/analyzer/test/src/dart/constant/evaluation_test.dart
+++ b/pkg/analyzer/test/src/dart/constant/evaluation_test.dart
@@ -802,6 +802,46 @@
EnableString.spread_collections
];
+ test_listLiteral_ifElement_false_withElse() async {
+ await _resolveTestCode('''
+const c = [1, if (1 < 0) 2 else 3, 4];
+''');
+ DartObjectImpl result = _evaluateConstant('c');
+ expect(
+ result.type, typeProvider.listType.instantiate([typeProvider.intType]));
+ expect(result.toListValue().map((e) => e.toIntValue()), [1, 3, 4]);
+ }
+
+ test_listLiteral_ifElement_false_withoutElse() async {
+ await _resolveTestCode('''
+const c = [1, if (1 < 0) 2, 3];
+''');
+ DartObjectImpl result = _evaluateConstant('c');
+ expect(
+ result.type, typeProvider.listType.instantiate([typeProvider.intType]));
+ expect(result.toListValue().map((e) => e.toIntValue()), [1, 3]);
+ }
+
+ test_listLiteral_ifElement_true_withElse() async {
+ await _resolveTestCode('''
+const c = [1, if (1 > 0) 2 else 3, 4];
+''');
+ DartObjectImpl result = _evaluateConstant('c');
+ expect(
+ result.type, typeProvider.listType.instantiate([typeProvider.intType]));
+ expect(result.toListValue().map((e) => e.toIntValue()), [1, 2, 4]);
+ }
+
+ test_listLiteral_ifElement_true_withoutElse() async {
+ await _resolveTestCode('''
+const c = [1, if (1 > 0) 2, 3];
+''');
+ DartObjectImpl result = _evaluateConstant('c');
+ expect(
+ result.type, typeProvider.listType.instantiate([typeProvider.intType]));
+ expect(result.toListValue().map((e) => e.toIntValue()), [1, 2, 3]);
+ }
+
test_listLiteral_nested() async {
await _resolveTestCode('''
const c = [1, if (1 > 0) if (2 > 1) 2, 3];
@@ -814,47 +854,7 @@
expect(result.toListValue().map((e) => e.toIntValue()), [1, 2, 3]);
}
- test_listLiteral_withIf_false_withElse() async {
- await _resolveTestCode('''
-const c = [1, if (1 < 0) 2 else 3, 4];
-''');
- DartObjectImpl result = _evaluateConstant('c');
- expect(
- result.type, typeProvider.listType.instantiate([typeProvider.intType]));
- expect(result.toListValue().map((e) => e.toIntValue()), [1, 3, 4]);
- }
-
- test_listLiteral_withIf_false_withoutElse() async {
- await _resolveTestCode('''
-const c = [1, if (1 < 0) 2, 3];
-''');
- DartObjectImpl result = _evaluateConstant('c');
- expect(
- result.type, typeProvider.listType.instantiate([typeProvider.intType]));
- expect(result.toListValue().map((e) => e.toIntValue()), [1, 3]);
- }
-
- test_listLiteral_withIf_true_withElse() async {
- await _resolveTestCode('''
-const c = [1, if (1 > 0) 2 else 3, 4];
-''');
- DartObjectImpl result = _evaluateConstant('c');
- expect(
- result.type, typeProvider.listType.instantiate([typeProvider.intType]));
- expect(result.toListValue().map((e) => e.toIntValue()), [1, 2, 4]);
- }
-
- test_listLiteral_withIf_true_withoutElse() async {
- await _resolveTestCode('''
-const c = [1, if (1 > 0) 2, 3];
-''');
- DartObjectImpl result = _evaluateConstant('c');
- expect(
- result.type, typeProvider.listType.instantiate([typeProvider.intType]));
- expect(result.toListValue().map((e) => e.toIntValue()), [1, 2, 3]);
- }
-
- test_listLiteral_withSpread() async {
+ test_listLiteral_spreadElement() async {
await _resolveTestCode('''
const c = [1, ...[2, 3], 4];
''');
@@ -864,6 +864,66 @@
expect(result.toListValue().map((e) => e.toIntValue()), [1, 2, 3, 4]);
}
+ test_mapLiteral_ifElement_false_withElse() async {
+ await _resolveTestCode('''
+const c = {'a' : 1, if (1 < 0) 'b' : 2 else 'c' : 3, 'd' : 4};
+''');
+ DartObjectImpl result = _evaluateConstant('c');
+ expect(
+ result.type,
+ typeProvider.mapType
+ .instantiate([typeProvider.stringType, typeProvider.intType]));
+ Map<DartObject, DartObject> value = result.toMapValue();
+ expect(value.keys.map((e) => e.toStringValue()),
+ unorderedEquals(['a', 'c', 'd']));
+ expect(value.values.map((e) => e.toIntValue()), unorderedEquals([1, 3, 4]));
+ }
+
+ test_mapLiteral_ifElement_false_withoutElse() async {
+ await _resolveTestCode('''
+const c = {'a' : 1, if (1 < 0) 'b' : 2, 'c' : 3};
+''');
+ DartObjectImpl result = _evaluateConstant('c');
+ expect(
+ result.type,
+ typeProvider.mapType
+ .instantiate([typeProvider.stringType, typeProvider.intType]));
+ Map<DartObject, DartObject> value = result.toMapValue();
+ expect(
+ value.keys.map((e) => e.toStringValue()), unorderedEquals(['a', 'c']));
+ expect(value.values.map((e) => e.toIntValue()), unorderedEquals([1, 3]));
+ }
+
+ test_mapLiteral_ifElement_true_withElse() async {
+ await _resolveTestCode('''
+const c = {'a' : 1, if (1 > 0) 'b' : 2 else 'c' : 3, 'd' : 4};
+''');
+ DartObjectImpl result = _evaluateConstant('c');
+ expect(
+ result.type,
+ typeProvider.mapType
+ .instantiate([typeProvider.stringType, typeProvider.intType]));
+ Map<DartObject, DartObject> value = result.toMapValue();
+ expect(value.keys.map((e) => e.toStringValue()),
+ unorderedEquals(['a', 'b', 'd']));
+ expect(value.values.map((e) => e.toIntValue()), unorderedEquals([1, 2, 4]));
+ }
+
+ test_mapLiteral_ifElement_true_withoutElse() async {
+ await _resolveTestCode('''
+const c = {'a' : 1, if (1 > 0) 'b' : 2, 'c' : 3};
+''');
+ DartObjectImpl result = _evaluateConstant('c');
+ expect(
+ result.type,
+ typeProvider.mapType
+ .instantiate([typeProvider.stringType, typeProvider.intType]));
+ Map<DartObject, DartObject> value = result.toMapValue();
+ expect(value.keys.map((e) => e.toStringValue()),
+ unorderedEquals(['a', 'b', 'c']));
+ expect(value.values.map((e) => e.toIntValue()), unorderedEquals([1, 2, 3]));
+ }
+
@failingTest
test_mapLiteral_nested() async {
// Fails because we're not yet parsing nested elements.
@@ -881,67 +941,7 @@
expect(value.values.map((e) => e.toIntValue()), unorderedEquals([1, 2, 3]));
}
- test_mapLiteral_withIf_false_withElse() async {
- await _resolveTestCode('''
-const c = {'a' : 1, if (1 < 0) 'b' : 2 else 'c' : 3, 'd' : 4};
-''');
- DartObjectImpl result = _evaluateConstant('c');
- expect(
- result.type,
- typeProvider.mapType
- .instantiate([typeProvider.stringType, typeProvider.intType]));
- Map<DartObject, DartObject> value = result.toMapValue();
- expect(value.keys.map((e) => e.toStringValue()),
- unorderedEquals(['a', 'c', 'd']));
- expect(value.values.map((e) => e.toIntValue()), unorderedEquals([1, 3, 4]));
- }
-
- test_mapLiteral_withIf_false_withoutElse() async {
- await _resolveTestCode('''
-const c = {'a' : 1, if (1 < 0) 'b' : 2, 'c' : 3};
-''');
- DartObjectImpl result = _evaluateConstant('c');
- expect(
- result.type,
- typeProvider.mapType
- .instantiate([typeProvider.stringType, typeProvider.intType]));
- Map<DartObject, DartObject> value = result.toMapValue();
- expect(
- value.keys.map((e) => e.toStringValue()), unorderedEquals(['a', 'c']));
- expect(value.values.map((e) => e.toIntValue()), unorderedEquals([1, 3]));
- }
-
- test_mapLiteral_withIf_true_withElse() async {
- await _resolveTestCode('''
-const c = {'a' : 1, if (1 > 0) 'b' : 2 else 'c' : 3, 'd' : 4};
-''');
- DartObjectImpl result = _evaluateConstant('c');
- expect(
- result.type,
- typeProvider.mapType
- .instantiate([typeProvider.stringType, typeProvider.intType]));
- Map<DartObject, DartObject> value = result.toMapValue();
- expect(value.keys.map((e) => e.toStringValue()),
- unorderedEquals(['a', 'b', 'd']));
- expect(value.values.map((e) => e.toIntValue()), unorderedEquals([1, 2, 4]));
- }
-
- test_mapLiteral_withIf_true_withoutElse() async {
- await _resolveTestCode('''
-const c = {'a' : 1, if (1 > 0) 'b' : 2, 'c' : 3};
-''');
- DartObjectImpl result = _evaluateConstant('c');
- expect(
- result.type,
- typeProvider.mapType
- .instantiate([typeProvider.stringType, typeProvider.intType]));
- Map<DartObject, DartObject> value = result.toMapValue();
- expect(value.keys.map((e) => e.toStringValue()),
- unorderedEquals(['a', 'b', 'c']));
- expect(value.values.map((e) => e.toIntValue()), unorderedEquals([1, 2, 3]));
- }
-
- test_mapLiteral_withSpread() async {
+ test_mapLiteral_spreadElement() async {
await _resolveTestCode('''
const c = {'a' : 1, ...{'b' : 2, 'c' : 3}, 'd' : 4};
''');
@@ -957,6 +957,46 @@
value.values.map((e) => e.toIntValue()), unorderedEquals([1, 2, 3, 4]));
}
+ test_setLiteral_ifElement_false_withElse() async {
+ await _resolveTestCode('''
+const c = {1, if (1 < 0) 2 else 3, 4};
+''');
+ DartObjectImpl result = _evaluateConstant('c');
+ expect(
+ result.type, typeProvider.setType.instantiate([typeProvider.intType]));
+ expect(result.toSetValue().map((e) => e.toIntValue()), [1, 3, 4]);
+ }
+
+ test_setLiteral_ifElement_false_withoutElse() async {
+ await _resolveTestCode('''
+const c = {1, if (1 < 0) 2, 3};
+''');
+ DartObjectImpl result = _evaluateConstant('c');
+ expect(
+ result.type, typeProvider.setType.instantiate([typeProvider.intType]));
+ expect(result.toSetValue().map((e) => e.toIntValue()), [1, 3]);
+ }
+
+ test_setLiteral_ifElement_true_withElse() async {
+ await _resolveTestCode('''
+const c = {1, if (1 > 0) 2 else 3, 4};
+''');
+ DartObjectImpl result = _evaluateConstant('c');
+ expect(
+ result.type, typeProvider.setType.instantiate([typeProvider.intType]));
+ expect(result.toSetValue().map((e) => e.toIntValue()), [1, 2, 4]);
+ }
+
+ test_setLiteral_ifElement_true_withoutElse() async {
+ await _resolveTestCode('''
+const c = {1, if (1 > 0) 2, 3};
+''');
+ DartObjectImpl result = _evaluateConstant('c');
+ expect(
+ result.type, typeProvider.setType.instantiate([typeProvider.intType]));
+ expect(result.toSetValue().map((e) => e.toIntValue()), [1, 2, 3]);
+ }
+
test_setLiteral_nested() async {
await _resolveTestCode('''
const c = {1, if (1 > 0) if (2 > 1) 2, 3};
@@ -967,47 +1007,7 @@
expect(result.toSetValue().map((e) => e.toIntValue()), [1, 2, 3]);
}
- test_setLiteral_withIf_false_withElse() async {
- await _resolveTestCode('''
-const c = {1, if (1 < 0) 2 else 3, 4};
-''');
- DartObjectImpl result = _evaluateConstant('c');
- expect(
- result.type, typeProvider.setType.instantiate([typeProvider.intType]));
- expect(result.toSetValue().map((e) => e.toIntValue()), [1, 3, 4]);
- }
-
- test_setLiteral_withIf_false_withoutElse() async {
- await _resolveTestCode('''
-const c = {1, if (1 < 0) 2, 3};
-''');
- DartObjectImpl result = _evaluateConstant('c');
- expect(
- result.type, typeProvider.setType.instantiate([typeProvider.intType]));
- expect(result.toSetValue().map((e) => e.toIntValue()), [1, 3]);
- }
-
- test_setLiteral_withIf_true_withElse() async {
- await _resolveTestCode('''
-const c = {1, if (1 > 0) 2 else 3, 4};
-''');
- DartObjectImpl result = _evaluateConstant('c');
- expect(
- result.type, typeProvider.setType.instantiate([typeProvider.intType]));
- expect(result.toSetValue().map((e) => e.toIntValue()), [1, 2, 4]);
- }
-
- test_setLiteral_withIf_true_withoutElse() async {
- await _resolveTestCode('''
-const c = {1, if (1 > 0) 2, 3};
-''');
- DartObjectImpl result = _evaluateConstant('c');
- expect(
- result.type, typeProvider.setType.instantiate([typeProvider.intType]));
- expect(result.toSetValue().map((e) => e.toIntValue()), [1, 2, 3]);
- }
-
- test_setLiteral_withSpread() async {
+ test_setLiteral_spreadElement() async {
await _resolveTestCode('''
const c = {1, ...{2, 3}, 4};
''');
diff --git a/pkg/analyzer/test/src/diagnostics/const_eval_throws_exception_test.dart b/pkg/analyzer/test/src/diagnostics/const_eval_throws_exception_test.dart
index c39bd8d..b7ca6b6 100644
--- a/pkg/analyzer/test/src/diagnostics/const_eval_throws_exception_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/const_eval_throws_exception_test.dart
@@ -2,6 +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/analysis/experiments.dart';
+import 'package:analyzer/src/generated/engine.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
@@ -10,6 +12,7 @@
main() {
defineReflectiveSuite(() {
defineReflectiveTests(ConstEvalThrowsExceptionTest);
+ defineReflectiveTests(ConstEvalThrowsExceptionWithUIAsCodeTest);
});
}
@@ -125,3 +128,28 @@
assertNoTestErrors();
}
}
+
+@reflectiveTest
+class ConstEvalThrowsExceptionWithUIAsCodeTest
+ extends ConstEvalThrowsExceptionTest {
+ @override
+ AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()
+ ..enabledExperiments = [
+ EnableString.control_flow_collections,
+ EnableString.spread_collections,
+ ];
+
+ test_ifElement_false_thenNotEvaluated() async {
+ assertNoErrorsInCode('''
+const dynamic nil = null;
+const c = [if (1 < 0) nil + 1];
+''');
+ }
+
+ test_ifElement_true_elseNotEvaluated() async {
+ assertNoErrorsInCode('''
+const dynamic nil = null;
+const c = [if (0 < 1) 3 else nil + 1];
+''');
+ }
+}
diff --git a/pkg/analyzer/test/src/diagnostics/equal_keys_in_map_test.dart b/pkg/analyzer/test/src/diagnostics/equal_keys_in_map_test.dart
index 2469819..361ac61 100644
--- a/pkg/analyzer/test/src/diagnostics/equal_keys_in_map_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/equal_keys_in_map_test.dart
@@ -24,44 +24,43 @@
EnableString.spread_collections,
];
- test_ifElement_elseBranch_evaluated_addsDuplicate() async {
+ test_ifElement_false_elseEvaluated() async {
await assertErrorsInCode('''
-void main() {
- const notTrue = false;
- const {1: null, if (notTrue) 2: null else 1: null};
-}
+const c = {1: null, if (1 < 0) 2: null else 1: null};
''', [StaticWarningCode.EQUAL_KEYS_IN_MAP]);
}
- test_ifElement_evaluated_addsDuplicate() async {
+ test_ifElement_false_onlyElseEvaluated() async {
+ assertNoErrorsInCode('''
+const c = {if (0 < 1) 1 : 1 else 1 : 2};
+''');
+ }
+
+ test_ifElement_false_thenNotEvaluated() async {
+ assertNoErrorsInCode('''
+const c = {2 : 1, if (1 < 0) 2 : 2};
+''');
+ }
+
+ test_ifElement_true_elseNotEvaluated() async {
+ assertNoErrorsInCode('''
+const c = {1 : 1, if (0 < 1) 2 : 2 else 1 : 2};
+''');
+ }
+
+ test_ifElement_true_onlyThenEvaluated() async {
+ assertNoErrorsInCode('''
+const c = {if (0 < 1) 1 : 1 else 1 : 2};
+''');
+ }
+
+ test_ifElement_true_thenEvaluated() async {
await assertErrorsInCode('''
-void main() {
- const {1: null, if (true) 1: null};
-}
+const c = {1: null, if (0 < 1) 1: null};
''', [StaticWarningCode.EQUAL_KEYS_IN_MAP]);
}
@failingTest
- test_ifElement_notEvaluated_doesntAddDuplicate() async {
- await assertNoErrorsInCode('''
-void main() {
- const notTrue = false;
- const {1: null, if (notTrue) 1: null};
-}
-''');
- }
-
- @failingTest
- test_ifElement_withElse_evaluated_doesntAddDuplicate() async {
- await assertNoErrorsInCode('''
-void main() {
- const isTrue = true;
- const {if (isTrue) 1: null : 1 :null};
-}
-''');
- }
-
- @failingTest
test_nonConst_noDuplicateReported() async {
await assertNoErrorsInCode('''
void main() {
diff --git a/pkg/analyzer/test/src/diagnostics/non_constant_map_element_test.dart b/pkg/analyzer/test/src/diagnostics/non_constant_map_element_test.dart
index 459422f..eb39184 100644
--- a/pkg/analyzer/test/src/diagnostics/non_constant_map_element_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/non_constant_map_element_test.dart
@@ -74,7 +74,6 @@
''', [CompileTimeErrorCode.NON_CONSTANT_MAP_ELEMENT]);
}
- @failingTest
test_ifElementWithElse_mayBeConst() async {
await assertNoErrorsInCode('''
void main() {
