Add avoid_direct_collection_equality_checks lint

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 (<https://github.com/dart-lang/sdk/issues/60119>).
 
 # 15.1.0
 

packages/leancode_lint/README.md

@@ -256,6 +256,56 @@ None.
 
 </details>
 
+<details>
+<summary><code>avoid_direct_collection_equality_checks</code></summary>
+
+### `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<int> a, List<int> b) {
+  return a == b;
+}
+```
+
+**GOOD:**
+
+```dart
+import 'package:flutter/foundation.dart';
+
+bool sameItems(List<int> a, List<int> b) {
+  return listEquals(a, b);
+}
+```
+
+**GOOD:**
+
+```dart
+import 'package:collection/collection.dart';
+
+bool sameItems(List<int> a, List<int> b) {
+  return const ListEquality<int>().equals(a, b);
+}
+```
+
+#### Configuration
+
+None.
+
+</details>
+
 <details>
 <summary><code>bloc_related_class_naming</code></summary>
 

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,

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<void> {
+  _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<BinaryExpression>();
+
+/// 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<String>? 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<void> 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<String>? 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<void> 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<int>`) so
+    // the generated constructor is `const ListEquality<int>()` 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 <DartType>[];
+
+    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));
+    });
+  }
+}