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 index 598537aa4e7c1f..466e02c1126ac4 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_dunder_call.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_dunder_call.py @@ -37,6 +37,13 @@ + 4 )) +print(2 * a.__add__(3)) # PLC2801 +x = 2 * a.__add__(3) # PLC2801 +x = 2 * -a.__add__(3) # PLC2801 +x = a.__add__(3) # PLC2801 +x = -a.__add__(3) # PLC2801 +x = (-a).__add__(3) # PLC2801 +x = -(-a).__add__(3) # 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 index 7ae459b76b7a29..bdfc8a855e0dcd 100644 --- 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,7 +2,6 @@ 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; @@ -167,53 +166,24 @@ pub(crate) fn unnecessary_dunder_call(checker: &mut Checker, call: &ast::ExprCal ); if let Some(mut fixed) = fixed { - // check if this attribute is preceded by a unary operator, e.g. `-a.__sub__(1)` - // if it is, the fix has to be handled differently to maintain semantics. - let is_in_unary = checker + // by the looks of it, we don't need to wrap the expression in parens if + // it's the only argument to a call expression. + // being in any other kind of expression though, we *will* add parens. + // e.g. `print(a.__add__(3))` -> `print(a + 3)` instead of `print((a + 3))` + // a multiplication expression example: `x = 2 * a.__add__(3)` -> `x = 2 * (a + 3)` + let wrap_in_paren = checker .semantic() .current_expression_parent() - .is_some_and(|parent| matches!(parent, Expr::UnaryOp(ast::ExprUnaryOp { .. }))); - - if is_in_unary { - let mut tokenizer = - SimpleTokenizer::starts_at(value.as_ref().end(), checker.locator().contents()) - .skip_trivia(); - - let token = tokenizer.next().unwrap(); - // we're going to start looking for the first immediate Right-Parentheses, - // and the first Dot token after that. - - // if our attribute is within parentheses with a unary, e.g. `-(-5).__sub__(1)` - // we have to add the fix within the existing parentheses, // ^ - // because `--5 - 1` is not the same as `-(-5 - 1)` - if token.kind != SimpleTokenKind::RParen { - // if we're not in parentheses, we're going to wrap the fix in parentheses - // to maintain semantic integrity. - // `-a.__sub__(1)` -> `-(a - 1)` - fixed = format!("({fixed})"); - } + .is_some_and(|parent| !parent.is_call_expr()); - // find the dot token for this call - let dot_start = if token.kind == SimpleTokenKind::Dot { - token.start() - } else { - tokenizer - .find(|token| token.kind == SimpleTokenKind::Dot) - .unwrap() - .start() - }; - - diagnostic.set_fix(Fix::unsafe_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::unsafe_edit(Edit::range_replacement( - fixed, - call.range(), - ))); + if wrap_in_paren { + fixed = format!("({})", fixed); } + + diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( + fixed, + call.range(), + ))); }; checker.diagnostics.push(diagnostic); diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2801_unnecessary_dunder_call.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2801_unnecessary_dunder_call.py.snap index 8ce0dd76a87308..ab2c34505b1ce1 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2801_unnecessary_dunder_call.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLC2801_unnecessary_dunder_call.py.snap @@ -578,6 +578,8 @@ unnecessary_dunder_call.py:35:7: PLC2801 [*] Unnecessary dunder call to `__rsub_ 38 | | 4 39 | | )) | |_^ PLC2801 +40 | print(2 * a.__add__(3)) # PLC2801 +41 | x = 2 * a.__add__(3) # PLC2801 | = help: Use `-` operator @@ -592,14 +594,158 @@ unnecessary_dunder_call.py:35:7: PLC2801 [*] Unnecessary dunder call to `__rsub_ 38 |- 4 39 |-)) 37 |+ 4) - a) -40 38 | -41 39 | -42 40 | class Thing: +40 38 | print(2 * a.__add__(3)) # PLC2801 +41 39 | x = 2 * a.__add__(3) # PLC2801 +42 40 | x = 2 * -a.__add__(3) # PLC2801 -unnecessary_dunder_call.py:51:16: PLC2801 Unnecessary dunder call to `__getattribute__`. Access attribute directly or use getattr built-in function. +unnecessary_dunder_call.py:40:11: PLC2801 [*] Unnecessary dunder call to `__add__`. Use `+` operator. | -50 | def do_thing(self, item): -51 | return object.__getattribute__(self, item) # PLC2801 +38 | 4 +39 | )) +40 | print(2 * a.__add__(3)) # PLC2801 + | ^^^^^^^^^^^^ PLC2801 +41 | x = 2 * a.__add__(3) # PLC2801 +42 | x = 2 * -a.__add__(3) # PLC2801 + | + = help: Use `+` operator + +ℹ Unsafe fix +37 37 | + +38 38 | 4 +39 39 | )) +40 |-print(2 * a.__add__(3)) # PLC2801 + 40 |+print(2 * (a + 3)) # PLC2801 +41 41 | x = 2 * a.__add__(3) # PLC2801 +42 42 | x = 2 * -a.__add__(3) # PLC2801 +43 43 | x = a.__add__(3) # PLC2801 + +unnecessary_dunder_call.py:41:9: PLC2801 [*] Unnecessary dunder call to `__add__`. Use `+` operator. + | +39 | )) +40 | print(2 * a.__add__(3)) # PLC2801 +41 | x = 2 * a.__add__(3) # PLC2801 + | ^^^^^^^^^^^^ PLC2801 +42 | x = 2 * -a.__add__(3) # PLC2801 +43 | x = a.__add__(3) # PLC2801 + | + = help: Use `+` operator + +ℹ Unsafe fix +38 38 | 4 +39 39 | )) +40 40 | print(2 * a.__add__(3)) # PLC2801 +41 |-x = 2 * a.__add__(3) # PLC2801 + 41 |+x = 2 * (a + 3) # PLC2801 +42 42 | x = 2 * -a.__add__(3) # PLC2801 +43 43 | x = a.__add__(3) # PLC2801 +44 44 | x = -a.__add__(3) # PLC2801 + +unnecessary_dunder_call.py:42:10: PLC2801 [*] Unnecessary dunder call to `__add__`. Use `+` operator. + | +40 | print(2 * a.__add__(3)) # PLC2801 +41 | x = 2 * a.__add__(3) # PLC2801 +42 | x = 2 * -a.__add__(3) # PLC2801 + | ^^^^^^^^^^^^ PLC2801 +43 | x = a.__add__(3) # PLC2801 +44 | x = -a.__add__(3) # PLC2801 + | + = help: Use `+` operator + +ℹ Unsafe fix +39 39 | )) +40 40 | print(2 * a.__add__(3)) # PLC2801 +41 41 | x = 2 * a.__add__(3) # PLC2801 +42 |-x = 2 * -a.__add__(3) # PLC2801 + 42 |+x = 2 * -(a + 3) # PLC2801 +43 43 | x = a.__add__(3) # PLC2801 +44 44 | x = -a.__add__(3) # PLC2801 +45 45 | x = (-a).__add__(3) # PLC2801 + +unnecessary_dunder_call.py:43:5: PLC2801 [*] Unnecessary dunder call to `__add__`. Use `+` operator. + | +41 | x = 2 * a.__add__(3) # PLC2801 +42 | x = 2 * -a.__add__(3) # PLC2801 +43 | x = a.__add__(3) # PLC2801 + | ^^^^^^^^^^^^ PLC2801 +44 | x = -a.__add__(3) # PLC2801 +45 | x = (-a).__add__(3) # PLC2801 + | + = help: Use `+` operator + +ℹ Unsafe fix +40 40 | print(2 * a.__add__(3)) # PLC2801 +41 41 | x = 2 * a.__add__(3) # PLC2801 +42 42 | x = 2 * -a.__add__(3) # PLC2801 +43 |-x = a.__add__(3) # PLC2801 + 43 |+x = a + 3 # PLC2801 +44 44 | x = -a.__add__(3) # PLC2801 +45 45 | x = (-a).__add__(3) # PLC2801 +46 46 | x = -(-a).__add__(3) # PLC2801 + +unnecessary_dunder_call.py:44:6: PLC2801 [*] Unnecessary dunder call to `__add__`. Use `+` operator. + | +42 | x = 2 * -a.__add__(3) # PLC2801 +43 | x = a.__add__(3) # PLC2801 +44 | x = -a.__add__(3) # PLC2801 + | ^^^^^^^^^^^^ PLC2801 +45 | x = (-a).__add__(3) # PLC2801 +46 | x = -(-a).__add__(3) # PLC2801 + | + = help: Use `+` operator + +ℹ Unsafe fix +41 41 | x = 2 * a.__add__(3) # PLC2801 +42 42 | x = 2 * -a.__add__(3) # PLC2801 +43 43 | x = a.__add__(3) # PLC2801 +44 |-x = -a.__add__(3) # PLC2801 + 44 |+x = -(a + 3) # PLC2801 +45 45 | x = (-a).__add__(3) # PLC2801 +46 46 | x = -(-a).__add__(3) # PLC2801 +47 47 | + +unnecessary_dunder_call.py:45:5: PLC2801 [*] Unnecessary dunder call to `__add__`. Use `+` operator. + | +43 | x = a.__add__(3) # PLC2801 +44 | x = -a.__add__(3) # PLC2801 +45 | x = (-a).__add__(3) # PLC2801 + | ^^^^^^^^^^^^^^^ PLC2801 +46 | x = -(-a).__add__(3) # PLC2801 + | + = help: Use `+` operator + +ℹ Unsafe fix +42 42 | x = 2 * -a.__add__(3) # PLC2801 +43 43 | x = a.__add__(3) # PLC2801 +44 44 | x = -a.__add__(3) # PLC2801 +45 |-x = (-a).__add__(3) # PLC2801 + 45 |+x = -a + 3 # PLC2801 +46 46 | x = -(-a).__add__(3) # PLC2801 +47 47 | +48 48 | + +unnecessary_dunder_call.py:46:6: PLC2801 [*] Unnecessary dunder call to `__add__`. Use `+` operator. + | +44 | x = -a.__add__(3) # PLC2801 +45 | x = (-a).__add__(3) # PLC2801 +46 | x = -(-a).__add__(3) # PLC2801 + | ^^^^^^^^^^^^^^^ PLC2801 + | + = help: Use `+` operator + +ℹ Unsafe fix +43 43 | x = a.__add__(3) # PLC2801 +44 44 | x = -a.__add__(3) # PLC2801 +45 45 | x = (-a).__add__(3) # PLC2801 +46 |-x = -(-a).__add__(3) # PLC2801 + 46 |+x = -(-a + 3) # PLC2801 +47 47 | +48 48 | +49 49 | class Thing: + +unnecessary_dunder_call.py:58:16: PLC2801 Unnecessary dunder call to `__getattribute__`. Access attribute directly or use getattr built-in function. + | +57 | def do_thing(self, item): +58 | return object.__getattribute__(self, item) # PLC2801 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLC2801 | = help: Access attribute directly or use getattr built-in function