From 586a3fc0c348ad91dbcdbb7ab825f011f537e928 Mon Sep 17 00:00:00 2001 From: Chammika Mannakkara Date: Sun, 8 Jan 2023 02:01:07 +0900 Subject: [PATCH 1/7] ifexpr checks : SIM210, SIM211, SIM212 --- src/checkers/ast.rs | 15 +++ src/flake8_simplify/plugins/ast_ifexp.rs | 142 +++++++++++++++++++++++ src/flake8_simplify/plugins/mod.rs | 4 + src/registry.rs | 26 +++++ 4 files changed, 187 insertions(+) create mode 100644 src/flake8_simplify/plugins/ast_ifexp.rs diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 0695f806bd394..fa078c88448ea 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -2714,6 +2714,21 @@ where } self.push_scope(Scope::new(ScopeKind::Lambda(Lambda { args, body }))); } + ExprKind::IfExp { test, body, orelse } => { + if self.settings.enabled.contains(&CheckCode::SIM210) { + flake8_simplify::plugins::explicit_ture_false_in_ifexpr( + self, expr, test, body, orelse, + ) + } + if self.settings.enabled.contains(&CheckCode::SIM211) { + flake8_simplify::plugins::explicit_false_true_in_ifexpr( + self, expr, test, body, orelse, + ) + } + if self.settings.enabled.contains(&CheckCode::SIM212) { + flake8_simplify::plugins::twisted_arms_in_ifexpr(self, expr, test, body, orelse) + } + } ExprKind::ListComp { elt, generators } | ExprKind::SetComp { elt, generators } => { if self.settings.enabled.contains(&CheckCode::C416) { if let Some(check) = flake8_comprehensions::checks::unnecessary_comprehension( diff --git a/src/flake8_simplify/plugins/ast_ifexp.rs b/src/flake8_simplify/plugins/ast_ifexp.rs new file mode 100644 index 0000000000000..da9bbf5e8a264 --- /dev/null +++ b/src/flake8_simplify/plugins/ast_ifexp.rs @@ -0,0 +1,142 @@ +use rustpython_ast::{Constant, Expr, ExprContext, ExprKind, 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}; + +/// SIM210 +pub fn explicit_ture_false_in_ifexpr( + checker: &mut Checker, + expr: &Expr, + test: &Expr, + body: &Expr, + orelse: &Expr, +) { + let ExprKind::Constant { value, .. } = &body.node else { + return; + }; + if !matches!(value, Constant::Bool(true)) { + return; + } + let ExprKind::Constant { value, .. } = &orelse.node else { + return; + }; + if !matches!(value, Constant::Bool(false)) { + return; + } + + let mut check = Check::new( + CheckKind::IfExprWithTrueFalse(unparse_expr(test, checker.style)), + Range::from_located(expr), + ); + if checker.patch(check.kind.code()) { + check.amend(Fix::replacement( + unparse_expr( + &create_expr(ExprKind::Call { + func: Box::new(create_expr(ExprKind::Name { + id: "bool".to_string(), + ctx: ExprContext::Load, + })), + args: vec![create_expr(test.node.clone())], + keywords: vec![], + }), + checker.style, + ), + expr.location, + expr.end_location.unwrap(), + )); + } + checker.add_check(check); +} + +/// SIM211 +pub fn explicit_false_true_in_ifexpr( + checker: &mut Checker, + expr: &Expr, + test: &Expr, + body: &Expr, + orelse: &Expr, +) { + let ExprKind::Constant { value, .. } = &body.node else { + return; + }; + if !matches!(value, Constant::Bool(false)) { + return; + } + let ExprKind::Constant { value, .. } = &orelse.node else { + return; + }; + if !matches!(value, Constant::Bool(true)) { + return; + } + + let mut check = Check::new( + CheckKind::IfExprWithFalseTrue(unparse_expr(test, checker.style)), + Range::from_located(expr), + ); + if checker.patch(check.kind.code()) { + check.amend(Fix::replacement( + unparse_expr( + &create_expr(ExprKind::UnaryOp { + op: Unaryop::Not, + operand: Box::new(create_expr(test.node.clone())), + }), + checker.style, + ), + expr.location, + expr.end_location.unwrap(), + )); + } + checker.add_check(check); +} + +/// SIM212 +pub fn twisted_arms_in_ifexpr( + checker: &mut Checker, + expr: &Expr, + test: &Expr, + body: &Expr, + orelse: &Expr, +) { + let ExprKind::UnaryOp { op, operand: test_operand } = &test.node else { + return; + }; + if !matches!(op, Unaryop::Not) { + return; + } + // Check if test operand and else branch is the same named variable + let ExprKind::Name { id: test_id, .. } = &test_operand.node else { + return; + }; + let ExprKind::Name {id: orelse_id, ..} = &orelse.node else { + return; + }; + if !test_id.eq(orelse_id) { + return; + } + + let mut check = Check::new( + CheckKind::NegateEqualOp( + unparse_expr(body, checker.style), + unparse_expr(orelse, checker.style), + ), + Range::from_located(expr), + ); + if checker.patch(check.kind.code()) { + check.amend(Fix::replacement( + unparse_expr( + &create_expr(ExprKind::IfExp { + test: Box::new(create_expr(orelse.node.clone())), + body: Box::new(create_expr(orelse.node.clone())), + orelse: Box::new(create_expr(body.node.clone())), + }), + checker.style, + ), + expr.location, + expr.end_location.unwrap(), + )); + } + checker.add_check(check); +} diff --git a/src/flake8_simplify/plugins/mod.rs b/src/flake8_simplify/plugins/mod.rs index b0c774311270b..f86d13e56f401 100644 --- a/src/flake8_simplify/plugins/mod.rs +++ b/src/flake8_simplify/plugins/mod.rs @@ -3,6 +3,9 @@ pub use ast_bool_op::{ }; pub use ast_for::convert_loop_to_any_all; pub use ast_if::{nested_if_statements, use_ternary_operator}; +pub use ast_ifexp::{ + explicit_false_true_in_ifexpr, explicit_ture_false_in_ifexpr, twisted_arms_in_ifexpr, +}; 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; @@ -13,6 +16,7 @@ pub use yoda_conditions::yoda_conditions; mod ast_bool_op; mod ast_for; mod ast_if; +mod ast_ifexp; mod ast_with; mod key_in_dict; mod return_in_try_except_finally; diff --git a/src/registry.rs b/src/registry.rs index 12b0525388d91..96d3077a09bae 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -231,6 +231,9 @@ pub enum CheckCode { SIM201, SIM202, SIM208, + SIM210, + SIM211, + SIM212, SIM220, SIM221, SIM222, @@ -974,6 +977,9 @@ pub enum CheckKind { NegateEqualOp(String, String), NegateNotEqualOp(String, String), DoubleNegation(String), + IfExprWithTrueFalse(String), + IfExprWithFalseTrue(String), + IfExprWithTwistedArms(String, String), AndFalse, ConvertLoopToAll(String), ConvertLoopToAny(String), @@ -1437,6 +1443,11 @@ impl CheckCode { CheckKind::NegateNotEqualOp("left".to_string(), "right".to_string()) } CheckCode::SIM208 => CheckKind::DoubleNegation("expr".to_string()), + CheckCode::SIM210 => CheckKind::IfExprWithTrueFalse("expr".to_string()), + CheckCode::SIM211 => CheckKind::IfExprWithFalseTrue("expr".to_string()), + CheckCode::SIM212 => { + CheckKind::IfExprWithTwistedArms("expr1".to_string(), "expr2".to_string()) + } CheckCode::SIM220 => CheckKind::AAndNotA("...".to_string()), CheckCode::SIM221 => CheckKind::AOrNotA("...".to_string()), CheckCode::SIM222 => CheckKind::OrTrue, @@ -1987,6 +1998,9 @@ impl CheckCode { CheckCode::SIM201 => CheckCategory::Flake8Simplify, CheckCode::SIM202 => CheckCategory::Flake8Simplify, CheckCode::SIM208 => CheckCategory::Flake8Simplify, + CheckCode::SIM210 => CheckCategory::Flake8Simplify, + CheckCode::SIM211 => CheckCategory::Flake8Simplify, + CheckCode::SIM212 => CheckCategory::Flake8Simplify, CheckCode::SIM220 => CheckCategory::Flake8Simplify, CheckCode::SIM221 => CheckCategory::Flake8Simplify, CheckCode::SIM222 => CheckCategory::Flake8Simplify, @@ -2246,6 +2260,9 @@ impl CheckKind { CheckKind::ConvertLoopToAny(..) => &CheckCode::SIM110, CheckKind::DoubleNegation(..) => &CheckCode::SIM208, CheckKind::DuplicateIsinstanceCall(..) => &CheckCode::SIM101, + CheckKind::IfExprWithFalseTrue(..) => &CheckCode::SIM211, + CheckKind::IfExprWithTrueFalse(..) => &CheckCode::SIM210, + CheckKind::IfExprWithTwistedArms(..) => &CheckCode::SIM212, CheckKind::KeyInDict(..) => &CheckCode::SIM118, CheckKind::MultipleWithStatements => &CheckCode::SIM117, CheckKind::NegateEqualOp(..) => &CheckCode::SIM201, @@ -3047,6 +3064,15 @@ impl CheckKind { CheckKind::DoubleNegation(expr) => { format!("Use `{expr}` instead of `not (not {expr})`") } + CheckKind::IfExprWithTrueFalse(expr) => { + format!("Use `bool({expr})` instead of `True if {expr} else False`") + } + CheckKind::IfExprWithFalseTrue(expr) => { + format!("Use `not {expr}` instead of `False if {expr} else True`") + } + CheckKind::IfExprWithTwistedArms(expr_body, expr_else) => { + format!("Use `{expr_else} if {expr_else} else {expr_body}` instead of `{expr_body} if not {expr_else} else {expr_else}`") + } CheckKind::KeyInDict(key, dict) => { format!("Use `{key} in {dict}` instead of `{key} in {dict}.keys()`") } From 8b5edbe8b41b96f7b733c1583acd9a13ae4634e3 Mon Sep 17 00:00:00 2001 From: Chammika Mannakkara Date: Sun, 8 Jan 2023 02:01:26 +0900 Subject: [PATCH 2/7] tests added --- resources/test/fixtures/flake8_simplify/SIM210.py | 10 ++++++++++ resources/test/fixtures/flake8_simplify/SIM211.py | 10 ++++++++++ resources/test/fixtures/flake8_simplify/SIM212.py | 9 +++++++++ 3 files changed, 29 insertions(+) create mode 100644 resources/test/fixtures/flake8_simplify/SIM210.py create mode 100644 resources/test/fixtures/flake8_simplify/SIM211.py create mode 100644 resources/test/fixtures/flake8_simplify/SIM212.py diff --git a/resources/test/fixtures/flake8_simplify/SIM210.py b/resources/test/fixtures/flake8_simplify/SIM210.py new file mode 100644 index 0000000000000..87be0a7da0095 --- /dev/null +++ b/resources/test/fixtures/flake8_simplify/SIM210.py @@ -0,0 +1,10 @@ + +a = True if b else False # SIM210 + +a = True if b!=c else False # SIM210 + +a = True if b+c else False # SIM210 + +a = False if b else True + + diff --git a/resources/test/fixtures/flake8_simplify/SIM211.py b/resources/test/fixtures/flake8_simplify/SIM211.py new file mode 100644 index 0000000000000..1823ac7fdfcae --- /dev/null +++ b/resources/test/fixtures/flake8_simplify/SIM211.py @@ -0,0 +1,10 @@ + +a = False if b else True # SIM211 + +a = False if b!=c else True # SIM211 -> SIM201 + +a = False if b+c else True # SIM211 + +a = True if b else False + + diff --git a/resources/test/fixtures/flake8_simplify/SIM212.py b/resources/test/fixtures/flake8_simplify/SIM212.py new file mode 100644 index 0000000000000..617bd48b4dbc9 --- /dev/null +++ b/resources/test/fixtures/flake8_simplify/SIM212.py @@ -0,0 +1,9 @@ + +c = b if not a else a # SIM212 + +c = b+c if not a else a # SIM212 + +c = b if not x else a + +c = a if a else b + From 42fd35c2b448cefe321a10923e59e2155a26044b Mon Sep 17 00:00:00 2001 From: Chammika Mannakkara Date: Sun, 8 Jan 2023 02:06:58 +0900 Subject: [PATCH 3/7] added tests and snapshots --- src/flake8_simplify/mod.rs | 3 + ...ke8_simplify__tests__SIM210_SIM210.py.snap | 56 +++++++++++++++++++ ...ke8_simplify__tests__SIM211_SIM211.py.snap | 56 +++++++++++++++++++ ...ke8_simplify__tests__SIM212_SIM212.py.snap | 29 ++++++++++ src/registry.rs | 5 +- 5 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM210_SIM210.py.snap create mode 100644 src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM211_SIM211.py.snap create mode 100644 src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM212_SIM212.py.snap diff --git a/src/flake8_simplify/mod.rs b/src/flake8_simplify/mod.rs index 67b41f3b961e7..d0de2565b0f34 100644 --- a/src/flake8_simplify/mod.rs +++ b/src/flake8_simplify/mod.rs @@ -23,6 +23,9 @@ mod tests { #[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::SIM210, Path::new("SIM210.py"); "SIM210")] + #[test_case(CheckCode::SIM211, Path::new("SIM211.py"); "SIM211")] + #[test_case(CheckCode::SIM212, Path::new("SIM212.py"); "SIM212")] #[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/snapshots/ruff__flake8_simplify__tests__SIM210_SIM210.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM210_SIM210.py.snap new file mode 100644 index 0000000000000..3ec3f3cbd953c --- /dev/null +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM210_SIM210.py.snap @@ -0,0 +1,56 @@ +--- +source: src/flake8_simplify/mod.rs +expression: checks +--- +- kind: + IfExprWithTrueFalse: b + location: + row: 2 + column: 4 + end_location: + row: 2 + column: 24 + fix: + content: bool(b) + location: + row: 2 + column: 4 + end_location: + row: 2 + column: 24 + parent: ~ +- kind: + IfExprWithTrueFalse: b != c + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 27 + fix: + content: bool(b != c) + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 27 + parent: ~ +- kind: + IfExprWithTrueFalse: b + c + location: + row: 6 + column: 4 + end_location: + row: 6 + column: 26 + fix: + content: bool(b + c) + location: + row: 6 + column: 4 + end_location: + row: 6 + column: 26 + parent: ~ + diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM211_SIM211.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM211_SIM211.py.snap new file mode 100644 index 0000000000000..e9f332e710a60 --- /dev/null +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM211_SIM211.py.snap @@ -0,0 +1,56 @@ +--- +source: src/flake8_simplify/mod.rs +expression: checks +--- +- kind: + IfExprWithFalseTrue: b + location: + row: 2 + column: 4 + end_location: + row: 2 + column: 24 + fix: + content: not b + location: + row: 2 + column: 4 + end_location: + row: 2 + column: 24 + parent: ~ +- kind: + IfExprWithFalseTrue: b != c + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 27 + fix: + content: not b != c + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 27 + parent: ~ +- kind: + IfExprWithFalseTrue: b + c + location: + row: 6 + column: 4 + end_location: + row: 6 + column: 26 + fix: + content: not b + c + location: + row: 6 + column: 4 + end_location: + row: 6 + column: 26 + parent: ~ + diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM212_SIM212.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM212_SIM212.py.snap new file mode 100644 index 0000000000000..f65d8fe3560a2 --- /dev/null +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM212_SIM212.py.snap @@ -0,0 +1,29 @@ +--- +source: src/flake8_simplify/mod.rs +expression: checks +--- +- kind: + NegateEqualOp: + - b + - a + location: + row: 2 + column: 4 + end_location: + row: 2 + column: 21 + fix: ~ + parent: ~ +- kind: + NegateEqualOp: + - b + c + - a + location: + row: 4 + column: 4 + end_location: + row: 4 + column: 23 + fix: ~ + parent: ~ + diff --git a/src/registry.rs b/src/registry.rs index 96d3077a09bae..67c6dd767cad0 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -3071,7 +3071,10 @@ impl CheckKind { format!("Use `not {expr}` instead of `False if {expr} else True`") } CheckKind::IfExprWithTwistedArms(expr_body, expr_else) => { - format!("Use `{expr_else} if {expr_else} else {expr_body}` instead of `{expr_body} if not {expr_else} else {expr_else}`") + format!( + "Use `{expr_else} if {expr_else} else {expr_body}` instead of `{expr_body} if \ + not {expr_else} else {expr_else}`" + ) } CheckKind::KeyInDict(key, dict) => { format!("Use `{key} in {dict}` instead of `{key} in {dict}.keys()`") From 3671401b64c39ed398d0c3d410dd8722bf2d3662 Mon Sep 17 00:00:00 2001 From: Chammika Mannakkara Date: Sun, 8 Jan 2023 02:17:47 +0900 Subject: [PATCH 4/7] consistent naming with flake8_simplify repo --- src/flake8_simplify/plugins/{unary_ops.rs => ast_unary_op.rs} | 0 src/flake8_simplify/plugins/mod.rs | 4 ++-- 2 files changed, 2 insertions(+), 2 deletions(-) rename src/flake8_simplify/plugins/{unary_ops.rs => ast_unary_op.rs} (100%) diff --git a/src/flake8_simplify/plugins/unary_ops.rs b/src/flake8_simplify/plugins/ast_unary_op.rs similarity index 100% rename from src/flake8_simplify/plugins/unary_ops.rs rename to src/flake8_simplify/plugins/ast_unary_op.rs diff --git a/src/flake8_simplify/plugins/mod.rs b/src/flake8_simplify/plugins/mod.rs index f086887f50c2e..d71b92b78ef01 100644 --- a/src/flake8_simplify/plugins/mod.rs +++ b/src/flake8_simplify/plugins/mod.rs @@ -6,10 +6,10 @@ pub use ast_if::{nested_if_statements, return_bool_condition_directly, use_terna pub use ast_ifexp::{ explicit_false_true_in_ifexpr, explicit_ture_false_in_ifexpr, twisted_arms_in_ifexpr, }; +pub use ast_unary_op::{double_negation, negation_with_equal_op, negation_with_not_equal_op}; 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; @@ -17,9 +17,9 @@ mod ast_bool_op; mod ast_for; mod ast_if; mod ast_ifexp; +mod ast_unary_op; mod ast_with; mod key_in_dict; mod return_in_try_except_finally; -mod unary_ops; mod use_contextlib_suppress; mod yoda_conditions; From 18f126866a93dde2f62ce7d9391b2e448077ccd6 Mon Sep 17 00:00:00 2001 From: Chammika Mannakkara Date: Sun, 8 Jan 2023 02:34:22 +0900 Subject: [PATCH 5/7] added to fixable, regenerate docs --- README.md | 3 +++ ruff.schema.json | 4 ++++ src/registry.rs | 8 ++++++++ 3 files changed, 15 insertions(+) diff --git a/README.md b/README.md index b23b6d4c64a67..27177d666787c 100644 --- a/README.md +++ b/README.md @@ -983,6 +983,9 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/ | 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)` | 🛠 | +| SIM210 | IfExprWithTrueFalse | Use `bool(expr)` instead of `True if expr else False` | 🛠 | +| SIM211 | IfExprWithFalseTrue | Use `not expr` instead of `False if expr else True` | 🛠 | +| SIM212 | IfExprWithTwistedArms | Use `expr2 if expr2 else expr1` instead of `expr1 if not expr2 else expr2` | 🛠 | | 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/ruff.schema.json b/ruff.schema.json index 0781ea7b7cf02..aff07733a8441 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -956,6 +956,10 @@ "SIM201", "SIM202", "SIM208", + "SIM21", + "SIM210", + "SIM211", + "SIM212", "SIM22", "SIM220", "SIM221", diff --git a/src/registry.rs b/src/registry.rs index 7bef06b893fe4..81da27454ac96 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -3768,6 +3768,9 @@ impl CheckKind { | CheckKind::EndsInPunctuation | CheckKind::FStringMissingPlaceholders | CheckKind::GetAttrWithConstant + | CheckKind::IfExprWithFalseTrue(..) + | CheckKind::IfExprWithTrueFalse(..) + | CheckKind::IfExprWithTwistedArms(..) | CheckKind::ImplicitReturn | CheckKind::ImplicitReturnValue | CheckKind::IncorrectFixtureParenthesesStyle(..) @@ -3927,6 +3930,11 @@ impl CheckKind { CheckKind::GetAttrWithConstant => { Some("Replace `getattr` with attribute access".to_string()) } + CheckKind::IfExprWithFalseTrue(expr) => Some(format!("Replace with `not {expr}`")), + CheckKind::IfExprWithTrueFalse(expr) => Some(format!("Replace with `bool({expr})`")), + CheckKind::IfExprWithTwistedArms(expr1, expr2) => { + Some(format!("Replace with `{expr1} if {expr1} else {expr2}`")) + } CheckKind::ImplicitReturnValue => Some("Add explicit `None` return value".to_string()), CheckKind::ImplicitReturn => Some("Add explicit `return` statement".to_string()), CheckKind::IncorrectMarkParenthesesStyle(..) => { From 70682e228c876245f506125449cde41d89156260 Mon Sep 17 00:00:00 2001 From: Chammika Mannakkara Date: Sun, 8 Jan 2023 02:43:44 +0900 Subject: [PATCH 6/7] fix typo --- src/checkers/ast.rs | 2 +- src/flake8_simplify/plugins/ast_ifexp.rs | 2 +- src/flake8_simplify/plugins/mod.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 18b3e537b3e3d..54583eff93e70 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -2711,7 +2711,7 @@ where } ExprKind::IfExp { test, body, orelse } => { if self.settings.enabled.contains(&CheckCode::SIM210) { - flake8_simplify::plugins::explicit_ture_false_in_ifexpr( + flake8_simplify::plugins::explicit_true_false_in_ifexpr( self, expr, test, body, orelse, ) } diff --git a/src/flake8_simplify/plugins/ast_ifexp.rs b/src/flake8_simplify/plugins/ast_ifexp.rs index 210a1e43f55f5..c2fe8ce5e8f02 100644 --- a/src/flake8_simplify/plugins/ast_ifexp.rs +++ b/src/flake8_simplify/plugins/ast_ifexp.rs @@ -7,7 +7,7 @@ use crate::checkers::ast::Checker; use crate::registry::{Check, CheckKind}; /// SIM210 -pub fn explicit_ture_false_in_ifexpr( +pub fn explicit_true_false_in_ifexpr( checker: &mut Checker, expr: &Expr, test: &Expr, diff --git a/src/flake8_simplify/plugins/mod.rs b/src/flake8_simplify/plugins/mod.rs index d71b92b78ef01..70042b4401bfb 100644 --- a/src/flake8_simplify/plugins/mod.rs +++ b/src/flake8_simplify/plugins/mod.rs @@ -4,7 +4,7 @@ pub use ast_bool_op::{ pub use ast_for::convert_loop_to_any_all; pub use ast_if::{nested_if_statements, return_bool_condition_directly, use_ternary_operator}; pub use ast_ifexp::{ - explicit_false_true_in_ifexpr, explicit_ture_false_in_ifexpr, twisted_arms_in_ifexpr, + explicit_false_true_in_ifexpr, explicit_true_false_in_ifexpr, twisted_arms_in_ifexpr, }; pub use ast_unary_op::{double_negation, negation_with_equal_op, negation_with_not_equal_op}; pub use ast_with::multiple_with_statements; From dbf4a9a8066ffa27dd1bc3d88091cd67313f523d Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 7 Jan 2023 15:30:56 -0500 Subject: [PATCH 7/7] Tweak fixtures --- README.md | 2 +- .../test/fixtures/flake8_simplify/SIM210.py | 11 +++---- .../test/fixtures/flake8_simplify/SIM211.py | 11 +++---- .../test/fixtures/flake8_simplify/SIM212.py | 10 +++--- src/flake8_simplify/plugins/ast_ifexp.rs | 3 +- ...ke8_simplify__tests__SIM210_SIM210.py.snap | 32 +++++++++---------- ...ke8_simplify__tests__SIM211_SIM211.py.snap | 32 +++++++++---------- ...ke8_simplify__tests__SIM212_SIM212.py.snap | 10 +++--- src/violations.rs | 2 +- 9 files changed, 53 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 587df1f57ecba..94dcb61671d16 100644 --- a/README.md +++ b/README.md @@ -985,7 +985,7 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/ | SIM208 | DoubleNegation | Use `expr` instead of `not (not expr)` | 🛠 | | SIM210 | IfExprWithTrueFalse | Use `bool(expr)` instead of `True if expr else False` | 🛠 | | SIM211 | IfExprWithFalseTrue | Use `not expr` instead of `False if expr else True` | 🛠 | -| SIM212 | IfExprWithTwistedArms | Use `body if body else else` instead of `else if not body else body` | 🛠 | +| SIM212 | IfExprWithTwistedArms | Use `b if b else a` instead of `a if not b else b` | 🛠 | | 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/SIM210.py b/resources/test/fixtures/flake8_simplify/SIM210.py index 87be0a7da0095..f8a6e808fea13 100644 --- a/resources/test/fixtures/flake8_simplify/SIM210.py +++ b/resources/test/fixtures/flake8_simplify/SIM210.py @@ -1,10 +1,7 @@ +a = True if b else False # SIM210 -a = True if b else False # SIM210 - -a = True if b!=c else False # SIM210 - -a = True if b+c else False # SIM210 - -a = False if b else True +a = True if b != c else False # SIM210 +a = True if b + c else False # SIM210 +a = False if b else True # OK diff --git a/resources/test/fixtures/flake8_simplify/SIM211.py b/resources/test/fixtures/flake8_simplify/SIM211.py index 1823ac7fdfcae..645997adba6ab 100644 --- a/resources/test/fixtures/flake8_simplify/SIM211.py +++ b/resources/test/fixtures/flake8_simplify/SIM211.py @@ -1,10 +1,7 @@ +a = False if b else True # SIM211 -a = False if b else True # SIM211 - -a = False if b!=c else True # SIM211 -> SIM201 - -a = False if b+c else True # SIM211 - -a = True if b else False +a = False if b != c else True # SIM211 +a = False if b + c else True # SIM211 +a = True if b else False # OK diff --git a/resources/test/fixtures/flake8_simplify/SIM212.py b/resources/test/fixtures/flake8_simplify/SIM212.py index 617bd48b4dbc9..c39c4db88993d 100644 --- a/resources/test/fixtures/flake8_simplify/SIM212.py +++ b/resources/test/fixtures/flake8_simplify/SIM212.py @@ -1,9 +1,7 @@ +c = b if not a else a # SIM212 -c = b if not a else a # SIM212 +c = b + c if not a else a # SIM212 -c = b+c if not a else a # SIM212 - -c = b if not x else a - -c = a if a else b +c = b if not x else a # OK +c = a if a else b # OK diff --git a/src/flake8_simplify/plugins/ast_ifexp.rs b/src/flake8_simplify/plugins/ast_ifexp.rs index b2b1e257305ba..03a93f3924a0e 100644 --- a/src/flake8_simplify/plugins/ast_ifexp.rs +++ b/src/flake8_simplify/plugins/ast_ifexp.rs @@ -107,7 +107,8 @@ pub fn twisted_arms_in_ifexpr( if !matches!(op, Unaryop::Not) { return; } - // Check if test operand and else branch is the same named variable + + // Check if the test operand and else branch use the same variable. let ExprKind::Name { id: test_id, .. } = &test_operand.node else { return; }; diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM210_SIM210.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM210_SIM210.py.snap index 3ec3f3cbd953c..db606bee62bb5 100644 --- a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM210_SIM210.py.snap +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM210_SIM210.py.snap @@ -5,52 +5,52 @@ expression: checks - kind: IfExprWithTrueFalse: b location: - row: 2 + row: 1 column: 4 end_location: - row: 2 + row: 1 column: 24 fix: content: bool(b) location: - row: 2 + row: 1 column: 4 end_location: - row: 2 + row: 1 column: 24 parent: ~ - kind: IfExprWithTrueFalse: b != c location: - row: 4 + row: 3 column: 4 end_location: - row: 4 - column: 27 + row: 3 + column: 29 fix: content: bool(b != c) location: - row: 4 + row: 3 column: 4 end_location: - row: 4 - column: 27 + row: 3 + column: 29 parent: ~ - kind: IfExprWithTrueFalse: b + c location: - row: 6 + row: 5 column: 4 end_location: - row: 6 - column: 26 + row: 5 + column: 28 fix: content: bool(b + c) location: - row: 6 + row: 5 column: 4 end_location: - row: 6 - column: 26 + row: 5 + column: 28 parent: ~ diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM211_SIM211.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM211_SIM211.py.snap index e9f332e710a60..c09285d349701 100644 --- a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM211_SIM211.py.snap +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM211_SIM211.py.snap @@ -5,52 +5,52 @@ expression: checks - kind: IfExprWithFalseTrue: b location: - row: 2 + row: 1 column: 4 end_location: - row: 2 + row: 1 column: 24 fix: content: not b location: - row: 2 + row: 1 column: 4 end_location: - row: 2 + row: 1 column: 24 parent: ~ - kind: IfExprWithFalseTrue: b != c location: - row: 4 + row: 3 column: 4 end_location: - row: 4 - column: 27 + row: 3 + column: 29 fix: content: not b != c location: - row: 4 + row: 3 column: 4 end_location: - row: 4 - column: 27 + row: 3 + column: 29 parent: ~ - kind: IfExprWithFalseTrue: b + c location: - row: 6 + row: 5 column: 4 end_location: - row: 6 - column: 26 + row: 5 + column: 28 fix: content: not b + c location: - row: 6 + row: 5 column: 4 end_location: - row: 6 - column: 26 + row: 5 + column: 28 parent: ~ diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM212_SIM212.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM212_SIM212.py.snap index f65d8fe3560a2..0fd162503d27f 100644 --- a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM212_SIM212.py.snap +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM212_SIM212.py.snap @@ -7,10 +7,10 @@ expression: checks - b - a location: - row: 2 + row: 1 column: 4 end_location: - row: 2 + row: 1 column: 21 fix: ~ parent: ~ @@ -19,11 +19,11 @@ expression: checks - b + c - a location: - row: 4 + row: 3 column: 4 end_location: - row: 4 - column: 23 + row: 3 + column: 25 fix: ~ parent: ~ diff --git a/src/violations.rs b/src/violations.rs index 0dab8fe1d1595..d26ec8fb63a2a 100644 --- a/src/violations.rs +++ b/src/violations.rs @@ -2985,7 +2985,7 @@ impl AlwaysAutofixableViolation for IfExprWithTwistedArms { } fn placeholder() -> Self { - IfExprWithTwistedArms("else".to_string(), "body".to_string()) + IfExprWithTwistedArms("a".to_string(), "b".to_string()) } }