[ruff] implement useless if-else (RUF034) (#13218)

This commit is contained in:
Lucas Vieira dos Santos 2024-09-04 08:22:17 +02:00 committed by GitHub
parent 862bd0c429
commit e37bde458e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 101 additions and 0 deletions

View file

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

View file

@ -1404,6 +1404,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::IfExpInsteadOfOrOperator) { if checker.enabled(Rule::IfExpInsteadOfOrOperator) {
refurb::rules::if_exp_instead_of_or_operator(checker, if_exp); 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( Expr::ListComp(
comp @ ast::ExprListComp { comp @ ast::ExprListComp {

View file

@ -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, "031") => (RuleGroup::Preview, rules::ruff::rules::IncorrectlyParenthesizedTupleInSubscript),
(Ruff, "032") => (RuleGroup::Preview, rules::ruff::rules::DecimalFromFloatLiteral), (Ruff, "032") => (RuleGroup::Preview, rules::ruff::rules::DecimalFromFloatLiteral),
(Ruff, "033") => (RuleGroup::Preview, rules::ruff::rules::PostInitDefault), (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, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
(Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA), (Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),

View file

@ -58,6 +58,7 @@ mod tests {
#[test_case(Rule::AssertWithPrintMessage, Path::new("RUF030.py"))] #[test_case(Rule::AssertWithPrintMessage, Path::new("RUF030.py"))]
#[test_case(Rule::IncorrectlyParenthesizedTupleInSubscript, Path::new("RUF031.py"))] #[test_case(Rule::IncorrectlyParenthesizedTupleInSubscript, Path::new("RUF031.py"))]
#[test_case(Rule::DecimalFromFloatLiteral, Path::new("RUF032.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::RedirectedNOQA, Path::new("RUF101.py"))]
#[test_case(Rule::PostInitDefault, Path::new("RUF033.py"))] #[test_case(Rule::PostInitDefault, Path::new("RUF033.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> { fn rules(rule_code: Rule, path: &Path) -> Result<()> {

View file

@ -29,6 +29,7 @@ pub(crate) use unnecessary_iterable_allocation_for_first_element::*;
pub(crate) use unnecessary_key_check::*; pub(crate) use unnecessary_key_check::*;
pub(crate) use unused_async::*; pub(crate) use unused_async::*;
pub(crate) use unused_noqa::*; pub(crate) use unused_noqa::*;
pub(crate) use useless_if_else::*;
pub(crate) use zip_instead_of_pairwise::*; pub(crate) use zip_instead_of_pairwise::*;
mod ambiguous_unicode_character; mod ambiguous_unicode_character;
@ -66,6 +67,7 @@ mod unnecessary_iterable_allocation_for_first_element;
mod unnecessary_key_check; mod unnecessary_key_check;
mod unused_async; mod unused_async;
mod unused_noqa; mod unused_noqa;
mod useless_if_else;
mod zip_instead_of_pairwise; mod zip_instead_of_pairwise;
#[derive(Clone, Copy)] #[derive(Clone, Copy)]

View file

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

View file

@ -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
ruff.schema.json generated
View file

@ -3740,6 +3740,7 @@
"RUF031", "RUF031",
"RUF032", "RUF032",
"RUF033", "RUF033",
"RUF034",
"RUF1", "RUF1",
"RUF10", "RUF10",
"RUF100", "RUF100",