From 0fb3b24f99c57dfdbb513e1abbd2a6789c497e3c Mon Sep 17 00:00:00 2001 From: cupofme Date: Wed, 15 Jul 2026 16:26:37 +0200 Subject: [PATCH 1/4] Add `avoid_direct_collection_equality_checks` lint --- packages/leancode_lint/CHANGELOG.md | 7 +- packages/leancode_lint/README.md | 50 ++++ packages/leancode_lint/lib/plugin.dart | 10 + ...oid_direct_collection_equality_checks.dart | 253 ++++++++++++++++++ ...irect_collection_equality_checks_test.dart | 151 +++++++++++ 5 files changed, 470 insertions(+), 1 deletion(-) create mode 100644 packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart create mode 100644 packages/leancode_lint/test/test_cases/avoid_direct_collection_equality_checks_test.dart diff --git a/packages/leancode_lint/CHANGELOG.md b/packages/leancode_lint/CHANGELOG.md index 8398ea77..2e016e5e 100644 --- a/packages/leancode_lint/CHANGELOG.md +++ b/packages/leancode_lint/CHANGELOG.md @@ -1,3 +1,8 @@ +# Unreleased + +- Add new custom lints: + - [`avoid_direct_collection_equality_checks`](https://github.com/leancodepl/flutter_corelibrary/tree/master/packages/leancode_lint#avoid_direct_collection_equality_checks) + # 24.0.0 - Add new custom lints: @@ -93,7 +98,7 @@ - Remove the following lints which have been removed from Dart: - [`package_api_docs`](https://dart.dev/tools/linter-rules/package_api_docs) - [`unsafe_html`](https://dart.dev/tools/linter-rules/unsafe_html) -- Disable the [`require_trailing_commas`](https://dart.dev/tools/linter-rules/require_trailing_commas) lint as it conflicts with Dart 3.7 formatter (https://github.com/dart-lang/sdk/issues/60119). +- Disable the [`require_trailing_commas`](https://dart.dev/tools/linter-rules/require_trailing_commas) lint as it conflicts with Dart 3.7 formatter (). # 15.1.0 diff --git a/packages/leancode_lint/README.md b/packages/leancode_lint/README.md index fa473322..a91dafa0 100644 --- a/packages/leancode_lint/README.md +++ b/packages/leancode_lint/README.md @@ -256,6 +256,56 @@ None. +
+avoid_direct_collection_equality_checks + +### `avoid_direct_collection_equality_checks` + +**AVOID** comparing collections directly with `==` or `!=`. + +For `List`, `Set`, and `Map`, `==` compares identity (reference equality), not +contents, so `[1, 2] == [1, 2]` is `false`. Use a content-equality helper +instead. + +The rule offers two quick fixes: one rewriting the comparison to Flutter's +`listEquals`/`setEquals`/`mapEquals` (from `package:flutter/foundation.dart`), +and one to `package:collection`'s `ListEquality`/`SetEquality`/`MapEquality`. +Both add the required import and negate the result for `!=`. + +**BAD:** + +```dart +bool sameItems(List a, List b) { + return a == b; +} +``` + +**GOOD:** + +```dart +import 'package:flutter/foundation.dart'; + +bool sameItems(List a, List b) { + return listEquals(a, b); +} +``` + +**GOOD:** + +```dart +import 'package:collection/collection.dart'; + +bool sameItems(List a, List b) { + return const ListEquality().equals(a, b); +} +``` + +#### Configuration + +None. + +
+
bloc_related_class_naming diff --git a/packages/leancode_lint/lib/plugin.dart b/packages/leancode_lint/lib/plugin.dart index 4d754127..35d25551 100644 --- a/packages/leancode_lint/lib/plugin.dart +++ b/packages/leancode_lint/lib/plugin.dart @@ -7,6 +7,7 @@ import 'package:leancode_lint/src/assists/convert_record_into_nominal_type.dart' import 'package:leancode_lint/src/lints/add_cubit_suffix_for_cubits.dart'; import 'package:leancode_lint/src/lints/avoid_catch_error.dart'; import 'package:leancode_lint/src/lints/avoid_conditional_hooks.dart'; +import 'package:leancode_lint/src/lints/avoid_direct_collection_equality_checks.dart'; import 'package:leancode_lint/src/lints/avoid_single_child_in_multi_child_widget.dart'; import 'package:leancode_lint/src/lints/bloc_related_class_naming.dart'; import 'package:leancode_lint/src/lints/bloc_subclasses_naming.dart'; @@ -64,6 +65,15 @@ final class LeanCodeLintPlugin extends Plugin { ) ..registerWarningRule(AvoidCatchError()) ..registerWarningRule(AvoidConditionalHooks()) + ..registerWarningRule(AvoidDirectCollectionEqualityChecks()) + ..registerFixForRule( + AvoidDirectCollectionEqualityChecks.code, + ReplaceWithFlutterFoundationEqualsFix.new, + ) + ..registerFixForRule( + AvoidDirectCollectionEqualityChecks.code, + ReplaceWithCollectionPackageEqualityFix.new, + ) ..registerWarningRule(HookWidgetDoesNotUseHooks()) ..registerFixForRule( HookWidgetDoesNotUseHooks.code, diff --git a/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart b/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart new file mode 100644 index 00000000..ff40e659 --- /dev/null +++ b/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart @@ -0,0 +1,253 @@ +import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; +import 'package:analysis_server_plugin/edit/dart/dart_fix_kind_priority.dart'; +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/token.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:analyzer_plugin/utilities/range_factory.dart'; +import 'package:leancode_lint/src/type_checker.dart'; + +/// Displays a warning when collections are compared directly with `==` or `!=`. +class AvoidDirectCollectionEqualityChecks extends AnalysisRule { + AvoidDirectCollectionEqualityChecks() + : super(name: code.lowerCaseName, description: code.problemMessage); + + static const code = LintCode( + 'avoid_direct_collection_equality_checks', + 'Avoid comparing {0}s directly with `==` or `!=`. This compares identity, not contents.', + correctionMessage: 'Use `{1}` or `const {2}().equals` instead.', + severity: .WARNING, + ); + + @override + LintCode get diagnosticCode => code; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + registry.addBinaryExpression(this, _Visitor(this)); + } +} + +class _Visitor extends SimpleAstVisitor { + _Visitor(this.rule); + + final AnalysisRule rule; + + @override + void visitBinaryExpression(BinaryExpression node) { + final operator = node.operator.type; + if (operator != TokenType.EQ_EQ && operator != TokenType.BANG_EQ) { + return; + } + + final leftKind = collectionKind(node.leftOperand.staticType); + final rightKind = collectionKind(node.rightOperand.staticType); + if (leftKind == null || leftKind != rightKind) { + return; + } + + rule.reportAtNode( + node, + arguments: [ + leftKind.displayName, + leftKind.flutterFn, + leftKind.collectionClass, + ], + ); + } +} + +/// The kind of a core collection type, used to pick the matching equality +/// helper for the quick fixes. +enum CollectionKind { + list('List', flutterFn: 'listEquals', collectionClass: 'ListEquality'), + set('Set', flutterFn: 'setEquals', collectionClass: 'SetEquality'), + map('Map', flutterFn: 'mapEquals', collectionClass: 'MapEquality'); + + const CollectionKind( + this.displayName, { + required this.flutterFn, + required this.collectionClass, + }); + + /// The name used in the diagnostic message, e.g. `List`. + final String displayName; + + /// The `package:flutter/foundation.dart` function, e.g. `listEquals`. + final String flutterFn; + + /// The `package:collection` equality class, e.g. `ListEquality`. + final String collectionClass; +} + +/// Returns the [CollectionKind] of [type] if it is (a subtype of) a core +/// `List`, `Set`, or `Map`, otherwise `null`. +CollectionKind? collectionKind(DartType? type) { + if (type == null) { + return null; + } + + // `Map` is checked first because it is not an `Iterable`, while `Set` and + // `List` both are. + const map = TypeChecker.fromName('Map', packageName: 'dart:core'); + const set = TypeChecker.fromName('Set', packageName: 'dart:core'); + const list = TypeChecker.fromName('List', packageName: 'dart:core'); + + if (map.isAssignableFromType(type)) { + return CollectionKind.map; + } + if (set.isAssignableFromType(type)) { + return CollectionKind.set; + } + if (list.isAssignableFromType(type)) { + return CollectionKind.list; + } + return null; +} + +/// Returns the [BinaryExpression] a collection-equality fix should rewrite, or +/// `null` if it cannot be resolved from [node]. +BinaryExpression? _targetBinary(AstNode node) => + node.thisOrAncestorOfType(); + +/// Replaces a direct collection equality check with the matching Flutter +/// `listEquals`/`setEquals`/`mapEquals` call. +class ReplaceWithFlutterFoundationEqualsFix extends ResolvedCorrectionProducer { + ReplaceWithFlutterFoundationEqualsFix({required super.context}); + + @override + FixKind get fixKind => const .new( + 'leancode_lint.fix.replaceWithFlutterFoundationEquals', + DartFixKindPriority.standard, + "Replace with '{0}'", + ); + + @override + List? get fixArguments { + final binary = _targetBinary(node); + final kind = binary == null + ? null + : collectionKind(binary.leftOperand.staticType); + return [(kind ?? CollectionKind.list).flutterFn]; + } + + @override + CorrectionApplicability get applicability => .automatically; + + @override + Future compute(ChangeBuilder builder) async { + final binary = _targetBinary(node); + if (binary == null) { + return; + } + + final kind = collectionKind(binary.leftOperand.staticType); + if (kind == null) { + return; + } + + final negate = binary.operator.type == TokenType.BANG_EQ; + final left = binary.leftOperand.toSource(); + final right = binary.rightOperand.toSource(); + + await builder.addDartFileEdit(file, (builder) { + builder + ..importLibraryElement(.parse('package:flutter/foundation.dart')) + ..addReplacement( + range.node(binary), + (builder) => builder.write( + '${negate ? '!' : ''}${kind.flutterFn}($left, $right)', + ), + ) + ..format(range.node(binary)); + }); + } +} + +/// Replaces a direct collection equality check with the matching +/// `package:collection` equality, e.g. `const ListEquality().equals(a, b)`. +class ReplaceWithCollectionPackageEqualityFix + extends ResolvedCorrectionProducer { + ReplaceWithCollectionPackageEqualityFix({required super.context}); + + @override + FixKind get fixKind => const .new( + 'leancode_lint.fix.replaceWithCollectionPackageEquality', + DartFixKindPriority.standard, + "Replace with '{0}'", + ); + + @override + List? get fixArguments { + final binary = _targetBinary(node); + final kind = binary == null + ? null + : collectionKind(binary.leftOperand.staticType); + return [(kind ?? CollectionKind.list).collectionClass]; + } + + @override + CorrectionApplicability get applicability => .automatically; + + @override + Future compute(ChangeBuilder builder) async { + final binary = _targetBinary(node); + if (binary == null) { + return; + } + + final leftType = binary.leftOperand.staticType; + final kind = collectionKind(leftType); + if (kind == null) { + return; + } + + // Resolve the collection's type arguments (e.g. `int` for `List`) so + // the generated constructor is `const ListEquality()` rather than a + // raw `const ListEquality()`, which fails type inference. + final collectionElement = switch (kind) { + CollectionKind.list => typeProvider.listElement, + CollectionKind.set => typeProvider.setElement, + CollectionKind.map => typeProvider.mapElement, + }; + final typeArguments = leftType is InterfaceType + ? leftType.asInstanceOf(collectionElement)?.typeArguments ?? const [] + : const []; + + final negate = binary.operator.type == TokenType.BANG_EQ; + final left = binary.leftOperand.toSource(); + final right = binary.rightOperand.toSource(); + + await builder.addDartFileEdit(file, (builder) { + builder + ..importLibraryElement(.parse('package:collection/collection.dart')) + ..addReplacement(range.node(binary), (builder) { + if (negate) { + builder.write('!'); + } + builder.write('const ${kind.collectionClass}'); + if (typeArguments.isNotEmpty) { + builder.write('<'); + for (var i = 0; i < typeArguments.length; i++) { + if (i > 0) { + builder.write(', '); + } + builder.writeType(typeArguments[i], shouldWriteDynamic: true); + } + builder.write('>'); + } + builder.write('().equals($left, $right)'); + }) + ..format(range.node(binary)); + }); + } +} diff --git a/packages/leancode_lint/test/test_cases/avoid_direct_collection_equality_checks_test.dart b/packages/leancode_lint/test/test_cases/avoid_direct_collection_equality_checks_test.dart new file mode 100644 index 00000000..c106cbf4 --- /dev/null +++ b/packages/leancode_lint/test/test_cases/avoid_direct_collection_equality_checks_test.dart @@ -0,0 +1,151 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:leancode_lint/src/lints/avoid_direct_collection_equality_checks.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../assert_ranges.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(AvoidDirectCollectionEqualityChecksTest); + }); +} + +@reflectiveTest +class AvoidDirectCollectionEqualityChecksTest extends AnalysisRuleTest { + @override + void setUp() { + rule = AvoidDirectCollectionEqualityChecks(); + + super.setUp(); + } + + Future test_list_equality_is_marked() async { + await assertDiagnosticsInRanges(''' +bool test(List a, List b) { + return [!a == b!]; +} +'''); + } + + Future test_list_inequality_is_marked() async { + await assertDiagnosticsInRanges(''' +bool test(List a, List b) { + return [!a != b!]; +} +'''); + } + + Future test_list_literals_are_marked() async { + await assertDiagnosticsInRanges(''' +bool test() { + return [![1] == [2]!]; +} +'''); + } + + Future test_set_equality_is_marked() async { + await assertDiagnosticsInRanges(''' +bool test(Set a, Set b) { + return [!a == b!]; +} +'''); + } + + Future test_map_equality_is_marked() async { + await assertDiagnosticsInRanges(''' +bool test(Map a, Map b) { + return [!a == b!]; +} +'''); + } + + Future test_nullable_lists_are_marked() async { + await assertDiagnosticsInRanges(''' +bool test(List? a, List? b) { + return [!a == b!]; +} +'''); + } + + Future test_subtype_of_list_is_marked() async { + await assertDiagnosticsInRanges(''' +abstract class MyList implements List {} + +bool test(MyList a, List b) { + return [!a == b!]; +} +'''); + } + + Future test_local_map_variables_are_marked() async { + await assertDiagnosticsInRanges(''' +bool test() { + final a = {'x': 1}; + final b = {'y': 2}; + return [!a == b!]; +} +'''); + } + + Future test_list_of_constructor_is_marked() async { + await assertDiagnosticsInRanges(''' +bool test() { + final a = [1, 2, 3]; + final b = [1, 2, 3]; + return [!a == List.of(b)!]; +} +'''); + } + + Future test_comparison_with_null_is_not_marked() async { + await assertNoDiagnostics(''' +bool test(List? a) { + return a == null; +} +'''); + } + + Future test_collection_vs_non_collection_is_not_marked() async { + await assertNoDiagnostics(''' +bool test(List a, Object b) { + return a == b; +} +'''); + } + + Future test_scalar_comparison_is_not_marked() async { + await assertNoDiagnostics(''' +bool test(int a, int b, String c, String d) { + return a == b && c == d; +} +'''); + } + + Future test_different_collection_kinds_are_not_marked() async { + await assertNoDiagnostics(''' +bool test(List a, Set b) { + return a == b; +} +'''); + } + + Future test_custom_class_with_equals_is_not_marked() async { + await assertNoDiagnostics(''' +class Value { + const Value(this.value); + + final int value; + + @override + bool operator ==(Object other) => other is Value && other.value == value; + + @override + int get hashCode => value.hashCode; +} + +bool test(Value a, Value b) { + return a == b; +} +'''); + } +} From 9f6012c6e424335b4b1d000079239b9f9e71a68f Mon Sep 17 00:00:00 2001 From: cupofme Date: Thu, 16 Jul 2026 13:14:22 +0200 Subject: [PATCH 2/4] Address PR comments --- packages/leancode_lint/README.md | 10 +- packages/leancode_lint/lib/plugin.dart | 4 + ...oid_direct_collection_equality_checks.dart | 93 +++++++++++++++++-- 3 files changed, 95 insertions(+), 12 deletions(-) diff --git a/packages/leancode_lint/README.md b/packages/leancode_lint/README.md index a91dafa0..13e59cc5 100644 --- a/packages/leancode_lint/README.md +++ b/packages/leancode_lint/README.md @@ -267,10 +267,14 @@ For `List`, `Set`, and `Map`, `==` compares identity (reference equality), not contents, so `[1, 2] == [1, 2]` is `false`. Use a content-equality helper instead. -The rule offers two quick fixes: one rewriting the comparison to Flutter's +The rule offers three quick fixes: one rewriting the comparison to Flutter's `listEquals`/`setEquals`/`mapEquals` (from `package:flutter/foundation.dart`), -and one to `package:collection`'s `ListEquality`/`SetEquality`/`MapEquality`. -Both add the required import and negate the result for `!=`. +one to `package:collection`'s `ListEquality`/`SetEquality`/`MapEquality`, and one +to `identical` for the cases where an identity comparison is actually intended. +The `package:collection` fix is only offered when the equality class is reachable +through a direct dependency — either `package:collection` itself or a package +that re-exports it — and imports it via that dependency. The content-equality +fixes add the required import, and all fixes negate the result for `!=`. **BAD:** diff --git a/packages/leancode_lint/lib/plugin.dart b/packages/leancode_lint/lib/plugin.dart index 35d25551..50ee27da 100644 --- a/packages/leancode_lint/lib/plugin.dart +++ b/packages/leancode_lint/lib/plugin.dart @@ -74,6 +74,10 @@ final class LeanCodeLintPlugin extends Plugin { AvoidDirectCollectionEqualityChecks.code, ReplaceWithCollectionPackageEqualityFix.new, ) + ..registerFixForRule( + AvoidDirectCollectionEqualityChecks.code, + ReplaceWithIdenticalFix.new, + ) ..registerWarningRule(HookWidgetDoesNotUseHooks()) ..registerFixForRule( HookWidgetDoesNotUseHooks.code, diff --git a/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart b/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart index ff40e659..abb8500f 100644 --- a/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart +++ b/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart @@ -8,10 +8,12 @@ import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/error/error.dart'; +import 'package:analyzer/workspace/workspace.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; import 'package:analyzer_plugin/utilities/range_factory.dart'; import 'package:leancode_lint/src/type_checker.dart'; +import 'package:yaml/yaml.dart'; /// Displays a warning when collections are compared directly with `==` or `!=`. class AvoidDirectCollectionEqualityChecks extends AnalysisRule { @@ -21,7 +23,8 @@ class AvoidDirectCollectionEqualityChecks extends AnalysisRule { static const code = LintCode( 'avoid_direct_collection_equality_checks', 'Avoid comparing {0}s directly with `==` or `!=`. This compares identity, not contents.', - correctionMessage: 'Use `{1}` or `const {2}().equals` instead.', + correctionMessage: + 'Use `{1}` or `const {2}().equals` to compare contents, or `identical` if an identity check is intended.', severity: .WARNING, ); @@ -59,7 +62,7 @@ class _Visitor extends SimpleAstVisitor { node, arguments: [ leftKind.displayName, - leftKind.flutterFn, + leftKind.flutterFunction, leftKind.collectionClass, ], ); @@ -69,13 +72,13 @@ class _Visitor extends SimpleAstVisitor { /// The kind of a core collection type, used to pick the matching equality /// helper for the quick fixes. enum CollectionKind { - list('List', flutterFn: 'listEquals', collectionClass: 'ListEquality'), - set('Set', flutterFn: 'setEquals', collectionClass: 'SetEquality'), - map('Map', flutterFn: 'mapEquals', collectionClass: 'MapEquality'); + list('List', flutterFunction: 'listEquals', collectionClass: 'ListEquality'), + set('Set', flutterFunction: 'setEquals', collectionClass: 'SetEquality'), + map('Map', flutterFunction: 'mapEquals', collectionClass: 'MapEquality'); const CollectionKind( this.displayName, { - required this.flutterFn, + required this.flutterFunction, required this.collectionClass, }); @@ -83,7 +86,7 @@ enum CollectionKind { final String displayName; /// The `package:flutter/foundation.dart` function, e.g. `listEquals`. - final String flutterFn; + final String flutterFunction; /// The `package:collection` equality class, e.g. `ListEquality`. final String collectionClass; @@ -137,7 +140,7 @@ class ReplaceWithFlutterFoundationEqualsFix extends ResolvedCorrectionProducer { final kind = binary == null ? null : collectionKind(binary.leftOperand.staticType); - return [(kind ?? CollectionKind.list).flutterFn]; + return [(kind ?? CollectionKind.list).flutterFunction]; } @override @@ -165,7 +168,7 @@ class ReplaceWithFlutterFoundationEqualsFix extends ResolvedCorrectionProducer { ..addReplacement( range.node(binary), (builder) => builder.write( - '${negate ? '!' : ''}${kind.flutterFn}($left, $right)', + '${negate ? '!' : ''}${kind.flutterFunction}($left, $right)', ), ) ..format(range.node(binary)); @@ -198,6 +201,36 @@ class ReplaceWithCollectionPackageEqualityFix @override CorrectionApplicability get applicability => .automatically; + /// Whether the package that owns the analyzed file directly depends on + /// `package:collection`. Transitive dependencies do not count, since the + /// rewrite must not introduce an import the package cannot resolve on its + /// own. + bool get _dependsOnCollection { + final WorkspacePackage? package = sessionHelper + .session + .analysisContext + .contextRoot + .workspace + .findPackageFor(file); + final pubspec = package?.root.getFile('pubspec.yaml'); + if (pubspec == null || !pubspec.exists) { + return false; + } + + final YamlNode document; + try { + document = loadYamlNode(pubspec.readAsStringSync()); + } on Exception { + return false; + } + if (document is! YamlMap) { + return false; + } + + final dependencies = document['dependencies']; + return dependencies is YamlMap && dependencies.containsKey('collection'); + } + @override Future compute(ChangeBuilder builder) async { final binary = _targetBinary(node); @@ -205,6 +238,10 @@ class ReplaceWithCollectionPackageEqualityFix return; } + if (!_dependsOnCollection) { + return; + } + final leftType = binary.leftOperand.staticType; final kind = collectionKind(leftType); if (kind == null) { @@ -251,3 +288,41 @@ class ReplaceWithCollectionPackageEqualityFix }); } } + +/// Replaces a direct collection equality check with an `identical` call, for +/// the cases where an identity comparison is actually intended. +class ReplaceWithIdenticalFix extends ResolvedCorrectionProducer { + ReplaceWithIdenticalFix({required super.context}); + + @override + FixKind get fixKind => const .new( + 'leancode_lint.fix.replaceWithIdentical', + DartFixKindPriority.standard, + "Replace with 'identical'", + ); + + @override + CorrectionApplicability get applicability => .automatically; + + @override + Future compute(ChangeBuilder builder) async { + final binary = _targetBinary(node); + if (binary == null) { + return; + } + + final negate = binary.operator.type == TokenType.BANG_EQ; + final left = binary.leftOperand.toSource(); + final right = binary.rightOperand.toSource(); + + await builder.addDartFileEdit(file, (builder) { + builder + ..addReplacement( + range.node(binary), + (builder) => + builder.write('${negate ? '!' : ''}identical($left, $right)'), + ) + ..format(range.node(binary)); + }); + } +} From 1c84805b68ae5e94b52df26824959d86aec141dc Mon Sep 17 00:00:00 2001 From: cupofme Date: Thu, 16 Jul 2026 13:54:27 +0200 Subject: [PATCH 3/4] Replace getFile with getChildAssumingFile --- .../lib/src/lints/avoid_direct_collection_equality_checks.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart b/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart index abb8500f..e15f5893 100644 --- a/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart +++ b/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart @@ -212,7 +212,7 @@ class ReplaceWithCollectionPackageEqualityFix .contextRoot .workspace .findPackageFor(file); - final pubspec = package?.root.getFile('pubspec.yaml'); + final pubspec = package?.root.getChildAssumingFile('pubspec.yaml'); if (pubspec == null || !pubspec.exists) { return false; } From 98d6bd9e0826e0b1259644a7589052ee7eb18f4c Mon Sep 17 00:00:00 2001 From: cupofme Date: Mon, 27 Jul 2026 12:58:27 +0200 Subject: [PATCH 4/4] Address PR comments --- packages/leancode_lint/README.md | 17 ++- ...oid_direct_collection_equality_checks.dart | 84 +++++++++++ packages/leancode_lint/lib/src/helpers.dart | 61 ++++++++ ...oid_direct_collection_equality_checks.dart | 81 +++-------- .../test_cases/depends_on_package_test.dart | 130 ++++++++++++++++++ 5 files changed, 309 insertions(+), 64 deletions(-) create mode 100644 packages/leancode_lint/example/default/lib/avoid_direct_collection_equality_checks.dart create mode 100644 packages/leancode_lint/test/test_cases/depends_on_package_test.dart diff --git a/packages/leancode_lint/README.md b/packages/leancode_lint/README.md index 609563b5..1dbfacd8 100644 --- a/packages/leancode_lint/README.md +++ b/packages/leancode_lint/README.md @@ -271,10 +271,10 @@ The rule offers three quick fixes: one rewriting the comparison to Flutter's `listEquals`/`setEquals`/`mapEquals` (from `package:flutter/foundation.dart`), one to `package:collection`'s `ListEquality`/`SetEquality`/`MapEquality`, and one to `identical` for the cases where an identity comparison is actually intended. -The `package:collection` fix is only offered when the equality class is reachable -through a direct dependency — either `package:collection` itself or a package -that re-exports it — and imports it via that dependency. The content-equality -fixes add the required import, and all fixes negate the result for `!=`. +The content-equality fixes add the required import, so each is only offered when +the package owning the analyzed file declares a direct dependency on the package +it imports (`flutter` and `collection` respectively). All fixes negate the result for +`!=`. **BAD:** @@ -304,6 +304,14 @@ bool sameItems(List a, List b) { } ``` +**GOOD:** + +```dart +bool isSameList(List a, List b) { + return identical(a, b); +} +``` + #### Configuration None. @@ -1045,6 +1053,7 @@ See linked source code containing explanation in dart doc. --- ## 🛠️ Maintained by LeanCode +
[LeanCode Logo][leancode-landing] diff --git a/packages/leancode_lint/example/default/lib/avoid_direct_collection_equality_checks.dart b/packages/leancode_lint/example/default/lib/avoid_direct_collection_equality_checks.dart new file mode 100644 index 00000000..6a09085c --- /dev/null +++ b/packages/leancode_lint/example/default/lib/avoid_direct_collection_equality_checks.dart @@ -0,0 +1,84 @@ +// Examples for the `avoid_direct_collection_equality_checks` lint. +// +// This package depends on `flutter` but not on `collection`, so only the +// `listEquals`/`setEquals`/`mapEquals` and `identical` fixes are offered. Add +// `collection: any` to the dependencies in `pubspec.yaml` and run +// `flutter pub get` to see the `const ListEquality().equals` fix appear too — +// the transitive dependency this package already has through `flutter` +// deliberately does not enable it. + +// --- Violations --- + +bool listsAreEqual(List a, List b) { + return identical(a, b); +} + +bool listsAreNotEqual(List a, List b) { + return a != b; +} + +bool listLiteralsAreEqual() { + return [1, 2] == [1, 2]; +} + +bool setsAreEqual(Set a, Set b) { + return a == b; +} + +bool mapsAreEqual(Map a, Map b) { + return a == b; +} + +bool nullableListsAreEqual(List? a, List? b) { + return a == b; +} + +abstract class IntList implements List {} + +bool listSubtypeIsEqualToList(IntList a, List b) { + return a == b; +} + +bool inferredMapsAreEqual() { + final a = {'x': 1}; + final b = {'x': 1}; + return a == b; +} + +// --- No violations --- + +bool listIsNull(List? a) { + return a == null; +} + +bool listIsEqualToObject(List a, Object b) { + return a == b; +} + +bool listIsEqualToSet(List a, Set b) { + // The point here is that the lint stays quiet on mismatched kinds. + // ignore: unrelated_type_equality_checks + return a == b; +} + +bool scalarsAreEqual(int a, int b) { + return a == b; +} + +class Point { + const Point(this.x, this.y); + + final int x; + final int y; + + @override + bool operator ==(Object other) => + other is Point && other.x == x && other.y == y; + + @override + int get hashCode => Object.hash(x, y); +} + +bool pointsAreEqual(Point a, Point b) { + return a == b; +} diff --git a/packages/leancode_lint/lib/src/helpers.dart b/packages/leancode_lint/lib/src/helpers.dart index 296106d8..c7c0fa4a 100644 --- a/packages/leancode_lint/lib/src/helpers.dart +++ b/packages/leancode_lint/lib/src/helpers.dart @@ -1,15 +1,23 @@ import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; import 'package:analysis_server_plugin/edit/dart/dart_fix_kind_priority.dart'; import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/pubspec.dart'; import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/dart/ast/visitor.dart'; +// `PubPackage`, the only way to reach the parsed pubspec of the package owning +// a file, has no public equivalent. The SDK's own +// `depend_on_referenced_packages` reaches for it the same way. +// ignore: implementation_imports +import 'package:analyzer/src/workspace/pub.dart'; +import 'package:analyzer/workspace/workspace.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; import 'package:analyzer_plugin/utilities/range_factory.dart'; import 'package:leancode_lint/src/type_checker.dart'; import 'package:leancode_lint/src/utils.dart'; +import 'package:meta/meta.dart'; String typeParametersString( Iterable typeParameters, { @@ -252,6 +260,59 @@ class _HookWidgetBodyVisitor extends SimpleAstVisitor { } } +extension PackageDependencies on ResolvedCorrectionProducer { + bool dependsOnPackage(String packageName) => packageDependsOn( + packageName, + package: sessionHelper.session.analysisContext.contextRoot.workspace + .findPackageFor(file), + filePath: file, + ); +} + +/// Whether [package] declares a direct dependency on [packageName], as seen +/// from the file at [filePath]. +/// +/// A transitive dependency does not count: an import of it resolves today, but +/// breaks as soon as the intermediate package stops depending on +/// [packageName]. The package config cannot tell the two apart, hence the +/// pubspec. Dev dependencies count only outside the package's public +/// directories, since code that ships to consumers cannot rely on them. +/// +/// This mirrors how the SDK's `depend_on_referenced_packages` answers the very +/// same question. +@visibleForTesting +bool packageDependsOn( + String packageName, { + required WorkspacePackage? package, + required String filePath, +}) { + if (package is! PubPackage) { + return false; + } + final pubspec = package.pubspec; + if (pubspec == null) { + return false; + } + + bool declares(Iterable? dependencies) => + dependencies?.any((dep) => dep.name?.text == packageName) ?? false; + + return declares(pubspec.dependencies) || + (!_isInPublicDir(filePath, package) && declares(pubspec.devDependencies)); +} + +/// Mirrors `isInPublicDir` from the SDK's linter. +bool _isInPublicDir(String filePath, WorkspacePackage package) { + final pathContext = package.root.provider.pathContext; + String inRoot(List parts) => + pathContext.joinAll([package.root.path, ...parts]); + + return pathContext.isWithin(inRoot(['lib']), filePath) || + pathContext.isWithin(inRoot(['bin']), filePath) || + filePath == inRoot(['hook', 'build.dart']) || + filePath == inRoot(['hook', 'link.dart']); +} + bool isExpressionExactlyType( Expression expression, String typeName, diff --git a/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart b/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart index e15f5893..a9ef725f 100644 --- a/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart +++ b/packages/leancode_lint/lib/src/lints/avoid_direct_collection_equality_checks.dart @@ -8,14 +8,11 @@ import 'package:analyzer/dart/ast/token.dart'; import 'package:analyzer/dart/ast/visitor.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:analyzer/error/error.dart'; -import 'package:analyzer/workspace/workspace.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; import 'package:analyzer_plugin/utilities/range_factory.dart'; -import 'package:leancode_lint/src/type_checker.dart'; -import 'package:yaml/yaml.dart'; +import 'package:leancode_lint/src/helpers.dart'; -/// Displays a warning when collections are compared directly with `==` or `!=`. class AvoidDirectCollectionEqualityChecks extends AnalysisRule { AvoidDirectCollectionEqualityChecks() : super(name: code.lowerCaseName, description: code.problemMessage); @@ -69,8 +66,6 @@ class _Visitor extends SimpleAstVisitor { } } -/// The kind of a core collection type, used to pick the matching equality -/// helper for the quick fixes. enum CollectionKind { list('List', flutterFunction: 'listEquals', collectionClass: 'ListEquality'), set('Set', flutterFunction: 'setEquals', collectionClass: 'SetEquality'), @@ -82,48 +77,43 @@ enum CollectionKind { required this.collectionClass, }); - /// The name used in the diagnostic message, e.g. `List`. final String displayName; - /// The `package:flutter/foundation.dart` function, e.g. `listEquals`. + /// From `package:flutter/foundation.dart`. final String flutterFunction; - /// The `package:collection` equality class, e.g. `ListEquality`. + /// From `package:collection`. final String collectionClass; } -/// Returns the [CollectionKind] of [type] if it is (a subtype of) a core -/// `List`, `Set`, or `Map`, otherwise `null`. +/// Also matches subtypes of `List`, `Set` and `Map`. CollectionKind? collectionKind(DartType? type) { - if (type == null) { + if (type is! InterfaceType) { return null; } + final types = [type, ...type.allSupertypes]; + // `Map` is checked first because it is not an `Iterable`, while `Set` and // `List` both are. - const map = TypeChecker.fromName('Map', packageName: 'dart:core'); - const set = TypeChecker.fromName('Set', packageName: 'dart:core'); - const list = TypeChecker.fromName('List', packageName: 'dart:core'); - - if (map.isAssignableFromType(type)) { + if (types.any((it) => it.isDartCoreMap)) { return CollectionKind.map; } - if (set.isAssignableFromType(type)) { + if (types.any((it) => it.isDartCoreSet)) { return CollectionKind.set; } - if (list.isAssignableFromType(type)) { + if (types.any((it) => it.isDartCoreList)) { return CollectionKind.list; } return null; } -/// Returns the [BinaryExpression] a collection-equality fix should rewrite, or -/// `null` if it cannot be resolved from [node]. +const _flutterFoundationUri = 'package:flutter/foundation.dart'; +const _collectionUri = 'package:collection/collection.dart'; + BinaryExpression? _targetBinary(AstNode node) => node.thisOrAncestorOfType(); -/// Replaces a direct collection equality check with the matching Flutter -/// `listEquals`/`setEquals`/`mapEquals` call. class ReplaceWithFlutterFoundationEqualsFix extends ResolvedCorrectionProducer { ReplaceWithFlutterFoundationEqualsFix({required super.context}); @@ -153,6 +143,10 @@ class ReplaceWithFlutterFoundationEqualsFix extends ResolvedCorrectionProducer { return; } + if (!dependsOnPackage('flutter')) { + return; + } + final kind = collectionKind(binary.leftOperand.staticType); if (kind == null) { return; @@ -164,7 +158,7 @@ class ReplaceWithFlutterFoundationEqualsFix extends ResolvedCorrectionProducer { await builder.addDartFileEdit(file, (builder) { builder - ..importLibraryElement(.parse('package:flutter/foundation.dart')) + ..importLibraryElement(.parse(_flutterFoundationUri)) ..addReplacement( range.node(binary), (builder) => builder.write( @@ -176,8 +170,6 @@ class ReplaceWithFlutterFoundationEqualsFix extends ResolvedCorrectionProducer { } } -/// Replaces a direct collection equality check with the matching -/// `package:collection` equality, e.g. `const ListEquality().equals(a, b)`. class ReplaceWithCollectionPackageEqualityFix extends ResolvedCorrectionProducer { ReplaceWithCollectionPackageEqualityFix({required super.context}); @@ -201,36 +193,6 @@ class ReplaceWithCollectionPackageEqualityFix @override CorrectionApplicability get applicability => .automatically; - /// Whether the package that owns the analyzed file directly depends on - /// `package:collection`. Transitive dependencies do not count, since the - /// rewrite must not introduce an import the package cannot resolve on its - /// own. - bool get _dependsOnCollection { - final WorkspacePackage? package = sessionHelper - .session - .analysisContext - .contextRoot - .workspace - .findPackageFor(file); - final pubspec = package?.root.getChildAssumingFile('pubspec.yaml'); - if (pubspec == null || !pubspec.exists) { - return false; - } - - final YamlNode document; - try { - document = loadYamlNode(pubspec.readAsStringSync()); - } on Exception { - return false; - } - if (document is! YamlMap) { - return false; - } - - final dependencies = document['dependencies']; - return dependencies is YamlMap && dependencies.containsKey('collection'); - } - @override Future compute(ChangeBuilder builder) async { final binary = _targetBinary(node); @@ -238,7 +200,7 @@ class ReplaceWithCollectionPackageEqualityFix return; } - if (!_dependsOnCollection) { + if (!dependsOnPackage('collection')) { return; } @@ -266,7 +228,7 @@ class ReplaceWithCollectionPackageEqualityFix await builder.addDartFileEdit(file, (builder) { builder - ..importLibraryElement(.parse('package:collection/collection.dart')) + ..importLibraryElement(.parse(_collectionUri)) ..addReplacement(range.node(binary), (builder) { if (negate) { builder.write('!'); @@ -289,8 +251,7 @@ class ReplaceWithCollectionPackageEqualityFix } } -/// Replaces a direct collection equality check with an `identical` call, for -/// the cases where an identity comparison is actually intended. +/// For the cases where an identity comparison is actually intended. class ReplaceWithIdenticalFix extends ResolvedCorrectionProducer { ReplaceWithIdenticalFix({required super.context}); diff --git a/packages/leancode_lint/test/test_cases/depends_on_package_test.dart b/packages/leancode_lint/test/test_cases/depends_on_package_test.dart new file mode 100644 index 00000000..0065f43e --- /dev/null +++ b/packages/leancode_lint/test/test_cases/depends_on_package_test.dart @@ -0,0 +1,130 @@ +import 'dart:convert'; + +import 'package:analyzer/file_system/memory_file_system.dart'; +import 'package:analyzer/src/context/packages.dart'; +import 'package:analyzer/src/workspace/pub.dart'; +import 'package:analyzer/workspace/workspace.dart'; +import 'package:leancode_lint/src/helpers.dart'; +import 'package:test/test.dart'; + +void main() { + late MemoryResourceProvider provider; + late String rootPath; + + String inRoot(List parts) => + provider.pathContext.joinAll([rootPath, ...parts]); + + WorkspacePackage? buildPackage({ + List dependencies = const [], + List devDependencies = const [], + }) { + String section(String name, List packages) => packages.isEmpty + ? '' + : '$name:\n${packages.map((p) => ' $p: any\n').join()}'; + + provider.newFile( + inRoot(['pubspec.yaml']), + ''' +name: my_app +${section('dependencies', dependencies)}${section('dev_dependencies', devDependencies)}''', + ); + + final packageConfigFile = provider.newFile( + inRoot(['.dart_tool', 'package_config.json']), + jsonEncode({ + 'configVersion': 2, + 'packages': [ + {'name': 'my_app', 'rootUri': '../', 'packageUri': 'lib/'}, + // Resolvable, but not declared by `my_app` unless a test says so. + { + 'name': 'collection', + 'rootUri': '/pub-cache/collection', + 'packageUri': 'lib/', + }, + ], + }), + ); + + return PackageConfigWorkspace( + provider, + rootPath, + packageConfigFile, + Packages.empty, + ).findPackageFor(inRoot(['lib', 'a.dart'])); + } + + setUp(() { + provider = MemoryResourceProvider(); + rootPath = provider.pathContext.join( + provider.pathContext.rootPrefix(provider.pathContext.current), + 'home', + 'my_app', + ); + }); + + test('finds a direct dependency', () { + expect( + packageDependsOn( + 'collection', + package: buildPackage(dependencies: ['collection']), + filePath: inRoot(['lib', 'a.dart']), + ), + isTrue, + ); + }); + + test('does not find a package that is only transitively available', () { + expect( + packageDependsOn( + 'collection', + package: buildPackage(dependencies: ['flutter']), + filePath: inRoot(['lib', 'a.dart']), + ), + isFalse, + ); + }); + + test('ignores dev dependencies for files in public directories', () { + final package = buildPackage(devDependencies: ['collection']); + + for (final path in [ + inRoot(['lib', 'a.dart']), + inRoot(['lib', 'src', 'a.dart']), + inRoot(['bin', 'a.dart']), + inRoot(['hook', 'build.dart']), + ]) { + expect( + packageDependsOn('collection', package: package, filePath: path), + isFalse, + reason: path, + ); + } + }); + + test('accepts dev dependencies elsewhere', () { + final package = buildPackage(devDependencies: ['collection']); + + for (final path in [ + inRoot(['test', 'a_test.dart']), + inRoot(['tool', 'a.dart']), + inRoot(['example', 'lib', 'a.dart']), + ]) { + expect( + packageDependsOn('collection', package: package, filePath: path), + isTrue, + reason: path, + ); + } + }); + + test('returns false when the file has no package', () { + expect( + packageDependsOn( + 'collection', + package: null, + filePath: inRoot(['lib', 'a.dart']), + ), + isFalse, + ); + }); +}