From 5f04be01cb2766eef43da38c73af1ab64c281c19 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 24 Feb 2022 21:02:41 +0800 Subject: [PATCH 01/26] add sccafold for new rule --- .../lint_analyzer/rules/rules_factory.dart | 3 +++ ...ion_methods_with_unrelated_types_rule.dart | 24 +++++++++++++++++++ .../visitor.dart | 5 ++++ 3 files changed, 32 insertions(+) create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart create mode 100644 lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart index 7e9cba5f06..c45012da33 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_factory.dart @@ -1,6 +1,7 @@ import 'models/rule.dart'; import 'rules_list/always_remove_listener/always_remove_listener_rule.dart'; import 'rules_list/avoid_border_all/avoid_border_all_rule.dart'; +import 'rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart'; import 'rules_list/avoid_dynamic/avoid_dynamic_rule.dart'; import 'rules_list/avoid_global_state/avoid_global_state_rule.dart'; 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 final _implementedRules = )>{ AlwaysRemoveListenerRule.ruleId: (config) => AlwaysRemoveListenerRule(config), AvoidBorderAllRule.ruleId: (config) => AvoidBorderAllRule(config), + AvoidCollectionMethodsWithUnrelatedTypesRule.ruleId: (config) => + AvoidCollectionMethodsWithUnrelatedTypesRule(config), AvoidDynamicRule.ruleId: (config) => AvoidDynamicRule(config), AvoidGlobalStateRule.ruleId: (config) => AvoidGlobalStateRule(config), AvoidIgnoringReturnValuesRule.ruleId: (config) => diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart new file mode 100644 index 0000000000..5dba506df3 --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart @@ -0,0 +1,24 @@ +// ignore_for_file: public_member_api_docs + +import 'package:analyzer/dart/ast/visitor.dart'; + +import '../../../lint_utils.dart'; +import '../../../models/severity.dart'; +import '../../models/common_rule.dart'; + +part 'visitor.dart'; + +class AvoidCollectionMethodsWithUnrelatedTypesRule extends CommonRule { + static const String ruleId = 'avoid-collection-methods-with-unrelated-types'; + + static const _warning = 'Avoid collection methods with unrelated types.'; + + AvoidCollectionMethodsWithUnrelatedTypesRule([Map config = const {}]) + : super( + id: ruleId, + severity: readSeverity(config, Severity.warning), + excludes: readExcludes(config), + ); + + // TODO +} diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart new file mode 100644 index 0000000000..c9f83df815 --- /dev/null +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart @@ -0,0 +1,5 @@ +part of 'avoid_collection_methods_with_unrelated_types_rule.dart'; + +class _Visitor extends RecursiveAstVisitor { + // TODO +} From 1759c443a9a4715e834e9fb48f98d3f8a8eb1635 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 24 Feb 2022 21:09:04 +0800 Subject: [PATCH 02/26] add scaffold for tests --- ...tion_methods_with_unrelated_types_rule_test.dart | 13 +++++++++++++ .../examples/example.dart | 1 + 2 files changed, 14 insertions(+) create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart create mode 100644 test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart new file mode 100644 index 0000000000..f838c20990 --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart @@ -0,0 +1,13 @@ +import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart'; +import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/avoid_dynamic/avoid_dynamic_rule.dart'; +import 'package:test/test.dart'; + +import '../../../../../helpers/rule_test_helper.dart'; + +const _examplePath = 'avoid_collection_methods_with_unrelated_types/examples/example.dart'; + +void main() { + group('AvoidCollectionMethodsWithUnrelatedTypesRule', () { + // TODO + }); +} diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart new file mode 100644 index 0000000000..0ffdd02fcb --- /dev/null +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart @@ -0,0 +1 @@ +// TODO \ No newline at end of file From 708d4d62ea6dfa47be969b48af111bd76dd1ca59 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 24 Feb 2022 21:27:48 +0800 Subject: [PATCH 03/26] try to implement the rule --- ...ion_methods_with_unrelated_types_rule.dart | 33 +++++++++++--- .../visitor.dart | 43 ++++++++++++++++++- 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart index 5dba506df3..1d4dceb10b 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart @@ -1,10 +1,17 @@ // ignore_for_file: public_member_api_docs +import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/type.dart'; +import '../../../../../utils/node_utils.dart'; import '../../../lint_utils.dart'; +import '../../../models/internal_resolved_unit_result.dart'; +import '../../../models/issue.dart'; import '../../../models/severity.dart'; import '../../models/common_rule.dart'; +import '../../rule_utils.dart'; part 'visitor.dart'; @@ -15,10 +22,26 @@ class AvoidCollectionMethodsWithUnrelatedTypesRule extends CommonRule { AvoidCollectionMethodsWithUnrelatedTypesRule([Map config = const {}]) : super( - id: ruleId, - severity: readSeverity(config, Severity.warning), - excludes: readExcludes(config), - ); + id: ruleId, + severity: readSeverity(config, Severity.warning), + excludes: readExcludes(config), + ); - // TODO + @override + Iterable check(InternalResolvedUnitResult source) { + final visitor = _Visitor(); + + source.unit.visitChildren(visitor); + + return visitor.expressions + .map((expression) => createIssue( + rule: this, + location: nodeLocation( + node: expression, + source: source, + ), + message: _warning, + )) + .toList(growable: false); + } } diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart index c9f83df815..02b09dceaf 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart @@ -1,5 +1,46 @@ part of 'avoid_collection_methods_with_unrelated_types_rule.dart'; class _Visitor extends RecursiveAstVisitor { - // TODO + final _expressions = []; + + Iterable get expressions => _expressions; + + @override + void visitIndexExpression(IndexExpression node) { + super.visitIndexExpression(node); + + final targetType = node.target?.staticType; + final mapKeyType = _getMapKeyType(targetType); + if (mapKeyType == null) { + return; + } + + final mapKeyElement = mapKeyType.element; + if (mapKeyElement is! ClassElement) { + return; + } + + final indexType = node.index.staticType; + if (indexType == null) { + return; + } + + final indexTypeIsSubClassOfMapKeyType = indexType.asInstanceOf(mapKeyElement) != null; + if (!indexTypeIsSubClassOfMapKeyType) { + _expressions.add(node); + } + } + + DartType? _getMapKeyType(DartType? type) { + if (type == null || !type.isDartCoreMap || type is! ParameterizedType) { + return null; + } + + final typeArguments = type.typeArguments; + if (typeArguments.length != 2) { + return null; + } + + return typeArguments.first; + } } From cc19297c6234d9fcaff48b3ad0f234a1653e0492 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 24 Feb 2022 21:33:01 +0800 Subject: [PATCH 04/26] add tests --- ...ethods_with_unrelated_types_rule_test.dart | 27 +++++++++++++++-- .../examples/example.dart | 30 ++++++++++++++++++- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart index f838c20990..9636c0e5a3 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart @@ -1,5 +1,5 @@ import 'package:dart_code_metrics/src/analyzers/lint_analyzer/models/severity.dart'; -import 'package:dart_code_metrics/src/analyzers/lint_analyzer/rules/rules_list/avoid_dynamic/avoid_dynamic_rule.dart'; +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'; import 'package:test/test.dart'; import '../../../../../helpers/rule_test_helper.dart'; @@ -8,6 +8,29 @@ const _examplePath = 'avoid_collection_methods_with_unrelated_types/examples/exa void main() { group('AvoidCollectionMethodsWithUnrelatedTypesRule', () { - // TODO + test('initialization', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + final issues = AvoidCollectionMethodsWithUnrelatedTypesRule().check(unit); + + RuleTestHelper.verifyInitialization( + issues: issues, + ruleId: 'avoid-collection-methods-with-unrelated-types', + severity: Severity.warning, + ); + }); + + test('reports about found issues', () async { + final unit = await RuleTestHelper.resolveFromFile(_examplePath); + final issues = AvoidCollectionMethodsWithUnrelatedTypesRule().check(unit); + + RuleTestHelper.verifyIssues( + issues: issues, + // TODO fill in + startLines: [], + startColumns: [], + locationTexts: [], + messages: [], + ); + }); }); } diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart index 0ffdd02fcb..a096503843 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart @@ -1 +1,29 @@ -// TODO \ No newline at end of file +void main() { + final primitiveMap = Map(); + // write + primitiveMap[42] = "value"; + primitiveMap["wrong_key_type"] = "value"; // LINT + // read + final a1 = primitiveMap[42]; + final a2 = primitiveMap["wrong_key_type"]; // LINT + + final inheritanceMap = Map(); + // write + inheritanceMap[Animal()] = "value"; + inheritanceMap[HumanExtendsAnimal()] = "value"; + inheritanceMap[DogImplementsAnimal()] = "value"; + inheritanceMap[Flower()] = "value"; // LINT + // read + final b1 = inheritanceMap[Animal()]; + final b2 = inheritanceMap[HumanExtendsAnimal()]; + final b3 = inheritanceMap[DogImplementsAnimal()]; + final b4 = inheritanceMap[Flower()]; // LINT +} + +class Animal {} + +class HumanExtendsAnimal extends Animal {} + +class DogImplementsAnimal implements Animal {} + +class Flower {} From 2d594ee08415734061b2e2f02657b6cb9b365fe7 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 24 Feb 2022 21:40:35 +0800 Subject: [PATCH 05/26] add toString to debug issues --- lib/src/analyzers/lint_analyzer/models/issue.dart | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/src/analyzers/lint_analyzer/models/issue.dart b/lib/src/analyzers/lint_analyzer/models/issue.dart index 3e0bf606a8..8c608ae17d 100644 --- a/lib/src/analyzers/lint_analyzer/models/issue.dart +++ b/lib/src/analyzers/lint_analyzer/models/issue.dart @@ -43,4 +43,14 @@ class Issue { this.verboseMessage, this.suggestion, }); + + @override + String toString() => 'Issue{' + 'startLine: ${location.start.line}, ' + 'startColumn: ${location.start.column}, ' + 'locationText: ${location.text}, ' + 'message: $message, ' + 'replacement: ${suggestion?.replacement}, ' + 'replacementComment: ${suggestion?.comment}, ' + '}'; } From c1e6183a4676d6b1e0ab64c4ab48f1b61796c770 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 24 Feb 2022 21:43:30 +0800 Subject: [PATCH 06/26] pass tests --- ...ethods_with_unrelated_types_rule_test.dart | 19 ++++++++++++++----- .../examples/example.dart | 4 ++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart index 9636c0e5a3..0128c83708 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart @@ -25,11 +25,20 @@ void main() { RuleTestHelper.verifyIssues( issues: issues, - // TODO fill in - startLines: [], - startColumns: [], - locationTexts: [], - messages: [], + startLines: [5, 8, 15, 20], + startColumns: [3, 14, 3, 14], + locationTexts: [ + 'primitiveMap["str"]', + 'primitiveMap["str"]', + 'inheritanceMap[Flower()]', + 'inheritanceMap[Flower()]', + ], + messages: [ + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + ], ); }); }); diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart index a096503843..4d853bb223 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart @@ -2,10 +2,10 @@ void main() { final primitiveMap = Map(); // write primitiveMap[42] = "value"; - primitiveMap["wrong_key_type"] = "value"; // LINT + primitiveMap["str"] = "value"; // LINT // read final a1 = primitiveMap[42]; - final a2 = primitiveMap["wrong_key_type"]; // LINT + final a2 = primitiveMap["str"]; // LINT final inheritanceMap = Map(); // write From 82b50b06f24cc11e2620893386feb07efb1edb4c Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 24 Feb 2022 21:51:24 +0800 Subject: [PATCH 07/26] add doc --- ...collection-methods-with-unrelated-types.md | 33 +++++++++++++++++++ website/docs/rules/overview.md | 4 +++ 2 files changed, 37 insertions(+) create mode 100644 website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md diff --git a/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md b/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md new file mode 100644 index 0000000000..a8b7ba466d --- /dev/null +++ b/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md @@ -0,0 +1,33 @@ +# Avoid collection methods with unrelated types + +## Rule id {#rule-id} + +avoid-collection-methods-with-unrelated-types + +## Severity {#severity} + +Warning + +## Description {#description} + +Avoid using collection methods with unrelated types, such as accessing a map of integers using a string key. + +This lint has been requested for a long time: Follow [this link](https://github.com/dart-lang/linter/issues/1307) to see the details. + +Related: Dart's built-in `list_remove_unrelated_type` and `iterable_contains_unrelated_type`. + +### Example {#example} + +Bad: + +```dart +final map = Map(); +map["str"] = "value"; +``` + +Good: + +```dart +final map = Map(); +map[42] = "value"; +``` diff --git a/website/docs/rules/overview.md b/website/docs/rules/overview.md index 6c64b1dd4e..c2956e4508 100644 --- a/website/docs/rules/overview.md +++ b/website/docs/rules/overview.md @@ -11,6 +11,10 @@ Rules configuration is [described here](../getting-started/configuration#configu ## Common {#common} +- [avoid-collection-methods-with-unrelated-types](./common/avoid-collection-methods-with-unrelated-types.md) + + Avoid using collection methods with unrelated types, such as accessing a map of integers using a string key. + - [avoid-dynamic](./common/avoid-dynamic.md) Warns when `dynamic` type is used as variable type in declaration, return type of a function, etc. From d7faff16fd508f029d3d2cefc087b4167a7b7b6c Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 24 Feb 2022 21:59:07 +0800 Subject: [PATCH 08/26] add more functions in example --- .../examples/example.dart | 60 +++++++++++++------ 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart index 4d853bb223..3301cbcd7b 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart @@ -1,23 +1,45 @@ void main() { - final primitiveMap = Map(); - // write - primitiveMap[42] = "value"; - primitiveMap["str"] = "value"; // LINT - // read - final a1 = primitiveMap[42]; - final a2 = primitiveMap["str"]; // LINT - - final inheritanceMap = Map(); - // write - inheritanceMap[Animal()] = "value"; - inheritanceMap[HumanExtendsAnimal()] = "value"; - inheritanceMap[DogImplementsAnimal()] = "value"; - inheritanceMap[Flower()] = "value"; // LINT - // read - final b1 = inheritanceMap[Animal()]; - final b2 = inheritanceMap[HumanExtendsAnimal()]; - final b3 = inheritanceMap[DogImplementsAnimal()]; - final b4 = inheritanceMap[Flower()]; // LINT + { + final primitiveMap = Map(); + + primitiveMap[42] = "value"; + primitiveMap["str"] = "value"; // LINT + + final a1 = primitiveMap[42]; + final a2 = primitiveMap["str"]; // LINT + + primitiveMap.containsKey(42); + primitiveMap.containsKey("str"); // LINT + + primitiveMap.containsValue("value"); + primitiveMap.containsValue(100); // LINT + + primitiveMap.remove(42); + primitiveMap.remove("str"); // LINT + } + + { + final inheritanceMap = Map(); + + inheritanceMap[Animal()] = "value"; + inheritanceMap[HumanExtendsAnimal()] = "value"; + inheritanceMap[DogImplementsAnimal()] = "value"; + inheritanceMap[Flower()] = "value"; // LINT + + final b1 = inheritanceMap[Animal()]; + final b2 = inheritanceMap[HumanExtendsAnimal()]; + final b3 = inheritanceMap[DogImplementsAnimal()]; + final b4 = inheritanceMap[Flower()]; // LINT + + inheritanceMap.containsKey(DogImplementsAnimal()); + inheritanceMap.containsKey(Flower()); // LINT + + inheritanceMap.containsValue(Flower()); + inheritanceMap.containsValue(DogImplementsAnimal()); // LINT + + inheritanceMap.remove(DogImplementsAnimal()); + inheritanceMap.remove(Flower()); // LINT + } } class Animal {} From b08a68724016d8cc0b73c96d6e1b614bbab25c28 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 24 Feb 2022 21:59:16 +0800 Subject: [PATCH 09/26] refactor visitor to extract more generic info --- .../visitor.dart | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart index 02b09dceaf..56ac1d9ee8 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart @@ -10,13 +10,8 @@ class _Visitor extends RecursiveAstVisitor { super.visitIndexExpression(node); final targetType = node.target?.staticType; - final mapKeyType = _getMapKeyType(targetType); - if (mapKeyType == null) { - return; - } - - final mapKeyElement = mapKeyType.element; - if (mapKeyElement is! ClassElement) { + final mapType = _getMapType(targetType); + if (mapType == null) { return; } @@ -25,13 +20,13 @@ class _Visitor extends RecursiveAstVisitor { return; } - final indexTypeIsSubClassOfMapKeyType = indexType.asInstanceOf(mapKeyElement) != null; + final indexTypeIsSubClassOfMapKeyType = indexType.asInstanceOf(mapType.keyElement) != null; if (!indexTypeIsSubClassOfMapKeyType) { _expressions.add(node); } } - DartType? _getMapKeyType(DartType? type) { + _MapType? _getMapType(DartType? type) { if (type == null || !type.isDartCoreMap || type is! ParameterizedType) { return null; } @@ -41,6 +36,25 @@ class _Visitor extends RecursiveAstVisitor { return null; } - return typeArguments.first; + final keyType = typeArguments.first; + final valueType = typeArguments[1]; + + final keyElement = keyType.element; + final valueElement = valueType.element; + if (keyElement is! ClassElement || valueElement is! ClassElement) { + return null; + } + + return _MapType(keyType, valueType, keyElement, valueElement); } } + +class _MapType { + final DartType keyType; + final DartType valueType; + + final ClassElement keyElement; + final ClassElement valueElement; + + _MapType(this.keyType, this.valueType, this.keyElement, this.valueElement); +} From a0791bf037960fd216c847b01ff9849b04bfc168 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 24 Feb 2022 22:08:14 +0800 Subject: [PATCH 10/26] try to implement containsKey, containsValue, remove --- ...ion_methods_with_unrelated_types_rule.dart | 1 + .../visitor.dart | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart index 1d4dceb10b..a8c4d067f8 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart @@ -4,6 +4,7 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; +import 'package:collection/collection.dart'; import '../../../../../utils/node_utils.dart'; import '../../../lint_utils.dart'; diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart index 56ac1d9ee8..48ad6a2bf0 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart @@ -5,6 +5,7 @@ class _Visitor extends RecursiveAstVisitor { Iterable get expressions => _expressions; + // for `operator []` and `operator []=` @override void visitIndexExpression(IndexExpression node) { super.visitIndexExpression(node); @@ -26,6 +27,30 @@ class _Visitor extends RecursiveAstVisitor { } } + // for things like `map.containsKey` + @override + void visitMethodInvocation(MethodInvocation node) { + super.visitMethodInvocation(node); + + switch (node.methodName.name) { + case 'containsKey': + case 'remove': + final mapType = _getMapType(node.target?.staticType); + final argType = node.argumentList.arguments.singleOrNull?.staticType; + if(mapType != null && argType != null && argType.asInstanceOf(mapType.keyElement) == null) { + _expressions.add(node); + } + break; + case 'containsValue': + final mapType = _getMapType(node.target?.staticType); + final argType = node.argumentList.arguments.singleOrNull?.staticType; + if(mapType != null && argType != null && argType.asInstanceOf(mapType.valueElement) == null) { + _expressions.add(node); + } + break; + } + } + _MapType? _getMapType(DartType? type) { if (type == null || !type.isDartCoreMap || type is! ParameterizedType) { return null; From fd5c6bd204f186990bdd5c023cd49124c8ff7c7b Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Thu, 24 Feb 2022 22:09:13 +0800 Subject: [PATCH 11/26] pass tests --- ...n_methods_with_unrelated_types_rule_test.dart | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart index 0128c83708..57c8964e24 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart @@ -25,19 +25,31 @@ void main() { RuleTestHelper.verifyIssues( issues: issues, - startLines: [5, 8, 15, 20], - startColumns: [3, 14, 3, 14], + startLines: [6, 9, 12, 15, 18, 27, 32, 35, 38, 41], + startColumns: [5, 16, 5, 5, 5, 5, 16, 5, 5, 5], locationTexts: [ 'primitiveMap["str"]', 'primitiveMap["str"]', + 'primitiveMap.containsKey("str")', + 'primitiveMap.containsValue(100)', + 'primitiveMap.remove("str")', 'inheritanceMap[Flower()]', 'inheritanceMap[Flower()]', + 'inheritanceMap.containsKey(Flower())', + 'inheritanceMap.containsValue(DogImplementsAnimal())', + 'inheritanceMap.remove(Flower())', ], messages: [ 'Avoid collection methods with unrelated types.', 'Avoid collection methods with unrelated types.', 'Avoid collection methods with unrelated types.', 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', ], ); }); From ef4f27dc74a7e21e675fb458fe98a83791dfbfd9 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 08:54:45 +0800 Subject: [PATCH 12/26] add checking `Iterable.contains`, pass tests --- ...ion_methods_with_unrelated_types_rule.dart | 1 + .../visitor.dart | 27 ++++++++++++++++--- ...ethods_with_unrelated_types_rule_test.dart | 8 ++++-- .../examples/example.dart | 8 ++++++ 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart index a8c4d067f8..4a88fc9583 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart @@ -6,6 +6,7 @@ import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:collection/collection.dart'; +import '../../../../../utils/dart_types_utils.dart'; import '../../../../../utils/node_utils.dart'; import '../../../lint_utils.dart'; import '../../../models/internal_resolved_unit_result.dart'; diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart index 48ad6a2bf0..101fd0ea31 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart @@ -32,19 +32,25 @@ class _Visitor extends RecursiveAstVisitor { void visitMethodInvocation(MethodInvocation node) { super.visitMethodInvocation(node); + final argType = node.argumentList.arguments.singleOrNull?.staticType; + switch (node.methodName.name) { case 'containsKey': case 'remove': final mapType = _getMapType(node.target?.staticType); - final argType = node.argumentList.arguments.singleOrNull?.staticType; - if(mapType != null && argType != null && argType.asInstanceOf(mapType.keyElement) == null) { + if (mapType != null && argType != null && argType.asInstanceOf(mapType.keyElement) == null) { _expressions.add(node); } break; case 'containsValue': final mapType = _getMapType(node.target?.staticType); - final argType = node.argumentList.arguments.singleOrNull?.staticType; - if(mapType != null && argType != null && argType.asInstanceOf(mapType.valueElement) == null) { + if (mapType != null && argType != null && argType.asInstanceOf(mapType.valueElement) == null) { + _expressions.add(node); + } + break; + case 'contains': + final iterableType = _getIterableTypeElement(node.target?.staticType); + if (iterableType != null && argType != null && argType.asInstanceOf(iterableType) == null) { _expressions.add(node); } break; @@ -72,6 +78,19 @@ class _Visitor extends RecursiveAstVisitor { return _MapType(keyType, valueType, keyElement, valueElement); } + + ClassElement? _getIterableTypeElement(DartType? type) { + if (type == null || !isIterableOrSubclass(type) || type is! ParameterizedType) { + return null; + } + + final typeArgElement = type.typeArguments.singleOrNull?.element; + if (typeArgElement is! ClassElement) { + return null; + } + + return typeArgElement; + } } class _MapType { diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart index 57c8964e24..186ee1d8f6 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart @@ -25,8 +25,8 @@ void main() { RuleTestHelper.verifyIssues( issues: issues, - startLines: [6, 9, 12, 15, 18, 27, 32, 35, 38, 41], - startColumns: [5, 16, 5, 5, 5, 5, 16, 5, 5, 5], + startLines: [6, 9, 12, 15, 18, 27, 32, 35, 38, 41, 46, 49], + startColumns: [5, 16, 5, 5, 5, 5, 16, 5, 5, 5, 5, 5], locationTexts: [ 'primitiveMap["str"]', 'primitiveMap["str"]', @@ -38,6 +38,8 @@ void main() { 'inheritanceMap.containsKey(Flower())', 'inheritanceMap.containsValue(DogImplementsAnimal())', 'inheritanceMap.remove(Flower())', + '[1, 2, 3].contains("str")', + 'Iterable.generate(10).contains("str")', ], messages: [ 'Avoid collection methods with unrelated types.', @@ -50,6 +52,8 @@ void main() { 'Avoid collection methods with unrelated types.', 'Avoid collection methods with unrelated types.', 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', ], ); }); diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart index 3301cbcd7b..19b5dee7e8 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart @@ -40,6 +40,14 @@ void main() { inheritanceMap.remove(DogImplementsAnimal()); inheritanceMap.remove(Flower()); // LINT } + + { + [1, 2, 3].contains(42); + [1, 2, 3].contains("str"); // LINT + + Iterable.generate(10).contains(42); + Iterable.generate(10).contains("str"); // LINT + } } class Animal {} From 9eec51ecd25a237d9014f184bfd2eb4a35e0d403 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 08:58:26 +0800 Subject: [PATCH 13/26] revert issue.tostring --- lib/src/analyzers/lint_analyzer/models/issue.dart | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/models/issue.dart b/lib/src/analyzers/lint_analyzer/models/issue.dart index 8c608ae17d..3e0bf606a8 100644 --- a/lib/src/analyzers/lint_analyzer/models/issue.dart +++ b/lib/src/analyzers/lint_analyzer/models/issue.dart @@ -43,14 +43,4 @@ class Issue { this.verboseMessage, this.suggestion, }); - - @override - String toString() => 'Issue{' - 'startLine: ${location.start.line}, ' - 'startColumn: ${location.start.column}, ' - 'locationText: ${location.text}, ' - 'message: $message, ' - 'replacement: ${suggestion?.replacement}, ' - 'replacementComment: ${suggestion?.comment}, ' - '}'; } From 3be6a3c260cbbbc4681a43641efa88bc20a51b6b Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 08:59:21 +0800 Subject: [PATCH 14/26] run dart format --- ...ion_methods_with_unrelated_types_rule.dart | 3 ++- .../visitor.dart | 19 ++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart index 4a88fc9583..7f82b05727 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule.dart @@ -22,7 +22,8 @@ class AvoidCollectionMethodsWithUnrelatedTypesRule extends CommonRule { static const _warning = 'Avoid collection methods with unrelated types.'; - AvoidCollectionMethodsWithUnrelatedTypesRule([Map config = const {}]) + AvoidCollectionMethodsWithUnrelatedTypesRule( + [Map config = const {}]) : super( id: ruleId, severity: readSeverity(config, Severity.warning), diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart index 101fd0ea31..7913224e68 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart @@ -21,7 +21,8 @@ class _Visitor extends RecursiveAstVisitor { return; } - final indexTypeIsSubClassOfMapKeyType = indexType.asInstanceOf(mapType.keyElement) != null; + final indexTypeIsSubClassOfMapKeyType = + indexType.asInstanceOf(mapType.keyElement) != null; if (!indexTypeIsSubClassOfMapKeyType) { _expressions.add(node); } @@ -38,19 +39,25 @@ class _Visitor extends RecursiveAstVisitor { case 'containsKey': case 'remove': final mapType = _getMapType(node.target?.staticType); - if (mapType != null && argType != null && argType.asInstanceOf(mapType.keyElement) == null) { + if (mapType != null && + argType != null && + argType.asInstanceOf(mapType.keyElement) == null) { _expressions.add(node); } break; case 'containsValue': final mapType = _getMapType(node.target?.staticType); - if (mapType != null && argType != null && argType.asInstanceOf(mapType.valueElement) == null) { + if (mapType != null && + argType != null && + argType.asInstanceOf(mapType.valueElement) == null) { _expressions.add(node); } break; case 'contains': final iterableType = _getIterableTypeElement(node.target?.staticType); - if (iterableType != null && argType != null && argType.asInstanceOf(iterableType) == null) { + if (iterableType != null && + argType != null && + argType.asInstanceOf(iterableType) == null) { _expressions.add(node); } break; @@ -80,7 +87,9 @@ class _Visitor extends RecursiveAstVisitor { } ClassElement? _getIterableTypeElement(DartType? type) { - if (type == null || !isIterableOrSubclass(type) || type is! ParameterizedType) { + if (type == null || + !isIterableOrSubclass(type) || + type is! ParameterizedType) { return null; } From b02a9f142518a122a8a9bd81c8d6e1ef52282ce1 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:01:27 +0800 Subject: [PATCH 15/26] refactor code --- .../visitor.dart | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart index 7913224e68..1bdc304f11 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart @@ -39,31 +39,28 @@ class _Visitor extends RecursiveAstVisitor { case 'containsKey': case 'remove': final mapType = _getMapType(node.target?.staticType); - if (mapType != null && - argType != null && - argType.asInstanceOf(mapType.keyElement) == null) { - _expressions.add(node); - } + _addIfNotSubType(argType, mapType?.keyElement, node); break; case 'containsValue': final mapType = _getMapType(node.target?.staticType); - if (mapType != null && - argType != null && - argType.asInstanceOf(mapType.valueElement) == null) { - _expressions.add(node); - } + _addIfNotSubType(argType, mapType?.valueElement, node); break; case 'contains': final iterableType = _getIterableTypeElement(node.target?.staticType); - if (iterableType != null && - argType != null && - argType.asInstanceOf(iterableType) == null) { - _expressions.add(node); - } + _addIfNotSubType(argType, iterableType, node); break; } } + void _addIfNotSubType( + DartType? childType, ClassElement? parentElement, MethodInvocation node) { + if (parentElement != null && + childType != null && + childType.asInstanceOf(parentElement) == null) { + _expressions.add(node); + } + } + _MapType? _getMapType(DartType? type) { if (type == null || !type.isDartCoreMap || type is! ParameterizedType) { return null; From c3557a41edcfb9c825c2dca32778324f5a59b9ea Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:03:59 +0800 Subject: [PATCH 16/26] improve doc --- ...collection-methods-with-unrelated-types.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md b/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md index a8b7ba466d..4748ce31d6 100644 --- a/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md +++ b/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md @@ -12,6 +12,15 @@ Warning Avoid using collection methods with unrelated types, such as accessing a map of integers using a string key. +Full list: + +* `Map.operator []` +* `Map.operator []=` +* `Map.containsKey` +* `Map.containsValue` +* `Map.remove` +* `Iterable.contains` + This lint has been requested for a long time: Follow [this link](https://github.com/dart-lang/linter/issues/1307) to see the details. Related: Dart's built-in `list_remove_unrelated_type` and `iterable_contains_unrelated_type`. @@ -23,6 +32,12 @@ Bad: ```dart final map = Map(); map["str"] = "value"; +var a = map["str"]; +map.containsKey("str"); +map.containsValue(42); +map.remove("str"); + +Iterable.empty().contains("str"); ``` Good: @@ -30,4 +45,10 @@ Good: ```dart final map = Map(); map[42] = "value"; +var a = map[42]; +map.containsKey(42); +map.containsValue("value"); +map.remove(42); + +Iterable.empty().contains(42); ``` From 6f611e87e49d937cc181e2cc1b0bf6defa806584 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:08:43 +0800 Subject: [PATCH 17/26] try to also check subclasses of map (not finished) --- .../visitor.dart | 7 +++++-- lib/src/utils/dart_types_utils.dart | 7 +++++++ .../examples/example.dart | 10 ++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart index 1bdc304f11..772198e0dd 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart @@ -53,7 +53,10 @@ class _Visitor extends RecursiveAstVisitor { } void _addIfNotSubType( - DartType? childType, ClassElement? parentElement, MethodInvocation node) { + DartType? childType, + ClassElement? parentElement, + MethodInvocation node, + ) { if (parentElement != null && childType != null && childType.asInstanceOf(parentElement) == null) { @@ -62,7 +65,7 @@ class _Visitor extends RecursiveAstVisitor { } _MapType? _getMapType(DartType? type) { - if (type == null || !type.isDartCoreMap || type is! ParameterizedType) { + if (type == null || !isMapOrSubclass(type) || type is! ParameterizedType) { return null; } diff --git a/lib/src/utils/dart_types_utils.dart b/lib/src/utils/dart_types_utils.dart index 1e53ea372c..30d320fd82 100644 --- a/lib/src/utils/dart_types_utils.dart +++ b/lib/src/utils/dart_types_utils.dart @@ -17,3 +17,10 @@ bool _isList(DartType? type) => type?.isDartCoreList ?? false; bool _isSubclassOfList(DartType? type) => type is InterfaceType && type.allSupertypes.any(_isList); + +bool isMapOrSubclass(DartType? type) => _isMap(type) || _isSubclassOfMap(type); + +bool _isMap(DartType? type) => type?.isDartCoreMap ?? false; + +bool _isSubclassOfMap(DartType? type) => + type is InterfaceType && type.allSupertypes.any(_isMap); diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart index 19b5dee7e8..f153b19185 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart @@ -41,6 +41,14 @@ void main() { inheritanceMap.remove(Flower()); // LINT } + { + final myMap = MyMapSubClass(); + myMap[42] = "value"; + myMap["str"] = "value"; // LINT + myMap.containsKey(42); + myMap.containsKey("str"); // LINT + } + { [1, 2, 3].contains(42); [1, 2, 3].contains("str"); // LINT @@ -57,3 +65,5 @@ class HumanExtendsAnimal extends Animal {} class DogImplementsAnimal implements Animal {} class Flower {} + +class MyMapSubClass extends Map {} From 6d2a9f57cfda60647716a33587cbbfa6c2d9bda1 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:14:12 +0800 Subject: [PATCH 18/26] refactor dart_type_utils to avoid duplication --- lib/src/utils/dart_types_utils.dart | 55 +++++++++++++++++++---------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/lib/src/utils/dart_types_utils.dart b/lib/src/utils/dart_types_utils.dart index 30d320fd82..f53060cf6c 100644 --- a/lib/src/utils/dart_types_utils.dart +++ b/lib/src/utils/dart_types_utils.dart @@ -1,26 +1,43 @@ // ignore_for_file: public_member_api_docs import 'package:analyzer/dart/element/type.dart'; +import 'package:collection/collection.dart'; bool isIterableOrSubclass(DartType? type) => - _isIterable(type) || _isSubclassOfIterable(type); - -bool _isIterable(DartType? type) => type?.isDartCoreIterable ?? false; - -bool _isSubclassOfIterable(DartType? type) => - type is InterfaceType && type.allSupertypes.any(_isIterable); + _checkSelfOrSupertypes(type, (t) => t?.isDartCoreIterable ?? false); bool isListOrSubclass(DartType? type) => - _isList(type) || _isSubclassOfList(type); - -bool _isList(DartType? type) => type?.isDartCoreList ?? false; - -bool _isSubclassOfList(DartType? type) => - type is InterfaceType && type.allSupertypes.any(_isList); - -bool isMapOrSubclass(DartType? type) => _isMap(type) || _isSubclassOfMap(type); - -bool _isMap(DartType? type) => type?.isDartCoreMap ?? false; - -bool _isSubclassOfMap(DartType? type) => - type is InterfaceType && type.allSupertypes.any(_isMap); + _checkSelfOrSupertypes(type, (t) => t?.isDartCoreList ?? false); + +bool isMapOrSubclass(DartType? type) => + _checkSelfOrSupertypes(type, (t) => t?.isDartCoreMap ?? false); + +DartType? getSupertypeIterable(DartType? type) => + _getSelfOrSupertypes(type, (t) => t?.isDartCoreIterable ?? false); + +DartType? getSupertypeList(DartType? type) => + _getSelfOrSupertypes(type, (t) => t?.isDartCoreList ?? false); + +DartType? getSupertypeMap(DartType? type) => + _getSelfOrSupertypes(type, (t) => t?.isDartCoreMap ?? false); + +bool _checkSelfOrSupertypes( + DartType? type, + bool Function(DartType?) predicate, +) => + predicate(type) || + (type is InterfaceType && type.allSupertypes.any(predicate)); + +DartType? _getSelfOrSupertypes( + DartType? type, + bool Function(DartType?) predicate, +) { + if (predicate(type)) { + return type; + } + if (type is InterfaceType) { + return type.allSupertypes.firstWhereOrNull(predicate); + } + + return null; +} From 7b9fb1e4213e0e44e3fab6e9622950f6590ed9df Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:18:48 +0800 Subject: [PATCH 19/26] make test passes --- .../visitor.dart | 12 ++++++------ ...ction_methods_with_unrelated_types_rule_test.dart | 8 ++++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart index 772198e0dd..a9c0517a61 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart @@ -65,11 +65,12 @@ class _Visitor extends RecursiveAstVisitor { } _MapType? _getMapType(DartType? type) { - if (type == null || !isMapOrSubclass(type) || type is! ParameterizedType) { + final mapType = getSupertypeMap(type); + if (mapType == null || mapType is! ParameterizedType) { return null; } - final typeArguments = type.typeArguments; + final typeArguments = mapType.typeArguments; if (typeArguments.length != 2) { return null; } @@ -87,13 +88,12 @@ class _Visitor extends RecursiveAstVisitor { } ClassElement? _getIterableTypeElement(DartType? type) { - if (type == null || - !isIterableOrSubclass(type) || - type is! ParameterizedType) { + final iterableType = getSupertypeIterable(type); + if (iterableType == null || iterableType is! ParameterizedType) { return null; } - final typeArgElement = type.typeArguments.singleOrNull?.element; + final typeArgElement = iterableType.typeArguments.singleOrNull?.element; if (typeArgElement is! ClassElement) { return null; } diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart index 186ee1d8f6..788acdc4aa 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart @@ -25,8 +25,8 @@ void main() { RuleTestHelper.verifyIssues( issues: issues, - startLines: [6, 9, 12, 15, 18, 27, 32, 35, 38, 41, 46, 49], - startColumns: [5, 16, 5, 5, 5, 5, 16, 5, 5, 5, 5, 5], + startLines: [6, 9, 12, 15, 18, 27, 32, 35, 38, 41, 47, 49, 54, 57], + startColumns: [5, 16, 5, 5, 5, 5, 16, 5, 5, 5, 5, 5, 5, 5], locationTexts: [ 'primitiveMap["str"]', 'primitiveMap["str"]', @@ -38,6 +38,8 @@ void main() { 'inheritanceMap.containsKey(Flower())', 'inheritanceMap.containsValue(DogImplementsAnimal())', 'inheritanceMap.remove(Flower())', + 'myMap["str"]', + 'myMap.containsKey("str")', '[1, 2, 3].contains("str")', 'Iterable.generate(10).contains("str")', ], @@ -54,6 +56,8 @@ void main() { 'Avoid collection methods with unrelated types.', 'Avoid collection methods with unrelated types.', 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', ], ); }); From 9b1262ae0651a83fc6d475991e00f553b354bbf3 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:19:55 +0800 Subject: [PATCH 20/26] minor change to tests --- ...id_collection_methods_with_unrelated_types_rule_test.dart | 4 ++-- .../examples/example.dart | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart index 788acdc4aa..2d365b24f0 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart @@ -25,8 +25,8 @@ void main() { RuleTestHelper.verifyIssues( issues: issues, - startLines: [6, 9, 12, 15, 18, 27, 32, 35, 38, 41, 47, 49, 54, 57], - startColumns: [5, 16, 5, 5, 5, 5, 16, 5, 5, 5, 5, 5, 5, 5], + startLines: [6, 9, 12, 15, 18, 27, 32, 35, 38, 41, 48, 50, 55, 58], + startColumns: [5, 16, 5, 5, 5, 5, 16, 5, 5, 5, 14, 5, 5, 5], locationTexts: [ 'primitiveMap["str"]', 'primitiveMap["str"]', diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart index f153b19185..a210e3394e 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart @@ -41,10 +41,11 @@ void main() { inheritanceMap.remove(Flower()); // LINT } + // sub-class of `Map`, `Iterable` etc should also be supported { final myMap = MyMapSubClass(); - myMap[42] = "value"; - myMap["str"] = "value"; // LINT + var c1 = myMap[42]; + var c2 = myMap["str"]; // LINT myMap.containsKey(42); myMap.containsKey("str"); // LINT } From 6d724f042a2ef774e9a7566948cf42317aa70897 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:24:56 +0800 Subject: [PATCH 21/26] refactor and make tests pass --- .../visitor.dart | 78 +++++-------------- 1 file changed, 21 insertions(+), 57 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart index a9c0517a61..28b82be386 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart @@ -10,22 +10,8 @@ class _Visitor extends RecursiveAstVisitor { void visitIndexExpression(IndexExpression node) { super.visitIndexExpression(node); - final targetType = node.target?.staticType; - final mapType = _getMapType(targetType); - if (mapType == null) { - return; - } - - final indexType = node.index.staticType; - if (indexType == null) { - return; - } - - final indexTypeIsSubClassOfMapKeyType = - indexType.asInstanceOf(mapType.keyElement) != null; - if (!indexTypeIsSubClassOfMapKeyType) { - _expressions.add(node); - } + final mapType = _getMapTypeElement(node.target?.staticType); + _addIfNotSubType(node.index.staticType, mapType?.first, node); } // for things like `map.containsKey` @@ -38,12 +24,12 @@ class _Visitor extends RecursiveAstVisitor { switch (node.methodName.name) { case 'containsKey': case 'remove': - final mapType = _getMapType(node.target?.staticType); - _addIfNotSubType(argType, mapType?.keyElement, node); + final mapType = _getMapTypeElement(node.target?.staticType); + _addIfNotSubType(argType, mapType?.first, node); break; case 'containsValue': - final mapType = _getMapType(node.target?.staticType); - _addIfNotSubType(argType, mapType?.valueElement, node); + final mapType = _getMapTypeElement(node.target?.staticType); + _addIfNotSubType(argType, mapType?[1], node); break; case 'contains': final iterableType = _getIterableTypeElement(node.target?.staticType); @@ -55,7 +41,7 @@ class _Visitor extends RecursiveAstVisitor { void _addIfNotSubType( DartType? childType, ClassElement? parentElement, - MethodInvocation node, + Expression node, ) { if (parentElement != null && childType != null && @@ -64,50 +50,28 @@ class _Visitor extends RecursiveAstVisitor { } } - _MapType? _getMapType(DartType? type) { - final mapType = getSupertypeMap(type); - if (mapType == null || mapType is! ParameterizedType) { - return null; - } + List? _getMapTypeElement(DartType? type) => + _getTypeArgElements(getSupertypeMap(type)); - final typeArguments = mapType.typeArguments; - if (typeArguments.length != 2) { - return null; - } + ClassElement? _getIterableTypeElement(DartType? type) => + _getTypeArgElements(getSupertypeIterable(type))?.singleOrNull; - final keyType = typeArguments.first; - final valueType = typeArguments[1]; + ClassElement? _getListTypeElement(DartType? type) => + _getTypeArgElements(getSupertypeList(type))?.singleOrNull; - final keyElement = keyType.element; - final valueElement = valueType.element; - if (keyElement is! ClassElement || valueElement is! ClassElement) { + List? _getTypeArgElements(DartType? type) { + if (type == null || type is! ParameterizedType) { return null; } - return _MapType(keyType, valueType, keyElement, valueElement); - } - - ClassElement? _getIterableTypeElement(DartType? type) { - final iterableType = getSupertypeIterable(type); - if (iterableType == null || iterableType is! ParameterizedType) { - return null; - } - - final typeArgElement = iterableType.typeArguments.singleOrNull?.element; - if (typeArgElement is! ClassElement) { + final typeArgElements = type.typeArguments + .map((arg) => arg.element) + .whereType() + .toList(); + if (typeArgElements.length < type.typeArguments.length) { return null; } - return typeArgElement; + return typeArgElements; } } - -class _MapType { - final DartType keyType; - final DartType valueType; - - final ClassElement keyElement; - final ClassElement valueElement; - - _MapType(this.keyType, this.valueType, this.keyElement, this.valueElement); -} From 7350ef89e162bc562c59ee553f093bf6f55462fd Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:27:35 +0800 Subject: [PATCH 22/26] support List.remove --- .../visitor.dart | 6 ++++++ ...d_collection_methods_with_unrelated_types_rule_test.dart | 6 ++++-- .../examples/example.dart | 6 ++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart index 28b82be386..70e5fecbdb 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart @@ -23,9 +23,15 @@ class _Visitor extends RecursiveAstVisitor { switch (node.methodName.name) { case 'containsKey': + final mapType = _getMapTypeElement(node.target?.staticType); + _addIfNotSubType(argType, mapType?.first, node); + break; case 'remove': final mapType = _getMapTypeElement(node.target?.staticType); _addIfNotSubType(argType, mapType?.first, node); + + final listType = _getListTypeElement(node.target?.staticType); + _addIfNotSubType(argType, listType, node); break; case 'containsValue': final mapType = _getMapTypeElement(node.target?.staticType); diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart index 2d365b24f0..4ca2b53505 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart @@ -25,8 +25,8 @@ void main() { RuleTestHelper.verifyIssues( issues: issues, - startLines: [6, 9, 12, 15, 18, 27, 32, 35, 38, 41, 48, 50, 55, 58], - startColumns: [5, 16, 5, 5, 5, 5, 16, 5, 5, 5, 14, 5, 5, 5], + startLines: [6, 9, 12, 15, 18, 27, 32, 35, 38, 41, 48, 50, 55, 58, 64], + startColumns: [5, 16, 5, 5, 5, 5, 16, 5, 5, 5, 14, 5, 5, 5, 5], locationTexts: [ 'primitiveMap["str"]', 'primitiveMap["str"]', @@ -42,6 +42,7 @@ void main() { 'myMap.containsKey("str")', '[1, 2, 3].contains("str")', 'Iterable.generate(10).contains("str")', + 'primitiveList.remove("str")', ], messages: [ 'Avoid collection methods with unrelated types.', @@ -58,6 +59,7 @@ void main() { 'Avoid collection methods with unrelated types.', 'Avoid collection methods with unrelated types.', 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', ], ); }); diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart index a210e3394e..5dcdacae92 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart @@ -57,6 +57,12 @@ void main() { Iterable.generate(10).contains(42); Iterable.generate(10).contains("str"); // LINT } + + { + final primitiveList = [10, 20, 30]; + primitiveList.remove(20); + primitiveList.remove("str"); // LINT + } } class Animal {} From 3535eef67e82a3d99fa608cab272bd8bd9c6d9c5 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:28:47 +0800 Subject: [PATCH 23/26] change doc --- ...avoid-collection-methods-with-unrelated-types.md | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md b/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md index 4748ce31d6..4f6dd5ddba 100644 --- a/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md +++ b/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md @@ -12,15 +12,6 @@ Warning Avoid using collection methods with unrelated types, such as accessing a map of integers using a string key. -Full list: - -* `Map.operator []` -* `Map.operator []=` -* `Map.containsKey` -* `Map.containsValue` -* `Map.remove` -* `Iterable.contains` - This lint has been requested for a long time: Follow [this link](https://github.com/dart-lang/linter/issues/1307) to see the details. Related: Dart's built-in `list_remove_unrelated_type` and `iterable_contains_unrelated_type`. @@ -38,6 +29,8 @@ map.containsValue(42); map.remove("str"); Iterable.empty().contains("str"); + +List().remove("str"); ``` Good: @@ -51,4 +44,6 @@ map.containsValue("value"); map.remove(42); Iterable.empty().contains(42); + +List().remove(42); ``` From b13fac09956847c6271ae6e1e993f940124bf683 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:39:57 +0800 Subject: [PATCH 24/26] support sets --- .../visitor.dart | 57 ++++++++++++++----- lib/src/utils/dart_types_utils.dart | 3 + .../examples/example.dart | 31 ++++++++++ ...collection-methods-with-unrelated-types.md | 38 ++++++++++--- 4 files changed, 106 insertions(+), 23 deletions(-) diff --git a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart index 70e5fecbdb..9469595398 100644 --- a/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart +++ b/lib/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/visitor.dart @@ -19,60 +19,82 @@ class _Visitor extends RecursiveAstVisitor { void visitMethodInvocation(MethodInvocation node) { super.visitMethodInvocation(node); + final mapType = _getMapTypeElement(node.target?.staticType); + final listType = _getListTypeElement(node.target?.staticType); + final setType = _getSetTypeElement(node.target?.staticType); + final iterableType = _getIterableTypeElement(node.target?.staticType); final argType = node.argumentList.arguments.singleOrNull?.staticType; switch (node.methodName.name) { case 'containsKey': - final mapType = _getMapTypeElement(node.target?.staticType); _addIfNotSubType(argType, mapType?.first, node); break; case 'remove': - final mapType = _getMapTypeElement(node.target?.staticType); _addIfNotSubType(argType, mapType?.first, node); - - final listType = _getListTypeElement(node.target?.staticType); _addIfNotSubType(argType, listType, node); + _addIfNotSubType(argType, setType, node); + break; + case 'lookup': + _addIfNotSubType(argType, setType, node); break; case 'containsValue': - final mapType = _getMapTypeElement(node.target?.staticType); _addIfNotSubType(argType, mapType?[1], node); break; case 'contains': - final iterableType = _getIterableTypeElement(node.target?.staticType); _addIfNotSubType(argType, iterableType, node); break; + case 'containsAll': + case 'removeAll': + case 'retainAll': + final argAsIterableParamType = _getIterableTypeElement(argType); + _addIfNotSubType(argAsIterableParamType?.type, setType, node); + break; + case 'difference': + case 'intersection': + final argAsSetParamType = _getSetTypeElement(argType); + _addIfNotSubType(argAsSetParamType?.type, setType, node); + break; } } void _addIfNotSubType( DartType? childType, - ClassElement? parentElement, + _TypedClassElement? parentElement, Expression node, ) { if (parentElement != null && childType != null && - childType.asInstanceOf(parentElement) == null) { + childType.asInstanceOf(parentElement.element) == null) { _expressions.add(node); } } - List? _getMapTypeElement(DartType? type) => + List<_TypedClassElement>? _getMapTypeElement(DartType? type) => _getTypeArgElements(getSupertypeMap(type)); - ClassElement? _getIterableTypeElement(DartType? type) => + _TypedClassElement? _getIterableTypeElement(DartType? type) => _getTypeArgElements(getSupertypeIterable(type))?.singleOrNull; - ClassElement? _getListTypeElement(DartType? type) => + _TypedClassElement? _getListTypeElement(DartType? type) => _getTypeArgElements(getSupertypeList(type))?.singleOrNull; - List? _getTypeArgElements(DartType? type) { + _TypedClassElement? _getSetTypeElement(DartType? type) => + _getTypeArgElements(getSupertypeSet(type))?.singleOrNull; + + List<_TypedClassElement>? _getTypeArgElements(DartType? type) { if (type == null || type is! ParameterizedType) { return null; } final typeArgElements = type.typeArguments - .map((arg) => arg.element) - .whereType() + .map((typeArg) { + final element = typeArg.element; + + return element is ClassElement + ? _TypedClassElement(typeArg, element) + : null; + }) + .whereNotNull() .toList(); if (typeArgElements.length < type.typeArguments.length) { return null; @@ -81,3 +103,10 @@ class _Visitor extends RecursiveAstVisitor { return typeArgElements; } } + +class _TypedClassElement { + final DartType type; + final ClassElement element; + + _TypedClassElement(this.type, this.element); +} diff --git a/lib/src/utils/dart_types_utils.dart b/lib/src/utils/dart_types_utils.dart index f53060cf6c..fee5c179ee 100644 --- a/lib/src/utils/dart_types_utils.dart +++ b/lib/src/utils/dart_types_utils.dart @@ -18,6 +18,9 @@ DartType? getSupertypeIterable(DartType? type) => DartType? getSupertypeList(DartType? type) => _getSelfOrSupertypes(type, (t) => t?.isDartCoreList ?? false); +DartType? getSupertypeSet(DartType? type) => + _getSelfOrSupertypes(type, (t) => t?.isDartCoreSet ?? false); + DartType? getSupertypeMap(DartType? type) => _getSelfOrSupertypes(type, (t) => t?.isDartCoreMap ?? false); diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart index 5dcdacae92..dfe7e9a06e 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart @@ -56,6 +56,9 @@ void main() { Iterable.generate(10).contains(42); Iterable.generate(10).contains("str"); // LINT + + {1, 2, 3}.contains(42); + {1, 2, 3}.contains("str"); // LINT } { @@ -63,6 +66,34 @@ void main() { primitiveList.remove(20); primitiveList.remove("str"); // LINT } + + { + final primitiveSet = {10, 20, 30}; + + primitiveSet.contains(42); + primitiveSet.contains("str"); // LINT + + primitiveSet.containsAll(Iterable.empty()); + primitiveSet.containsAll(Iterable.empty()); // LINT + + primitiveSet.difference({}); + primitiveSet.difference({}); // LINT + + primitiveSet.intersection({}); + primitiveSet.intersection({}); // LINT + + primitiveSet.lookup(42); + primitiveSet.lookup("str"); // LINT + + primitiveList.remove(42); + primitiveList.remove("str"); // LINT + + primitiveSet.removeAll(Iterable.empty()); + primitiveSet.removeAll(Iterable.empty()); // LINT + + primitiveSet.retainAll(Iterable.empty()); + primitiveSet.retainAll(Iterable.empty()); // LINT + } } class Animal {} diff --git a/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md b/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md index 4f6dd5ddba..e0f932a146 100644 --- a/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md +++ b/website/docs/rules/common/avoid-collection-methods-with-unrelated-types.md @@ -22,15 +22,25 @@ Bad: ```dart final map = Map(); -map["str"] = "value"; -var a = map["str"]; -map.containsKey("str"); -map.containsValue(42); -map.remove("str"); - -Iterable.empty().contains("str"); - -List().remove("str"); +map["str"] = "value"; // LINT +var a = map["str"]; // LINT +map.containsKey("str"); // LINT +map.containsValue(42); // LINT +map.remove("str"); // LINT + +Iterable.empty().contains("str"); // LINT + +List().remove("str"); // LINT + +final set = {10, 20, 30}; +set.contains("str"); // LINT +set.containsAll(Iterable.empty()); // LINT +set.difference({}); // LINT +primitiveSet.intersection({}); // LINT +set.lookup("str"); // LINT +primitiveList.remove("str"); // LINT +set.removeAll(Iterable.empty()); // LINT +set.retainAll(Iterable.empty()); // LINT ``` Good: @@ -46,4 +56,14 @@ map.remove(42); Iterable.empty().contains(42); List().remove(42); + +final set = {10, 20, 30}; +set.contains(42); +set.containsAll(Iterable.empty()); +set.difference({}); +primitiveSet.intersection({}); +set.lookup(42); +primitiveList.remove(42); +set.removeAll(Iterable.empty()); +set.retainAll(Iterable.empty()); ``` From d41ac40bf527f44f8b94327d2e1eac54e68d930b Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Fri, 25 Feb 2022 09:42:21 +0800 Subject: [PATCH 25/26] make tests pass --- ...ethods_with_unrelated_types_rule_test.dart | 75 ++++++++++++++++++- .../examples/example.dart | 4 +- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart index 4ca2b53505..1925c8babb 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/avoid_collection_methods_with_unrelated_types_rule_test.dart @@ -4,7 +4,8 @@ import 'package:test/test.dart'; import '../../../../../helpers/rule_test_helper.dart'; -const _examplePath = 'avoid_collection_methods_with_unrelated_types/examples/example.dart'; +const _examplePath = + 'avoid_collection_methods_with_unrelated_types/examples/example.dart'; void main() { group('AvoidCollectionMethodsWithUnrelatedTypesRule', () { @@ -25,8 +26,58 @@ void main() { RuleTestHelper.verifyIssues( issues: issues, - startLines: [6, 9, 12, 15, 18, 27, 32, 35, 38, 41, 48, 50, 55, 58, 64], - startColumns: [5, 16, 5, 5, 5, 5, 16, 5, 5, 5, 14, 5, 5, 5, 5], + startLines: [ + 6, + 9, + 12, + 15, + 18, + 27, + 32, + 35, + 38, + 41, + 48, + 50, + 55, + 58, + 61, + 67, + 74, + 77, + 80, + 83, + 86, + 89, + 92, + 95, + ], + startColumns: [ + 5, + 16, + 5, + 5, + 5, + 5, + 16, + 5, + 5, + 5, + 14, + 5, + 5, + 5, + 5, + 5, + 5, + 5, + 5, + 5, + 5, + 5, + 5, + 5, + ], locationTexts: [ 'primitiveMap["str"]', 'primitiveMap["str"]', @@ -42,7 +93,16 @@ void main() { 'myMap.containsKey("str")', '[1, 2, 3].contains("str")', 'Iterable.generate(10).contains("str")', + '{1, 2, 3}.contains("str")', 'primitiveList.remove("str")', + 'primitiveSet.contains("str")', + 'primitiveSet.containsAll(Iterable.empty())', + 'primitiveSet.difference({})', + 'primitiveSet.intersection({})', + 'primitiveSet.lookup("str")', + 'primitiveSet.remove("str")', + 'primitiveSet.removeAll(Iterable.empty())', + 'primitiveSet.retainAll(Iterable.empty())', ], messages: [ 'Avoid collection methods with unrelated types.', @@ -60,6 +120,15 @@ void main() { 'Avoid collection methods with unrelated types.', 'Avoid collection methods with unrelated types.', 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', + 'Avoid collection methods with unrelated types.', ], ); }); diff --git a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart index dfe7e9a06e..9f2c62c726 100644 --- a/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart +++ b/test/src/analyzers/lint_analyzer/rules/rules_list/avoid_collection_methods_with_unrelated_types/examples/example.dart @@ -85,8 +85,8 @@ void main() { primitiveSet.lookup(42); primitiveSet.lookup("str"); // LINT - primitiveList.remove(42); - primitiveList.remove("str"); // LINT + primitiveSet.remove(42); + primitiveSet.remove("str"); // LINT primitiveSet.removeAll(Iterable.empty()); primitiveSet.removeAll(Iterable.empty()); // LINT From 1a5579f9b2690aeb03d4bcc6c130b4faa12f1755 Mon Sep 17 00:00:00 2001 From: fzyzcjy Date: Sat, 26 Feb 2022 09:04:13 +0800 Subject: [PATCH 26/26] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3979f04abf..6248f580c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * **Breaking Change:** cli arguments `--fatal-unused` and `--fatal-warnings` activate by default. * chore: restrict `analyzer` version to `>=2.8.0 <3.3.0`. +* feat: add static code diagnostic `avoid-collection-methods-with-unrelated-types` ## 4.11.0