diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a89b0b1a57..ea46d95e39 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -65,17 +65,13 @@ understand how other, similar rules are implemented. To add a test fixture, create a file under `resources/test/fixtures`, named to match the `CheckCode` you defined earlier (e.g., `E402.py`). This file should contain a variety of violations and -non-violations designed to evaluate and demonstrate the behavior of your lint rule. Run Ruff locally -with (e.g.) `cargo run resources/test/fixtures/E402.py --no-cache --select E402`. Once you're satisfied with the -output, codify the behavior as a snapshot test by adding a new `testcase` macro to the `mod tests` -section of `src/linter.rs`, like so: +non-violations designed to evaluate and demonstrate the behavior of your lint rule. -```rust -use test_case::test_case; +Run `cargo +nightly dev generate-all` to generate the code for your new fixture. Then run Ruff locally +with (e.g.) `cargo run resources/test/fixtures/E402.py --no-cache --select E402`. -#[test_case(CheckCode::A001, Path::new("A001.py"); "A001")] -... -``` +Once you're satisified with the output, codify the behavior as a snapshot test by +adding a new `test_case` macro in `src/[test-suite-name]/mod.rs` file. Then, run `cargo test`. Your test will fail, but you'll be prompted to follow-up with `cargo insta review`. Accept the generated snapshot, then commit the snapshot file alongside the diff --git a/README.md b/README.md index 3c14a8333d..f74dafab45 100644 --- a/README.md +++ b/README.md @@ -923,6 +923,7 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/ | Code | Name | Message | Fix | | ---- | ---- | ------- | --- | | SIM118 | KeyInDict | Use `key in dict` instead of `key in dict.keys()` | 🛠 | +| SIM300 | YodaConditions | Use `left == right` instead of `right == left (Yoda-conditions)` | | ### flake8-tidy-imports (TID) diff --git a/resources/test/fixtures/flake8_simplify/SIM300.py b/resources/test/fixtures/flake8_simplify/SIM300.py new file mode 100644 index 0000000000..4b6c1fcef7 --- /dev/null +++ b/resources/test/fixtures/flake8_simplify/SIM300.py @@ -0,0 +1,11 @@ +# Errors +"yoda" == compare # SIM300 +42 == age # SIM300 + +# OK +compare == "yoda" +age == 42 +x == y +"yoda" == compare == 1 +"yoda" == compare == someothervar +"yoda" == "yoda" diff --git a/ruff.schema.json b/ruff.schema.json index 585fd07cbe..9ff5639a49 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -848,6 +848,9 @@ "SIM1", "SIM11", "SIM118", + "SIM3", + "SIM30", + "SIM300", "T", "T1", "T10", diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index b4b7bdaeaf..58611670de 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -2402,6 +2402,10 @@ where comparators, ); } + + if self.settings.enabled.contains(&CheckCode::SIM300) { + flake8_simplify::plugins::yoda_conditions(self, expr, left, ops, comparators); + } } ExprKind::Constant { value: Constant::Str(value), diff --git a/src/checks.rs b/src/checks.rs index c4ee0b107b..bd0b734f62 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -213,6 +213,7 @@ pub enum CheckCode { YTT303, // flake8-simplify SIM118, + SIM300, // pyupgrade UP001, UP003, @@ -893,6 +894,7 @@ pub enum CheckKind { SysVersionSlice1Referenced, // flake8-simplify KeyInDict(String, String), + YodaConditions(String, String), // pyupgrade TypeOfPrimitive(Primitive), UselessMetaclassType, @@ -1287,6 +1289,7 @@ impl CheckCode { CheckCode::BLE001 => CheckKind::BlindExcept("Exception".to_string()), // flake8-simplify CheckCode::SIM118 => CheckKind::KeyInDict("key".to_string(), "dict".to_string()), + CheckCode::SIM300 => CheckKind::YodaConditions("left".to_string(), "right".to_string()), // pyupgrade CheckCode::UP001 => CheckKind::UselessMetaclassType, CheckCode::UP003 => CheckKind::TypeOfPrimitive(Primitive::Str), @@ -1721,6 +1724,7 @@ impl CheckCode { CheckCode::S106 => CheckCategory::Flake8Bandit, CheckCode::S107 => CheckCategory::Flake8Bandit, CheckCode::SIM118 => CheckCategory::Flake8Simplify, + CheckCode::SIM300 => CheckCategory::Flake8Simplify, CheckCode::T100 => CheckCategory::Flake8Debugger, CheckCode::T201 => CheckCategory::Flake8Print, CheckCode::T203 => CheckCategory::Flake8Print, @@ -1950,6 +1954,7 @@ impl CheckKind { CheckKind::SysVersionSlice1Referenced => &CheckCode::YTT303, // flake8-simplify CheckKind::KeyInDict(..) => &CheckCode::SIM118, + CheckKind::YodaConditions(..) => &CheckCode::SIM300, // pyupgrade CheckKind::TypeOfPrimitive(..) => &CheckCode::UP003, CheckKind::UselessMetaclassType => &CheckCode::UP001, @@ -2673,6 +2678,9 @@ impl CheckKind { CheckKind::KeyInDict(key, dict) => { format!("Use `{key} in {dict}` instead of `{key} in {dict}.keys()`") } + CheckKind::YodaConditions(left, right) => { + format!("Use `{left} == {right}` instead of `{right} == {left} (Yoda-conditions)`") + } // pyupgrade CheckKind::TypeOfPrimitive(primitive) => { format!("Use `{}` instead of `type(...)`", primitive.builtin()) diff --git a/src/checks_gen.rs b/src/checks_gen.rs index bf85f7c9a8..996871ada2 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -479,6 +479,9 @@ pub enum CheckCodePrefix { SIM1, SIM11, SIM118, + SIM3, + SIM30, + SIM300, T, T1, T10, @@ -754,6 +757,7 @@ impl CheckCodePrefix { CheckCode::YTT302, CheckCode::YTT303, CheckCode::SIM118, + CheckCode::SIM300, CheckCode::UP001, CheckCode::UP003, CheckCode::UP004, @@ -2411,10 +2415,13 @@ impl CheckCodePrefix { CheckCodePrefix::S105 => vec![CheckCode::S105], CheckCodePrefix::S106 => vec![CheckCode::S106], CheckCodePrefix::S107 => vec![CheckCode::S107], - CheckCodePrefix::SIM => vec![CheckCode::SIM118], + CheckCodePrefix::SIM => vec![CheckCode::SIM118, CheckCode::SIM300], CheckCodePrefix::SIM1 => vec![CheckCode::SIM118], CheckCodePrefix::SIM11 => vec![CheckCode::SIM118], CheckCodePrefix::SIM118 => vec![CheckCode::SIM118], + CheckCodePrefix::SIM3 => vec![CheckCode::SIM300], + CheckCodePrefix::SIM30 => vec![CheckCode::SIM300], + CheckCodePrefix::SIM300 => vec![CheckCode::SIM300], CheckCodePrefix::T => vec![CheckCode::T100, CheckCode::T201, CheckCode::T203], CheckCodePrefix::T1 => vec![CheckCode::T100], CheckCodePrefix::T10 => vec![CheckCode::T100], @@ -3315,6 +3322,9 @@ impl CheckCodePrefix { CheckCodePrefix::SIM1 => SuffixLength::One, CheckCodePrefix::SIM11 => SuffixLength::Two, CheckCodePrefix::SIM118 => SuffixLength::Three, + CheckCodePrefix::SIM3 => SuffixLength::One, + CheckCodePrefix::SIM30 => SuffixLength::Two, + CheckCodePrefix::SIM300 => SuffixLength::Three, CheckCodePrefix::T => SuffixLength::Zero, CheckCodePrefix::T1 => SuffixLength::One, CheckCodePrefix::T10 => SuffixLength::Two, diff --git a/src/flake8_simplify/mod.rs b/src/flake8_simplify/mod.rs index c6d337ce20..358cf06b96 100644 --- a/src/flake8_simplify/mod.rs +++ b/src/flake8_simplify/mod.rs @@ -13,6 +13,7 @@ mod tests { use crate::settings; #[test_case(CheckCode::SIM118, Path::new("SIM118.py"); "SIM118")] + #[test_case(CheckCode::SIM300, Path::new("SIM300.py"); "SIM300")] fn checks(check_code: CheckCode, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy()); let checks = test_path( diff --git a/src/flake8_simplify/plugins/mod.rs b/src/flake8_simplify/plugins/mod.rs index ef30eec235..164b029001 100644 --- a/src/flake8_simplify/plugins/mod.rs +++ b/src/flake8_simplify/plugins/mod.rs @@ -1,3 +1,5 @@ pub use key_in_dict::{key_in_dict_compare, key_in_dict_for}; +pub use yoda_conditions::yoda_conditions; mod key_in_dict; +mod yoda_conditions; diff --git a/src/flake8_simplify/plugins/yoda_conditions.rs b/src/flake8_simplify/plugins/yoda_conditions.rs new file mode 100644 index 0000000000..2a58554c45 --- /dev/null +++ b/src/flake8_simplify/plugins/yoda_conditions.rs @@ -0,0 +1,40 @@ +use rustpython_ast::{Cmpop, Expr, ExprKind}; + +use crate::ast::types::Range; +use crate::checkers::ast::Checker; +use crate::checks::{Check, CheckKind}; + +/// SIM300 +pub fn yoda_conditions( + checker: &mut Checker, + expr: &Expr, + left: &Expr, + ops: &[Cmpop], + comparators: &[Expr], +) { + if !matches!(ops[..], [Cmpop::Eq]) { + return; + } + + if comparators.len() != 1 { + return; + } + + if !matches!(left.node, ExprKind::Constant { .. }) { + return; + } + + let right = comparators.first().unwrap(); + if matches!(left.node, ExprKind::Constant { .. }) + & matches!(right.node, ExprKind::Constant { .. }) + { + return; + } + + let check = Check::new( + CheckKind::YodaConditions(left.to_string(), right.to_string()), + Range::from_located(expr), + ); + + checker.add_check(check); +} diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM300_SIM300.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM300_SIM300.py.snap new file mode 100644 index 0000000000..a154666829 --- /dev/null +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM300_SIM300.py.snap @@ -0,0 +1,29 @@ +--- +source: src/flake8_simplify/mod.rs +expression: checks +--- +- kind: + YodaConditions: + - "'yoda'" + - compare + location: + row: 2 + column: 0 + end_location: + row: 2 + column: 17 + fix: ~ + parent: ~ +- kind: + YodaConditions: + - "42" + - age + location: + row: 3 + column: 0 + end_location: + row: 3 + column: 9 + fix: ~ + parent: ~ +