diff --git a/README.md b/README.md index 2f493d0ee0ba1..8ae76e798902f 100644 --- a/README.md +++ b/README.md @@ -978,6 +978,9 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/ | SIM111 | ConvertLoopToAll | Use `return all(x for x in y)` instead of `for` loop | 🛠 | | SIM117 | MultipleWithStatements | Use a single `with` statement with multiple contexts instead of nested `with` statements | | | SIM118 | KeyInDict | Use `key in dict` instead of `key in dict.keys()` | 🛠 | +| SIM201 | NegateEqualOp | Use `left != right` instead of `not left == right` | 🛠 | +| SIM202 | NegateNotEqualOp | Use `left == right` instead of `not left != right` | 🛠 | +| SIM208 | DoubleNegation | Use `expr` instead of `not (not expr)` | 🛠 | | SIM220 | AAndNotA | Use `False` instead of `... and not ...` | 🛠 | | SIM221 | AOrNotA | Use `True` instead of `... or not ...` | 🛠 | | SIM222 | OrTrue | Use `True` instead of `... or True` | 🛠 | diff --git a/resources/test/fixtures/flake8_simplify/SIM201.py b/resources/test/fixtures/flake8_simplify/SIM201.py new file mode 100644 index 0000000000000..6bccebc348ba3 --- /dev/null +++ b/resources/test/fixtures/flake8_simplify/SIM201.py @@ -0,0 +1,17 @@ +if not a == b: # SIM201 + pass + +if not a == (b + c): # SIM201 + pass + +if not (a + b) == c: # SIM201 + pass + +if not a != b: # OK + pass + +if a == b: # OK + pass + +if not a == b: # OK + raise ValueError() diff --git a/resources/test/fixtures/flake8_simplify/SIM202.py b/resources/test/fixtures/flake8_simplify/SIM202.py new file mode 100644 index 0000000000000..23baab04ff6c0 --- /dev/null +++ b/resources/test/fixtures/flake8_simplify/SIM202.py @@ -0,0 +1,14 @@ +if not a != b: # SIM202 + pass + +if not a != (b + c): # SIM202 + pass + +if not (a + b) != c: # SIM202 + pass + +if not a == b: # OK + pass + +if a != b: # OK + pass diff --git a/resources/test/fixtures/flake8_simplify/SIM208.py b/resources/test/fixtures/flake8_simplify/SIM208.py new file mode 100644 index 0000000000000..1a23f70773a01 --- /dev/null +++ b/resources/test/fixtures/flake8_simplify/SIM208.py @@ -0,0 +1,14 @@ +if not (not a): # SIM208 + pass + +if not (not (a == b)): # SIM208 + pass + +if not a: # OK + pass + +if not a == b: # OK + pass + +if not a != b: # OK + pass diff --git a/ruff.schema.json b/ruff.schema.json index 3fa37aa8e9dc6..9729f1d8e6e31 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -950,6 +950,10 @@ "SIM117", "SIM118", "SIM2", + "SIM20", + "SIM201", + "SIM202", + "SIM208", "SIM22", "SIM220", "SIM221", diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 5f977d0071120..0695f806bd394 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -2548,6 +2548,16 @@ where if self.settings.enabled.contains(&CheckCode::B002) { flake8_bugbear::plugins::unary_prefix_increment(self, expr, op, operand); } + + if self.settings.enabled.contains(&CheckCode::SIM201) { + flake8_simplify::plugins::negation_with_equal_op(self, expr, op, operand); + } + if self.settings.enabled.contains(&CheckCode::SIM202) { + flake8_simplify::plugins::negation_with_not_equal_op(self, expr, op, operand); + } + if self.settings.enabled.contains(&CheckCode::SIM208) { + flake8_simplify::plugins::double_negation(self, expr, op, operand); + } } ExprKind::Compare { left, diff --git a/src/flake8_simplify/mod.rs b/src/flake8_simplify/mod.rs index 7b04ff0440225..67b41f3b961e7 100644 --- a/src/flake8_simplify/mod.rs +++ b/src/flake8_simplify/mod.rs @@ -20,6 +20,9 @@ mod tests { #[test_case(CheckCode::SIM110, Path::new("SIM110.py"); "SIM110")] #[test_case(CheckCode::SIM111, Path::new("SIM111.py"); "SIM111")] #[test_case(CheckCode::SIM117, Path::new("SIM117.py"); "SIM117")] + #[test_case(CheckCode::SIM201, Path::new("SIM201.py"); "SIM201")] + #[test_case(CheckCode::SIM202, Path::new("SIM202.py"); "SIM202")] + #[test_case(CheckCode::SIM208, Path::new("SIM208.py"); "SIM208")] #[test_case(CheckCode::SIM118, Path::new("SIM118.py"); "SIM118")] #[test_case(CheckCode::SIM220, Path::new("SIM220.py"); "SIM220")] #[test_case(CheckCode::SIM221, Path::new("SIM221.py"); "SIM221")] diff --git a/src/flake8_simplify/plugins/mod.rs b/src/flake8_simplify/plugins/mod.rs index 88b20f5496196..b0c774311270b 100644 --- a/src/flake8_simplify/plugins/mod.rs +++ b/src/flake8_simplify/plugins/mod.rs @@ -6,6 +6,7 @@ pub use ast_if::{nested_if_statements, use_ternary_operator}; pub use ast_with::multiple_with_statements; pub use key_in_dict::{key_in_dict_compare, key_in_dict_for}; pub use return_in_try_except_finally::return_in_try_except_finally; +pub use unary_ops::{double_negation, negation_with_equal_op, negation_with_not_equal_op}; pub use use_contextlib_suppress::use_contextlib_suppress; pub use yoda_conditions::yoda_conditions; @@ -15,5 +16,6 @@ mod ast_if; mod ast_with; mod key_in_dict; mod return_in_try_except_finally; +mod unary_ops; mod use_contextlib_suppress; mod yoda_conditions; diff --git a/src/flake8_simplify/plugins/unary_ops.rs b/src/flake8_simplify/plugins/unary_ops.rs new file mode 100644 index 0000000000000..6e66cd3c52c64 --- /dev/null +++ b/src/flake8_simplify/plugins/unary_ops.rs @@ -0,0 +1,129 @@ +use rustpython_ast::{Cmpop, Expr, ExprKind, Stmt, StmtKind, Unaryop}; + +use crate::ast::helpers::{create_expr, unparse_expr}; +use crate::ast::types::Range; +use crate::autofix::Fix; +use crate::checkers::ast::Checker; +use crate::registry::{Check, CheckKind}; + +fn is_exception_check(stmt: &Stmt) -> bool { + let StmtKind::If {test: _, body, orelse: _} = &stmt.node else { + return false; + }; + if body.len() != 1 { + return false; + } + if matches!(body[0].node, StmtKind::Raise { .. }) { + return true; + } + false +} + +/// SIM201 +pub fn negation_with_equal_op(checker: &mut Checker, expr: &Expr, op: &Unaryop, operand: &Expr) { + if !matches!(op, Unaryop::Not) { + return; + } + let ExprKind::Compare{ left, ops, comparators} = &operand.node else { + return; + }; + if !matches!(&ops[..], [Cmpop::Eq]) { + return; + } + if is_exception_check(checker.current_stmt()) { + return; + } + + let mut check = Check::new( + CheckKind::NegateEqualOp( + unparse_expr(left, checker.style), + unparse_expr(&comparators[0], checker.style), + ), + Range::from_located(operand), + ); + if checker.patch(check.kind.code()) { + check.amend(Fix::replacement( + unparse_expr( + &create_expr(ExprKind::Compare { + left: left.clone(), + ops: vec![Cmpop::NotEq], + comparators: comparators.clone(), + }), + checker.style, + ), + expr.location, + expr.end_location.unwrap(), + )); + } + checker.add_check(check); +} + +/// SIM202 +pub fn negation_with_not_equal_op( + checker: &mut Checker, + expr: &Expr, + op: &Unaryop, + operand: &Expr, +) { + if !matches!(op, Unaryop::Not) { + return; + } + let ExprKind::Compare{ left, ops, comparators} = &operand.node else { + return; + }; + if !matches!(&ops[..], [Cmpop::NotEq]) { + return; + } + if is_exception_check(checker.current_stmt()) { + return; + } + + let mut check = Check::new( + CheckKind::NegateNotEqualOp( + unparse_expr(left, checker.style), + unparse_expr(&comparators[0], checker.style), + ), + Range::from_located(operand), + ); + if checker.patch(check.kind.code()) { + check.amend(Fix::replacement( + unparse_expr( + &create_expr(ExprKind::Compare { + left: left.clone(), + ops: vec![Cmpop::Eq], + comparators: comparators.clone(), + }), + checker.style, + ), + expr.location, + expr.end_location.unwrap(), + )); + } + checker.add_check(check); +} + +/// SIM208 +pub fn double_negation(checker: &mut Checker, expr: &Expr, op: &Unaryop, operand: &Expr) { + if !matches!(op, Unaryop::Not) { + return; + } + let ExprKind::UnaryOp { op: operand_op, operand } = &operand.node else { + return; + }; + if !matches!(operand_op, Unaryop::Not) { + return; + } + + let mut check = Check::new( + CheckKind::DoubleNegation(operand.to_string()), + Range::from_located(operand), + ); + if checker.patch(check.kind.code()) { + check.amend(Fix::replacement( + unparse_expr(operand, checker.style), + expr.location, + expr.end_location.unwrap(), + )); + } + checker.add_check(check); +} diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM201_SIM201.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM201_SIM201.py.snap new file mode 100644 index 0000000000000..de3043c436853 --- /dev/null +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM201_SIM201.py.snap @@ -0,0 +1,62 @@ +--- +source: src/flake8_simplify/mod.rs +expression: checks +--- +- kind: + NegateEqualOp: + - a + - b + location: + row: 1 + column: 7 + end_location: + row: 1 + column: 13 + fix: + content: a != b + location: + row: 1 + column: 3 + end_location: + row: 1 + column: 13 + parent: ~ +- kind: + NegateEqualOp: + - a + - b + c + location: + row: 4 + column: 7 + end_location: + row: 4 + column: 19 + fix: + content: a != b + c + location: + row: 4 + column: 3 + end_location: + row: 4 + column: 19 + parent: ~ +- kind: + NegateEqualOp: + - a + b + - c + location: + row: 7 + column: 7 + end_location: + row: 7 + column: 19 + fix: + content: a + b != c + location: + row: 7 + column: 3 + end_location: + row: 7 + column: 19 + parent: ~ + diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM201_SIM201_2.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM201_SIM201_2.py.snap new file mode 100644 index 0000000000000..22ec04aee93e5 --- /dev/null +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM201_SIM201_2.py.snap @@ -0,0 +1,6 @@ +--- +source: src/flake8_simplify/mod.rs +expression: checks +--- +[] + diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM202_SIM202.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM202_SIM202.py.snap new file mode 100644 index 0000000000000..1a038bec17be5 --- /dev/null +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM202_SIM202.py.snap @@ -0,0 +1,62 @@ +--- +source: src/flake8_simplify/mod.rs +expression: checks +--- +- kind: + NegateNotEqualOp: + - a + - b + location: + row: 1 + column: 7 + end_location: + row: 1 + column: 13 + fix: + content: a == b + location: + row: 1 + column: 3 + end_location: + row: 1 + column: 13 + parent: ~ +- kind: + NegateNotEqualOp: + - a + - b + c + location: + row: 4 + column: 7 + end_location: + row: 4 + column: 19 + fix: + content: a == b + c + location: + row: 4 + column: 3 + end_location: + row: 4 + column: 19 + parent: ~ +- kind: + NegateNotEqualOp: + - a + b + - c + location: + row: 7 + column: 7 + end_location: + row: 7 + column: 19 + fix: + content: a + b == c + location: + row: 7 + column: 3 + end_location: + row: 7 + column: 19 + parent: ~ + diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM208_SIM208.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM208_SIM208.py.snap new file mode 100644 index 0000000000000..de677fbd127d7 --- /dev/null +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM208_SIM208.py.snap @@ -0,0 +1,39 @@ +--- +source: src/flake8_simplify/mod.rs +expression: checks +--- +- kind: + DoubleNegation: a + location: + row: 1 + column: 12 + end_location: + row: 1 + column: 13 + fix: + content: a + location: + row: 1 + column: 3 + end_location: + row: 1 + column: 14 + parent: ~ +- kind: + DoubleNegation: a == b + location: + row: 4 + column: 13 + end_location: + row: 4 + column: 19 + fix: + content: a == b + location: + row: 4 + column: 3 + end_location: + row: 4 + column: 21 + parent: ~ + diff --git a/src/pycodestyle/plugins.rs b/src/pycodestyle/plugins.rs index ec03f83960079..4c909803b696c 100644 --- a/src/pycodestyle/plugins.rs +++ b/src/pycodestyle/plugins.rs @@ -4,7 +4,9 @@ use rustpython_ast::{Arguments, Location, StmtKind}; use rustpython_parser::ast::{Cmpop, Constant, Expr, ExprKind, Stmt, Unaryop}; use crate::ast::helpers; -use crate::ast::helpers::{match_leading_content, match_trailing_content}; +use crate::ast::helpers::{ + create_expr, match_leading_content, match_trailing_content, unparse_expr, +}; use crate::ast::types::Range; use crate::ast::whitespace::leading_space; use crate::autofix::Fix; @@ -13,24 +15,20 @@ use crate::registry::{Check, CheckKind}; use crate::source_code_generator::SourceCodeGenerator; use crate::source_code_style::SourceCodeStyleDetector; -fn compare( +pub fn compare( left: &Expr, ops: &[Cmpop], comparators: &[Expr], stylist: &SourceCodeStyleDetector, ) -> String { - let cmp = Expr::new( - Location::default(), - Location::default(), - ExprKind::Compare { + unparse_expr( + &create_expr(ExprKind::Compare { left: Box::new(left.clone()), ops: ops.to_vec(), comparators: comparators.to_vec(), - }, - ); - let mut generator: SourceCodeGenerator = stylist.into(); - generator.unparse_expr(&cmp, 0); - generator.generate() + }), + stylist, + ) } /// E711, E712 diff --git a/src/registry.rs b/src/registry.rs index c81f12288d7f2..12b0525388d91 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -228,6 +228,9 @@ pub enum CheckCode { SIM111, SIM117, SIM118, + SIM201, + SIM202, + SIM208, SIM220, SIM221, SIM222, @@ -968,6 +971,9 @@ pub enum CheckKind { DuplicateIsinstanceCall(String), AAndNotA(String), AOrNotA(String), + NegateEqualOp(String, String), + NegateNotEqualOp(String, String), + DoubleNegation(String), AndFalse, ConvertLoopToAll(String), ConvertLoopToAny(String), @@ -1426,6 +1432,11 @@ impl CheckCode { } CheckCode::SIM117 => CheckKind::MultipleWithStatements, CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()), + CheckCode::SIM201 => CheckKind::NegateEqualOp("left".to_string(), "right".to_string()), + CheckCode::SIM202 => { + CheckKind::NegateNotEqualOp("left".to_string(), "right".to_string()) + } + CheckCode::SIM208 => CheckKind::DoubleNegation("expr".to_string()), CheckCode::SIM220 => CheckKind::AAndNotA("...".to_string()), CheckCode::SIM221 => CheckKind::AOrNotA("...".to_string()), CheckCode::SIM222 => CheckKind::OrTrue, @@ -1973,6 +1984,9 @@ impl CheckCode { CheckCode::SIM111 => CheckCategory::Flake8Simplify, CheckCode::SIM117 => CheckCategory::Flake8Simplify, CheckCode::SIM118 => CheckCategory::Flake8Simplify, + CheckCode::SIM201 => CheckCategory::Flake8Simplify, + CheckCode::SIM202 => CheckCategory::Flake8Simplify, + CheckCode::SIM208 => CheckCategory::Flake8Simplify, CheckCode::SIM220 => CheckCategory::Flake8Simplify, CheckCode::SIM221 => CheckCategory::Flake8Simplify, CheckCode::SIM222 => CheckCategory::Flake8Simplify, @@ -2230,9 +2244,12 @@ impl CheckKind { CheckKind::CompareWithTuple(..) => &CheckCode::SIM109, CheckKind::ConvertLoopToAll(..) => &CheckCode::SIM111, CheckKind::ConvertLoopToAny(..) => &CheckCode::SIM110, + CheckKind::DoubleNegation(..) => &CheckCode::SIM208, CheckKind::DuplicateIsinstanceCall(..) => &CheckCode::SIM101, CheckKind::KeyInDict(..) => &CheckCode::SIM118, CheckKind::MultipleWithStatements => &CheckCode::SIM117, + CheckKind::NegateEqualOp(..) => &CheckCode::SIM201, + CheckKind::NegateNotEqualOp(..) => &CheckCode::SIM202, CheckKind::NestedIfStatements => &CheckCode::SIM102, CheckKind::OrTrue => &CheckCode::SIM222, CheckKind::ReturnInTryExceptFinally => &CheckCode::SIM107, @@ -3021,6 +3038,15 @@ impl CheckKind { CheckKind::ConvertLoopToAny(any) => { format!("Use `{any}` instead of `for` loop") } + CheckKind::NegateEqualOp(left, right) => { + format!("Use `{left} != {right}` instead of `not {left} == {right}`") + } + CheckKind::NegateNotEqualOp(left, right) => { + format!("Use `{left} == {right}` instead of `not {left} != {right}`") + } + CheckKind::DoubleNegation(expr) => { + format!("Use `{expr}` instead of `not (not {expr})`") + } CheckKind::KeyInDict(key, dict) => { format!("Use `{key} in {dict}` instead of `{key} in {dict}.keys()`") } @@ -3686,6 +3712,7 @@ impl CheckKind { | CheckKind::DeprecatedUnittestAlias(..) | CheckKind::DoNotAssertFalse | CheckKind::DoNotAssignLambda(..) + | CheckKind::DoubleNegation(..) | CheckKind::DupeClassFieldDefinitions(..) | CheckKind::DuplicateHandlerException(..) | CheckKind::DuplicateIsinstanceCall(..) @@ -3703,6 +3730,8 @@ impl CheckKind { | CheckKind::MisplacedComparisonConstant(..) | CheckKind::MissingReturnTypeSpecialMethod(..) | CheckKind::NativeLiterals(..) + | CheckKind::NegateEqualOp(..) + | CheckKind::NegateNotEqualOp(..) | CheckKind::NewLineAfterLastParagraph | CheckKind::NewLineAfterSectionName(..) | CheckKind::NoBlankLineAfterFunction(..) @@ -3831,6 +3860,7 @@ impl CheckKind { } CheckKind::DoNotAssertFalse => Some("Replace `assert False`".to_string()), CheckKind::DoNotAssignLambda(name) => Some(format!("Rewrite `{name}` as a `def`")), + CheckKind::DoubleNegation(expr) => Some(format!("Replace with `{expr}`")), CheckKind::DupeClassFieldDefinitions(name) => { Some(format!("Remove duplicate field definition for `{name}`")) } @@ -3872,11 +3902,13 @@ impl CheckKind { CheckKind::NativeLiterals(literal_type) => { Some(format!("Replace with `{literal_type}`")) } - CheckKind::OpenAlias => Some("Replace with builtin `open`".to_string()), - CheckKind::OrTrue => Some("Replace with `True`".to_string()), + CheckKind::NegateEqualOp(..) => Some("Replace with `!=` operator".to_string()), + CheckKind::NegateNotEqualOp(..) => Some("Replace with `==` operator".to_string()), CheckKind::NewLineAfterLastParagraph => { Some("Move closing quotes to new line".to_string()) } + CheckKind::OpenAlias => Some("Replace with builtin `open`".to_string()), + CheckKind::OrTrue => Some("Replace with `True`".to_string()), CheckKind::ReplaceUniversalNewlines => { Some("Replace with `text` keyword argument".to_string()) }