From e37bde458e9928f1997cba249471459d31136aee Mon Sep 17 00:00:00 2001 From: Lucas Vieira dos Santos Date: Wed, 4 Sep 2024 08:22:17 +0200 Subject: [PATCH] [ruff] implement useless if-else (RUF034) (#13218) --- .../resources/test/fixtures/ruff/RUF034.py | 11 ++++ .../src/checkers/ast/analyze/expression.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/ruff/mod.rs | 1 + .../ruff_linter/src/rules/ruff/rules/mod.rs | 2 + .../src/rules/ruff/rules/useless_if_else.rs | 55 +++++++++++++++++++ ..._rules__ruff__tests__RUF034_RUF034.py.snap | 27 +++++++++ ruff.schema.json | 1 + 8 files changed, 101 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF034.py create mode 100644 crates/ruff_linter/src/rules/ruff/rules/useless_if_else.rs create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF034_RUF034.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF034.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF034.py new file mode 100644 index 0000000000000..89fcf458ee44c --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF034.py @@ -0,0 +1,11 @@ +# Valid +x = 1 if True else 2 + +# Invalid +x = 1 if True else 1 + +# Invalid +x = "a" if True else "a" + +# Invalid +x = 0.1 if False else 0.1 diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index 57a64933b4b69..ba8bb634a903a 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -1404,6 +1404,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::IfExpInsteadOfOrOperator) { refurb::rules::if_exp_instead_of_or_operator(checker, if_exp); } + if checker.enabled(Rule::UselessIfElse) { + ruff::rules::useless_if_else(checker, if_exp); + } } Expr::ListComp( comp @ ast::ExprListComp { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index b458629c3d96b..25c150dfe556d 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -961,6 +961,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "031") => (RuleGroup::Preview, rules::ruff::rules::IncorrectlyParenthesizedTupleInSubscript), (Ruff, "032") => (RuleGroup::Preview, rules::ruff::rules::DecimalFromFloatLiteral), (Ruff, "033") => (RuleGroup::Preview, rules::ruff::rules::PostInitDefault), + (Ruff, "034") => (RuleGroup::Preview, rules::ruff::rules::UselessIfElse), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA), diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index b6966c2574805..84b7fd0d8ff7d 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -58,6 +58,7 @@ mod tests { #[test_case(Rule::AssertWithPrintMessage, Path::new("RUF030.py"))] #[test_case(Rule::IncorrectlyParenthesizedTupleInSubscript, Path::new("RUF031.py"))] #[test_case(Rule::DecimalFromFloatLiteral, Path::new("RUF032.py"))] + #[test_case(Rule::UselessIfElse, Path::new("RUF034.py"))] #[test_case(Rule::RedirectedNOQA, Path::new("RUF101.py"))] #[test_case(Rule::PostInitDefault, Path::new("RUF033.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index b95b436221fea..49b40b0b7900d 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -29,6 +29,7 @@ pub(crate) use unnecessary_iterable_allocation_for_first_element::*; pub(crate) use unnecessary_key_check::*; pub(crate) use unused_async::*; pub(crate) use unused_noqa::*; +pub(crate) use useless_if_else::*; pub(crate) use zip_instead_of_pairwise::*; mod ambiguous_unicode_character; @@ -66,6 +67,7 @@ mod unnecessary_iterable_allocation_for_first_element; mod unnecessary_key_check; mod unused_async; mod unused_noqa; +mod useless_if_else; mod zip_instead_of_pairwise; #[derive(Clone, Copy)] diff --git a/crates/ruff_linter/src/rules/ruff/rules/useless_if_else.rs b/crates/ruff_linter/src/rules/ruff/rules/useless_if_else.rs new file mode 100644 index 0000000000000..aa3caad384942 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/useless_if_else.rs @@ -0,0 +1,55 @@ +use crate::checkers::ast::Checker; +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; +use ruff_python_ast::comparable::ComparableExpr; + +/// ## What it does +/// Checks for useless if-else conditions with identical arms. +/// +/// ## Why is this bad? +/// Useless if-else conditions add unnecessary complexity to the code without +/// providing any logical benefit. +/// +/// Assigning the value directly is clearer and more explicit, and +/// should be preferred. +/// +/// ## Example +/// ```python +/// # Bad +/// foo = x if y else x +/// ``` +/// +/// Use instead: +/// ```python +/// # Good +/// foo = x +/// ``` +#[violation] +pub struct UselessIfElse; + +impl Violation for UselessIfElse { + #[derive_message_formats] + fn message(&self) -> String { + format!("Useless if-else condition") + } +} + +/// RUF031 +pub(crate) fn useless_if_else(checker: &mut Checker, if_expr: &ast::ExprIf) { + let ast::ExprIf { + body, + orelse, + range, + .. + } = if_expr; + + // Skip if the body and orelse are not the same + if ComparableExpr::from(body) != ComparableExpr::from(orelse) { + return; + } + + checker + .diagnostics + .push(Diagnostic::new(UselessIfElse, *range)); +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF034_RUF034.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF034_RUF034.py.snap new file mode 100644 index 0000000000000..9feaa0bea123f --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF034_RUF034.py.snap @@ -0,0 +1,27 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF034.py:5:5: RUF034 Useless if-else condition + | +4 | # Invalid +5 | x = 1 if True else 1 + | ^^^^^^^^^^^^^^^^ RUF034 +6 | +7 | # Invalid + | + +RUF034.py:8:5: RUF034 Useless if-else condition + | + 7 | # Invalid + 8 | x = "a" if True else "a" + | ^^^^^^^^^^^^^^^^^^^^ RUF034 + 9 | +10 | # Invalid + | + +RUF034.py:11:5: RUF034 Useless if-else condition + | +10 | # Invalid +11 | x = 0.1 if False else 0.1 + | ^^^^^^^^^^^^^^^^^^^^^ RUF034 + | diff --git a/ruff.schema.json b/ruff.schema.json index eba40f1aa42ea..c39a68ebd6aa7 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3740,6 +3740,7 @@ "RUF031", "RUF032", "RUF033", + "RUF034", "RUF1", "RUF10", "RUF100",