Check types of values in constant List/Set against the element types.
R=brianwilkerson@google.com, paulberry@google.com
Change-Id: I19c2b37596dae8fd9f3afb1fadf842be4c944b44
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/96983
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 8531759..7234756 100644
--- a/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart
+++ b/pkg/analyzer/lib/src/dart/constant/constant_verifier.dart
@@ -144,6 +144,8 @@
void visitListLiteral(ListLiteral node) {
super.visitListLiteral(node);
if (node.isConst) {
+ InterfaceType nodeType = node.staticType;
+ DartType elementType = nodeType.typeArguments[0];
// Dummy sets of keys to accommodate the fact that
// `_validateCollectionElement` handles map literals.
HashSet<DartObject> keys = new HashSet<DartObject>();
@@ -151,7 +153,7 @@
for (CollectionElement element in node.elements2) {
bool isValid = _validateCollectionElement(element, true, keys,
invalidKeys, CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT,
- forList: true);
+ forList: true, listElementType: elementType);
if (isValid && element is Expression) {
// TODO(brianwilkerson) Handle the other kinds of elements.
_reportErrorIfFromDeferredLibrary(
@@ -176,41 +178,20 @@
HashSet<DartObject> keys = new HashSet<DartObject>();
List<Expression> invalidKeys = new List<Expression>();
if (node.isSet) {
- HashSet<DartObject> elements = new HashSet<DartObject>();
- List<Expression> invalidElements = new List<Expression>();
if (isConst) {
+ InterfaceType nodeType = node.staticType;
+ var elementType = nodeType.typeArguments[0];
+ var uniqueValues = Set<DartObject>();
+ var duplicateElements = <Expression>[];
for (CollectionElement element in node.elements2) {
- if (element is Expression) {
- // TODO(mfairhurst): unify this with _validateCollectionElemet
- DartObject result = _validate(
- element, CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT);
- if (result != null) {
- _reportErrorIfFromDeferredLibrary(
- element,
- CompileTimeErrorCode
- .NON_CONSTANT_SET_ELEMENT_FROM_DEFERRED_LIBRARY);
- if (!elements.add(result)) {
- invalidElements.add(element);
- }
- DartType type = result.type;
- if (_implementsEqualsWhenNotAllowed(type)) {
- _errorReporter.reportErrorForNode(
- CompileTimeErrorCode
- .CONST_SET_ELEMENT_TYPE_IMPLEMENTS_EQUALS,
- element,
- [type.displayName]);
- }
- }
- } else {
- bool isValid = _validateCollectionElement(element, isConst, keys,
- invalidKeys, CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT,
- forSet: true);
- if (isValid) {
- // TODO(mfairhurst) report deferred library error
- }
- }
+ _validateCollectionElement(element, isConst, keys, invalidKeys,
+ CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT,
+ forSet: true,
+ setElementType: elementType,
+ setElements: uniqueValues,
+ setElementsDuplicate: duplicateElements);
}
- for (var invalidElement in invalidElements) {
+ for (var invalidElement in duplicateElements) {
_errorReporter.reportErrorForNode(
StaticWarningCode.EQUAL_VALUES_IN_CONST_SET, invalidElement);
}
@@ -462,12 +443,62 @@
HashSet<DartObject> keys,
List<Expression> invalidKeys,
ErrorCode errorCode, {
+ DartType listElementType,
+ DartType mapKeyType,
+ DartType mapValueType,
+ DartType setElementType,
+ Set<DartObject> setElements,
+ List<CollectionElement> setElementsDuplicate,
bool forList = false,
bool forMap = false,
bool forSet = false,
}) {
if (element is Expression) {
- return !isConst || _validate(element, errorCode) != null;
+ if (!isConst) return true;
+
+ var value = _validate(element, errorCode);
+ if (value == null) return false;
+
+ if (forList) {
+ if (!_evaluationEngine.runtimeTypeMatch(value, listElementType)) {
+ _errorReporter.reportErrorForNode(
+ StaticWarningCode.LIST_ELEMENT_TYPE_NOT_ASSIGNABLE,
+ element,
+ [value.type, listElementType],
+ );
+ return false;
+ }
+ }
+
+ if (forSet) {
+ if (!_evaluationEngine.runtimeTypeMatch(value, setElementType)) {
+ _errorReporter.reportErrorForNode(
+ StaticWarningCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE,
+ element,
+ [value.type, setElementType],
+ );
+ return false;
+ }
+
+ if (_implementsEqualsWhenNotAllowed(value.type)) {
+ _errorReporter.reportErrorForNode(
+ CompileTimeErrorCode.CONST_SET_ELEMENT_TYPE_IMPLEMENTS_EQUALS,
+ element,
+ [value.type.displayName]);
+ return false;
+ }
+
+ _reportErrorIfFromDeferredLibrary(
+ element,
+ CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT_FROM_DEFERRED_LIBRARY,
+ );
+
+ if (!setElements.add(value)) {
+ setElementsDuplicate.add(element);
+ }
+ }
+
+ return true;
} else if (element is ForElement) {
if (isConst) {
_errorReporter.reportErrorForNode(errorCode, element);
@@ -483,11 +514,39 @@
return false;
} else if (conditionValue) {
return _validateCollectionElement(
- element.thenElement, isConst, keys, invalidKeys, errorCode) !=
+ element.thenElement,
+ isConst,
+ keys,
+ invalidKeys,
+ errorCode,
+ forList: forList,
+ forMap: forMap,
+ forSet: forSet,
+ listElementType: listElementType,
+ mapKeyType: mapKeyType,
+ mapValueType: mapValueType,
+ setElementType: setElementType,
+ setElements: setElements,
+ setElementsDuplicate: setElementsDuplicate,
+ ) !=
null;
} else if (element.elseElement != null) {
return _validateCollectionElement(
- element.elseElement, isConst, keys, invalidKeys, errorCode) !=
+ element.elseElement,
+ isConst,
+ keys,
+ invalidKeys,
+ errorCode,
+ forList: forList,
+ forMap: forMap,
+ forSet: forSet,
+ listElementType: listElementType,
+ mapKeyType: mapKeyType,
+ mapValueType: mapValueType,
+ setElementType: setElementType,
+ setElements: setElements,
+ setElementsDuplicate: setElementsDuplicate,
+ ) !=
null;
} else {
return true;
diff --git a/pkg/analyzer/lib/src/generated/error_verifier.dart b/pkg/analyzer/lib/src/generated/error_verifier.dart
index 8dc85c4..8bd952c 100644
--- a/pkg/analyzer/lib/src/generated/error_verifier.dart
+++ b/pkg/analyzer/lib/src/generated/error_verifier.dart
@@ -3934,6 +3934,9 @@
* [StaticWarningCode.LIST_ELEMENT_TYPE_NOT_ASSIGNABLE].
*/
void _checkForListElementTypeNotAssignable(ListLiteral literal) {
+ // Constants are verified using their actual values.
+ if (literal.isConst) return;
+
// Determine the list's element type. We base this on the static type and
// not the literal's type arguments because in strong mode, the type
// arguments may be inferred.
@@ -5229,6 +5232,9 @@
* [StaticWarningCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE].
*/
void _checkForSetElementTypeNotAssignable3(SetOrMapLiteral literal) {
+ // Constants are verified using their actual values.
+ if (literal.isConst) return;
+
// Determine the set's element type. We base this on the static type and
// not the literal's type arguments because in strong mode, the type
// arguments may be inferred.
diff --git a/pkg/analyzer/test/src/diagnostics/invalid_cast_new_expr_test.dart b/pkg/analyzer/test/src/diagnostics/invalid_cast_new_expr_test.dart
index f1c94b4..7ea31a2 100644
--- a/pkg/analyzer/test/src/diagnostics/invalid_cast_new_expr_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/invalid_cast_new_expr_test.dart
@@ -29,7 +29,10 @@
class B extends A {
const B();
}
-''', [StrongModeCode.INVALID_CAST_NEW_EXPR]);
+''', [
+ StaticWarningCode.LIST_ELEMENT_TYPE_NOT_ASSIGNABLE,
+ StrongModeCode.INVALID_CAST_NEW_EXPR,
+ ]);
}
test_listLiteral_nonConst() async {
@@ -53,7 +56,10 @@
class B extends A {
const B();
}
-''', [StrongModeCode.INVALID_CAST_NEW_EXPR]);
+''', [
+ StaticWarningCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE,
+ StrongModeCode.INVALID_CAST_NEW_EXPR,
+ ]);
}
test_setLiteral_nonConst() async {
diff --git a/pkg/analyzer/test/src/diagnostics/list_element_type_not_assignable_test.dart b/pkg/analyzer/test/src/diagnostics/list_element_type_not_assignable_test.dart
index 148844c..4d3cc5f 100644
--- a/pkg/analyzer/test/src/diagnostics/list_element_type_not_assignable_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/list_element_type_not_assignable_test.dart
@@ -30,12 +30,11 @@
''');
}
- @FailingTest(issue: 'https://github.com/dart-lang/sdk/issues/21119')
test_explicitTypeArgs_const_actualTypeMismatch() async {
await assertErrorsInCode('''
const dynamic x = 42;
var v = const <String>[x];
-''', [CheckedModeCompileTimeErrorCode.LIST_ELEMENT_TYPE_NOT_ASSIGNABLE]);
+''', [StaticWarningCode.LIST_ELEMENT_TYPE_NOT_ASSIGNABLE]);
}
test_explicitTypeArgs_notConst() async {
@@ -52,15 +51,82 @@
AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()
..enabledExperiments = ['control-flow-collections', 'spread-collections'];
- test_spread_valid_const() async {
+ test_const_ifElement_thenElseFalse_intInt() async {
await assertNoErrorsInCode('''
-var v = const <String>[...['a', 'b']];
+const dynamic a = 0;
+const dynamic b = 0;
+var v = <int>[if (1 < 0) a else b];
''');
}
- test_spread_valid_nonConst() async {
+ test_const_ifElement_thenElseFalse_intString() async {
+ await assertErrorsInCode('''
+const dynamic a = 0;
+const dynamic b = 'b';
+var v = const <int>[if (1 < 0) a else b];
+''', [StaticWarningCode.LIST_ELEMENT_TYPE_NOT_ASSIGNABLE]);
+ }
+
+ test_const_ifElement_thenFalse_intString() async {
await assertNoErrorsInCode('''
-var v = <String>[...['a', 'b']];
+const dynamic a = 'a';
+var v = const <int>[if (1 < 0) a];
+''');
+ }
+
+ test_const_ifElement_thenTrue_intInt() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+var v = <int>[if (true) a];
+''');
+ }
+
+ test_const_ifElement_thenTrue_intString() async {
+ await assertErrorsInCode('''
+const dynamic a = 'a';
+var v = const <int>[if (true) a];
+''', [StaticWarningCode.LIST_ELEMENT_TYPE_NOT_ASSIGNABLE]);
+ }
+
+ test_const_spread_intInt() async {
+ await assertNoErrorsInCode('''
+var v = const <int>[...[0, 1]];
+''');
+ }
+
+ test_nonConst_ifElement_thenElseFalse_intDynamic() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 'a';
+const dynamic b = 'b';
+var v = <int>[if (1 < 0) a else b];
+''');
+ }
+
+ test_nonConst_ifElement_thenElseFalse_intInt() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+const dynamic b = 0;
+var v = <int>[if (1 < 0) a else b];
+''');
+ }
+
+ test_nonConst_ifElement_thenTrue_intDynamic() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 'a';
+var v = <int>[if (true) a];
+''');
+ }
+
+ test_nonConst_ifElement_thenTrue_intInt() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+var v = <int>[if (true) a];
+''');
+ }
+
+ test_nonConst_spread_intInt() async {
+ await assertNoErrorsInCode('''
+var v = <int>[...[0, 1]];
''');
}
}
diff --git a/pkg/analyzer/test/src/diagnostics/set_element_type_not_assignable_test.dart b/pkg/analyzer/test/src/diagnostics/set_element_type_not_assignable_test.dart
index 61ebabe..54f9514 100644
--- a/pkg/analyzer/test/src/diagnostics/set_element_type_not_assignable_test.dart
+++ b/pkg/analyzer/test/src/diagnostics/set_element_type_not_assignable_test.dart
@@ -52,15 +52,82 @@
AnalysisOptionsImpl get analysisOptions => AnalysisOptionsImpl()
..enabledExperiments = ['control-flow-collections', 'spread-collections'];
- test_spread_valid_const() async {
+ test_const_ifElement_thenElseFalse_intInt() async {
await assertNoErrorsInCode('''
-var v = const <String>{...['a', 'b']};
+const dynamic a = 0;
+const dynamic b = 0;
+var v = <int>{if (1 < 0) a else b};
''');
}
- test_spread_valid_nonConst() async {
+ test_const_ifElement_thenElseFalse_intString() async {
+ await assertErrorsInCode('''
+const dynamic a = 0;
+const dynamic b = 'b';
+var v = const <int>{if (1 < 0) a else b};
+''', [StaticWarningCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE]);
+ }
+
+ test_const_ifElement_thenFalse_intString() async {
await assertNoErrorsInCode('''
-var v = <String>{...['a', 'b']};
+const dynamic a = 'a';
+var v = const <int>{if (1 < 0) a};
+''');
+ }
+
+ test_const_ifElement_thenTrue_intInt() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+var v = <int>{if (true) a};
+''');
+ }
+
+ test_const_ifElement_thenTrue_intString() async {
+ await assertErrorsInCode('''
+const dynamic a = 'a';
+var v = const <int>{if (true) a};
+''', [StaticWarningCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE]);
+ }
+
+ test_const_spread_intInt() async {
+ await assertNoErrorsInCode('''
+var v = const <int>{...[0, 1]};
+''');
+ }
+
+ test_nonConst_ifElement_thenElseFalse_intDynamic() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 'a';
+const dynamic b = 'b';
+var v = <int>{if (1 < 0) a else b};
+''');
+ }
+
+ test_nonConst_ifElement_thenElseFalse_intInt() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+const dynamic b = 0;
+var v = <int>{if (1 < 0) a else b};
+''');
+ }
+
+ test_nonConst_ifElement_thenTrue_intDynamic() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 'a';
+var v = <int>{if (true) a};
+''');
+ }
+
+ test_nonConst_ifElement_thenTrue_intInt() async {
+ await assertNoErrorsInCode('''
+const dynamic a = 0;
+var v = <int>{if (true) a};
+''');
+ }
+
+ test_nonConst_spread_intInt() async {
+ await assertNoErrorsInCode('''
+var v = <int>{...[0, 1]};
''');
}
}