Add avoid_direct_collection_equality_checks lint

packages/leancode_lint/CHANGELOG.md

@@ -1,8 +1,9 @@
 # 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)
   - [`avoid_build_context_in_blocs`](https://github.com/leancodepl/flutter_corelibrary/tree/master/packages/leancode_lint#avoid_build_context_in_blocs)
-  
+
 # 25.0.0
 
 - Add new custom lint [`prefer_abstract_final_class`](https://github.com/leancodepl/flutter_corelibrary/tree/master/packages/leancode_lint#prefer_abstract_final_class)

packages/leancode_lint/README.md

@@ -306,6 +306,68 @@ 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 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 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:**
+
+```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);
+}
+```
+
+**GOOD:**
+
+```dart
+bool isSameList(List<int> a, List<int> b) {
+  return identical(a, b);
+}
+```
+
+#### Configuration
+
+None.
+
+</details>
+
 <details>
 <summary><code>bloc_related_class_naming</code></summary>
 
@@ -1041,6 +1103,7 @@ See linked source code containing explanation in dart doc.
 ---
 
 ## 🛠️ Maintained by LeanCode
+
 <div align="center">
 
   [<img src="https://leancodepublic.blob.core.windows.net/public/wide.png" alt="LeanCode Logo" height="100" />][leancode-landing]

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<int> a, List<int> b) {
+  return identical(a, b);
+}
+
+bool listsAreNotEqual(List<int> a, List<int> b) {
+  return a != b;
+}
+
+bool listLiteralsAreEqual() {
+  return <int>[1, 2] == <int>[1, 2];
+}
+
+bool setsAreEqual(Set<String> a, Set<String> b) {
+  return a == b;
+}
+
+bool mapsAreEqual(Map<String, int> a, Map<String, int> b) {
+  return a == b;
+}
+
+bool nullableListsAreEqual(List<int>? a, List<int>? b) {
+  return a == b;
+}
+
+abstract class IntList implements List<int> {}
+
+bool listSubtypeIsEqualToList(IntList a, List<int> b) {
+  return a == b;
+}
+
+bool inferredMapsAreEqual() {
+  final a = {'x': 1};
+  final b = {'x': 1};
+  return a == b;
+}
+
+// --- No violations ---
+
+bool listIsNull(List<int>? a) {
+  return a == null;
+}
+
+bool listIsEqualToObject(List<int> a, Object b) {
+  return a == b;
+}
+
+bool listIsEqualToSet(List<int> a, Set<int> 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;
+}

packages/leancode_lint/lib/plugin.dart

@@ -8,6 +8,7 @@ import 'package:leancode_lint/src/lints/add_cubit_suffix_for_cubits.dart';
 import 'package:leancode_lint/src/lints/avoid_build_context_in_blocs.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';
@@ -67,6 +68,19 @@ final class LeanCodeLintPlugin extends Plugin {
       ..registerWarningRule(AvoidCatchError())
       ..registerWarningRule(AvoidBuildContextInBlocs())
       ..registerWarningRule(AvoidConditionalHooks())
+      ..registerWarningRule(AvoidDirectCollectionEqualityChecks())
+      ..registerFixForRule(
+        AvoidDirectCollectionEqualityChecks.code,
+        ReplaceWithFlutterFoundationEqualsFix.new,
+      )
+      ..registerFixForRule(
+        AvoidDirectCollectionEqualityChecks.code,
+        ReplaceWithCollectionPackageEqualityFix.new,
+      )
+      ..registerFixForRule(
+        AvoidDirectCollectionEqualityChecks.code,
+        ReplaceWithIdenticalFix.new,
+      )
       ..registerWarningRule(HookWidgetDoesNotUseHooks())
       ..registerFixForRule(
         HookWidgetDoesNotUseHooks.code,

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<TypeParameter> typeParameters, {
@@ -252,6 +260,59 @@ class _HookWidgetBodyVisitor extends SimpleAstVisitor<void> {
   }
 }
 
+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<PubspecDependency>? 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<String> 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,