diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM300.py b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM300.py index a9cd7df872..d97692d793 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM300.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM300.py @@ -13,9 +13,10 @@ YODA > age # SIM300 YODA >= age # SIM300 JediOrder.YODA == age # SIM300 0 < (number - 100) # SIM300 -SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 B 0 +UPPER_LIST == ['upper'] +DummyHandler.CONFIG == {} +{"thats": "acceptable"} == DummyHandler.CONFIG +SECONDS_IN_DAY == 60 * 60 * 24 +SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) \ No newline at end of file diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/yoda_conditions.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/yoda_conditions.rs index c196823de7..68f38713bc 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/rules/yoda_conditions.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/rules/yoda_conditions.rs @@ -1,3 +1,5 @@ +use std::cmp; + use anyhow::Result; use libcst_native::CompOp; @@ -78,18 +80,51 @@ impl Violation for YodaConditions { } } -/// Return `true` if an [`Expr`] is a constant or a constant-like name. -fn is_constant_like(expr: &Expr) -> bool { +/// Comparisons left hand side must not be more [`ConstantLikelihood`] than their right hand side +#[derive(PartialEq, Eq, PartialOrd, Ord, Debug)] +enum ConstantLikelihood { + Unlikely = 0, + Probably = 1, // CAMEL_CASED vars + Definitely = 2, // literals, empty dicts and tuples... +} + +fn how_likely_constant_str(s: &str) -> ConstantLikelihood { + if str::is_cased_uppercase(s) { + ConstantLikelihood::Probably + } else { + ConstantLikelihood::Unlikely + } +} + +/// Return [`Expr`] [`ConstantLikelihood`] level depending on simple heuristics. +fn how_likely_constant(expr: &Expr) -> ConstantLikelihood { + if expr.is_literal_expr() { + return ConstantLikelihood::Definitely; + } match expr { - Expr::Attribute(ast::ExprAttribute { attr, .. }) => str::is_cased_uppercase(attr), - Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.iter().all(is_constant_like), - Expr::Name(ast::ExprName { id, .. }) => str::is_cased_uppercase(id), + Expr::Attribute(ast::ExprAttribute { attr, .. }) => how_likely_constant_str(attr), + Expr::Name(ast::ExprName { id, .. }) => how_likely_constant_str(id), + Expr::Tuple(ast::ExprTuple { elts, .. }) | Expr::List(ast::ExprList { elts, .. }) => elts + .iter() + .map(how_likely_constant) + .min() + .unwrap_or(ConstantLikelihood::Definitely), + Expr::Dict(ast::ExprDict { values: vs, .. }) => { + if vs.is_empty() { + ConstantLikelihood::Definitely + } else { + ConstantLikelihood::Probably + } + } + Expr::BinOp(ast::ExprBinOp { left, right, .. }) => { + cmp::min(how_likely_constant(left), how_likely_constant(right)) + } Expr::UnaryOp(ast::ExprUnaryOp { op: UnaryOp::UAdd | UnaryOp::USub | UnaryOp::Invert, operand, range: _, - }) => operand.is_literal_expr(), - _ => expr.is_literal_expr(), + }) => how_likely_constant(operand), + _ => ConstantLikelihood::Unlikely, } } @@ -180,7 +215,7 @@ pub(crate) fn yoda_conditions( return; } - if !is_constant_like(left) || is_constant_like(right) { + if how_likely_constant(left) <= how_likely_constant(right) { return; } diff --git a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM300_SIM300.py.snap b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM300_SIM300.py.snap index 036cabca9d..0cf1810323 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM300_SIM300.py.snap +++ b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM300_SIM300.py.snap @@ -247,7 +247,7 @@ SIM300.py:13:1: SIM300 [*] Yoda conditions are discouraged, use `age <= YODA` in 13 |+age <= YODA # SIM300 14 14 | JediOrder.YODA == age # SIM300 15 15 | 0 < (number - 100) # SIM300 -16 16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 +16 16 | B (60 * 60) # SIM300 +16 | B (60 * 60) # SIM300 -17 17 | B 0` instead | @@ -276,8 +276,8 @@ SIM300.py:15:1: SIM300 [*] Yoda conditions are discouraged, use `(number - 100) 14 | JediOrder.YODA == age # SIM300 15 | 0 < (number - 100) # SIM300 | ^^^^^^^^^^^^^^^^^^ SIM300 -16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 -17 | B 0` @@ -287,70 +287,91 @@ SIM300.py:15:1: SIM300 [*] Yoda conditions are discouraged, use `(number - 100) 14 14 | JediOrder.YODA == age # SIM300 15 |-0 < (number - 100) # SIM300 15 |+(number - 100) > 0 # SIM300 -16 16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 -17 17 | B B` instead | 14 | JediOrder.YODA == age # SIM300 15 | 0 < (number - 100) # SIM300 -16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM300 -17 | B B` ℹ Safe fix 13 13 | YODA >= age # SIM300 14 14 | JediOrder.YODA == age # SIM300 15 15 | 0 < (number - 100) # SIM300 -16 |-SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 - 16 |+(60 * 60) < SomeClass().settings.SOME_CONSTANT_VALUE # SIM300 -17 17 | B B or B +17 17 | B or(B) B` instead +SIM300.py:17:5: SIM300 [*] Yoda conditions are discouraged, use `A[0][0] > (B)` instead | 15 | 0 < (number - 100) # SIM300 -16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 -17 | B B` - -ℹ Safe fix -14 14 | JediOrder.YODA == age # SIM300 -15 15 | 0 < (number - 100) # SIM300 -16 16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 -17 |-B B or B -18 18 | B or(B) (B)` instead - | -16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 -17 | B (B)` ℹ Safe fix +14 14 | JediOrder.YODA == age # SIM300 15 15 | 0 < (number - 100) # SIM300 -16 16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 -17 17 | B (B) -19 19 | -20 20 | # OK -21 21 | compare == "yoda" +16 16 | B (B) +18 18 | ['upper'] == UPPER_LIST +19 19 | {} == DummyHandler.CONFIG +20 20 | + +SIM300.py:18:1: SIM300 [*] Yoda conditions are discouraged, use `UPPER_LIST == ['upper']` instead + | +16 | B