Treat unary operations on constants as constant-like (#3348)

crates/ruff/resources/test/fixtures/flake8_simplify/SIM300.py

@@ -6,6 +6,8 @@
 "yoda" <= compare  # SIM300
 "yoda" < compare  # SIM300
 42 > age  # SIM300
+-42 > age  # SIM300
++42 > age  # SIM300
 YODA == age  # SIM300
 YODA > age  # SIM300
 YODA >= age  # SIM300

crates/ruff/resources/test/fixtures/pylint/magic_value_comparison.py

@@ -11,6 +11,9 @@ if 10 == 100:  # [comparison-of-constants] R0133
 if 1 == 3:  # [comparison-of-constants] R0133
     pass
 
+if 1 == -3:  # [comparison-of-constants] R0133
+    pass
+
 x = 0
 if 4 == 3 == x:  # [comparison-of-constants] R0133
     pass
@@ -35,6 +38,12 @@ if argc != 1:  # correct
 if argc != 2:  # [magic-value-comparison]
     pass
 
+if argc != -2:  # [magic-value-comparison]
+    pass
+
+if argc != +2:  # [magic-value-comparison]
+    pass
+
 if __name__ == "__main__":  # correct
     pass
 

crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs

@@ -2,7 +2,7 @@ use anyhow::Result;
 use libcst_native::{Codegen, CodegenState, CompOp};
 use ruff_macros::{define_violation, derive_message_formats};
 use ruff_python::str::{self};
-use rustpython_parser::ast::{Cmpop, Expr, ExprKind};
+use rustpython_parser::ast::{Cmpop, Expr, ExprKind, Unaryop};
 
 use crate::ast::types::Range;
 use crate::checkers::ast::Checker;
@@ -50,6 +50,10 @@ fn is_constant_like(expr: &Expr) -> bool {
         ExprKind::Constant { .. } => true,
         ExprKind::Tuple { elts, .. } => elts.iter().all(is_constant_like),
         ExprKind::Name { id, .. } => str::is_upper(id),
+        ExprKind::UnaryOp {
+            op: Unaryop::UAdd | Unaryop::USub | Unaryop::Invert,
+            operand,
+        } => matches!(operand.node, ExprKind::Constant { .. }),
         _ => false,
     }
 }

crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap

@@ -130,110 +130,146 @@ expression: diagnostics
   parent: ~
 - kind:
     YodaConditions:
-      suggestion: age == YODA
+      suggestion: age < -42
   location:
     row: 9
     column: 0
   end_location:
     row: 9
+    column: 9
+  fix:
+    content: age < -42
+    location:
+      row: 9
+      column: 0
+    end_location:
+      row: 9
+      column: 9
+  parent: ~
+- kind:
+    YodaConditions:
+      suggestion: age < +42
+  location:
+    row: 10
+    column: 0
+  end_location:
+    row: 10
+    column: 9
+  fix:
+    content: age < +42
+    location:
+      row: 10
+      column: 0
+    end_location:
+      row: 10
+      column: 9
+  parent: ~
+- kind:
+    YodaConditions:
+      suggestion: age == YODA
+  location:
+    row: 11
+    column: 0
+  end_location:
+    row: 11
     column: 11
   fix:
     content: age == YODA
     location:
-      row: 9
+      row: 11
       column: 0
     end_location:
-      row: 9
+      row: 11
       column: 11
   parent: ~
 - kind:
     YodaConditions:
       suggestion: age < YODA
   location:
-    row: 10
+    row: 12
     column: 0
   end_location:
-    row: 10
+    row: 12
     column: 10
   fix:
     content: age < YODA
     location:
-      row: 10
+      row: 12
       column: 0
     end_location:
-      row: 10
+      row: 12
       column: 10
   parent: ~
 - kind:
     YodaConditions:
       suggestion: age <= YODA
   location:
-    row: 11
+    row: 13
     column: 0
   end_location:
-    row: 11
+    row: 13
     column: 11
   fix:
     content: age <= YODA
     location:
-      row: 11
+      row: 13
       column: 0
     end_location:
-      row: 11
+      row: 13
       column: 11
   parent: ~
 - kind:
     YodaConditions:
       suggestion: age == JediOrder.YODA
   location:
-    row: 12
+    row: 14
     column: 0
   end_location:
-    row: 12
+    row: 14
     column: 21
   fix:
     content: age == JediOrder.YODA
     location:
-      row: 12
+      row: 14
       column: 0
     end_location:
-      row: 12
+      row: 14
       column: 21
   parent: ~
 - kind:
     YodaConditions:
       suggestion: (number - 100) > 0
   location:
-    row: 13
+    row: 15
     column: 0
   end_location:
-    row: 13
+    row: 15
     column: 18
   fix:
     content: (number - 100) > 0
     location:
-      row: 13
+      row: 15
       column: 0
     end_location:
-      row: 13
+      row: 15
       column: 18
   parent: ~
 - kind:
     YodaConditions:
       suggestion: (60 * 60) < SomeClass().settings.SOME_CONSTANT_VALUE
   location:
-    row: 14
+    row: 16
     column: 0
   end_location:
-    row: 14
+    row: 16
     column: 52
   fix:
     content: (60 * 60) < SomeClass().settings.SOME_CONSTANT_VALUE
     location:
-      row: 14
+      row: 16
       column: 0
     end_location:
-      row: 14
+      row: 16
       column: 52
   parent: ~
 

crates/ruff/src/rules/pylint/rules/magic_value_comparison.rs

@@ -1,8 +1,9 @@
 use itertools::Itertools;
-use ruff_macros::{define_violation, derive_message_formats};
-use rustpython_parser::ast::{Constant, Expr, ExprKind};
+use rustpython_parser::ast::{Constant, Expr, ExprKind, Unaryop};
 
-use crate::ast::helpers::unparse_constant;
+use ruff_macros::{define_violation, derive_message_formats};
+
+use crate::ast::helpers::unparse_expr;
 use crate::ast::types::Range;
 use crate::checkers::ast::Checker;
 use crate::registry::Diagnostic;
@@ -24,6 +25,22 @@ impl Violation for MagicValueComparison {
     }
 }
 
+/// If an [`Expr`] is a constant (or unary operation on a constant), return the [`Constant`].
+fn as_constant(expr: &Expr) -> Option<&Constant> {
+    match &expr.node {
+        ExprKind::Constant { value, .. } => Some(value),
+        ExprKind::UnaryOp {
+            op: Unaryop::UAdd | Unaryop::USub | Unaryop::Invert,
+            operand,
+        } => match &operand.node {
+            ExprKind::Constant { value, .. } => Some(value),
+            _ => None,
+        },
+        _ => None,
+    }
+}
+
+/// Return `true` if a [`Constant`] is a magic value.
 fn is_magic_value(constant: &Constant, allowed_types: &[ConstantType]) -> bool {
     if let Ok(constant_type) = ConstantType::try_from(constant) {
         if allowed_types.contains(&constant_type) {
@@ -37,7 +54,7 @@ fn is_magic_value(constant: &Constant, allowed_types: &[ConstantType]) -> bool {
         Constant::Ellipsis => false,
         // Otherwise, special-case some common string and integer types.
         Constant::Str(value) => !matches!(value.as_str(), "" | "__main__"),
-        Constant::Int(value) => !matches!(value.try_into(), Ok(-1 | 0 | 1)),
+        Constant::Int(value) => !matches!(value.try_into(), Ok(0 | 1)),
         Constant::Bytes(_) => true,
         Constant::Tuple(_) => true,
         Constant::Float(_) => true,
@@ -53,19 +70,17 @@ pub fn magic_value_comparison(checker: &mut Checker, left: &Expr, comparators: &
     {
         // If both of the comparators are constant, skip rule for the whole expression.
         // R0133: comparison-of-constants
-        if matches!(left.node, ExprKind::Constant { .. })
-            && matches!(right.node, ExprKind::Constant { .. })
-        {
+        if as_constant(left).is_some() && as_constant(right).is_some() {
             return;
         }
     }
 
     for comparison_expr in std::iter::once(left).chain(comparators.iter()) {
-        if let ExprKind::Constant { value, .. } = &comparison_expr.node {
+        if let Some(value) = as_constant(comparison_expr) {
             if is_magic_value(value, &checker.settings.pylint.allow_magic_value_types) {
                 checker.diagnostics.push(Diagnostic::new(
                     MagicValueComparison {
-                        value: unparse_constant(value, checker.stylist),
+                        value: unparse_expr(comparison_expr, checker.stylist),
                     },
                     Range::from_located(comparison_expr),
                 ));

crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR2004_magic_value_comparison.py.snap

@@ -1,5 +1,5 @@
 ---
-source: src/rules/pylint/mod.rs
+source: crates/ruff/src/rules/pylint/mod.rs
 expression: diagnostics
 ---
 - kind:
@@ -17,21 +17,43 @@ expression: diagnostics
     MagicValueComparison:
       value: "2"
   location:
-    row: 35
+    row: 38
     column: 11
   end_location:
-    row: 35
+    row: 38
     column: 12
   fix: ~
   parent: ~
+- kind:
+    MagicValueComparison:
+      value: "-2"
+  location:
+    row: 41
+    column: 11
+  end_location:
+    row: 41
+    column: 13
+  fix: ~
+  parent: ~
+- kind:
+    MagicValueComparison:
+      value: "+2"
+  location:
+    row: 44
+    column: 11
+  end_location:
+    row: 44
+    column: 13
+  fix: ~
+  parent: ~
 - kind:
     MagicValueComparison:
       value: "3.141592653589793"
   location:
-    row: 56
+    row: 65
     column: 20
   end_location:
-    row: 56
+    row: 65
     column: 40
   fix: ~
   parent: ~

crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__allow_magic_value_types.snap

@@ -6,10 +6,10 @@ expression: diagnostics
     MagicValueComparison:
       value: "\"Hunter2\""
   location:
-    row: 50
+    row: 59
     column: 21
   end_location:
-    row: 50
+    row: 59
     column: 30
   fix: ~
   parent: ~
@@ -17,10 +17,10 @@ expression: diagnostics
     MagicValueComparison:
       value: "3.141592653589793"
   location:
-    row: 56
+    row: 65
     column: 20
   end_location:
-    row: 56
+    row: 65
     column: 40
   fix: ~
   parent: ~
@@ -28,10 +28,10 @@ expression: diagnostics
     MagicValueComparison:
       value: "b\"something\""
   location:
-    row: 65
+    row: 74
     column: 17
   end_location:
-    row: 65
+    row: 74
     column: 29
   fix: ~
   parent: ~