Implement misplaced-comparison-constant

README.md

@@ -767,6 +767,7 @@ For more, see [Pylint](https://pypi.org/project/pylint/2.15.7/) on PyPI.
 
 | Code | Name | Message | Fix |
 | ---- | ---- | ------- | --- |
+| PLC2201 | MisplacedComparisonConstant | Comparison should be ... |  |
 | PLC3002 | UnnecessaryDirectLambdaCall | Lambda expression called directly. Execute the expression inline instead. |  |
 | PLE1142 | AwaitOutsideAsync | `await` should be used within an async function |  |
 | PLR0206 | PropertyWithParameters | Cannot have defined parameters for properties |  |

resources/test/fixtures/pylint/misplaced_comparison_constant.py

@@ -0,0 +1,50 @@
+"""Check that the constants are on the right side of the comparisons"""
+
+# pylint: disable=singleton-comparison, missing-docstring, too-few-public-methods
+# pylint: disable=comparison-of-constants
+
+class MyClass:
+    def __init__(self):
+        self.attr = 1
+
+    def dummy_return(self):
+        return self.attr
+
+def dummy_return():
+    return 2
+
+def bad_comparisons():
+    """this is not ok"""
+    instance = MyClass()
+    for i in range(10):
+        if 5 <= i:  # [misplaced-comparison-constant]
+            pass
+        if 1 == i:  # [misplaced-comparison-constant]
+            pass
+        if 3 < dummy_return():  # [misplaced-comparison-constant]
+            pass
+        if 4 != instance.dummy_return():  # [misplaced-comparison-constant]
+            pass
+        if 1 == instance.attr:  # [misplaced-comparison-constant]
+            pass
+        if "aaa" == instance.attr: # [misplaced-comparison-constant]
+            pass
+
+def good_comparison():
+    """this is ok"""
+    for i in range(10):
+        if i == 5:
+            pass
+
+def double_comparison():
+    """Check that we return early for non-binary comparison"""
+    for i in range(10):
+        if i == 1 == 2:
+            pass
+        if 2 <= i <= 8:
+            print("Between 2 and 8 inclusive")
+
+def const_comparison():
+    """Check that we return early for comparison of two constants"""
+    if 1 == 2:
+        pass

src/check_ast.rs

@@ -2001,6 +2001,16 @@ where
                         .into_iter(),
                     );
                 }
+
+                if self.settings.enabled.contains(&CheckCode::PLC2201) {
+                    pylint::plugins::misplaced_comparison_constant(
+                        self,
+                        expr,
+                        left,
+                        ops,
+                        comparators,
+                    );
+                }
             }
             ExprKind::Constant {
                 value: Constant::Str(value),

src/checks.rs

@@ -91,6 +91,7 @@ pub enum CheckCode {
     F841,
     F901,
     // pylint
+    PLC2201,
     PLC3002,
     PLE1142,
     PLR0206,
@@ -575,6 +576,7 @@ pub enum CheckKind {
     YieldOutsideFunction(DeferralKeyword),
     // pylint
     ConsiderMergingIsinstance(String, Vec<String>),
+    MisplacedComparisonConstant(String),
     UnnecessaryDirectLambdaCall,
     PropertyWithParameters,
     ConsiderUsingFromImport(String, String),
@@ -872,6 +874,7 @@ impl CheckCode {
             CheckCode::F841 => CheckKind::UnusedVariable("...".to_string()),
             CheckCode::F901 => CheckKind::RaiseNotImplemented,
             // pylint
+            CheckCode::PLC2201 => CheckKind::MisplacedComparisonConstant("...".to_string()),
             CheckCode::PLC3002 => CheckKind::UnnecessaryDirectLambdaCall,
             CheckCode::PLE1142 => CheckKind::AwaitOutsideAsync,
             CheckCode::PLR0402 => {
@@ -1301,6 +1304,7 @@ impl CheckCode {
             CheckCode::N817 => CheckCategory::PEP8Naming,
             CheckCode::N818 => CheckCategory::PEP8Naming,
             CheckCode::PGH001 => CheckCategory::PygrepHooks,
+            CheckCode::PLC2201 => CheckCategory::Pylint,
             CheckCode::PLC3002 => CheckCategory::Pylint,
             CheckCode::PLE1142 => CheckCategory::Pylint,
             CheckCode::PLR0206 => CheckCategory::Pylint,
@@ -1426,11 +1430,12 @@ impl CheckKind {
             CheckKind::NoNewLineAtEndOfFile => &CheckCode::W292,
             CheckKind::InvalidEscapeSequence(_) => &CheckCode::W605,
             // pylint
+            CheckKind::MisplacedComparisonConstant(..) => &CheckCode::PLC2201,
+            CheckKind::UnnecessaryDirectLambdaCall => &CheckCode::PLC3002,
             CheckKind::AwaitOutsideAsync => &CheckCode::PLE1142,
             CheckKind::ConsiderMergingIsinstance(..) => &CheckCode::PLR1701,
             CheckKind::PropertyWithParameters => &CheckCode::PLR0206,
             CheckKind::ConsiderUsingFromImport(..) => &CheckCode::PLR0402,
-            CheckKind::UnnecessaryDirectLambdaCall => &CheckCode::PLC3002,
             // flake8-builtins
             CheckKind::BuiltinVariableShadowing(_) => &CheckCode::A001,
             CheckKind::BuiltinArgumentShadowing(_) => &CheckCode::A002,
@@ -1816,6 +1821,9 @@ impl CheckKind {
                 let types = types.join(", ");
                 format!("Consider merging these isinstance calls: `isinstance({obj}, ({types}))`")
             }
+            CheckKind::MisplacedComparisonConstant(comprison) => {
+                format!("Comparison should be {comprison}")
+            }
             CheckKind::UnnecessaryDirectLambdaCall => "Lambda expression called directly. Execute \
                                                        the expression inline instead."
                 .to_string(),

src/checks_gen.rs

@@ -283,6 +283,10 @@ pub enum CheckCodePrefix {
     PGH00,
     PGH001,
     PLC,
+    PLC2,
+    PLC22,
+    PLC220,
+    PLC2201,
     PLC3,
     PLC30,
     PLC300,
@@ -1184,7 +1188,11 @@ impl CheckCodePrefix {
             CheckCodePrefix::PGH0 => vec![CheckCode::PGH001],
             CheckCodePrefix::PGH00 => vec![CheckCode::PGH001],
             CheckCodePrefix::PGH001 => vec![CheckCode::PGH001],
-            CheckCodePrefix::PLC => vec![CheckCode::PLC3002],
+            CheckCodePrefix::PLC => vec![CheckCode::PLC2201, CheckCode::PLC3002],
+            CheckCodePrefix::PLC2 => vec![CheckCode::PLC2201],
+            CheckCodePrefix::PLC22 => vec![CheckCode::PLC2201],
+            CheckCodePrefix::PLC220 => vec![CheckCode::PLC2201],
+            CheckCodePrefix::PLC2201 => vec![CheckCode::PLC2201],
             CheckCodePrefix::PLC3 => vec![CheckCode::PLC3002],
             CheckCodePrefix::PLC30 => vec![CheckCode::PLC3002],
             CheckCodePrefix::PLC300 => vec![CheckCode::PLC3002],
@@ -1713,6 +1721,10 @@ impl CheckCodePrefix {
             CheckCodePrefix::PGH00 => SuffixLength::Two,
             CheckCodePrefix::PGH001 => SuffixLength::Three,
             CheckCodePrefix::PLC => SuffixLength::Zero,
+            CheckCodePrefix::PLC2 => SuffixLength::One,
+            CheckCodePrefix::PLC22 => SuffixLength::Two,
+            CheckCodePrefix::PLC220 => SuffixLength::Three,
+            CheckCodePrefix::PLC2201 => SuffixLength::Four,
             CheckCodePrefix::PLC3 => SuffixLength::One,
             CheckCodePrefix::PLC30 => SuffixLength::Two,
             CheckCodePrefix::PLC300 => SuffixLength::Three,

src/pylint/mod.rs

@@ -11,6 +11,7 @@ mod tests {
     use crate::linter::test_path;
     use crate::Settings;
 
+    #[test_case(CheckCode::PLC2201, Path::new("misplaced_comparison_constant.py"); "PLC2201")]
     #[test_case(CheckCode::PLC3002, Path::new("unnecessary_direct_lambda_call.py"); "PLC3002")]
     #[test_case(CheckCode::PLE1142, Path::new("await_outside_async.py"); "PLE1142")]
     #[test_case(CheckCode::PLR0206, Path::new("property_with_parameters.py"); "PLR0206")]

src/pylint/plugins.rs

@@ -1,12 +1,54 @@
 use itertools::Itertools;
 use rustc_hash::{FxHashMap, FxHashSet};
-use rustpython_ast::{Alias, Arguments, Boolop, Expr, ExprKind, Stmt};
+use rustpython_ast::{Alias, Arguments, Boolop, Cmpop, Expr, ExprKind, Stmt};
 
 use crate::ast::types::{FunctionScope, Range, ScopeKind};
+use crate::autofix::Fix;
 use crate::check_ast::Checker;
 use crate::checks::CheckKind;
 use crate::Check;
 
+/// PLC2201
+pub fn misplaced_comparison_constant(
+    checker: &mut Checker,
+    expr: &Expr,
+    left: &Expr,
+    ops: &[Cmpop],
+    comparators: &[Expr],
+) {
+    if let ([op], [right]) = (ops, comparators) {
+        if matches!(
+            op,
+            Cmpop::Eq | Cmpop::NotEq | Cmpop::Lt | Cmpop::LtE | Cmpop::Gt | Cmpop::GtE,
+        ) && matches!(&left.node, &ExprKind::Constant { .. })
+            && !matches!(&right.node, &ExprKind::Constant { .. })
+        {
+            let reversed_op = match op {
+                Cmpop::Eq => "==",
+                Cmpop::NotEq => "!=",
+                Cmpop::Lt => ">",
+                Cmpop::LtE => ">=",
+                Cmpop::Gt => "<",
+                Cmpop::GtE => "<=",
+                _ => unreachable!("Expected comparison operator"),
+            };
+            let suggestion = format!("{right} {reversed_op} {left}");
+            let mut check = Check::new(
+                CheckKind::MisplacedComparisonConstant(suggestion.clone()),
+                Range::from_located(expr),
+            );
+            if checker.patch(check.kind.code()) {
+                check.amend(Fix::replacement(
+                    suggestion,
+                    expr.location,
+                    expr.end_location.unwrap(),
+                ));
+            }
+            checker.add_check(check);
+        }
+    }
+}
+
 /// PLC3002
 pub fn unnecessary_direct_lambda_call(checker: &mut Checker, expr: &Expr, func: &Expr) {
     if let ExprKind::Lambda { .. } = &func.node {

src/pylint/snapshots/ruff__pylint__tests__misplaced_comparison_constant.py.snap

@@ -0,0 +1,101 @@
+---
+source: src/pylint/mod.rs
+expression: checks
+---
+- kind:
+    MisplacedComparisonConstant: i >= 5
+  location:
+    row: 20
+    column: 11
+  end_location:
+    row: 20
+    column: 17
+  fix:
+    content: i >= 5
+    location:
+      row: 20
+      column: 11
+    end_location:
+      row: 20
+      column: 17
+- kind:
+    MisplacedComparisonConstant: i == 1
+  location:
+    row: 22
+    column: 11
+  end_location:
+    row: 22
+    column: 17
+  fix:
+    content: i == 1
+    location:
+      row: 22
+      column: 11
+    end_location:
+      row: 22
+      column: 17
+- kind:
+    MisplacedComparisonConstant: dummy_return() > 3
+  location:
+    row: 24
+    column: 11
+  end_location:
+    row: 24
+    column: 29
+  fix:
+    content: dummy_return() > 3
+    location:
+      row: 24
+      column: 11
+    end_location:
+      row: 24
+      column: 29
+- kind:
+    MisplacedComparisonConstant: instance.dummy_return() != 4
+  location:
+    row: 26
+    column: 11
+  end_location:
+    row: 26
+    column: 39
+  fix:
+    content: instance.dummy_return() != 4
+    location:
+      row: 26
+      column: 11
+    end_location:
+      row: 26
+      column: 39
+- kind:
+    MisplacedComparisonConstant: instance.attr == 1
+  location:
+    row: 28
+    column: 11
+  end_location:
+    row: 28
+    column: 29
+  fix:
+    content: instance.attr == 1
+    location:
+      row: 28
+      column: 11
+    end_location:
+      row: 28
+      column: 29
+- kind:
+    MisplacedComparisonConstant: "instance.attr == 'aaa'"
+  location:
+    row: 30
+    column: 11
+  end_location:
+    row: 30
+    column: 33
+  fix:
+    content: "instance.attr == 'aaa'"
+    location:
+      row: 30
+      column: 11
+    end_location:
+      row: 30
+      column: 33
+