diff --git a/README.md b/README.md index 17392d9d2f183..5aed4c08535b3 100644 --- a/README.md +++ b/README.md @@ -960,6 +960,8 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/ | Code | Name | Message | Fix | | ---- | ---- | ------- | --- | +| SIM220 | AAndNotA | Use `False` instead of `... and not ...` | 🛠 | +| SIM221 | AOrNotA | Use `True` instead of `... or not ...` | 🛠 | | SIM118 | KeyInDict | Use `key in dict` instead of `key in dict.keys()` | 🛠 | | SIM222 | OrTrue | Use `True` instead of `... or True` | 🛠 | | SIM223 | AndFalse | Use `False` instead of `... and False` | 🛠 | @@ -1352,9 +1354,9 @@ natively, including: - [`flake8-print`](https://pypi.org/project/flake8-print/) - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/) - [`flake8-return`](https://pypi.org/project/flake8-return/) -- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (4/37) +- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (6/30) - [`flake8-super`](https://pypi.org/project/flake8-super/) -- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/) (1/3) +- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/) - [`isort`](https://pypi.org/project/isort/) - [`mccabe`](https://pypi.org/project/mccabe/) - [`pep8-naming`](https://pypi.org/project/pep8-naming/) @@ -1410,9 +1412,9 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl - [`flake8-print`](https://pypi.org/project/flake8-print/) - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/) - [`flake8-return`](https://pypi.org/project/flake8-return/) -- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (4/37) +- [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) (6/30) - [`flake8-super`](https://pypi.org/project/flake8-super/) -- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/) (1/3) +- [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/) - [`mccabe`](https://pypi.org/project/mccabe/) - [`pep8-naming`](https://pypi.org/project/pep8-naming/) - [`pydocstyle`](https://pypi.org/project/pydocstyle/) diff --git a/resources/test/fixtures/flake8_simplify/SIM220.py b/resources/test/fixtures/flake8_simplify/SIM220.py new file mode 100644 index 0000000000000..867f5797efb43 --- /dev/null +++ b/resources/test/fixtures/flake8_simplify/SIM220.py @@ -0,0 +1,17 @@ +if a and not a: + pass + +if (a and not a) and b: + pass + +if (a and not a) or b: + pass + +if a: + pass + +if not a: + pass + +if a and not b: + pass diff --git a/resources/test/fixtures/flake8_simplify/SIM221.py b/resources/test/fixtures/flake8_simplify/SIM221.py new file mode 100644 index 0000000000000..ca981c66995cc --- /dev/null +++ b/resources/test/fixtures/flake8_simplify/SIM221.py @@ -0,0 +1,17 @@ +if a or not a: + pass + +if (a or not a) or b: + pass + +if (a or not a) and b: + pass + +if a: + pass + +if not a: + pass + +if a or not b: + pass diff --git a/ruff.schema.json b/ruff.schema.json index 895c7c930a625..1199176e5bd77 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -899,6 +899,8 @@ "SIM118", "SIM2", "SIM22", + "SIM220", + "SIM221", "SIM222", "SIM223", "SIM3", diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index b19b11f443c80..8fdf100ce14da 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -2578,6 +2578,12 @@ where if self.settings.enabled.contains(&CheckCode::PLR1701) { pylint::plugins::merge_isinstance(self, expr, op, values); } + if self.settings.enabled.contains(&CheckCode::SIM220) { + flake8_simplify::plugins::a_and_not_a(self, expr); + } + if self.settings.enabled.contains(&CheckCode::SIM221) { + flake8_simplify::plugins::a_or_not_a(self, expr); + } if self.settings.enabled.contains(&CheckCode::SIM222) { flake8_simplify::plugins::or_true(self, expr); } diff --git a/src/flake8_simplify/mod.rs b/src/flake8_simplify/mod.rs index 92c3edb372711..829f6d2e40b9b 100644 --- a/src/flake8_simplify/mod.rs +++ b/src/flake8_simplify/mod.rs @@ -16,6 +16,8 @@ mod tests { #[test_case(CheckCode::SIM222, Path::new("SIM222.py"); "SIM222")] #[test_case(CheckCode::SIM223, Path::new("SIM223.py"); "SIM223")] #[test_case(CheckCode::SIM300, Path::new("SIM300.py"); "SIM300")] + #[test_case(CheckCode::SIM221, Path::new("SIM221.py"); "SIM221")] + #[test_case(CheckCode::SIM220, Path::new("SIM220.py"); "SIM220")] fn checks(check_code: CheckCode, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy()); let checks = test_path( diff --git a/src/flake8_simplify/plugins/bool_ops.rs b/src/flake8_simplify/plugins/bool_ops.rs index 16028543e015a..b98777ca46ce4 100644 --- a/src/flake8_simplify/plugins/bool_ops.rs +++ b/src/flake8_simplify/plugins/bool_ops.rs @@ -1,10 +1,115 @@ -use rustpython_ast::{Boolop, Constant, Expr, ExprKind}; +use rustpython_ast::{Boolop, Constant, Expr, ExprKind, Unaryop}; use crate::ast::types::Range; use crate::autofix::Fix; use crate::checkers::ast::Checker; use crate::registry::{Check, CheckCode, CheckKind}; +fn is_same_expr<'a>(a: &'a Expr, b: &'a Expr) -> Option<&'a str> { + if let (ExprKind::Name { id: a, .. }, ExprKind::Name { id: b, .. }) = (&a.node, &b.node) { + if a == b { + return Some(a); + } + } + None +} + +/// SIM220 +pub fn a_and_not_a(checker: &mut Checker, expr: &Expr) { + let ExprKind::BoolOp { op: Boolop::And, values, } = &expr.node else { + return; + }; + if values.len() < 2 { + return; + } + + // Collect all negated and non-negated expressions. + let mut negated_expr = vec![]; + let mut non_negated_expr = vec![]; + for expr in values { + if let ExprKind::UnaryOp { + op: Unaryop::Not, + operand, + } = &expr.node + { + negated_expr.push(operand); + } else { + non_negated_expr.push(expr); + } + } + + if negated_expr.is_empty() { + return; + } + + for negate_expr in negated_expr { + for non_negate_expr in &non_negated_expr { + if let Some(id) = is_same_expr(negate_expr, non_negate_expr) { + let mut check = Check::new( + CheckKind::AAndNotA(id.to_string()), + Range::from_located(expr), + ); + if checker.patch(&CheckCode::SIM220) { + check.amend(Fix::replacement( + "False".to_string(), + expr.location, + expr.end_location.unwrap(), + )); + } + checker.add_check(check); + } + } + } +} + +/// SIM221 +pub fn a_or_not_a(checker: &mut Checker, expr: &Expr) { + let ExprKind::BoolOp { op: Boolop::Or, values, } = &expr.node else { + return; + }; + if values.len() < 2 { + return; + } + + // Collect all negated and non-negated expressions. + let mut negated_expr = vec![]; + let mut non_negated_expr = vec![]; + for expr in values { + if let ExprKind::UnaryOp { + op: Unaryop::Not, + operand, + } = &expr.node + { + negated_expr.push(operand); + } else { + non_negated_expr.push(expr); + } + } + + if negated_expr.is_empty() { + return; + } + + for negate_expr in negated_expr { + for non_negate_expr in &non_negated_expr { + if let Some(id) = is_same_expr(negate_expr, non_negate_expr) { + let mut check = Check::new( + CheckKind::AOrNotA(id.to_string()), + Range::from_located(expr), + ); + if checker.patch(&CheckCode::SIM220) { + check.amend(Fix::replacement( + "True".to_string(), + expr.location, + expr.end_location.unwrap(), + )); + } + checker.add_check(check); + } + } + } +} + /// SIM222 pub fn or_true(checker: &mut Checker, expr: &Expr) { let ExprKind::BoolOp { op: Boolop::Or, values, } = &expr.node else { diff --git a/src/flake8_simplify/plugins/mod.rs b/src/flake8_simplify/plugins/mod.rs index cb786b1a7372a..27c0a0352d18f 100644 --- a/src/flake8_simplify/plugins/mod.rs +++ b/src/flake8_simplify/plugins/mod.rs @@ -1,4 +1,4 @@ -pub use bool_ops::{and_false, or_true}; +pub use bool_ops::{a_and_not_a, a_or_not_a, and_false, or_true}; pub use key_in_dict::{key_in_dict_compare, key_in_dict_for}; pub use yoda_conditions::yoda_conditions; diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM220_SIM220.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM220_SIM220.py.snap new file mode 100644 index 0000000000000..e500a1e803734 --- /dev/null +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM220_SIM220.py.snap @@ -0,0 +1,56 @@ +--- +source: src/flake8_simplify/mod.rs +expression: checks +--- +- kind: + AAndNotA: a + location: + row: 1 + column: 3 + end_location: + row: 1 + column: 14 + fix: + content: "False" + location: + row: 1 + column: 3 + end_location: + row: 1 + column: 14 + parent: ~ +- kind: + AAndNotA: a + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 15 + fix: + content: "False" + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 15 + parent: ~ +- kind: + AAndNotA: a + location: + row: 7 + column: 4 + end_location: + row: 7 + column: 15 + fix: + content: "False" + location: + row: 7 + column: 4 + end_location: + row: 7 + column: 15 + parent: ~ + diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM221_SIM221.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM221_SIM221.py.snap new file mode 100644 index 0000000000000..dccbea38a6561 --- /dev/null +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM221_SIM221.py.snap @@ -0,0 +1,35 @@ +--- +source: src/flake8_simplify/mod.rs +expression: checks +--- +- kind: + AOrNotA: a + location: + row: 1 + column: 3 + end_location: + row: 1 + column: 13 + fix: ~ + parent: ~ +- kind: + AOrNotA: a + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 14 + fix: ~ + parent: ~ +- kind: + AOrNotA: a + location: + row: 7 + column: 4 + end_location: + row: 7 + column: 14 + fix: ~ + parent: ~ + diff --git a/src/registry.rs b/src/registry.rs index f2f161a7a3388..8c1f7187be4bb 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -217,6 +217,8 @@ pub enum CheckCode { YTT302, YTT303, // flake8-simplify + SIM220, + SIM221, SIM118, SIM222, SIM223, @@ -945,6 +947,8 @@ pub enum CheckKind { SysVersionCmpStr10, SysVersionSlice1Referenced, // flake8-simplify + AAndNotA(String), + AOrNotA(String), KeyInDict(String, String), AndFalse, OrTrue, @@ -1374,6 +1378,8 @@ impl CheckCode { CheckCode::BLE001 => CheckKind::BlindExcept("Exception".to_string()), // flake8-simplify CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()), + CheckCode::SIM220 => CheckKind::AAndNotA("...".to_string()), + CheckCode::SIM221 => CheckKind::AOrNotA("...".to_string()), CheckCode::SIM222 => CheckKind::OrTrue, CheckCode::SIM223 => CheckKind::AndFalse, CheckCode::SIM300 => CheckKind::YodaConditions("left".to_string(), "right".to_string()), @@ -1898,6 +1904,8 @@ impl CheckCode { CheckCode::S107 => CheckCategory::Flake8Bandit, // flake8-simplify CheckCode::SIM118 => CheckCategory::Flake8Simplify, + CheckCode::SIM220 => CheckCategory::Flake8Simplify, + CheckCode::SIM221 => CheckCategory::Flake8Simplify, CheckCode::SIM222 => CheckCategory::Flake8Simplify, CheckCode::SIM223 => CheckCategory::Flake8Simplify, CheckCode::SIM300 => CheckCategory::Flake8Simplify, @@ -2147,6 +2155,8 @@ impl CheckKind { CheckKind::SysVersionSlice1Referenced => &CheckCode::YTT303, // flake8-simplify CheckKind::KeyInDict(..) => &CheckCode::SIM118, + CheckKind::AAndNotA(..) => &CheckCode::SIM220, + CheckKind::AOrNotA(..) => &CheckCode::SIM221, CheckKind::OrTrue => &CheckCode::SIM222, CheckKind::AndFalse => &CheckCode::SIM223, CheckKind::YodaConditions(..) => &CheckCode::SIM300, @@ -2904,6 +2914,8 @@ impl CheckKind { CheckKind::KeyInDict(key, dict) => { format!("Use `{key} in {dict}` instead of `{key} in {dict}.keys()`") } + CheckKind::AAndNotA(name) => format!("Use `False` instead of `{name} and not {name}`"), + CheckKind::AOrNotA(name) => format!("Use `True` instead of `{name} or not {name}`"), CheckKind::OrTrue => "Use `True` instead of `... or True`".to_string(), CheckKind::AndFalse => "Use `False` instead of `... and False`".to_string(), CheckKind::YodaConditions(left, right) => { @@ -3495,9 +3507,11 @@ impl CheckKind { pub fn fixable(&self) -> bool { matches!( self, - CheckKind::AmbiguousUnicodeCharacterString(..) + CheckKind::AAndNotA(..) + | CheckKind::AOrNotA(..) | CheckKind::AmbiguousUnicodeCharacterComment(..) | CheckKind::AmbiguousUnicodeCharacterDocstring(..) + | CheckKind::AmbiguousUnicodeCharacterString(..) | CheckKind::AndFalse | CheckKind::BlankLineAfterLastSection(..) | CheckKind::BlankLineAfterSection(..) @@ -3611,6 +3625,8 @@ impl CheckKind { /// The message used to describe the fix action for a given `CheckKind`. pub fn commit(&self) -> Option { match self { + CheckKind::AAndNotA(..) => Some("Replace with `False`".to_string()), + CheckKind::AOrNotA(..) => Some("Replace with `True`".to_string()), CheckKind::AmbiguousUnicodeCharacterString(confusable, representant) | CheckKind::AmbiguousUnicodeCharacterDocstring(confusable, representant) | CheckKind::AmbiguousUnicodeCharacterComment(confusable, representant) => { diff --git a/src/registry_gen.rs b/src/registry_gen.rs index 891e927488e54..438c1831ea512 100644 --- a/src/registry_gen.rs +++ b/src/registry_gen.rs @@ -519,6 +519,8 @@ pub enum CheckCodePrefix { SIM118, SIM2, SIM22, + SIM220, + SIM221, SIM222, SIM223, SIM3, @@ -799,6 +801,8 @@ impl CheckCodePrefix { CheckCode::YTT301, CheckCode::YTT302, CheckCode::YTT303, + CheckCode::SIM220, + CheckCode::SIM221, CheckCode::SIM118, CheckCode::SIM222, CheckCode::SIM223, @@ -2608,6 +2612,8 @@ impl CheckCodePrefix { CheckCodePrefix::S106 => vec![CheckCode::S106], CheckCodePrefix::S107 => vec![CheckCode::S107], CheckCodePrefix::SIM => vec![ + CheckCode::SIM220, + CheckCode::SIM221, CheckCode::SIM118, CheckCode::SIM222, CheckCode::SIM223, @@ -2616,8 +2622,20 @@ impl CheckCodePrefix { CheckCodePrefix::SIM1 => vec![CheckCode::SIM118], CheckCodePrefix::SIM11 => vec![CheckCode::SIM118], CheckCodePrefix::SIM118 => vec![CheckCode::SIM118], - CheckCodePrefix::SIM2 => vec![CheckCode::SIM222, CheckCode::SIM223], - CheckCodePrefix::SIM22 => vec![CheckCode::SIM222, CheckCode::SIM223], + CheckCodePrefix::SIM2 => vec![ + CheckCode::SIM220, + CheckCode::SIM221, + CheckCode::SIM222, + CheckCode::SIM223, + ], + CheckCodePrefix::SIM22 => vec![ + CheckCode::SIM220, + CheckCode::SIM221, + CheckCode::SIM222, + CheckCode::SIM223, + ], + CheckCodePrefix::SIM220 => vec![CheckCode::SIM220], + CheckCodePrefix::SIM221 => vec![CheckCode::SIM221], CheckCodePrefix::SIM222 => vec![CheckCode::SIM222], CheckCodePrefix::SIM223 => vec![CheckCode::SIM223], CheckCodePrefix::SIM3 => vec![CheckCode::SIM300], @@ -3569,6 +3587,8 @@ impl CheckCodePrefix { CheckCodePrefix::SIM118 => SuffixLength::Three, CheckCodePrefix::SIM2 => SuffixLength::One, CheckCodePrefix::SIM22 => SuffixLength::Two, + CheckCodePrefix::SIM220 => SuffixLength::Three, + CheckCodePrefix::SIM221 => SuffixLength::Three, CheckCodePrefix::SIM222 => SuffixLength::Three, CheckCodePrefix::SIM223 => SuffixLength::Three, CheckCodePrefix::SIM3 => SuffixLength::One,