Add flake8-simplify SIM300 check for Yoda Conditions (#1539)

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
+non-violations designed to evaluate and demonstrate the behavior of your lint rule.
-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:
 
-```rust
+Run `cargo +nightly dev generate-all` to generate the code for your new fixture. Then run Ruff locally
-use test_case::test_case;
+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

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)
 

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"

ruff.schema.json

@@ -848,6 +848,9 @@
         "SIM1",
         "SIM11",
         "SIM118",
+        "SIM3",
+        "SIM30",
+        "SIM300",
         "T",
         "T1",
         "T10",

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),

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())

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,

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(

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;

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

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: ~
+