Skip to content

Commit eb0ca50

Browse files
authored
Account for records in type relatedness checks (#4266)
1 parent 0b4787f commit eb0ca50

File tree

3 files changed

+110
-32
lines changed

3 files changed

+110
-32
lines changed

lib/src/util/dart_type_utilities.dart

+44-32
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ import '../analyzer.dart';
1111
import '../ast.dart';
1212
import '../extensions.dart';
1313

14-
typedef AstNodePredicate = bool Function(AstNode node);
15-
1614
bool argumentsMatchParameters(
1715
NodeList<Expression> arguments, NodeList<FormalParameter> parameters) {
1816
var namedParameters = <String, Element?>{};
@@ -145,7 +143,9 @@ bool canonicalElementsFromIdentifiersAreEqual(
145143
/// * are related if their supertypes are equal, e.g. `List<dynamic>` and
146144
/// `Set<dynamic>`.
147145
/// * Two type variables are related if their bounds are related.
148-
/// * Otherwise, the types are related.
146+
/// * A record type is unrelated to any other type except a record type of
147+
/// the same shape.
148+
/// * Otherwise, any two types are related.
149149
// TODO(srawlins): typedefs and functions in general.
150150
bool typesAreUnrelated(
151151
TypeSystem typeSystem, DartType? leftType, DartType? rightType) {
@@ -167,35 +167,8 @@ bool typesAreUnrelated(
167167
return false;
168168
}
169169
if (promotedLeftType is InterfaceType && promotedRightType is InterfaceType) {
170-
// In this case, [leftElement] and [rightElement] each represent
171-
// the same class, like `int`, or `Iterable<String>`.
172-
var leftElement = promotedLeftType.element;
173-
var rightElement = promotedRightType.element;
174-
if (leftElement == rightElement) {
175-
// In this case, [leftElement] and [rightElement] represent the same
176-
// class, modulo generics, e.g. `List<int>` and `List<dynamic>`. Now we
177-
// need to check type arguments.
178-
var leftTypeArguments = promotedLeftType.typeArguments;
179-
var rightTypeArguments = promotedRightType.typeArguments;
180-
if (leftTypeArguments.length != rightTypeArguments.length) {
181-
// I cannot think of how we would enter this block, but it guards
182-
// against RangeError below.
183-
return false;
184-
}
185-
for (var i = 0; i < leftTypeArguments.length; i++) {
186-
// If any of the pair-wise type arguments are unrelated, then
187-
// [leftType] and [rightType] are unrelated.
188-
if (typesAreUnrelated(
189-
typeSystem, leftTypeArguments[i], rightTypeArguments[i])) {
190-
return true;
191-
}
192-
}
193-
// Otherwise, they might be related.
194-
return false;
195-
} else {
196-
return (leftElement.supertype?.isDartCoreObject ?? false) ||
197-
leftElement.supertype != rightElement.supertype;
198-
}
170+
return typeSystem.interfaceTypesAreUnrelated(
171+
promotedLeftType, promotedRightType);
199172
} else if (promotedLeftType is TypeParameterType &&
200173
promotedRightType is TypeParameterType) {
201174
return typesAreUnrelated(typeSystem, promotedLeftType.element.bound,
@@ -208,6 +181,10 @@ bool typesAreUnrelated(
208181
if (_isFunctionTypeUnrelatedToType(promotedRightType, promotedLeftType)) {
209182
return true;
210183
}
184+
} else if (promotedLeftType is RecordType ||
185+
promotedRightType is RecordType) {
186+
return !typeSystem.isAssignableTo(promotedLeftType, promotedRightType) &&
187+
!typeSystem.isAssignableTo(promotedRightType, promotedLeftType);
211188
}
212189
return false;
213190
}
@@ -226,6 +203,8 @@ bool _isFunctionTypeUnrelatedToType(FunctionType type1, DartType type2) {
226203
return true;
227204
}
228205

206+
typedef AstNodePredicate = bool Function(AstNode node);
207+
229208
class DartTypeUtilities {
230209
@Deprecated('Replace with `type.extendsClass`')
231210
static bool extendsClass(
@@ -282,6 +261,39 @@ class InterfaceTypeDefinition {
282261
}
283262
}
284263

264+
extension on TypeSystem {
265+
bool interfaceTypesAreUnrelated(
266+
InterfaceType leftType, InterfaceType rightType) {
267+
var leftElement = leftType.element;
268+
var rightElement = rightType.element;
269+
if (leftElement == rightElement) {
270+
// In this case, [leftElement] and [rightElement] represent the same
271+
// class, modulo generics, e.g. `List<int>` and `List<dynamic>`. Now we
272+
// need to check type arguments.
273+
var leftTypeArguments = leftType.typeArguments;
274+
var rightTypeArguments = rightType.typeArguments;
275+
if (leftTypeArguments.length != rightTypeArguments.length) {
276+
// I cannot think of how we would enter this block, but it guards
277+
// against RangeError below.
278+
return false;
279+
}
280+
for (var i = 0; i < leftTypeArguments.length; i++) {
281+
// If any of the pair-wise type arguments are unrelated, then
282+
// [leftType] and [rightType] are unrelated.
283+
if (typesAreUnrelated(
284+
this, leftTypeArguments[i], rightTypeArguments[i])) {
285+
return true;
286+
}
287+
}
288+
// Otherwise, they might be related.
289+
return false;
290+
} else {
291+
return (leftElement.supertype?.isDartCoreObject ?? false) ||
292+
leftElement.supertype != rightElement.supertype;
293+
}
294+
}
295+
}
296+
285297
extension DartTypeExtensions on DartType {
286298
/// Returns the type which should be used when conducting "interface checks"
287299
/// on `this`.

test/rules/collection_methods_unrelated_type_test.dart

+16
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ var x = <M>[].contains(C());
7171
await assertNoDiagnostics('var x = <num>[].contains(Object());');
7272
}
7373

74+
test_contains_related_records() async {
75+
await assertNoDiagnostics('var x = <(num, num)>[].contains((1, 2));');
76+
}
77+
7478
test_contains_related_subclassOfList() async {
7579
await assertNoDiagnostics('''
7680
abstract class C implements List<num> {}
@@ -118,6 +122,18 @@ abstract class C implements List<num> {
118122
''', [lint(66, 3)]);
119123
}
120124

125+
test_contains_unrelated_records() async {
126+
await assertDiagnostics("var x = <(int, int)>[].contains(('hi', 'hey'));", [
127+
lint(32, 13),
128+
]);
129+
}
130+
131+
test_contains_unrelated_recordAndNonRecord() async {
132+
await assertDiagnostics("var x = <(int, int)>[].contains('hi');", [
133+
lint(32, 4),
134+
]);
135+
}
136+
121137
test_contains_unrelated_subclassOfList() async {
122138
await assertDiagnostics('''
123139
abstract class C implements List<num> {}

test/rules/unrelated_type_equality_checks_test.dart

+50
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,56 @@ class UnrelatedTypeEqualityChecksTestLanguage300 extends LintRuleTest
1818
@override
1919
String get lintRule => 'unrelated_type_equality_checks';
2020

21+
test_recordAndInterfaceType_unrelated() async {
22+
await assertDiagnostics(r'''
23+
bool f((int, int) a, String b) => a == b;
24+
''', [
25+
lint(34, 6),
26+
]);
27+
}
28+
29+
test_records_related() async {
30+
await assertNoDiagnostics(r'''
31+
bool f((int, int) a, (num, num) b) => a == b;
32+
''');
33+
}
34+
35+
test_records_unrelated() async {
36+
await assertDiagnostics(r'''
37+
bool f((int, int) a, (String, String) b) => a == b;
38+
''', [
39+
lint(44, 6),
40+
]);
41+
}
42+
43+
test_recordsWithNamed_related() async {
44+
await assertNoDiagnostics(r'''
45+
bool f(({int one, int two}) a, ({num two, num one}) b) => a == b;
46+
''');
47+
}
48+
49+
test_recordsWithNamed_unrelated() async {
50+
await assertDiagnostics(r'''
51+
bool f(({int one, int two}) a, ({String one, String two}) b) => a == b;
52+
''', [
53+
lint(64, 6),
54+
]);
55+
}
56+
57+
test_recordsWithNamedAndPositional_related() async {
58+
await assertNoDiagnostics(r'''
59+
bool f((int, {int two}) a, (num one, {num two}) b) => a == b;
60+
''');
61+
}
62+
63+
test_recordsWithNamedAndPositional_unrelated() async {
64+
await assertDiagnostics(r'''
65+
bool f((int, {int two}) a, (String one, {String two}) b) => a == b;
66+
''', [
67+
lint(60, 6),
68+
]);
69+
}
70+
2171
@FailingTest(
2272
reason: 'Error code refactoring',
2373
issue: 'https://github.com/dart-lang/linter/issues/4256')

0 commit comments

Comments
 (0)