[pylint] - add fix for unary expressions in PLC2801

astral-sh · Jan 20, 2024 · 5a85f1d · 5a85f1d
1 parent b3a6f0c
commit 5a85f1d
Show file tree

Hide file tree

Showing 3 changed files with 205 additions and 12 deletions.
diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_dunder_call.py b/crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_dunder_call.py
@@ -1,6 +1,6 @@
 from typing import Any
 
-
+a = 2
 print((3.0).__add__(4.0))  # PLC2801
 print((3.0).__sub__(4.0))  # PLC2801
 print((3.0).__mul__(4.0))  # PLC2801
@@ -17,6 +17,13 @@
 print((3.0).__repr__())  # PLC2801
 print([1, 2, 3].__len__())  # PLC2801
 print((1).__neg__())  # PLC2801
+print(-a.__sub__(1))  # PLC2801
+print(-(a).__sub__(1))  # PLC2801
+print(-(-a.__sub__(1)))  # PLC2801
+print((5 - a).__sub__(1))  # PLC2801
+print(-(5 - a).__sub__(1))  # PLC2801
+print(-(-5 - a).__sub__(1))  # PLC2801
+print(+-+-+-a.__sub__(1))  # PLC2801
 
 
 class Thing:

diff --git a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_dunder_call.rs b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_dunder_call.rs
@@ -2,6 +2,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::{self as ast, Expr, Stmt};
 use ruff_python_semantic::SemanticModel;
+use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
 use ruff_text_size::Ranged;
 
 use crate::checkers::ast::Checker;
@@ -156,8 +157,44 @@ pub(crate) fn unnecessary_dunder_call(checker: &mut Checker, call: &ast::ExprCal
         call.range(),
     );
 
-    if let Some(fixed) = fixed {
-        diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(fixed, call.range())));
+    if let Some(mut fixed) = fixed {
+        let is_in_unary = checker
+            .semantic()
+            .current_expression_parent()
+            .is_some_and(|parent| matches!(parent, Expr::UnaryOp(ast::ExprUnaryOp { .. })));
+
+        // find the first ")" before our dunder method
+        let rparen = SimpleTokenizer::starts_at(value.as_ref().end(), checker.locator().contents())
+            .find(|token| token.kind == SimpleTokenKind::RParen);
+
+        // find the "." before our dunder method
+        let dot = SimpleTokenizer::starts_at(value.as_ref().end(), checker.locator().contents())
+            .find(|token| token.kind == SimpleTokenKind::Dot)
+            .unwrap();
+
+        if is_in_unary {
+            if rparen.is_some() {
+                // if we're within parentheses with a unary, we're going to take
+                // the value operand, and insert the fix in its place within its
+                // existing parentheses.
+                // for example, `-(-a).__sub__(1)` -> `-(-a - 1)`
+
+                diagnostic.set_fix(Fix::safe_edits(
+                    Edit::range_replacement(fixed, value.as_ref().range()),
+                    [Edit::deletion(dot.start(), call.end())],
+                ));
+            } else {
+                // `-a.__sub__(1)` -> `-(a - 1)`
+                fixed = format!("({fixed})");
+                diagnostic.set_fix(Fix::safe_edits(
+                    Edit::range_replacement(fixed, value.as_ref().range()),
+                    [Edit::deletion(dot.start(), call.end())],
+                ));
+            }
+        } else {
+            // `(3).__add__(1)` -> `3 + 1`
+            diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(fixed, call.range())));
+        }
     };
 
     checker.diagnostics.push(diagnostic);

diff --git a/...lint/snapshots/ruff_linter__rules__pylint__tests__PLC2801_unnecessary_dunder_call.py.snap b/...lint/snapshots/ruff_linter__rules__pylint__tests__PLC2801_unnecessary_dunder_call.py.snap
@@ -3,6 +3,7 @@ source: crates/ruff_linter/src/rules/pylint/mod.rs
 ---
 unnecessary_dunder_call.py:4:7: PLC2801 [*] Unnecessary dunder call to `__add__`. Use `+` operator.
   |
+3 | a = 2
 4 | print((3.0).__add__(4.0))  # PLC2801
   |       ^^^^^^^^^^^^^^^^^^ PLC2801
 5 | print((3.0).__sub__(4.0))  # PLC2801
@@ -13,7 +14,7 @@ unnecessary_dunder_call.py:4:7: PLC2801 [*] Unnecessary dunder call to `__add__`
 ℹ Safe fix
 1 1 | from typing import Any
 2 2 | 
-3 3 | 
+3 3 | a = 2
 4   |-print((3.0).__add__(4.0))  # PLC2801
   4 |+print(3.0 + 4.0)  # PLC2801
 5 5 | print((3.0).__sub__(4.0))  # PLC2801
@@ -22,6 +23,7 @@ unnecessary_dunder_call.py:4:7: PLC2801 [*] Unnecessary dunder call to `__add__`
 
 unnecessary_dunder_call.py:5:7: PLC2801 [*] Unnecessary dunder call to `__sub__`. Use `-` operator.
   |
+3 | a = 2
 4 | print((3.0).__add__(4.0))  # PLC2801
 5 | print((3.0).__sub__(4.0))  # PLC2801
   |       ^^^^^^^^^^^^^^^^^^ PLC2801
@@ -32,7 +34,7 @@ unnecessary_dunder_call.py:5:7: PLC2801 [*] Unnecessary dunder call to `__sub__`
 
 ℹ Safe fix
 2 2 | 
-3 3 | 
+3 3 | a = 2
 4 4 | print((3.0).__add__(4.0))  # PLC2801
 5   |-print((3.0).__sub__(4.0))  # PLC2801
   5 |+print(3.0 - 4.0)  # PLC2801
@@ -52,7 +54,7 @@ unnecessary_dunder_call.py:6:7: PLC2801 [*] Unnecessary dunder call to `__mul__`
   = help: Use `*` operator
 
 ℹ Safe fix
-3 3 | 
+3 3 | a = 2
 4 4 | print((3.0).__add__(4.0))  # PLC2801
 5 5 | print((3.0).__sub__(4.0))  # PLC2801
 6   |-print((3.0).__mul__(4.0))  # PLC2801
@@ -290,7 +292,7 @@ unnecessary_dunder_call.py:17:7: PLC2801 [*] Unnecessary dunder call to `__repr_
    17 |+print(repr(3.0))  # PLC2801
 18 18 | print([1, 2, 3].__len__())  # PLC2801
 19 19 | print((1).__neg__())  # PLC2801
-20 20 | 
+20 20 | print(-a.__sub__(1))  # PLC2801
 
 unnecessary_dunder_call.py:18:7: PLC2801 [*] Unnecessary dunder call to `__len__`. Use `len()` builtin.
    |
@@ -299,6 +301,7 @@ unnecessary_dunder_call.py:18:7: PLC2801 [*] Unnecessary dunder call to `__len__
 18 | print([1, 2, 3].__len__())  # PLC2801
    |       ^^^^^^^^^^^^^^^^^^^ PLC2801
 19 | print((1).__neg__())  # PLC2801
+20 | print(-a.__sub__(1))  # PLC2801
    |
    = help: Use `len()` builtin
 
@@ -309,22 +312,168 @@ unnecessary_dunder_call.py:18:7: PLC2801 [*] Unnecessary dunder call to `__len__
 18    |-print([1, 2, 3].__len__())  # PLC2801
    18 |+print(len([1, 2, 3]))  # PLC2801
 19 19 | print((1).__neg__())  # PLC2801
-20 20 | 
-21 21 | 
+20 20 | print(-a.__sub__(1))  # PLC2801
+21 21 | print(-(a).__sub__(1))  # PLC2801
 
 unnecessary_dunder_call.py:19:7: PLC2801 Unnecessary dunder call to `__neg__`. Multiply by -1 instead.
    |
 17 | print((3.0).__repr__())  # PLC2801
 18 | print([1, 2, 3].__len__())  # PLC2801
 19 | print((1).__neg__())  # PLC2801
    |       ^^^^^^^^^^^^^ PLC2801
+20 | print(-a.__sub__(1))  # PLC2801
+21 | print(-(a).__sub__(1))  # PLC2801
    |
    = help: Multiply by -1 instead
 
-unnecessary_dunder_call.py:31:16: PLC2801 Unnecessary dunder call to `__getattribute__`. Access attribute directly or use getattr built-in function.
+unnecessary_dunder_call.py:20:8: PLC2801 [*] Unnecessary dunder call to `__sub__`. Use `-` operator.
    |
-30 |     def do_thing(self, item):
-31 |         return object.__getattribute__(self, item)  # PLC2801
+18 | print([1, 2, 3].__len__())  # PLC2801
+19 | print((1).__neg__())  # PLC2801
+20 | print(-a.__sub__(1))  # PLC2801
+   |        ^^^^^^^^^^^^ PLC2801
+21 | print(-(a).__sub__(1))  # PLC2801
+22 | print(-(-a.__sub__(1)))  # PLC2801
+   |
+   = help: Use `-` operator
+
+ℹ Safe fix
+17 17 | print((3.0).__repr__())  # PLC2801
+18 18 | print([1, 2, 3].__len__())  # PLC2801
+19 19 | print((1).__neg__())  # PLC2801
+20    |-print(-a.__sub__(1))  # PLC2801
+   20 |+print(-(a - 1))  # PLC2801
+21 21 | print(-(a).__sub__(1))  # PLC2801
+22 22 | print(-(-a.__sub__(1)))  # PLC2801
+23 23 | print((5 - a).__sub__(1))  # PLC2801
+
+unnecessary_dunder_call.py:21:8: PLC2801 [*] Unnecessary dunder call to `__sub__`. Use `-` operator.
+   |
+19 | print((1).__neg__())  # PLC2801
+20 | print(-a.__sub__(1))  # PLC2801
+21 | print(-(a).__sub__(1))  # PLC2801
+   |        ^^^^^^^^^^^^^^ PLC2801
+22 | print(-(-a.__sub__(1)))  # PLC2801
+23 | print((5 - a).__sub__(1))  # PLC2801
+   |
+   = help: Use `-` operator
+
+ℹ Safe fix
+18 18 | print([1, 2, 3].__len__())  # PLC2801
+19 19 | print((1).__neg__())  # PLC2801
+20 20 | print(-a.__sub__(1))  # PLC2801
+21    |-print(-(a).__sub__(1))  # PLC2801
+   21 |+print(-(a - 1))  # PLC2801
+22 22 | print(-(-a.__sub__(1)))  # PLC2801
+23 23 | print((5 - a).__sub__(1))  # PLC2801
+24 24 | print(-(5 - a).__sub__(1))  # PLC2801
+
+unnecessary_dunder_call.py:22:10: PLC2801 [*] Unnecessary dunder call to `__sub__`. Use `-` operator.
+   |
+20 | print(-a.__sub__(1))  # PLC2801
+21 | print(-(a).__sub__(1))  # PLC2801
+22 | print(-(-a.__sub__(1)))  # PLC2801
+   |          ^^^^^^^^^^^^ PLC2801
+23 | print((5 - a).__sub__(1))  # PLC2801
+24 | print(-(5 - a).__sub__(1))  # PLC2801
+   |
+   = help: Use `-` operator
+
+ℹ Safe fix
+19 19 | print((1).__neg__())  # PLC2801
+20 20 | print(-a.__sub__(1))  # PLC2801
+21 21 | print(-(a).__sub__(1))  # PLC2801
+22    |-print(-(-a.__sub__(1)))  # PLC2801
+   22 |+print(-(-(a - 1)))  # PLC2801
+23 23 | print((5 - a).__sub__(1))  # PLC2801
+24 24 | print(-(5 - a).__sub__(1))  # PLC2801
+25 25 | print(-(-5 - a).__sub__(1))  # PLC2801
+
+unnecessary_dunder_call.py:23:7: PLC2801 [*] Unnecessary dunder call to `__sub__`. Use `-` operator.
+   |
+21 | print(-(a).__sub__(1))  # PLC2801
+22 | print(-(-a.__sub__(1)))  # PLC2801
+23 | print((5 - a).__sub__(1))  # PLC2801
+   |       ^^^^^^^^^^^^^^^^^^ PLC2801
+24 | print(-(5 - a).__sub__(1))  # PLC2801
+25 | print(-(-5 - a).__sub__(1))  # PLC2801
+   |
+   = help: Use `-` operator
+
+ℹ Safe fix
+20 20 | print(-a.__sub__(1))  # PLC2801
+21 21 | print(-(a).__sub__(1))  # PLC2801
+22 22 | print(-(-a.__sub__(1)))  # PLC2801
+23    |-print((5 - a).__sub__(1))  # PLC2801
+   23 |+print(5 - a - 1)  # PLC2801
+24 24 | print(-(5 - a).__sub__(1))  # PLC2801
+25 25 | print(-(-5 - a).__sub__(1))  # PLC2801
+26 26 | print(+-+-+-a.__sub__(1))  # PLC2801
+
+unnecessary_dunder_call.py:24:8: PLC2801 [*] Unnecessary dunder call to `__sub__`. Use `-` operator.
+   |
+22 | print(-(-a.__sub__(1)))  # PLC2801
+23 | print((5 - a).__sub__(1))  # PLC2801
+24 | print(-(5 - a).__sub__(1))  # PLC2801
+   |        ^^^^^^^^^^^^^^^^^^ PLC2801
+25 | print(-(-5 - a).__sub__(1))  # PLC2801
+26 | print(+-+-+-a.__sub__(1))  # PLC2801
+   |
+   = help: Use `-` operator
+
+ℹ Safe fix
+21 21 | print(-(a).__sub__(1))  # PLC2801
+22 22 | print(-(-a.__sub__(1)))  # PLC2801
+23 23 | print((5 - a).__sub__(1))  # PLC2801
+24    |-print(-(5 - a).__sub__(1))  # PLC2801
+   24 |+print(-(5 - a - 1))  # PLC2801
+25 25 | print(-(-5 - a).__sub__(1))  # PLC2801
+26 26 | print(+-+-+-a.__sub__(1))  # PLC2801
+27 27 | 
+
+unnecessary_dunder_call.py:25:8: PLC2801 [*] Unnecessary dunder call to `__sub__`. Use `-` operator.
+   |
+23 | print((5 - a).__sub__(1))  # PLC2801
+24 | print(-(5 - a).__sub__(1))  # PLC2801
+25 | print(-(-5 - a).__sub__(1))  # PLC2801
+   |        ^^^^^^^^^^^^^^^^^^^ PLC2801
+26 | print(+-+-+-a.__sub__(1))  # PLC2801
+   |
+   = help: Use `-` operator
+
+ℹ Safe fix
+22 22 | print(-(-a.__sub__(1)))  # PLC2801
+23 23 | print((5 - a).__sub__(1))  # PLC2801
+24 24 | print(-(5 - a).__sub__(1))  # PLC2801
+25    |-print(-(-5 - a).__sub__(1))  # PLC2801
+   25 |+print(-(-5 - a - 1))  # PLC2801
+26 26 | print(+-+-+-a.__sub__(1))  # PLC2801
+27 27 | 
+28 28 | 
+
+unnecessary_dunder_call.py:26:13: PLC2801 [*] Unnecessary dunder call to `__sub__`. Use `-` operator.
+   |
+24 | print(-(5 - a).__sub__(1))  # PLC2801
+25 | print(-(-5 - a).__sub__(1))  # PLC2801
+26 | print(+-+-+-a.__sub__(1))  # PLC2801
+   |             ^^^^^^^^^^^^ PLC2801
+   |
+   = help: Use `-` operator
+
+ℹ Safe fix
+23 23 | print((5 - a).__sub__(1))  # PLC2801
+24 24 | print(-(5 - a).__sub__(1))  # PLC2801
+25 25 | print(-(-5 - a).__sub__(1))  # PLC2801
+26    |-print(+-+-+-a.__sub__(1))  # PLC2801
+   26 |+print(+-+-+-(a - 1))  # PLC2801
+27 27 | 
+28 28 | 
+29 29 | class Thing:
+
+unnecessary_dunder_call.py:38:16: PLC2801 Unnecessary dunder call to `__getattribute__`. Access attribute directly or use getattr built-in function.
+   |
+37 |     def do_thing(self, item):
+38 |         return object.__getattribute__(self, item)  # PLC2801
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC2801
    |
    = help: Access attribute directly or use getattr built-in function