diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB149.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB149.py new file mode 100644 index 0000000000000..b389fc75f973e --- /dev/null +++ b/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: + ... diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 9ca9a6df71838..278712f4b6c47 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/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); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index c471c890a0f1a..b7f15c0d25908 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/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), diff --git a/crates/ruff_linter/src/rules/refurb/mod.rs b/crates/ruff_linter/src/rules/refurb/mod.rs index de5f0bdde06dd..7bedb7c313137 100644 --- a/crates/ruff_linter/src/rules/refurb/mod.rs +++ b/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"))] diff --git a/crates/ruff_linter/src/rules/refurb/rules/mod.rs b/crates/ruff_linter/src/rules/refurb/rules/mod.rs index a69a798cf5ec8..632257328be3a 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/mod.rs +++ b/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; diff --git a/crates/ruff_linter/src/rules/refurb/rules/no_bool_literal_compare.rs b/crates/ruff_linter/src/rules/refurb/rules/no_bool_literal_compare.rs new file mode 100644 index 0000000000000..56748548b732e --- /dev/null +++ b/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, +} diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB149_FURB149.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB149_FURB149.py.snap new file mode 100644 index 0000000000000..3c938e811590e --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB149_FURB149.py.snap @@ -0,0 +1,443 @@ +--- +source: crates/ruff_linter/src/rules/refurb/mod.rs +--- +FURB149.py:3:4: FURB149 [*] Comparison to `True` should be `cond` + | +1 | failed = True +2 | +3 | if True == failed: # FURB149 + | ^^^^^^^^^^^^^^ FURB149 +4 | print("You failed") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +1 1 | failed = True +2 2 | +3 |-if True == failed: # FURB149 + 3 |+if failed: # FURB149 +4 4 | print("You failed") +5 5 | +6 6 | if True != failed: # FURB149 + +FURB149.py:6:4: FURB149 [*] Comparison to `False` should be `not cond` + | +4 | print("You failed") +5 | +6 | if True != failed: # FURB149 + | ^^^^^^^^^^^^^^ FURB149 +7 | print("You did not fail") + | + = help: Use shorthand falsey check + +ℹ Unsafe fix +3 3 | if True == failed: # FURB149 +4 4 | print("You failed") +5 5 | +6 |-if True != failed: # FURB149 + 6 |+if not failed: # FURB149 +7 7 | print("You did not fail") +8 8 | +9 9 | if False == failed: # FURB149 + +FURB149.py:9:4: FURB149 [*] Comparison to `False` should be `not cond` + | + 7 | print("You did not fail") + 8 | + 9 | if False == failed: # FURB149 + | ^^^^^^^^^^^^^^^ FURB149 +10 | print("You did not fail") + | + = help: Use shorthand falsey check + +ℹ Unsafe fix +6 6 | if True != failed: # FURB149 +7 7 | print("You did not fail") +8 8 | +9 |-if False == failed: # FURB149 + 9 |+if not failed: # FURB149 +10 10 | print("You did not fail") +11 11 | +12 12 | if False != failed: # FURB149 + +FURB149.py:12:4: FURB149 [*] Comparison to `True` should be `cond` + | +10 | print("You did not fail") +11 | +12 | if False != failed: # FURB149 + | ^^^^^^^^^^^^^^^ FURB149 +13 | print("You failed") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +9 9 | if False == failed: # FURB149 +10 10 | print("You did not fail") +11 11 | +12 |-if False != failed: # FURB149 + 12 |+if failed: # FURB149 +13 13 | print("You failed") +14 14 | +15 15 | if True is failed: # FURB149 + +FURB149.py:15:4: FURB149 [*] Comparison to `True` should be `cond` + | +13 | print("You failed") +14 | +15 | if True is failed: # FURB149 + | ^^^^^^^^^^^^^^ FURB149 +16 | print("You failed") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +12 12 | if False != failed: # FURB149 +13 13 | print("You failed") +14 14 | +15 |-if True is failed: # FURB149 + 15 |+if failed: # FURB149 +16 16 | print("You failed") +17 17 | +18 18 | if True is not failed: # FURB149 + +FURB149.py:18:4: FURB149 [*] Comparison to `False` should be `not cond` + | +16 | print("You failed") +17 | +18 | if True is not failed: # FURB149 + | ^^^^^^^^^^^^^^^^^^ FURB149 +19 | print("You did not fail") + | + = help: Use shorthand falsey check + +ℹ Unsafe fix +15 15 | if True is failed: # FURB149 +16 16 | print("You failed") +17 17 | +18 |-if True is not failed: # FURB149 + 18 |+if not failed: # FURB149 +19 19 | print("You did not fail") +20 20 | +21 21 | if False is failed: # FURB149 + +FURB149.py:21:4: FURB149 [*] Comparison to `False` should be `not cond` + | +19 | print("You did not fail") +20 | +21 | if False is failed: # FURB149 + | ^^^^^^^^^^^^^^^ FURB149 +22 | print("You did not fail") + | + = help: Use shorthand falsey check + +ℹ Unsafe fix +18 18 | if True is not failed: # FURB149 +19 19 | print("You did not fail") +20 20 | +21 |-if False is failed: # FURB149 + 21 |+if not failed: # FURB149 +22 22 | print("You did not fail") +23 23 | +24 24 | if False is not failed: # FURB149 + +FURB149.py:24:4: FURB149 [*] Comparison to `True` should be `cond` + | +22 | print("You did not fail") +23 | +24 | if False is not failed: # FURB149 + | ^^^^^^^^^^^^^^^^^^^ FURB149 +25 | print("You failed") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +21 21 | if False is failed: # FURB149 +22 22 | print("You did not fail") +23 23 | +24 |-if False is not failed: # FURB149 + 24 |+if failed: # FURB149 +25 25 | print("You failed") +26 26 | +27 27 | if failed is True: # FURB149 + +FURB149.py:27:4: FURB149 [*] Comparison to `True` should be `cond` + | +25 | print("You failed") +26 | +27 | if failed is True: # FURB149 + | ^^^^^^^^^^^^^^ FURB149 +28 | print("You failed") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +24 24 | if False is not failed: # FURB149 +25 25 | print("You failed") +26 26 | +27 |-if failed is True: # FURB149 + 27 |+if failed: # FURB149 +28 28 | print("You failed") +29 29 | +30 30 | if failed is not True: # FURB149 + +FURB149.py:30:4: FURB149 [*] Comparison to `False` should be `not cond` + | +28 | print("You failed") +29 | +30 | if failed is not True: # FURB149 + | ^^^^^^^^^^^^^^^^^^ FURB149 +31 | print("You did not fail") + | + = help: Use shorthand falsey check + +ℹ Unsafe fix +27 27 | if failed is True: # FURB149 +28 28 | print("You failed") +29 29 | +30 |-if failed is not True: # FURB149 + 30 |+if not failed: # FURB149 +31 31 | print("You did not fail") +32 32 | +33 33 | if failed is False: # FURB149 + +FURB149.py:33:4: FURB149 [*] Comparison to `False` should be `not cond` + | +31 | print("You did not fail") +32 | +33 | if failed is False: # FURB149 + | ^^^^^^^^^^^^^^^ FURB149 +34 | print("You did not fail") + | + = help: Use shorthand falsey check + +ℹ Unsafe fix +30 30 | if failed is not True: # FURB149 +31 31 | print("You did not fail") +32 32 | +33 |-if failed is False: # FURB149 + 33 |+if not failed: # FURB149 +34 34 | print("You did not fail") +35 35 | +36 36 | if failed is not False: # FURB149 + +FURB149.py:36:4: FURB149 [*] Comparison to `True` should be `cond` + | +34 | print("You did not fail") +35 | +36 | if failed is not False: # FURB149 + | ^^^^^^^^^^^^^^^^^^^ FURB149 +37 | print("You failed") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +33 33 | if failed is False: # FURB149 +34 34 | print("You did not fail") +35 35 | +36 |-if failed is not False: # FURB149 + 36 |+if failed: # FURB149 +37 37 | print("You failed") +38 38 | +39 39 | if failed == True: # FURB149 + +FURB149.py:39:4: FURB149 [*] Comparison to `True` should be `cond` + | +37 | print("You failed") +38 | +39 | if failed == True: # FURB149 + | ^^^^^^^^^^^^^^ FURB149 +40 | print("You failed") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +36 36 | if failed is not False: # FURB149 +37 37 | print("You failed") +38 38 | +39 |-if failed == True: # FURB149 + 39 |+if failed: # FURB149 +40 40 | print("You failed") +41 41 | +42 42 | if failed != True: # FURB149 + +FURB149.py:42:4: FURB149 [*] Comparison to `False` should be `not cond` + | +40 | print("You failed") +41 | +42 | if failed != True: # FURB149 + | ^^^^^^^^^^^^^^ FURB149 +43 | print("You did not fail") + | + = help: Use shorthand falsey check + +ℹ Unsafe fix +39 39 | if failed == True: # FURB149 +40 40 | print("You failed") +41 41 | +42 |-if failed != True: # FURB149 + 42 |+if not failed: # FURB149 +43 43 | print("You did not fail") +44 44 | +45 45 | if failed == False: # FURB149 + +FURB149.py:45:4: FURB149 [*] Comparison to `False` should be `not cond` + | +43 | print("You did not fail") +44 | +45 | if failed == False: # FURB149 + | ^^^^^^^^^^^^^^^ FURB149 +46 | print("You did not fail") + | + = help: Use shorthand falsey check + +ℹ Unsafe fix +42 42 | if failed != True: # FURB149 +43 43 | print("You did not fail") +44 44 | +45 |-if failed == False: # FURB149 + 45 |+if not failed: # FURB149 +46 46 | print("You did not fail") +47 47 | +48 48 | if failed != False: # FURB149 + +FURB149.py:48:4: FURB149 [*] Comparison to `True` should be `cond` + | +46 | print("You did not fail") +47 | +48 | if failed != False: # FURB149 + | ^^^^^^^^^^^^^^^ FURB149 +49 | print("You failed") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +45 45 | if failed == False: # FURB149 +46 46 | print("You did not fail") +47 47 | +48 |-if failed != False: # FURB149 + 48 |+if failed: # FURB149 +49 49 | print("You failed") +50 50 | +51 51 | if (failed == True) or (failed == False): # FURB149 + +FURB149.py:51:5: FURB149 [*] Comparison to `True` should be `cond` + | +49 | print("You failed") +50 | +51 | if (failed == True) or (failed == False): # FURB149 + | ^^^^^^^^^^^^^^ FURB149 +52 | print("wat") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +48 48 | if failed != False: # FURB149 +49 49 | print("You failed") +50 50 | +51 |-if (failed == True) or (failed == False): # FURB149 + 51 |+if (failed) or (failed == False): # FURB149 +52 52 | print("wat") +53 53 | +54 54 | if(True) == failed: # FURB149 + +FURB149.py:51:25: FURB149 [*] Comparison to `False` should be `not cond` + | +49 | print("You failed") +50 | +51 | if (failed == True) or (failed == False): # FURB149 + | ^^^^^^^^^^^^^^^ FURB149 +52 | print("wat") + | + = help: Use shorthand falsey check + +ℹ Unsafe fix +48 48 | if failed != False: # FURB149 +49 49 | print("You failed") +50 50 | +51 |-if (failed == True) or (failed == False): # FURB149 + 51 |+if (failed == True) or (not failed): # FURB149 +52 52 | print("wat") +53 53 | +54 54 | if(True) == failed: # FURB149 + +FURB149.py:54:3: FURB149 [*] Comparison to `True` should be `cond` + | +52 | print("wat") +53 | +54 | if(True) == failed: # FURB149 + | ^^^^^^^^^^^^^^^^ FURB149 +55 | print("You failed") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +51 51 | if (failed == True) or (failed == False): # FURB149 +52 52 | print("wat") +53 53 | +54 |-if(True) == failed: # FURB149 + 54 |+if(failed): # FURB149 +55 55 | print("You failed") +56 56 | +57 57 | if(failed) == True: # FURB149 + +FURB149.py:57:3: FURB149 [*] Comparison to `True` should be `cond` + | +55 | print("You failed") +56 | +57 | if(failed) == True: # FURB149 + | ^^^^^^^^^^^^^^^^ FURB149 +58 | print("You failed") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +54 54 | if(True) == failed: # FURB149 +55 55 | print("You failed") +56 56 | +57 |-if(failed) == True: # FURB149 + 57 |+if(failed): # FURB149 +58 58 | print("You failed") +59 59 | +60 60 | if(((failed))) == True: # FURB149 + +FURB149.py:60:3: FURB149 [*] Comparison to `True` should be `cond` + | +58 | print("You failed") +59 | +60 | if(((failed))) == True: # FURB149 + | ^^^^^^^^^^^^^^^^^^^^ FURB149 +61 | print("You failed") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +57 57 | if(failed) == True: # FURB149 +58 58 | print("You failed") +59 59 | +60 |-if(((failed))) == True: # FURB149 + 60 |+if(failed): # FURB149 +61 61 | print("You failed") +62 62 | +63 63 | if(yield failed) == True: # FURB149 + +FURB149.py:63:3: FURB149 [*] Comparison to `True` should be `cond` + | +61 | print("You failed") +62 | +63 | if(yield failed) == True: # FURB149 + | ^^^^^^^^^^^^^^^^^^^^^^ FURB149 +64 | print("You failed") + | + = help: Use shorthand truthy check + +ℹ Unsafe fix +60 60 | if(((failed))) == True: # FURB149 +61 61 | print("You failed") +62 62 | +63 |-if(yield failed) == True: # FURB149 + 63 |+if(yield failed): # FURB149 +64 64 | print("You failed") +65 65 | +66 66 | + + diff --git a/ruff.schema.json b/ruff.schema.json index 7b65b8f158596..119eab8b77ba2 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3016,6 +3016,7 @@ "FURB140", "FURB145", "FURB148", + "FURB149", "FURB15", "FURB152", "FURB16",