[refurb] - add bool-literal-compare with fix (FURB149)

crates/ruff_linter/resources/test/fixtures/refurb/FURB149.py

@@ -0,0 +1,84 @@
+failed = True
+
+if True == failed:  # FURB149
+    print("You failed")
+
+if True != failed:  # FURB149
+    print("You did not fail")
+
+if False == failed:  # FURB149
+    print("You did not fail")
+
+if False != failed:  # FURB149
+    print("You failed")
+
+if True is failed:  # FURB149
+    print("You failed")
+
+if True is not failed:  # FURB149
+    print("You did not fail")
+
+if False is failed:  # FURB149
+    print("You did not fail")
+
+if False is not failed:  # FURB149
+    print("You failed")
+
+if failed is True:  # FURB149
+    print("You failed")
+
+if failed is not True:  # FURB149
+    print("You did not fail")
+
+if failed is False:  # FURB149
+    print("You did not fail")
+
+if failed is not False:  # FURB149
+    print("You failed")
+
+if failed == True:  # FURB149
+    print("You failed")
+
+if failed != True:  # FURB149
+    print("You did not fail")
+
+if failed == False:  # FURB149
+    print("You did not fail")
+
+if failed != False:  # FURB149
+    print("You failed")
+
+if (failed == True) or (failed == False):  # FURB149
+    print("wat")
+
+if(True) == failed:  # FURB149
+    print("You failed")
+
+if(failed) == True:  # FURB149
+    print("You failed")
+
+if(((failed))) == True:  # FURB149
+    print("You failed")
+
+if(yield failed) == True:  # FURB149
+    print("You failed")
+
+
+# OK
+if failed:
+    print("You failed")
+
+if not failed:
+    print("You did not fail")
+
+if True is False:
+    ...
+
+if True is not False:
+    ...
+
+if True == False:
+    ...
+
+if True != False:
+    ...

crates/ruff_linter/src/checkers/ast/analyze/expression.rs

@@ -1243,6 +1243,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
                 range: _,
             },
         ) => {
+            if checker.enabled(Rule::BoolLiteralCompare) {
+                refurb::rules::bool_literal_compare(checker, compare);
+            }
             if checker.any_enabled(&[Rule::NoneComparison, Rule::TrueFalseComparison]) {
                 pycodestyle::rules::literal_comparisons(checker, compare);
             }

crates/ruff_linter/src/codes.rs

@@ -1042,6 +1042,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
         (Refurb, "140") => (RuleGroup::Preview, rules::refurb::rules::ReimplementedStarmap),
         (Refurb, "145") => (RuleGroup::Preview, rules::refurb::rules::SliceCopy),
         (Refurb, "148") => (RuleGroup::Preview, rules::refurb::rules::UnnecessaryEnumerate),
+        (Refurb, "149") => (RuleGroup::Preview, rules::refurb::rules::BoolLiteralCompare),
         (Refurb, "152") => (RuleGroup::Preview, rules::refurb::rules::MathConstant),
         (Refurb, "161") => (RuleGroup::Preview, rules::refurb::rules::BitCount),
         (Refurb, "163") => (RuleGroup::Preview, rules::refurb::rules::RedundantLogBase),

crates/ruff_linter/src/rules/refurb/mod.rs

@@ -24,6 +24,7 @@ mod tests {
     #[test_case(Rule::ReimplementedStarmap, Path::new("FURB140.py"))]
     #[test_case(Rule::SliceCopy, Path::new("FURB145.py"))]
     #[test_case(Rule::UnnecessaryEnumerate, Path::new("FURB148.py"))]
+    #[test_case(Rule::BoolLiteralCompare, Path::new("FURB149.py"))]
     #[test_case(Rule::MathConstant, Path::new("FURB152.py"))]
     #[test_case(Rule::PrintEmptyString, Path::new("FURB105.py"))]
     #[test_case(Rule::ImplicitCwd, Path::new("FURB177.py"))]

crates/ruff_linter/src/rules/refurb/rules/mod.rs

@@ -7,6 +7,7 @@ pub(crate) use implicit_cwd::*;
 pub(crate) use isinstance_type_none::*;
 pub(crate) use math_constant::*;
 pub(crate) use metaclass_abcmeta::*;
+pub(crate) use no_bool_literal_compare::*;
 pub(crate) use print_empty_string::*;
 pub(crate) use read_whole_file::*;
 pub(crate) use readlines_in_for::*;
@@ -29,6 +30,7 @@ mod implicit_cwd;
 mod isinstance_type_none;
 mod math_constant;
 mod metaclass_abcmeta;
+mod no_bool_literal_compare;
 mod print_empty_string;
 mod read_whole_file;
 mod readlines_in_for;

crates/ruff_linter/src/rules/refurb/rules/no_bool_literal_compare.rs

@@ -0,0 +1,150 @@
+use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
+use ruff_macros::{derive_message_formats, violation};
+use ruff_python_ast::{self as ast, Expr};
+use ruff_text_size::Ranged;
+
+use crate::checkers::ast::Checker;
+
+/// ## What it does
+/// Checks for uses of long-form truthy and falsey checks.
+///
+/// ## Why is this bad?
+/// Python has a shorthand for checking if a value is truthy or falsey, which
+/// is more concise and idiomatic.
+///
+/// ## Example
+/// ```python
+/// failed = True
+/// if failed == True:
+///     print("failed")
+/// # or
+/// if failed is True:
+///     print("failed")
+/// # or
+/// if failed == False:
+///     print("passed")
+/// # or
+/// if failed == False:
+///     print("passed")
+/// ```
+///
+/// Use instead:
+/// ```python
+/// failed = True
+/// if failed:  # for truthy checks
+///     print("failed")
+/// # or
+/// if not failed:  # for falsey checks
+///     print("passed")
+/// ```
+///
+#[violation]
+pub struct BoolLiteralCompare {
+    check_type: CheckType,
+}
+
+impl AlwaysFixableViolation for BoolLiteralCompare {
+    #[derive_message_formats]
+    fn message(&self) -> String {
+        let BoolLiteralCompare { check_type } = self;
+
+        match check_type {
+            CheckType::Truthy => format!("Comparison to `True` should be `cond`"),
+            CheckType::Falsey => format!("Comparison to `False` should be `not cond`"),
+        }
+    }
+
+    fn fix_title(&self) -> String {
+        let BoolLiteralCompare { check_type } = self;
+
+        match check_type {
+            CheckType::Truthy => format!("Use shorthand truthy check"),
+            CheckType::Falsey => format!("Use shorthand falsey check"),
+        }
+    }
+}
+
+/// FURB149
+pub(crate) fn bool_literal_compare(checker: &mut Checker, compare: &ast::ExprCompare) {
+    if compare.ops.len() != 1 {
+        // don't do chained comparisons
+        return;
+    }
+    let comparator = compare.left.as_ref();
+
+    let [op, ..] = &*compare.ops else {
+        return;
+    };
+
+    let [next, ..] = &*compare.comparators else {
+        return;
+    };
+
+    // we'll try to determine what they're doing, whether or not they're using a yoda comparison
+    let (expr, boolvalue, in_parentheses) = match (comparator, next) {
+        (Expr::BooleanLiteral(_), Expr::BooleanLiteral(_)) => {
+            // they're comparing two bools, so we can't do anything here
+            return;
+        }
+        (
+            Expr::BooleanLiteral(ast::ExprBooleanLiteral {
+                value: boolvalue,
+                range: boolrange,
+            }),
+            expr,
+        ) => {
+            let in_parentheses =
+                compare.start() != boolrange.start() || compare.end() != expr.end();
+            (expr, boolvalue, in_parentheses)
+        }
+        (
+            expr,
+            Expr::BooleanLiteral(ast::ExprBooleanLiteral {
+                value: boolvalue,
+                range: boolrange,
+            }),
+        ) => {
+            let in_parentheses =
+                compare.start() != expr.start() || compare.end() != boolrange.end();
+            (expr, boolvalue, in_parentheses)
+        }
+        _ => {
+            return;
+        }
+    };
+
+    let (lparen, rparen) = if in_parentheses { ("(", ")") } else { ("", "") };
+
+    let (content, check_type) = match (op, boolvalue) {
+        (ast::CmpOp::Is | ast::CmpOp::Eq, true)
+        | (ast::CmpOp::IsNot | ast::CmpOp::NotEq, false) => (
+            format!("{lparen}{}{rparen}", checker.generator().expr(expr)),
+            CheckType::Truthy,
+        ),
+        (ast::CmpOp::Is | ast::CmpOp::Eq, false)
+        | (ast::CmpOp::IsNot | ast::CmpOp::NotEq, true) => (
+            format!("{lparen}not {}{rparen}", checker.generator().expr(expr)),
+            CheckType::Falsey,
+        ),
+        _ => return,
+    };
+
+    let mut diagnostic = Diagnostic::new(BoolLiteralCompare { check_type }, compare.range());
+
+    let edit = Edit::range_replacement(content, compare.range());
+
+    let fix = match check_type {
+        CheckType::Truthy => Fix::unsafe_edit(edit),
+        CheckType::Falsey => Fix::unsafe_edit(edit),
+    };
+
+    diagnostic.set_fix(fix);
+
+    checker.diagnostics.push(diagnostic);
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum CheckType {
+    Truthy,
+    Falsey,
+}