Replace misplaced-comparison-constant with SIM300

astral-sh · Jan 18, 2023 · 7f40ad8 · 7f40ad8
1 parent 7628876
commit 7f40ad8
Show file tree

Hide file tree

Showing 13 changed files with 95 additions and 224 deletions.
diff --git a/BREAKING_CHANGES.md b/BREAKING_CHANGES.md
@@ -1,5 +1,11 @@
 # Breaking Changes
 
+## 0.0.226
+
+### `misplaced-comparison-constant` (`PLC2201`) was deprecated in favor of `SIM300` ()
+
+
+
 ## 0.0.225
 
 ### `@functools.cache` rewrites have been moved to a standalone rule (`UP033`) ([#1938](https://github.com/charliermarsh/ruff/pull/1938))

diff --git a/README.md b/README.md
@@ -1112,7 +1112,6 @@ For more, see [Pylint](https://pypi.org/project/pylint/2.15.7/) on PyPI.
 | Code | Name | Message | Fix |
 | ---- | ---- | ------- | --- |
 | PLC0414 | UselessImportAlias | Import alias does not rename original package | 🛠 |
-| PLC2201 | MisplacedComparisonConstant | Comparison should be ... | 🛠 |
 | PLC3002 | UnnecessaryDirectLambdaCall | Lambda expression called directly. Execute the expression inline instead. |  |
 
 #### Error (PLE)

diff --git a/resources/test/fixtures/flake8_simplify/SIM300.py b/resources/test/fixtures/flake8_simplify/SIM300.py
@@ -2,6 +2,9 @@
 "yoda" == compare  # SIM300
 'yoda' == compare  # SIM300
 42 == age  # SIM300
+"yoda" <= compare  # SIM300
+'yoda' < compare  # SIM300
+42 > age  # SIM300
 
 # OK
 compare == "yoda"

diff --git a/ruff.schema.json b/ruff.schema.json
@@ -1484,10 +1484,6 @@
         "PLC04",
         "PLC041",
         "PLC0414",
-        "PLC2",
-        "PLC22",
-        "PLC220",
-        "PLC2201",
         "PLC3",
         "PLC30",
         "PLC300",

diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs
@@ -2552,16 +2552,6 @@ where
                     );
                 }
 
-                if self.settings.rules.enabled(&RuleCode::PLC2201) {
-                    pylint::rules::misplaced_comparison_constant(
-                        self,
-                        expr,
-                        left,
-                        ops,
-                        comparators,
-                    );
-                }
-
                 if self.settings.rules.enabled(&RuleCode::PLR0133) {
                     pylint::rules::constant_comparison(self, left, ops, comparators);
                 }

diff --git a/src/registry.rs b/src/registry.rs
@@ -80,7 +80,6 @@ ruff_macros::define_rule_mapping!(
     F901 => violations::RaiseNotImplemented,
     // pylint
     PLC0414 => violations::UselessImportAlias,
-    PLC2201 => violations::MisplacedComparisonConstant,
     PLC3002 => violations::UnnecessaryDirectLambdaCall,
     PLE0117 => violations::NonlocalWithoutBinding,
     PLE0118 => violations::UsedPriorGlobalDeclaration,

diff --git a/src/rules/flake8_simplify/rules/yoda_conditions.rs b/src/rules/flake8_simplify/rules/yoda_conditions.rs
@@ -14,17 +14,20 @@ pub fn yoda_conditions(
     ops: &[Cmpop],
     comparators: &[Expr],
 ) {
-    if !matches!(ops[..], [Cmpop::Eq]) {
+    let ([op], [right]) = (ops, comparators) else {
         return;
-    }
-    if comparators.len() != 1 {
+    };
+
+    if !matches!(
+        op,
+        Cmpop::Eq | Cmpop::NotEq | Cmpop::Lt | Cmpop::LtE | Cmpop::Gt | Cmpop::GtE,
+    ) {
         return;
     }
-    if !matches!(left.node, ExprKind::Constant { .. }) {
+    if !matches!(&left.node, &ExprKind::Constant { .. }) {
         return;
     }
-    let right = comparators.first().unwrap();
-    if matches!(right.node, ExprKind::Constant { .. }) {
+    if matches!(&right.node, &ExprKind::Constant { .. }) {
         return;
     }
 
@@ -36,16 +39,27 @@ pub fn yoda_conditions(
         .locator
         .slice_source_code_range(&Range::from_located(right));
 
+    // Reverse the operation.
+    let reversed_op = match op {
+        Cmpop::Eq => "==",
+        Cmpop::NotEq => "!=",
+        Cmpop::Lt => ">",
+        Cmpop::LtE => ">=",
+        Cmpop::Gt => "<",
+        Cmpop::GtE => "<=",
+        _ => unreachable!("Expected comparison operator"),
+    };
+
+    let suggestion = format!("{variable} {reversed_op} {constant}");
     let mut diagnostic = Diagnostic::new(
         violations::YodaConditions {
-            constant: constant.to_string(),
-            variable: variable.to_string(),
+            suggestion: suggestion.to_string(),
         },
         Range::from_located(expr),
     );
     if checker.patch(diagnostic.kind.code()) {
         diagnostic.amend(Fix::replacement(
-            format!("{variable} == {constant}"),
+            suggestion,
             left.location,
             right.end_location.unwrap(),
         ));

diff --git a/...ules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap b/...ules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap
@@ -4,8 +4,7 @@ expression: diagnostics
 ---
 - kind:
     YodaConditions:
-      variable: compare
-      constant: "\"yoda\""
+      suggestion: "compare == \"yoda\""
   location:
     row: 2
     column: 0
@@ -23,8 +22,7 @@ expression: diagnostics
   parent: ~
 - kind:
     YodaConditions:
-      variable: compare
-      constant: "'yoda'"
+      suggestion: "compare == 'yoda'"
   location:
     row: 3
     column: 0
@@ -42,8 +40,7 @@ expression: diagnostics
   parent: ~
 - kind:
     YodaConditions:
-      variable: age
-      constant: "42"
+      suggestion: age == 42
   location:
     row: 4
     column: 0
@@ -59,4 +56,58 @@ expression: diagnostics
       row: 4
       column: 9
   parent: ~
+- kind:
+    YodaConditions:
+      suggestion: "compare >= \"yoda\""
+  location:
+    row: 5
+    column: 0
+  end_location:
+    row: 5
+    column: 17
+  fix:
+    content: "compare >= \"yoda\""
+    location:
+      row: 5
+      column: 0
+    end_location:
+      row: 5
+      column: 17
+  parent: ~
+- kind:
+    YodaConditions:
+      suggestion: "compare > 'yoda'"
+  location:
+    row: 6
+    column: 0
+  end_location:
+    row: 6
+    column: 16
+  fix:
+    content: "compare > 'yoda'"
+    location:
+      row: 6
+      column: 0
+    end_location:
+      row: 6
+      column: 16
+  parent: ~
+- kind:
+    YodaConditions:
+      suggestion: age < 42
+  location:
+    row: 7
+    column: 0
+  end_location:
+    row: 7
+    column: 8
+  fix:
+    content: age < 42
+    location:
+      row: 7
+      column: 0
+    end_location:
+      row: 7
+      column: 8
+  parent: ~
 
diff --git a/src/rules/pylint/mod.rs b/src/rules/pylint/mod.rs
@@ -13,7 +13,6 @@ mod tests {
     use crate::settings::Settings;
 
     #[test_case(RuleCode::PLC0414, Path::new("import_aliasing.py"); "PLC0414")]
-    #[test_case(RuleCode::PLC2201, Path::new("misplaced_comparison_constant.py"); "PLC2201")]
     #[test_case(RuleCode::PLC3002, Path::new("unnecessary_direct_lambda_call.py"); "PLC3002")]
     #[test_case(RuleCode::PLE0117, Path::new("nonlocal_without_binding.py"); "PLE0117")]
     #[test_case(RuleCode::PLE0118, Path::new("used_prior_global_declaration.py"); "PLE0118")]

diff --git a/src/rules/pylint/rules/misplaced_comparison_constant.rs b/src/rules/pylint/rules/misplaced_comparison_constant.rs
diff --git a/src/rules/pylint/rules/mod.rs b/src/rules/pylint/rules/mod.rs
@@ -2,7 +2,6 @@ pub use await_outside_async::await_outside_async;
 pub use constant_comparison::constant_comparison;
 pub use magic_value_comparison::magic_value_comparison;
 pub use merge_isinstance::merge_isinstance;
-pub use misplaced_comparison_constant::misplaced_comparison_constant;
 pub use property_with_parameters::property_with_parameters;
 pub use unnecessary_direct_lambda_call::unnecessary_direct_lambda_call;
 pub use use_from_import::use_from_import;
@@ -15,7 +14,6 @@ mod await_outside_async;
 mod constant_comparison;
 mod magic_value_comparison;
 mod merge_isinstance;
-mod misplaced_comparison_constant;
 mod property_with_parameters;
 mod unnecessary_direct_lambda_call;
 mod use_from_import;