[analysis_server] associate UNNECESSARY_TYPE_CHECK_* with RemoveComparison

Fixes #47793
Fixes #51013

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

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

@@ -15,40 +15,48 @@ import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
 import 'package:analyzer_plugin/utilities/range_factory.dart';
 
 class RemoveComparison extends CorrectionProducer {
+  @override
+  final FixKind fixKind;
+
+  @override
+  final FixKind multiFixKind;
+
+  /// Initialize a newly created instance with [DartFixKind.REMOVE_COMPARISON].
+  RemoveComparison()
+      : fixKind = DartFixKind.REMOVE_COMPARISON,
+        multiFixKind = DartFixKind.REMOVE_COMPARISON_MULTI;
+
+  /// Initialize a newly created instance with [DartFixKind.REMOVE_TYPE_CHECK].
+  RemoveComparison.typeCheck()
+      : fixKind = DartFixKind.REMOVE_TYPE_CHECK,
+        multiFixKind = DartFixKind.REMOVE_TYPE_CHECK_MULTI;
+
   @override
   bool get canBeAppliedInBulk => true;
 
   @override
   bool get canBeAppliedToFile => true;
 
-  @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`.
+  /// Return `true` if the condition will always return `false`.
   bool get _conditionIsFalse {
     var errorCode = (diagnostic as AnalysisError).errorCode;
     return errorCode == HintCode.UNNECESSARY_NAN_COMPARISON_FALSE ||
-        errorCode == HintCode.UNNECESSARY_NULL_COMPARISON_FALSE;
+        errorCode == HintCode.UNNECESSARY_NULL_COMPARISON_FALSE ||
+        errorCode == HintCode.UNNECESSARY_TYPE_CHECK_FALSE;
   }
 
-  /// Return `true` if the null comparison will always return `true`.
+  /// Return `true` if the condition will always return `true`.
   bool get _conditionIsTrue {
     var errorCode = (diagnostic as AnalysisError).errorCode;
     return errorCode == HintCode.UNNECESSARY_NAN_COMPARISON_TRUE ||
         errorCode == HintCode.UNNECESSARY_NULL_COMPARISON_TRUE ||
+        errorCode == HintCode.UNNECESSARY_TYPE_CHECK_TRUE ||
         errorCode.name == LintNames.avoid_null_checks_in_equality_operators;
   }
 
   @override
   Future<void> compute(ChangeBuilder builder) async {
-    var binaryExpression = node;
-    if (binaryExpression is! BinaryExpression) {
-      return;
-    }
-    var parent = binaryExpression.parent;
+    var parent = node.parent;
     if (parent is AssertInitializer && _conditionIsTrue) {
       var constructor = parent.parent as ConstructorDeclaration;
       var list = constructor.initializers;
@@ -66,12 +74,10 @@ class RemoveComparison extends CorrectionProducer {
         builder.addDeletion(utils.getLinesRange(range.node(parent)));
       });
     } else if (parent is BinaryExpression) {
-      if (parent.operator.type == TokenType.AMPERSAND_AMPERSAND &&
-          _conditionIsTrue) {
-        await _removeOperatorAndOperand(builder, parent, binaryExpression);
-      } else if (parent.operator.type == TokenType.BAR_BAR &&
-          _conditionIsFalse) {
-        await _removeOperatorAndOperand(builder, parent, binaryExpression);
+      var type = parent.operator.type;
+      if ((type == TokenType.AMPERSAND_AMPERSAND && _conditionIsTrue) ||
+          (type == TokenType.BAR_BAR && _conditionIsFalse)) {
+        await _removeOperatorAndOperand(builder, parent);
       }
     } else if (parent is IfStatement) {
       if (parent.elseStatement == null && _conditionIsTrue) {
@@ -109,8 +115,8 @@ class RemoveComparison extends CorrectionProducer {
 
   /// Use the [builder] to add an edit to delete the operator and given
   /// [operand] from the [binary] expression.
-  Future<void> _removeOperatorAndOperand(ChangeBuilder builder,
-      BinaryExpression binary, Expression operand) async {
+  Future<void> _removeOperatorAndOperand(
+      ChangeBuilder builder, BinaryExpression binary) async {
     SourceRange operatorAndOperand;
     if (binary.leftOperand == node) {
       operatorAndOperand = range.startStart(node, binary.rightOperand);

pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml

@@ -1630,11 +1630,9 @@ HintCode.UNNECESSARY_NULL_COMPARISON_TRUE:
 HintCode.UNNECESSARY_QUESTION_MARK:
   status: hasFix
 HintCode.UNNECESSARY_TYPE_CHECK_FALSE:
-  status: needsFix
-  issue: https://github.com/dart-lang/sdk/issues/47793
+  status: hasFix
 HintCode.UNNECESSARY_TYPE_CHECK_TRUE:
-  status: needsFix
-  issue: https://github.com/dart-lang/sdk/issues/47793
+  status: hasFix
 HintCode.TEXT_DIRECTION_CODE_POINT_IN_COMMENT:
   status: hasFix
 HintCode.TEXT_DIRECTION_CODE_POINT_IN_LITERAL:

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

@@ -1198,6 +1198,16 @@ class DartFixKind {
     49,
     'Remove type arguments',
   );
+  static const REMOVE_TYPE_CHECK = FixKind(
+    'dart.fix.remove.typeCheck',
+    DartFixKindPriority.DEFAULT,
+    'Remove type check',
+  );
+  static const REMOVE_TYPE_CHECK_MULTI = FixKind(
+    'dart.fix.remove.comparison.multi',
+    DartFixKindPriority.IN_FILE,
+    'Remove type check everywhere in file',
+  );
   static const REMOVE_UNNECESSARY_CAST = FixKind(
     'dart.fix.remove.unnecessaryCast',
     DartFixKindPriority.DEFAULT,

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

@@ -1465,12 +1465,12 @@ class FixProcessor extends BaseProcessor {
     HintCode.UNNECESSARY_QUESTION_MARK: [
       RemoveQuestionMark.new,
     ],
-//    HintCode.UNNECESSARY_TYPE_CHECK_FALSE: [
-// TODO(brianwilkerson) Add a fix to remove the type check.
-//    ],
-//    HintCode.UNNECESSARY_TYPE_CHECK_TRUE: [
-// TODO(brianwilkerson) Add a fix to remove the type check.
-//    ],
+    HintCode.UNNECESSARY_TYPE_CHECK_FALSE: [
+      RemoveComparison.typeCheck,
+    ],
+    HintCode.UNNECESSARY_TYPE_CHECK_TRUE: [
+      RemoveComparison.typeCheck,
+    ],
     HintCode.UNUSED_CATCH_CLAUSE: [
       RemoveUnusedCatchClause.new,
     ],

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

@@ -4,7 +4,6 @@
 
 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';
 
@@ -13,6 +12,8 @@ import 'fix_processor.dart';
 void main() {
   defineReflectiveSuite(() {
     defineReflectiveTests(RemoveComparisonTest);
+    defineReflectiveTests(RemoveTypeCheckTest);
+    defineReflectiveTests(RemoveTypeCheckBulkTest);
     defineReflectiveTests(RemoveNullCheckComparisonTest);
     defineReflectiveTests(RemoveNullCheckComparisonBulkTest);
   });
@@ -372,66 +373,74 @@ class Person {
 ''');
     await assertNoFix();
   }
+}
 
-  /// todo(pq): consider implementing
-  @FailingTest(reason: 'Fix unimplemented')
-  Future<void> test_ifNullStatement() async {
+@reflectiveTest
+class RemoveTypeCheckBulkTest extends BulkFixProcessorTest {
+  Future<void> test_singleFile() async {
     await resolveTestCode('''
-class Person {
-  final String name = '';
-
-  @override
-  operator ==(Object? other) {
-    if (other is! Person) return false;
-    final toCompare = other ?? Person();
-    return toCompare.name == name;
+void f(int a, int b) {
+  if (a is! num || b == 0) {
+    print('');
   }
 }
-''');
-
-    await assertHasFix('''
-class Person {
-  final String name = '';
-
-  @override
-  operator ==(Object? 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 ==(Object? other) {
-    if (other == null) return false;
-    return other is Person &&
-          name == other.name;
+void g(int a, int b) {
+  if (b == 0 && a is num) {
+    print('');
   }
 }
 ''');
     await assertHasFix('''
-class Person {
-  final String name = '';
+void f(int a, int b) {
+  if (b == 0) {
+    print('');
+  }
+}
+void g(int a, int b) {
+  if (b == 0) {
+    print('');
+  }
+}
+''');
+  }
+}
 
+@reflectiveTest
+class RemoveTypeCheckTest extends FixProcessorTest {
   @override
-  operator ==(Object? other) {
-    return other is Person &&
-          name == other.name;
+  FixKind get kind => DartFixKind.REMOVE_TYPE_CHECK;
+
+  Future<void> test_unnecessaryTypeCheck_false() async {
+    await resolveTestCode('''
+void f(int a, int b) {
+  if (a is! num || b == 0) {
+    print('');
   }
 }
-''',
-        errorFilter: (error) =>
-            error.errorCode == HintCode.UNNECESSARY_NULL_COMPARISON_FALSE);
+''');
+    await assertHasFix('''
+void f(int a, int b) {
+  if (b == 0) {
+    print('');
+  }
+}
+''');
+  }
+
+  Future<void> test_unnecessaryTypeCheck_true() async {
+    await resolveTestCode('''
+void f(int a, int b) {
+  if (b == 0 && a is num) {
+    print('');
+  }
+}
+''');
+    await assertHasFix('''
+void f(int a, int b) {
+  if (b == 0) {
+    print('');
+  }
+}
+''');
   }
 }