Implement PLR0133 (ComparisonOfConstants) (#1841)

This PR adds [Pylint
`R0133`](https://pylint.pycqa.org/en/latest/user_guide/messages/refactor/comparison-of-constants.html)

Feel free to suggest changes and additions, I have tried to maintain
parity with the Pylint implementation
[`comparison_checker.py`](https://github.com/PyCQA/pylint/blob/main/pylint/checkers/base/comparison_checker.py#L247)

See #970

README.md

@@ -1094,6 +1094,7 @@ For more, see [Pylint](https://pypi.org/project/pylint/2.15.7/) on PyPI.
 | PLE1142 | AwaitOutsideAsync | `await` should be used within an async function |  |
 | PLR0206 | PropertyWithParameters | Cannot have defined parameters for properties |  |
 | PLR0402 | ConsiderUsingFromImport | Use `from ... import ...` in lieu of alias |  |
+| PLR0133 | ConstantComparison | Two constants compared in a comparison, consider replacing `0 == 0` |  |
 | PLR1701 | ConsiderMergingIsinstance | Merge these isinstance calls: `isinstance(..., (...))` |  |
 | PLR1722 | UseSysExit | Use `sys.exit()` instead of `exit` | 🛠 |
 | PLR2004 | MagicValueComparison | Magic number used in comparison, consider replacing magic with a constant variable |  |

resources/test/fixtures/pylint/constant_comparison.py

@@ -0,0 +1,59 @@
+"""Check that magic values are not used in comparisons"""
+
+if 100 == 100: # [comparison-of-constants]
+    pass
+
+if 1 == 3: # [comparison-of-constants]
+    pass
+
+if 1 != 3: # [comparison-of-constants]
+    pass
+
+x = 0
+if 4 == 3 == x: # [comparison-of-constants]
+    pass
+
+if x == 0: # correct
+    pass
+
+y = 1
+if x == y: # correct
+    pass
+
+if 1 > 0: # [comparison-of-constants]
+    pass
+
+if x > 0: # correct
+    pass
+
+if 1 >= 0: # [comparison-of-constants]
+    pass
+
+if x >= 0: # correct
+    pass
+
+if 1 < 0: # [comparison-of-constants]
+    pass
+
+if x < 0: # correct
+    pass
+
+if 1 <= 0: # [comparison-of-constants]
+    pass
+
+if x <= 0: # correct
+    pass
+
+word = "hello"
+if word == "": # correct
+    pass
+
+if "hello" == "": # [comparison-of-constants]
+    pass
+
+truthy = True
+if truthy == True: # correct
+    pass
+
+if True == False: # [comparison-of-constants]
+    pass

ruff.schema.json

@@ -1443,6 +1443,9 @@
         "PLE1142",
         "PLR",
         "PLR0",
+        "PLR01",
+        "PLR013",
+        "PLR0133",
         "PLR02",
         "PLR020",
         "PLR0206",

src/checkers/ast.rs

@@ -2590,6 +2590,10 @@ where
                     );
                 }
 
+                if self.settings.enabled.contains(&RuleCode::PLR0133) {
+                    pylint::rules::constant_comparison(self, left, ops, comparators);
+                }
+
                 if self.settings.enabled.contains(&RuleCode::PLR2004) {
                     pylint::rules::magic_value_comparison(self, left, comparators);
                 }

src/pylint/mod.rs

@@ -17,6 +17,7 @@ mod tests {
     #[test_case(RuleCode::PLE0117, Path::new("nonlocal_without_binding.py"); "PLE0117")]
     #[test_case(RuleCode::PLE0118, Path::new("used_prior_global_declaration.py"); "PLE0118")]
     #[test_case(RuleCode::PLE1142, Path::new("await_outside_async.py"); "PLE1142")]
+    #[test_case(RuleCode::PLR0133, Path::new("constant_comparison.py"); "PLR0133")]
     #[test_case(RuleCode::PLR0206, Path::new("property_with_parameters.py"); "PLR0206")]
     #[test_case(RuleCode::PLR0402, Path::new("import_aliasing.py"); "PLR0402")]
     #[test_case(RuleCode::PLR1701, Path::new("consider_merging_isinstance.py"); "PLR1701")]

src/pylint/rules/constant_comparison.rs

@@ -0,0 +1,44 @@
+use itertools::Itertools;
+use rustpython_ast::{Cmpop, Expr, ExprKind, Located};
+
+use crate::ast::types::Range;
+use crate::checkers::ast::Checker;
+use crate::registry::Diagnostic;
+use crate::violations;
+
+/// PLR0133
+pub fn constant_comparison(
+    checker: &mut Checker,
+    left: &Expr,
+    ops: &[Cmpop],
+    comparators: &[Expr],
+) {
+    for ((left, right), op) in std::iter::once(left)
+        .chain(comparators.iter())
+        .tuple_windows::<(&Located<_>, &Located<_>)>()
+        .zip(ops)
+    {
+        if let (
+            ExprKind::Constant {
+                value: left_constant,
+                ..
+            },
+            ExprKind::Constant {
+                value: right_constant,
+                ..
+            },
+        ) = (&left.node, &right.node)
+        {
+            let diagnostic = Diagnostic::new(
+                violations::ConstantComparison {
+                    left_constant: left_constant.to_string(),
+                    op: op.into(),
+                    right_constant: right_constant.to_string(),
+                },
+                Range::from_located(left),
+            );
+
+            checker.diagnostics.push(diagnostic);
+        };
+    }
+}

src/pylint/rules/mod.rs

@@ -1,4 +1,5 @@
 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;
@@ -11,6 +12,7 @@ pub use useless_else_on_loop::useless_else_on_loop;
 pub use useless_import_alias::useless_import_alias;
 
 mod await_outside_async;
+mod constant_comparison;
 mod magic_value_comparison;
 mod merge_isinstance;
 mod misplaced_comparison_constant;

src/pylint/snapshots/ruff__pylint__tests__PLR0133_constant_comparison.py.snap

@@ -0,0 +1,135 @@
+---
+source: src/pylint/mod.rs
+expression: diagnostics
+---
+- kind:
+    ConstantComparison:
+      left_constant: "100"
+      op: Eq
+      right_constant: "100"
+  location:
+    row: 3
+    column: 3
+  end_location:
+    row: 3
+    column: 6
+  fix: ~
+  parent: ~
+- kind:
+    ConstantComparison:
+      left_constant: "1"
+      op: Eq
+      right_constant: "3"
+  location:
+    row: 6
+    column: 3
+  end_location:
+    row: 6
+    column: 4
+  fix: ~
+  parent: ~
+- kind:
+    ConstantComparison:
+      left_constant: "1"
+      op: NotEq
+      right_constant: "3"
+  location:
+    row: 9
+    column: 3
+  end_location:
+    row: 9
+    column: 4
+  fix: ~
+  parent: ~
+- kind:
+    ConstantComparison:
+      left_constant: "4"
+      op: Eq
+      right_constant: "3"
+  location:
+    row: 13
+    column: 3
+  end_location:
+    row: 13
+    column: 4
+  fix: ~
+  parent: ~
+- kind:
+    ConstantComparison:
+      left_constant: "1"
+      op: Gt
+      right_constant: "0"
+  location:
+    row: 23
+    column: 3
+  end_location:
+    row: 23
+    column: 4
+  fix: ~
+  parent: ~
+- kind:
+    ConstantComparison:
+      left_constant: "1"
+      op: GtE
+      right_constant: "0"
+  location:
+    row: 29
+    column: 3
+  end_location:
+    row: 29
+    column: 4
+  fix: ~
+  parent: ~
+- kind:
+    ConstantComparison:
+      left_constant: "1"
+      op: Lt
+      right_constant: "0"
+  location:
+    row: 35
+    column: 3
+  end_location:
+    row: 35
+    column: 4
+  fix: ~
+  parent: ~
+- kind:
+    ConstantComparison:
+      left_constant: "1"
+      op: LtE
+      right_constant: "0"
+  location:
+    row: 41
+    column: 3
+  end_location:
+    row: 41
+    column: 4
+  fix: ~
+  parent: ~
+- kind:
+    ConstantComparison:
+      left_constant: "'hello'"
+      op: Eq
+      right_constant: "''"
+  location:
+    row: 51
+    column: 3
+  end_location:
+    row: 51
+    column: 10
+  fix: ~
+  parent: ~
+- kind:
+    ConstantComparison:
+      left_constant: "True"
+      op: Eq
+      right_constant: "False"
+  location:
+    row: 58
+    column: 3
+  end_location:
+    row: 58
+    column: 7
+  fix: ~
+  parent: ~
+

src/registry.rs

@@ -187,6 +187,7 @@ define_rule_mapping!(
     PLE1142 => violations::AwaitOutsideAsync,
     PLR0206 => violations::PropertyWithParameters,
     PLR0402 => violations::ConsiderUsingFromImport,
+    PLR0133 => violations::ConstantComparison,
     PLR1701 => violations::ConsiderMergingIsinstance,
     PLR1722 => violations::UseSysExit,
     PLR2004 => violations::MagicValueComparison,

src/violations.rs

@@ -1157,6 +1157,85 @@ impl Violation for ConsiderUsingFromImport {
     }
 }
 
+#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]
+pub enum ViolationsCmpop {
+    Eq,
+    NotEq,
+    Lt,
+    LtE,
+    Gt,
+    GtE,
+    Is,
+    IsNot,
+    In,
+    NotIn,
+}
+
+impl From<&Cmpop> for ViolationsCmpop {
+    fn from(cmpop: &Cmpop) -> Self {
+        match cmpop {
+            Cmpop::Eq => Self::Eq,
+            Cmpop::NotEq => Self::NotEq,
+            Cmpop::Lt => Self::Lt,
+            Cmpop::LtE => Self::LtE,
+            Cmpop::Gt => Self::Gt,
+            Cmpop::GtE => Self::GtE,
+            Cmpop::Is => Self::Is,
+            Cmpop::IsNot => Self::IsNot,
+            Cmpop::In => Self::In,
+            Cmpop::NotIn => Self::NotIn,
+        }
+    }
+}
+
+impl fmt::Display for ViolationsCmpop {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        let representation = match self {
+            Self::Eq => "==",
+            Self::NotEq => "!=",
+            Self::Lt => "<",
+            Self::LtE => "<=",
+            Self::Gt => ">",
+            Self::GtE => ">=",
+            Self::Is => "is",
+            Self::IsNot => "is not",
+            Self::In => "in",
+            Self::NotIn => "not in",
+        };
+        write!(f, "{representation}")
+    }
+}
+
+define_violation!(
+    pub struct ConstantComparison {
+        pub left_constant: String,
+        pub op: ViolationsCmpop,
+        pub right_constant: String,
+    }
+);
+impl Violation for ConstantComparison {
+    fn message(&self) -> String {
+        let ConstantComparison {
+            left_constant,
+            op,
+            right_constant,
+        } = self;
+
+        format!(
+            "Two constants compared in a comparison, consider replacing `{left_constant} {op} \
+             {right_constant}`"
+        )
+    }
+
+    fn placeholder() -> Self {
+        ConstantComparison {
+            left_constant: "0".to_string(),
+            op: ViolationsCmpop::Eq,
+            right_constant: "0".to_string(),
+        }
+    }
+}
+
 define_violation!(
     pub struct ConsiderMergingIsinstance(pub String, pub Vec<String>);
 );