Skip to content
This repository was archived by the owner on Jul 16, 2023. It is now read-only.

Commit 6e5e136

Browse files
fzyzcjydkrutskikh
fzyzcjy
authored and
dkrutskikh
committed
feat: Create new rule avoid_collection_methods_with_unrelated_types (#705)
* add sccafold for new rule * add scaffold for tests * try to implement the rule * add tests * add toString to debug issues * pass tests * add doc * add more functions in example * refactor visitor to extract more generic info * try to implement containsKey, containsValue, remove * pass tests * add checking `Iterable.contains`, pass tests * revert issue.tostring * run dart format * refactor code * improve doc * try to also check subclasses of map (not finished) * refactor dart_type_utils to avoid duplication * make test passes * minor change to tests * refactor and make tests pass * support List.remove * change doc * support sets * make tests pass * changelog
1 parent d9abbf0 commit 6e5e136

File tree

9 files changed

+521
-9
lines changed

9 files changed

+521
-9
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
* feat: add static code diagnostic `avoid-collection-methods-with-unrelated-types`
6+
37
## 4.11.0
48

59
* feat: add static code diagnostics `format-comment`.

‎lib/src/analyzers/lint_analyzer/rules/rules_factory.dart‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'models/rule.dart';
22
import 'rules_list/always_remove_listener/always_remove_listener_rule.dart';
33
import 'rules_list/avoid_border_all/avoid_border_all_rule.dart';
4+
import 'rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart';
45
import 'rules_list/avoid_dynamic/avoid_dynamic_rule.dart';
56
import 'rules_list/avoid_global_state/avoid_global_state_rule.dart';
67
import 'rules_list/avoid_ignoring_return_values/avoid_ignoring_return_values_rule.dart';
@@ -48,6 +49,8 @@ import 'rules_list/provide_correct_intl_args/provide_correct_intl_args_rule.dart
4849
final _implementedRules = <String, Rule Function(Map<String, Object>)>{
4950
AlwaysRemoveListenerRule.ruleId: (config) => AlwaysRemoveListenerRule(config),
5051
AvoidBorderAllRule.ruleId: (config) => AvoidBorderAllRule(config),
52+
AvoidCollectionMethodsWithUnrelatedTypesRule.ruleId: (config) =>
53+
AvoidCollectionMethodsWithUnrelatedTypesRule(config),
5154
AvoidDynamicRule.ruleId: (config) => AvoidDynamicRule(config),
5255
AvoidGlobalStateRule.ruleId: (config) => AvoidGlobalStateRule(config),
5356
AvoidIgnoringReturnValuesRule.ruleId: (config) =>

‎lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// ignore_for_file: public_member_api_docs
2+
3+
import 'package:analyzer/dart/ast/ast.dart';
4+
import 'package:analyzer/dart/ast/visitor.dart';
5+
import 'package:analyzer/dart/element/element.dart';
6+
import 'package:analyzer/dart/element/type.dart';
7+
import 'package:collection/collection.dart';
8+
9+
import '../../../../../utils/dart_types_utils.dart';
10+
import '../../../../../utils/node_utils.dart';
11+
import '../../../lint_utils.dart';
12+
import '../../../models/internal_resolved_unit_result.dart';
13+
import '../../../models/issue.dart';
14+
import '../../../models/severity.dart';
15+
import '../../models/common_rule.dart';
16+
import '../../rule_utils.dart';
17+
18+
part 'visitor.dart';
19+
20+
class AvoidCollectionMethodsWithUnrelatedTypesRule extends CommonRule {
21+
static const String ruleId = 'avoid-collection-methods-with-unrelated-types';
22+
23+
static const _warning = 'Avoid collection methods with unrelated types.';
24+
25+
AvoidCollectionMethodsWithUnrelatedTypesRule(
26+
[Map<String, Object> config = const {}])
27+
: super(
28+
id: ruleId,
29+
severity: readSeverity(config, Severity.warning),
30+
excludes: readExcludes(config),
31+
);
32+
33+
@override
34+
Iterable<Issue> check(InternalResolvedUnitResult source) {
35+
final visitor = _Visitor();
36+
37+
source.unit.visitChildren(visitor);
38+
39+
return visitor.expressions
40+
.map((expression) => createIssue(
41+
rule: this,
42+
location: nodeLocation(
43+
node: expression,
44+
source: source,
45+
),
46+
message: _warning,
47+
))
48+
.toList(growable: false);
49+
}
50+
}

‎lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
part of 'avoid_collection_methods_with_unrelated_types_rule.dart';
2+
3+
class _Visitor extends RecursiveAstVisitor<void> {
4+
final _expressions = <Expression>[];
5+
6+
Iterable<Expression> get expressions => _expressions;
7+
8+
// for `operator []` and `operator []=`
9+
@override
10+
void visitIndexExpression(IndexExpression node) {
11+
super.visitIndexExpression(node);
12+
13+
final mapType = _getMapTypeElement(node.target?.staticType);
14+
_addIfNotSubType(node.index.staticType, mapType?.first, node);
15+
}
16+
17+
// for things like `map.containsKey`
18+
@override
19+
void visitMethodInvocation(MethodInvocation node) {
20+
super.visitMethodInvocation(node);
21+
22+
final mapType = _getMapTypeElement(node.target?.staticType);
23+
final listType = _getListTypeElement(node.target?.staticType);
24+
final setType = _getSetTypeElement(node.target?.staticType);
25+
final iterableType = _getIterableTypeElement(node.target?.staticType);
26+
final argType = node.argumentList.arguments.singleOrNull?.staticType;
27+
28+
switch (node.methodName.name) {
29+
case 'containsKey':
30+
_addIfNotSubType(argType, mapType?.first, node);
31+
break;
32+
case 'remove':
33+
_addIfNotSubType(argType, mapType?.first, node);
34+
_addIfNotSubType(argType, listType, node);
35+
_addIfNotSubType(argType, setType, node);
36+
break;
37+
case 'lookup':
38+
_addIfNotSubType(argType, setType, node);
39+
break;
40+
case 'containsValue':
41+
_addIfNotSubType(argType, mapType?[1], node);
42+
break;
43+
case 'contains':
44+
_addIfNotSubType(argType, iterableType, node);
45+
break;
46+
case 'containsAll':
47+
case 'removeAll':
48+
case 'retainAll':
49+
final argAsIterableParamType = _getIterableTypeElement(argType);
50+
_addIfNotSubType(argAsIterableParamType?.type, setType, node);
51+
break;
52+
case 'difference':
53+
case 'intersection':
54+
final argAsSetParamType = _getSetTypeElement(argType);
55+
_addIfNotSubType(argAsSetParamType?.type, setType, node);
56+
break;
57+
}
58+
}
59+
60+
void _addIfNotSubType(
61+
DartType? childType,
62+
_TypedClassElement? parentElement,
63+
Expression node,
64+
) {
65+
if (parentElement != null &&
66+
childType != null &&
67+
childType.asInstanceOf(parentElement.element) == null) {
68+
_expressions.add(node);
69+
}
70+
}
71+
72+
List<_TypedClassElement>? _getMapTypeElement(DartType? type) =>
73+
_getTypeArgElements(getSupertypeMap(type));
74+
75+
_TypedClassElement? _getIterableTypeElement(DartType? type) =>
76+
_getTypeArgElements(getSupertypeIterable(type))?.singleOrNull;
77+
78+
_TypedClassElement? _getListTypeElement(DartType? type) =>
79+
_getTypeArgElements(getSupertypeList(type))?.singleOrNull;
80+
81+
_TypedClassElement? _getSetTypeElement(DartType? type) =>
82+
_getTypeArgElements(getSupertypeSet(type))?.singleOrNull;
83+
84+
List<_TypedClassElement>? _getTypeArgElements(DartType? type) {
85+
if (type == null || type is! ParameterizedType) {
86+
return null;
87+
}
88+
89+
final typeArgElements = type.typeArguments
90+
.map((typeArg) {
91+
final element = typeArg.element;
92+
93+
return element is ClassElement
94+
? _TypedClassElement(typeArg, element)
95+
: null;
96+
})
97+
.whereNotNull()
98+
.toList();
99+
if (typeArgElements.length < type.typeArguments.length) {
100+
return null;
101+
}
102+
103+
return typeArgElements;
104+
}
105+
}
106+
107+
class _TypedClassElement {
108+
final DartType type;
109+
final ClassElement element;
110+
111+
_TypedClassElement(this.type, this.element);
112+
}

‎lib/src/utils/dart_types_utils.dart‎

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,46 @@
11
// ignore_for_file: public_member_api_docs
22

33
import 'package:analyzer/dart/element/type.dart';
4+
import 'package:collection/collection.dart';
45

56
bool isIterableOrSubclass(DartType? type) =>
6-
_isIterable(type) || _isSubclassOfIterable(type);
7+
_checkSelfOrSupertypes(type, (t) => t?.isDartCoreIterable ?? false);
78

8-
bool _isIterable(DartType? type) => type?.isDartCoreIterable ?? false;
9+
bool isListOrSubclass(DartType? type) =>
10+
_checkSelfOrSupertypes(type, (t) => t?.isDartCoreList ?? false);
911

10-
bool _isSubclassOfIterable(DartType? type) =>
11-
type is InterfaceType && type.allSupertypes.any(_isIterable);
12+
bool isMapOrSubclass(DartType? type) =>
13+
_checkSelfOrSupertypes(type, (t) => t?.isDartCoreMap ?? false);
1214

13-
bool isListOrSubclass(DartType? type) =>
14-
_isList(type) || _isSubclassOfList(type);
15+
DartType? getSupertypeIterable(DartType? type) =>
16+
_getSelfOrSupertypes(type, (t) => t?.isDartCoreIterable ?? false);
17+
18+
DartType? getSupertypeList(DartType? type) =>
19+
_getSelfOrSupertypes(type, (t) => t?.isDartCoreList ?? false);
20+
21+
DartType? getSupertypeSet(DartType? type) =>
22+
_getSelfOrSupertypes(type, (t) => t?.isDartCoreSet ?? false);
23+
24+
DartType? getSupertypeMap(DartType? type) =>
25+
_getSelfOrSupertypes(type, (t) => t?.isDartCoreMap ?? false);
26+
27+
bool _checkSelfOrSupertypes(
28+
DartType? type,
29+
bool Function(DartType?) predicate,
30+
) =>
31+
predicate(type) ||
32+
(type is InterfaceType && type.allSupertypes.any(predicate));
1533

16-
bool _isList(DartType? type) => type?.isDartCoreList ?? false;
34+
DartType? _getSelfOrSupertypes(
35+
DartType? type,
36+
bool Function(DartType?) predicate,
37+
) {
38+
if (predicate(type)) {
39+
return type;
40+
}
41+
if (type is InterfaceType) {
42+
return type.allSupertypes.firstWhereOrNull(predicate);
43+
}
1744

18-
bool _isSubclassOfList(DartType? type) =>
19-
type is InterfaceType && type.allSupertypes.any(_isList);
45+
return null;
46+
}

‎test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart';
2+
import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart';
3+
import 'package:test/test.dart';
4+
5+
import '../../../../../helpers/rule_test_helper.dart';
6+
7+
const _examplePath =
8+
'avoid_collection_methods_with_unrelated_types/examples/example.dart';
9+
10+
void main() {
11+
group('AvoidCollectionMethodsWithUnrelatedTypesRule', () {
12+
test('initialization', () async {
13+
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
14+
final issues = AvoidCollectionMethodsWithUnrelatedTypesRule().check(unit);
15+
16+
RuleTestHelper.verifyInitialization(
17+
issues: issues,
18+
ruleId: 'avoid-collection-methods-with-unrelated-types',
19+
severity: Severity.warning,
20+
);
21+
});
22+
23+
test('reports about found issues', () async {
24+
final unit = await RuleTestHelper.resolveFromFile(_examplePath);
25+
final issues = AvoidCollectionMethodsWithUnrelatedTypesRule().check(unit);
26+
27+
RuleTestHelper.verifyIssues(
28+
issues: issues,
29+
startLines: [
30+
6,
31+
9,
32+
12,
33+
15,
34+
18,
35+
27,
36+
32,
37+
35,
38+
38,
39+
41,
40+
48,
41+
50,
42+
55,
43+
58,
44+
61,
45+
67,
46+
74,
47+
77,
48+
80,
49+
83,
50+
86,
51+
89,
52+
92,
53+
95,
54+
],
55+
startColumns: [
56+
5,
57+
16,
58+
5,
59+
5,
60+
5,
61+
5,
62+
16,
63+
5,
64+
5,
65+
5,
66+
14,
67+
5,
68+
5,
69+
5,
70+
5,
71+
5,
72+
5,
73+
5,
74+
5,
75+
5,
76+
5,
77+
5,
78+
5,
79+
5,
80+
],
81+
locationTexts: [
82+
'primitiveMap["str"]',
83+
'primitiveMap["str"]',
84+
'primitiveMap.containsKey("str")',
85+
'primitiveMap.containsValue(100)',
86+
'primitiveMap.remove("str")',
87+
'inheritanceMap[Flower()]',
88+
'inheritanceMap[Flower()]',
89+
'inheritanceMap.containsKey(Flower())',
90+
'inheritanceMap.containsValue(DogImplementsAnimal())',
91+
'inheritanceMap.remove(Flower())',
92+
'myMap["str"]',
93+
'myMap.containsKey("str")',
94+
'<int>[1, 2, 3].contains("str")',
95+
'Iterable<int>.generate(10).contains("str")',
96+
'<int>{1, 2, 3}.contains("str")',
97+
'primitiveList.remove("str")',
98+
'primitiveSet.contains("str")',
99+
'primitiveSet.containsAll(Iterable<String>.empty())',
100+
'primitiveSet.difference(<String>{})',
101+
'primitiveSet.intersection(<String>{})',
102+
'primitiveSet.lookup("str")',
103+
'primitiveSet.remove("str")',
104+
'primitiveSet.removeAll(Iterable<String>.empty())',
105+
'primitiveSet.retainAll(Iterable<String>.empty())',
106+
],
107+
messages: [
108+
'Avoid collection methods with unrelated types.',
109+
'Avoid collection methods with unrelated types.',
110+
'Avoid collection methods with unrelated types.',
111+
'Avoid collection methods with unrelated types.',
112+
'Avoid collection methods with unrelated types.',
113+
'Avoid collection methods with unrelated types.',
114+
'Avoid collection methods with unrelated types.',
115+
'Avoid collection methods with unrelated types.',
116+
'Avoid collection methods with unrelated types.',
117+
'Avoid collection methods with unrelated types.',
118+
'Avoid collection methods with unrelated types.',
119+
'Avoid collection methods with unrelated types.',
120+
'Avoid collection methods with unrelated types.',
121+
'Avoid collection methods with unrelated types.',
122+
'Avoid collection methods with unrelated types.',
123+
'Avoid collection methods with unrelated types.',
124+
'Avoid collection methods with unrelated types.',
125+
'Avoid collection methods with unrelated types.',
126+
'Avoid collection methods with unrelated types.',
127+
'Avoid collection methods with unrelated types.',
128+
'Avoid collection methods with unrelated types.',
129+
'Avoid collection methods with unrelated types.',
130+
'Avoid collection methods with unrelated types.',
131+
'Avoid collection methods with unrelated types.',
132+
],
133+
);
134+
});
135+
});
136+
}

0 commit comments

Comments
 (0)