Skip to content

Commit

Permalink
[ruff] implement useless if-else (RUF034) (#13218)
Browse files Browse the repository at this point in the history
  • Loading branch information
iamlucasvieira committed Sep 4, 2024
1 parent 862bd0c commit e37bde4
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 0 deletions.
11 changes: 11 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF034.py
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()> {
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)]
Expand Down
55 changes: 55 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/useless_if_else.rs
Original file line number Diff line number Diff line change
@@ -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));
}
Original file line number Diff line number Diff line change
@@ -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
|
1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit e37bde4

Please sign in to comment.