Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Simplify SIM201, SIM202, SIM208 (#1666) 

Flake8 simplify #998 

SIM201, SIM202 and SIM208 is done here with fixes.

Note: SIM203 == E713 

Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
Chammika Mannakkara 2023-01-07 00:47:48 +09:00 committed by GitHub
parent 0a940b3cb4
commit 1392170dbf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 408 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
README.md
View File

@ -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` | 🛠 |

17
resources/test/fixtures/flake8_simplify/SIM201.py vendored Normal file
View File

@ -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()

14
resources/test/fixtures/flake8_simplify/SIM202.py vendored Normal file
View File

@ -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

14
resources/test/fixtures/flake8_simplify/SIM208.py vendored Normal file
View File

@ -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

4
ruff.schema.json
View File

@ -950,6 +950,10 @@
"SIM117",
"SIM118",
"SIM2",
"SIM20",
"SIM201",
"SIM202",
"SIM208",
"SIM22",
"SIM220",
"SIM221",

10
src/checkers/ast.rs
View File

@ -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,

3
src/flake8_simplify/mod.rs
View File

@ -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")]

2
src/flake8_simplify/plugins/mod.rs
View File

@ -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;

129
src/flake8_simplify/plugins/unary_ops.rs Normal file
View File

@ -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);
}

62
src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM201_SIM201.py.snap Normal file
View File

@ -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: ~

6
src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM201_SIM201_2.py.snap Normal file
View File

@ -0,0 +1,6 @@
---
source: src/flake8_simplify/mod.rs
expression: checks
---
[]

62
src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM202_SIM202.py.snap Normal file
View File

@ -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: ~

39
src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM208_SIM208.py.snap Normal file
View File

@ -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: ~

20
src/pycodestyle/plugins.rs
View File

@ -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

36
src/registry.rs
View File

@ -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())
}