Replace misplaced-comparison-constant with SIM300 (#1980)

Closes: #1954.

BREAKING_CHANGES.md

@@ -1,5 +1,12 @@
 # Breaking Changes
 
+## 0.0.226
+
+### `misplaced-comparison-constant` (`PLC2201`) was deprecated in favor of `SIM300` ([#1980](https://github.com/charliermarsh/ruff/pull/1980))
+
+These two rules contain (nearly) identical logic. To deduplicate the rule set, we've upgraded
+`SIM300` to handle a few more cases, and deprecated `PLC2201` 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))

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)

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"

ruff.schema.json

@@ -1484,10 +1484,6 @@
  "PLC04",
  "PLC041",
  "PLC0414",
- "PLC2",
- "PLC22",
- "PLC220",
- "PLC2201",
  "PLC3",
  "PLC30",
  "PLC300",

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

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,

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

src/rules/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: ~
 

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")]

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;