fix for avoid_null_checks_in_equality_operators

Fixes https://github.com/dart-lang/sdk/issues/45919

(The bulk fix test is not awesome but the lint doesn't fire multiply in one `==` so overlaps are infeasible.)

Change-Id: I6fec933f0ba62e794c1b71a6ccb255ed92c4887b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/209661
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

pkg/analysis_server/lib/src/services/correction/dart/remove_comparison.dart

@@ -4,6 +4,7 @@
 
 import 'package:analysis_server/src/services/correction/dart/abstract_producer.dart';
 import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analysis_server/src/services/linter/lint_names.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/token.dart';
 import 'package:analyzer/error/error.dart';
@@ -14,18 +15,31 @@ import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
 import 'package:analyzer_plugin/utilities/range_factory.dart';
 
 class RemoveComparison extends CorrectionProducer {
+  @override
+  bool canBeAppliedInBulk;
+
+  @override
+  bool canBeAppliedToFile;
+
+  RemoveComparison(this.canBeAppliedInBulk, this.canBeAppliedToFile);
+
   @override
   FixKind get fixKind => DartFixKind.REMOVE_COMPARISON;
 
+  @override
+  FixKind get multiFixKind => DartFixKind.REMOVE_COMPARISON_MULTI;
+
   /// Return `true` if the null comparison will always return `false`.
   bool get _conditionIsFalse =>
       (diagnostic as AnalysisError).errorCode ==
       HintCode.UNNECESSARY_NULL_COMPARISON_FALSE;
 
   /// Return `true` if the null comparison will always return `true`.
-  bool get _conditionIsTrue =>
-      (diagnostic as AnalysisError).errorCode ==
-      HintCode.UNNECESSARY_NULL_COMPARISON_TRUE;
+  bool get _conditionIsTrue {
+    var errorCode = (diagnostic as AnalysisError).errorCode;
+    return errorCode == HintCode.UNNECESSARY_NULL_COMPARISON_TRUE ||
+        errorCode.name == LintNames.avoid_null_checks_in_equality_operators;
+  }
 
   @override
   Future<void> compute(ChangeBuilder builder) async {
@@ -108,5 +122,10 @@ class RemoveComparison extends CorrectionProducer {
   }
 
   /// Return an instance of this class. Used as a tear-off in `FixProcessor`.
-  static RemoveComparison newInstance() => RemoveComparison();
+  static RemoveComparison newInstance() => RemoveComparison(false, false);
+
+  /// Return an instance of this class that can apply bulk and in-file fixes.
+  /// Used as a tear-off in `FixProcessor`.
+  static RemoveComparison newInstanceBulkFixable() =>
+      RemoveComparison(true, true);
 }

pkg/analysis_server/lib/src/services/correction/fix.dart

@@ -126,8 +126,11 @@ class DartFixKind {
       'dart.fix.add.neNull', DartFixKindPriority.DEFAULT, 'Add != null');
   static const ADD_NE_NULL_MULTI = FixKind('dart.fix.add.neNull.multi',
       DartFixKindPriority.IN_FILE, 'Add != null everywhere in file');
-  static const ADD_NULL_CHECK = FixKind('dart.fix.add.nullCheck',
-      DartFixKindPriority.DEFAULT - 1, 'Add a null check (!)',);
+  static const ADD_NULL_CHECK = FixKind(
+    'dart.fix.add.nullCheck',
+    DartFixKindPriority.DEFAULT - 1,
+    'Add a null check (!)',
+  );
   static const ADD_OVERRIDE = FixKind('dart.fix.add.override',
       DartFixKindPriority.DEFAULT, "Add '@override' annotation");
   static const ADD_OVERRIDE_MULTI = FixKind(
@@ -454,6 +457,10 @@ class DartFixKind {
       DartFixKindPriority.IN_FILE, 'Remove awaits in file');
   static const REMOVE_COMPARISON = FixKind('dart.fix.remove.comparison',
       DartFixKindPriority.DEFAULT, 'Remove comparison');
+  static const REMOVE_COMPARISON_MULTI = FixKind(
+      'dart.fix.remove.comparison.multi',
+      DartFixKindPriority.IN_FILE,
+      'Remove comparisons in file');
   static const REMOVE_CONST = FixKind(
       'dart.fix.remove.const', DartFixKindPriority.DEFAULT, 'Remove const');
   static const REMOVE_DEAD_CODE = FixKind('dart.fix.remove.deadCode',

pkg/analysis_server/lib/src/services/correction/fix_internal.dart

@@ -344,6 +344,9 @@ class FixProcessor extends BaseProcessor {
     LintNames.avoid_init_to_null: [
       RemoveInitializer.newInstance,
     ],
+    LintNames.avoid_null_checks_in_equality_operators: [
+      RemoveComparison.newInstanceBulkFixable,
+    ],
     LintNames.avoid_private_typedef_functions: [
       InlineTypedef.newInstance,
     ],

pkg/analysis_server/lib/src/services/linter/lint_names.dart

@@ -16,6 +16,8 @@ class LintNames {
   static const String avoid_function_literals_in_foreach_calls =
       'avoid_function_literals_in_foreach_calls';
   static const String avoid_init_to_null = 'avoid_init_to_null';
+  static const String avoid_null_checks_in_equality_operators =
+      'avoid_null_checks_in_equality_operators';
   static const String avoid_private_typedef_functions =
       'avoid_private_typedef_functions';
   static const String avoid_redundant_argument_values =

pkg/analysis_server/test/src/services/correction/fix/remove_comparison_test.dart

@@ -3,6 +3,8 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analysis_server/src/services/linter/lint_names.dart';
+import 'package:analyzer/src/error/codes.dart';
 import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
 import 'package:test_reflective_loader/test_reflective_loader.dart';
 
@@ -12,6 +14,8 @@ import 'fix_processor.dart';
 void main() {
   defineReflectiveSuite(() {
     defineReflectiveTests(RemoveComparisonTest);
+    defineReflectiveTests(RemoveNullCheckComparisonTest);
+    defineReflectiveTests(RemoveNullCheckComparisonBulkTest);
   });
 }
 
@@ -188,3 +192,219 @@ void f(String s) {
 ''');
   }
 }
+
+@reflectiveTest
+class RemoveNullCheckComparisonBulkTest extends BulkFixProcessorTest {
+  @override
+  String get lintCode => LintNames.avoid_null_checks_in_equality_operators;
+
+  Future<void> test_singleFile() async {
+    await resolveTestCode('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) =>
+          other != null &&
+          other is Person &&
+          name == other.name;
+}
+
+class Person2 {
+  final String name = '';
+
+  @override
+  operator ==(other) =>
+          other != null &&
+          other is Person &&
+          name == other.name;
+}
+''');
+    await assertHasFix('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) =>
+          other is Person &&
+          name == other.name;
+}
+
+class Person2 {
+  final String name = '';
+
+  @override
+  operator ==(other) =>
+          other is Person &&
+          name == other.name;
+}
+''');
+  }
+
+  @FailingTest(reason: 'Only the first comparison is removed')
+  Future<void> test_singleFile_overlapping() async {
+    await resolveTestCode('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) =>
+          other != null &&
+          other != null &&
+          other is Person &&
+          name == other.name;
+}
+''');
+    await assertHasFix('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) =>
+          other is Person &&
+          name == other.name;
+}
+''');
+  }
+}
+
+@reflectiveTest
+class RemoveNullCheckComparisonTest extends FixProcessorLintTest {
+  @override
+  FixKind get kind => DartFixKind.REMOVE_COMPARISON;
+
+  @override
+  String get lintCode => LintNames.avoid_null_checks_in_equality_operators;
+
+  Future<void> test_expressionBody() async {
+    await resolveTestCode('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) =>
+          other != null &&
+          other is Person &&
+          name == other.name;
+}
+''');
+    await assertHasFix('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) =>
+          other is Person &&
+          name == other.name;
+}
+''',
+        errorFilter: (error) =>
+            error.errorCode == HintCode.UNNECESSARY_NULL_COMPARISON_TRUE);
+  }
+
+  Future<void> test_functionBody() async {
+    await resolveTestCode('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) {
+    return other != null &&
+          other is Person &&
+          name == other.name;
+  }
+}
+''');
+    await assertHasFix('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) {
+    return other is Person &&
+          name == other.name;
+  }
+}
+''',
+        errorFilter: (error) =>
+            error.errorCode == HintCode.UNNECESSARY_NULL_COMPARISON_TRUE);
+  }
+
+  Future<void> test_ifNullAssignmentStatement() async {
+    await resolveTestCode('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) {
+    if (other is! Person) return false;
+    other ??= Person();
+    return other.name == name;
+  }
+}
+''');
+    await assertNoFix();
+  }
+
+  /// todo(pq): consider implementing
+  @FailingTest(reason: 'Fix unimplemented')
+  Future<void> test_ifNullStatement() async {
+    await resolveTestCode('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) {
+    if (other is! Person) return false;
+    final toCompare = other ?? Person();
+    return toCompare.name == name;
+  }
+}
+''');
+
+    await assertHasFix('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) {
+    if (other is! Person) return false;
+    final toCompare = other;
+    return toCompare.name == name;
+  }
+}
+''',
+        errorFilter: (error) =>
+            error.errorCode == StaticWarningCode.DEAD_NULL_AWARE_EXPRESSION);
+  }
+
+  /// todo(pq): implement or remove the lint (see: https://github.com/dart-lang/linter/issues/2864)
+  @FailingTest(reason: 'Fix unimplemented')
+  Future<void> test_ifStatement() async {
+    await resolveTestCode('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) {
+    if (other == null) return false;
+    return other is Person &&
+          name == other.name;
+  }
+}
+''');
+    await assertHasFix('''
+class Person {
+  final String name = '';
+
+  @override
+  operator ==(other) {
+    return other is Person &&
+          name == other.name;
+  }
+}
+''',
+        errorFilter: (error) =>
+            error.errorCode == HintCode.UNNECESSARY_NULL_COMPARISON_FALSE);
+  }
+}