Report NON_CONSTANT_XYZ in not taken branches of IfElement.
R=brianwilkerson@google.com
Change-Id: I27e8a3b99140046399cdcc8daaa7ecc28ffa62df
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/97010
Reviewed-by: Brian Wilkerson <brianwilkerson@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 8f20c32..a8459c1 100644
--- a/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart
+++ b/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart
@@ -13,6 +13,7 @@
import 'package:analyzer/error/listener.dart';
import 'package:analyzer/src/dart/ast/utilities.dart';
import 'package:analyzer/src/dart/constant/evaluation.dart';
+import 'package:analyzer/src/dart/constant/potentially_constant.dart';
import 'package:analyzer/src/dart/constant/value.dart';
import 'package:analyzer/src/dart/element/element.dart';
import 'package:analyzer/src/error/codes.dart';
@@ -707,13 +708,19 @@
// The errors have already been reported.
if (conditionBool == null) return false;
- var thenValid = !conditionBool || verify(element.thenElement);
-
- var elseValid = conditionBool ||
- element.elseElement == null ||
- verify(element.elseElement);
-
- // TODO(scheglov) Check that not taken branches are constants
+ var thenValid = true;
+ var elseValid = true;
+ if (conditionBool) {
+ thenValid = verify(element.thenElement);
+ if (element.elseElement != null) {
+ elseValid = _reportNotPotentialConstants(element.elseElement);
+ }
+ } else {
+ thenValid = _reportNotPotentialConstants(element.thenElement);
+ if (element.elseElement != null) {
+ elseValid = verify(element.elseElement);
+ }
+ }
return thenValid && elseValid;
} else if (element is MapLiteralEntry) {
@@ -739,6 +746,36 @@
);
}
+ /// Return `true` if the [node] is a potential constant.
+ bool _reportNotPotentialConstants(AstNode node) {
+ var notPotentiallyConstants = getNotPotentiallyConstants(node);
+ if (notPotentiallyConstants.isEmpty) return true;
+
+ for (var notConst in notPotentiallyConstants) {
+ CompileTimeErrorCode errorCode;
+ if (forList) {
+ errorCode = CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT;
+ } else if (forMap) {
+ errorCode = CompileTimeErrorCode.NON_CONSTANT_MAP_ELEMENT;
+ for (var parent = notConst; parent != null; parent = parent.parent) {
+ if (parent is MapLiteralEntry) {
+ if (parent.key == notConst) {
+ errorCode = CompileTimeErrorCode.NON_CONSTANT_MAP_KEY;
+ } else {
+ errorCode = CompileTimeErrorCode.NON_CONSTANT_MAP_VALUE;
+ }
+ break;
+ }
+ }
+ } else if (forSet) {
+ errorCode = CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT;
+ }
+ verifier._errorReporter.reportErrorForNode(errorCode, notConst);
+ }
+
+ return false;
+ }
+
bool _validateListExpression(Expression expression, DartObjectImpl value) {
if (!verifier._evaluationEngine.runtimeTypeMatch(value, listElementType)) {
verifier._errorReporter.reportErrorForNode(
diff --git a/pkg/analyzer/test/src/diagnostics/map_key_type_not_assignable_test.dart b/pkg/analyzer/test/src/diagnostics/map_key_type_not_assignable_test.dart
index 9167018..30eea8f 100644
--- a/pkg/analyzer/test/src/diagnostics/map_key_type_not_assignable_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/map_key_type_not_assignable_test.dart
@@ -94,14 +94,6 @@
''', [StaticWarningCode.MAP_KEY_TYPE_NOT_ASSIGNABLE]);
}
- @failingTest
- test_const_ifElement_thenFalse_notConst() async {
- await assertErrorsInCode('''
-final a = 0;
-var v = const <int, bool>{if (1 > 2) a: true};
-''', [CompileTimeErrorCode.NON_CONSTANT_MAP_KEY]);
- }
-
test_const_ifElement_thenTrue_intInt_dynamic() async {
await assertNoErrorsInCode('''
const dynamic a = 0;
diff --git a/pkg/analyzer/test/src/diagnostics/map_value_type_not_assignable_test.dart b/pkg/analyzer/test/src/diagnostics/map_value_type_not_assignable_test.dart
index 62ae81e..35e036a 100644
--- a/pkg/analyzer/test/src/diagnostics/map_value_type_not_assignable_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/map_value_type_not_assignable_test.dart
@@ -94,14 +94,6 @@
''', [StaticWarningCode.MAP_VALUE_TYPE_NOT_ASSIGNABLE]);
}
- @failingTest
- test_const_ifElement_thenFalse_notConst() async {
- await assertErrorsInCode('''
-final a = 0;
-var v = const <bool, int>{if (1 > 2) true: a};
-''', [CompileTimeErrorCode.NON_CONSTANT_MAP_VALUE]);
- }
-
test_const_ifElement_thenTrue_intInt_dynamic() async {
await assertNoErrorsInCode('''
const dynamic a = 0;
diff --git a/pkg/analyzer/test/src/diagnostics/non_constant_list_element_test.dart b/pkg/analyzer/test/src/diagnostics/non_constant_list_element_test.dart
new file mode 100644
index 0000000..f90a4c4
--- /dev/null
+++ b/pkg/analyzer/test/src/diagnostics/non_constant_list_element_test.dart
@@ -0,0 +1,101 @@
+// 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/dart/analysis/experiments.dart';
+import 'package:analyzer/src/error/codes.dart';
+import 'package:analyzer/src/generated/engine.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/driver_resolution.dart';
+
+main() {
+ defineReflectiveSuite(() {
+ defineReflectiveTests(NonConstantListElementTest);
+ defineReflectiveTests(NonConstantListElementWithUiAsCodeTest);
+ });
+}
+
+@reflectiveTest
+class NonConstantListElementTest extends DriverResolutionTest {
+ test_const_topVar() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const [a];
+''', [CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT]);
+ }
+
+ test_nonConst_topVar() async {
+ await assertNoErrorsInCode('''
+final dynamic a = 0;
+var v = [a];
+''');
+ }
+}
+
+@reflectiveTest
+class NonConstantListElementWithUiAsCodeTest
+ extends NonConstantListElementTest {
+ @override
+ AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()
+ ..enabledExperiments = [
+ EnableString.control_flow_collections,
+ EnableString.spread_collections,
+ ];
+
+ test_const_ifElement_thenElseFalse_finalElse() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const [if (1 < 0) 0 else a];
+''', [CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT]);
+ }
+
+ test_const_ifElement_thenElseFalse_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const [if (1 < 0) a else 0];
+''', [CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT]);
+ }
+
+ test_const_ifElement_thenElseTrue_finalElse() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const [if (1 > 0) 0 else a];
+''', [CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT]);
+ }
+
+ test_const_ifElement_thenElseTrue_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const [if (1 > 0) a else 0];
+''', [CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT]);
+ }
+
+ test_const_ifElement_thenFalse_constThen() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+var v = const [if (1 < 0) a];
+''');
+ }
+
+ test_const_ifElement_thenFalse_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const [if (1 < 0) a];
+''', [CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT]);
+ }
+
+ test_const_ifElement_thenTrue_constThen() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+var v = const [if (1 > 0) a];
+''');
+ }
+
+ test_const_ifElement_thenTrue_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const [if (1 > 0) a];
+''', [CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT]);
+ }
+}
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 eb39184..ddff55a 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
@@ -12,6 +12,10 @@
main() {
defineReflectiveSuite(() {
defineReflectiveTests(NonConstantMapElementWithUiAsCodeTest);
+ defineReflectiveTests(NonConstantMapKeyTest);
+ defineReflectiveTests(NonConstantMapKeyWithUiAsCodeTest);
+ defineReflectiveTests(NonConstantMapValueTest);
+ defineReflectiveTests(NonConstantMapValueWithUiAsCodeTest);
});
}
@@ -100,3 +104,169 @@
''', [CompileTimeErrorCode.NON_CONSTANT_MAP_ELEMENT]);
}
}
+
+@reflectiveTest
+class NonConstantMapKeyTest extends DriverResolutionTest {
+ test_const_topVar() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{a: 0};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_KEY]);
+ }
+
+ test_nonConst_topVar() async {
+ await assertNoErrorsInCode('''
+final dynamic a = 0;
+var v = <int, int>{a: 0};
+''');
+ }
+}
+
+@reflectiveTest
+class NonConstantMapKeyWithUiAsCodeTest extends NonConstantMapKeyTest {
+ @override
+ AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()
+ ..enabledExperiments = [
+ EnableString.control_flow_collections,
+ EnableString.spread_collections,
+ ];
+
+ test_const_ifElement_thenElseFalse_finalElse() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{if (1 < 0) 0: 0 else a: 0};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_KEY]);
+ }
+
+ test_const_ifElement_thenElseFalse_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{if (1 < 0) a: 0 else 0: 0};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_KEY]);
+ }
+
+ test_const_ifElement_thenElseTrue_finalElse() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{if (1 > 0) 0: 0 else a: 0};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_KEY]);
+ }
+
+ test_const_ifElement_thenElseTrue_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{if (1 > 0) a: 0 else 0: 0};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_KEY]);
+ }
+
+ test_const_ifElement_thenFalse_constThen() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+var v = const <int, int>{if (1 < 0) a: 0};
+''');
+ }
+
+ test_const_ifElement_thenFalse_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{if (1 < 0) a: 0};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_KEY]);
+ }
+
+ test_const_ifElement_thenTrue_constThen() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+var v = const <int, int>{if (1 > 0) a: 0};
+''');
+ }
+
+ test_const_ifElement_thenTrue_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{if (1 > 0) a: 0};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_KEY]);
+ }
+}
+
+@reflectiveTest
+class NonConstantMapValueTest extends DriverResolutionTest {
+ test_const_topVar() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{0: a};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_VALUE]);
+ }
+
+ test_nonConst_topVar() async {
+ await assertNoErrorsInCode('''
+final dynamic a = 0;
+var v = <int, int>{0: a};
+''');
+ }
+}
+
+@reflectiveTest
+class NonConstantMapValueWithUiAsCodeTest extends NonConstantMapValueTest {
+ @override
+ AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()
+ ..enabledExperiments = [
+ EnableString.control_flow_collections,
+ EnableString.spread_collections,
+ ];
+
+ test_const_ifElement_thenElseFalse_finalElse() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{if (1 < 0) 0: 0 else 0: a};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_VALUE]);
+ }
+
+ test_const_ifElement_thenElseFalse_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{if (1 < 0) 0: a else 0: 0};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_VALUE]);
+ }
+
+ test_const_ifElement_thenElseTrue_finalElse() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{if (1 > 0) 0: 0 else 0: a};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_VALUE]);
+ }
+
+ test_const_ifElement_thenElseTrue_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{if (1 > 0) 0: a else 0: 0};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_VALUE]);
+ }
+
+ test_const_ifElement_thenFalse_constThen() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+var v = const <int, int>{if (1 < 0) 0: a};
+''');
+ }
+
+ test_const_ifElement_thenFalse_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{if (1 < 0) 0: a};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_VALUE]);
+ }
+
+ test_const_ifElement_thenTrue_constThen() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+var v = const <int, int>{if (1 > 0) 0: a};
+''');
+ }
+
+ test_const_ifElement_thenTrue_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int, int>{if (1 > 0) 0: a};
+''', [CompileTimeErrorCode.NON_CONSTANT_MAP_VALUE]);
+ }
+}
diff --git a/pkg/analyzer/test/src/diagnostics/non_constant_set_element_test.dart b/pkg/analyzer/test/src/diagnostics/non_constant_set_element_test.dart
new file mode 100644
index 0000000..1d8c475
--- /dev/null
+++ b/pkg/analyzer/test/src/diagnostics/non_constant_set_element_test.dart
@@ -0,0 +1,100 @@
+// 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/dart/analysis/experiments.dart';
+import 'package:analyzer/src/error/codes.dart';
+import 'package:analyzer/src/generated/engine.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/driver_resolution.dart';
+
+main() {
+ defineReflectiveSuite(() {
+ defineReflectiveTests(NonConstantSetElementTest);
+ defineReflectiveTests(NonConstantSetElementWithUiAsCodeTest);
+ });
+}
+
+@reflectiveTest
+class NonConstantSetElementTest extends DriverResolutionTest {
+ test_const_topVar() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int>{a};
+''', [CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT]);
+ }
+
+ test_nonConst_topVar() async {
+ await assertNoErrorsInCode('''
+final dynamic a = 0;
+var v = <int>{a};
+''');
+ }
+}
+
+@reflectiveTest
+class NonConstantSetElementWithUiAsCodeTest extends NonConstantSetElementTest {
+ @override
+ AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()
+ ..enabledExperiments = [
+ EnableString.control_flow_collections,
+ EnableString.spread_collections,
+ ];
+
+ test_const_ifElement_thenElseFalse_finalElse() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int>{if (1 < 0) 0 else a};
+''', [CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT]);
+ }
+
+ test_const_ifElement_thenElseFalse_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int>{if (1 < 0) a else 0};
+''', [CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT]);
+ }
+
+ test_const_ifElement_thenElseTrue_finalElse() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int>{if (1 > 0) 0 else a};
+''', [CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT]);
+ }
+
+ test_const_ifElement_thenElseTrue_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int>{if (1 > 0) a else 0};
+''', [CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT]);
+ }
+
+ test_const_ifElement_thenFalse_constThen() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+var v = const <int>{if (1 < 0) a};
+''');
+ }
+
+ test_const_ifElement_thenFalse_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int>{if (1 < 0) a};
+''', [CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT]);
+ }
+
+ test_const_ifElement_thenTrue_constThen() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+var v = const <int>{if (1 > 0) a};
+''');
+ }
+
+ test_const_ifElement_thenTrue_finalThen() async {
+ await assertErrorsInCode('''
+final dynamic a = 0;
+var v = const <int>{if (1 > 0) a};
+''', [CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT]);
+ }
+}
diff --git a/pkg/analyzer/test/src/diagnostics/test_all.dart b/pkg/analyzer/test/src/diagnostics/test_all.dart
index 7f6fdae..27a10b6 100644
--- a/pkg/analyzer/test/src/diagnostics/test_all.dart
+++ b/pkg/analyzer/test/src/diagnostics/test_all.dart
@@ -43,7 +43,9 @@
import 'must_be_immutable_test.dart' as must_be_immutable;
import 'must_call_super_test.dart' as must_call_super;
import 'non_bool_condition_test.dart' as non_bool_condition;
+import 'non_constant_list_element_test.dart' as non_constant_list_element;
import 'non_constant_map_element_test.dart' as non_constant_map_element;
+import 'non_constant_set_element_test.dart' as non_constant_set_element;
import 'set_element_type_not_assignable_test.dart'
as set_element_type_not_assignable;
import 'subtype_of_sealed_class_test.dart' as subtype_of_sealed_class;
@@ -106,7 +108,9 @@
must_be_immutable.main();
must_call_super.main();
non_bool_condition.main();
+ non_constant_list_element.main();
non_constant_map_element.main();
+ non_constant_set_element.main();
set_element_type_not_assignable.main();
subtype_of_sealed_class.main();
top_level_instance_getter.main();
