Revert two recent changes to no_literal_bool_comparisons:

* "[DAS] Broadens the scope of `no_literal_bool_comparisons`"
  commit 0541ad3.
* "[DAS] Fixes broken cases for "Convert to boolean expression" fix"
  commit 0444838.

The first commit may have lead to broken customer tests. See
flutter/flutter#172087.

Change-Id: Iba2c936a32ecf678318fa3aea929787dca046e09
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/440261
Reviewed-by: Siva Annamalai <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: srawlins

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

@@ -6,8 +6,6 @@ import 'package:analysis_server/src/services/correction/fix.dart';
 import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/precedence.dart';
-import 'package:analyzer/dart/ast/token.dart';
-import 'package:analyzer/source/source_range.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';
@@ -29,223 +27,49 @@ class ConvertToBooleanExpression extends ResolvedCorrectionProducer {
   Future<void> compute(ChangeBuilder builder) async {
     AstNode? node = this.node;
     if (node is BooleanLiteral) node = node.parent;
+    if (node is! BinaryExpression) return;
 
-    if (node case BinaryExpression(
-      :var rightOperand,
-      :var leftOperand,
-      operator: Token(type: var operator),
-    )) {
-      if (rightOperand is BooleanLiteral) {
-        return await _processBinaryExp(
-          builder,
-          node,
-          operator,
-          rightOperand,
-          leftOperand,
-          currentIsLeft: false,
-        );
-      }
-      if (leftOperand is BooleanLiteral) {
-        return await _processBinaryExp(
-          builder,
-          node,
-          operator,
-          leftOperand,
-          rightOperand,
-          currentIsLeft: true,
-        );
-      }
-    } else if (node
-        case ConditionalExpression(
-              :var condition,
-              :var thenExpression,
-              :var elseExpression,
-            ) &&
-            var conditionalExp) {
-      await (switch ((thenExpression, elseExpression)) {
-        (BooleanLiteral then, BooleanLiteral elseExp) => () async {
-          var equalValues = then.value == elseExp.value;
-          var rangeStart =
-              equalValues
-                  // keep `then`
-                  ? range.startStart(condition, then)
-                  // keep `condition`
-                  : range.endEnd(condition, then);
-          // remove ` : elseExp`
-          var rangeEnd = range.endEnd(then, elseExp);
-          await _addEdit(
-            builder,
-            parensRange: range.node(equalValues ? then : condition),
-            deleteRanges: [rangeStart, rangeEnd],
-            negated: !then.value && elseExp.value,
-            needsParens: !equalValues && condition.needsParens(),
-            bangBeforeParens: false,
-          );
-        }(),
-        (BooleanLiteral then, Expression elseExp) => () async {
-          var replaceRange = range.endStart(condition, elseExp);
-          var operator = then.ifBarElseAmpersand;
-          await _addEdit(
-            builder,
-            parensRange: range.node(conditionalExp.condition),
-            deleteRanges: const <SourceRange>[],
-            negated: !then.value,
-            replace: (replaceRange, ' ${operator.lexeme} '),
-            needsParens: conditionalExp.condition.needsParens(
-              then.value ? operator : null,
-            ),
-            bangBeforeParens: true,
-            parensRange2:
-                elseExp.needsParens(operator) ? range.node(elseExp) : null,
-          );
-        }(),
-        (Expression then, BooleanLiteral elseExp) => () async {
-          var rangeStart = range.endStart(condition, then);
-          var rangeEnd = range.endEnd(then, elseExp);
-          var operator = elseExp.ifBarElseAmpersand;
-          await _addEdit(
-            builder,
-            parensRange: range.node(conditionalExp.condition),
-            deleteRanges: [rangeEnd],
-            negated: elseExp.value,
-            replace: (rangeStart, ' ${operator.lexeme} '),
-            needsParens: conditionalExp.condition.needsParens(
-              elseExp.value ? null : operator,
-            ),
-            bangBeforeParens: true,
-            parensRange2: then.needsParens(operator) ? range.node(then) : null,
-          );
-        }(),
-        (_, _) => null,
-      });
+    var rightOperand = node.rightOperand;
+    var leftOperand = node.leftOperand;
+
+    Expression expression;
+    BooleanLiteral literal;
+
+    var deleteRange = range.endEnd(leftOperand, rightOperand);
+
+    if (rightOperand is BooleanLiteral) {
+      literal = rightOperand;
+      expression = node.leftOperand;
+    } else if (leftOperand is BooleanLiteral) {
+      literal = leftOperand;
+      expression = node.rightOperand;
+      deleteRange = range.startStart(leftOperand, rightOperand);
+    } else {
+      return;
     }
-  }
 
-  /// This correction producer can act on a variety of code, and various edits
-  /// might be applied.
-  ///
-  /// The [deleteRanges] are ranges that should be deleted from the code.
-  ///
-  /// The [replace] will be normally used to replace part of the expression
-  /// with a new operator.
-  ///
-  /// The [parensRange] is related to [needsParens] and [bangBeforeParens]. The
-  /// `bang` is added only if [negated] is true. It was meant for adding the
-  /// parens when the `bang` or the new operator given by [replace] makes the
-  /// precedence of the expression different than the original one.
-  ///
-  /// The [parensRange2] is meant to be used when converting conditional
-  /// expressions and should be non-`null` if the second expression needs
-  /// parentheses considering the new operator added by the given [replace].
-  Future<void> _addEdit(
-    ChangeBuilder builder, {
-    required SourceRange parensRange,
-    required bool needsParens,
-    required List<SourceRange> deleteRanges,
-    required bool negated,
-    required bool bangBeforeParens,
-    (SourceRange, String)? replace,
-    SourceRange? parensRange2,
-  }) async {
+    var negated = !isPositiveCase(node, literal);
     await builder.addDartFileEdit(file, (builder) {
-      if (bangBeforeParens) {
-        if (negated) {
-          builder.addSimpleInsertion(parensRange.offset, TokenType.BANG.lexeme);
-        }
-        if (needsParens) {
-          builder.addSimpleInsertion(parensRange.offset, '(');
-          builder.addSimpleInsertion(parensRange.end, ')');
-        }
-      } else {
-        if (needsParens) {
-          builder.addSimpleInsertion(parensRange.offset, '(');
-        }
-        if (negated) {
-          builder.addSimpleInsertion(parensRange.offset, TokenType.BANG.lexeme);
+      if (negated) {
+        builder.addSimpleInsertion(expression.offset, '!');
+        if (expression.needsParens) {
+          builder.addSimpleInsertion(expression.offset, '(');
+          builder.addSimpleInsertion(expression.end, ')');
         }
-        if (needsParens) {
-          builder.addSimpleInsertion(parensRange.end, ')');
-        }
-      }
-      if (replace != null) {
-        builder.addSimpleReplacement(replace.$1, replace.$2);
-      }
-      if (parensRange2 != null) {
-        builder.addSimpleInsertion(parensRange2.offset, '(');
-        builder.addSimpleInsertion(parensRange2.end, ')');
-      }
-      for (var range in deleteRanges) {
-        builder.addDeletion(range);
       }
+      builder.addDeletion(deleteRange);
     });
   }
 
-  Future<void> _processBinaryExp(
-    ChangeBuilder builder,
-    BinaryExpression binaryExp,
-    TokenType operator,
-    BooleanLiteral current,
-    Expression other, {
-    required bool currentIsLeft,
-  }) async {
-    List<SourceRange> deleteRanges;
-    SourceRange parensRange;
-    bool needsParens;
-    switch (operator) {
-      case TokenType.BAR || TokenType.BAR_BAR when current.value:
-      case TokenType.AMPERSAND || TokenType.AMPERSAND_AMPERSAND
-          when !current.value:
-        deleteRanges = [
-          currentIsLeft
-              ? range.endEnd(current, other)
-              : range.startStart(other, current),
-        ];
-        parensRange = range.node(current);
-        needsParens = current.needsParens();
-      default:
-        deleteRanges = [
-          currentIsLeft
-              ? range.startStart(current, other)
-              : range.endEnd(other, current),
-        ];
-        parensRange = range.node(other);
-        needsParens = other.needsParens();
-    }
-    await _addEdit(
-      builder,
-      negated: !isPositiveCase(binaryExp, current),
-      parensRange: parensRange,
-      deleteRanges: deleteRanges,
-      needsParens: needsParens,
-      bangBeforeParens: true,
-    );
-  }
-
   static bool isPositiveCase(
     BinaryExpression expression,
     BooleanLiteral literal,
   ) {
-    return switch (expression.operator.type) {
-      TokenType.BAR ||
-      TokenType.BAR_BAR ||
-      TokenType.AMPERSAND ||
-      TokenType.AMPERSAND_AMPERSAND => true,
-      TokenType.EQ_EQ => literal.value,
-      TokenType.BANG_EQ || TokenType.CARET => !literal.value,
-      _ => throw StateError('Unexpected operator ${expression.operator.type}'),
-    };
+    if (expression.operator.lexeme == '==') return literal.value;
+    return !literal.value;
   }
 }
 
-extension on BooleanLiteral {
-  TokenType get ifBarElseAmpersand =>
-      value ? TokenType.BAR_BAR : TokenType.AMPERSAND_AMPERSAND;
-}
-
 extension on Expression {
-  bool needsParens([TokenType? tokenType]) =>
-      precedence <=
-      (tokenType != null
-          ? Precedence.forTokenType(tokenType)
-          : Precedence.prefix);
+  bool get needsParens => precedence <= Precedence.prefix;
 }

pkg/analysis_server/test/abstract_single_unit.dart

@@ -18,11 +18,6 @@ import 'abstract_context.dart';
 class AbstractSingleUnitTest extends AbstractContextTest {
   bool verifyNoTestUnitErrors = true;
 
-  /// Whether the test code should parse carrets as position shorthands.
-  ///
-  /// Set this to `true` when the test code is using the carret operator.
-  bool keepCaret = false;
-
   TestCode? _parsedTestCode;
   late ParsedUnitResult testParsedResult;
   late ResolvedLibraryResult? testLibraryResult;
@@ -43,10 +38,7 @@ class AbstractSingleUnitTest extends AbstractContextTest {
 
   String get testCode => parsedTestCode.code;
   set testCode(String value) {
-    parsedTestCode = TestCode.parse(
-      normalizeSource(value),
-      positionShorthand: !keepCaret,
-    );
+    parsedTestCode = TestCode.parse(normalizeSource(value));
   }
 
   void addTestSource(String code) {