Skip to content

Commit f2dd40e

Browse files
[pyupgrade] Properly trigger super change in nested class (UP008) (#22677)
While debugging, I noticed that the function parameters were of type `Expr::Name` in the normal case and `Expr::Attribute` when the function was in a nested class. Reading the [documentation](https://docs.python.org/3/library/ast.html#ast.expr), it seems that being an `Expr::Attribute` makes sense because `Inner` is an attribute of `Outer`. I did some thinking and could not identify any additional `Expr` types I should be pattern-matching here, but I would like some input from you. The change seems to fix the issue without any regression. ```console echo 'class Base: def __init__(self, foo): self.foo = foo class Outer(Base): def __init__(self, foo): super().__init__(foo) # Should not trigger UP008 class Inner(Base): def __init__(self, foo): super().__init__(foo) # Should not trigger UP008 class InnerInner(Base): def __init__(self, foo): super(Outer.Inner.InnerInner, self).__init__(foo) # Should trigger UP008 ' | cargo run -p ruff -- check --select UP008 - UP008 Use `super()` instead of `super(__class__, self)` --> -:16:18 | 14 | class InnerInner(Base): 15 | def __init__(self, foo): 16 | super(Outer.Inner.InnerInner, self).__init__(foo) # Should trigger UP008 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | help: Remove `super()` parameters Found 1 error. No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option). ``` Closes #22597 --------- Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
1 parent 4f836b4 commit f2dd40e

3 files changed

Lines changed: 139 additions & 16 deletions

File tree

crates/ruff_linter/resources/test/fixtures/pyupgrade/UP008.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,3 +303,43 @@ class C(B):
303303
def f(self):
304304
__class__ = B # Local variable __class__ shadows the implicit __class__
305305
return super(__class__, self).f() # Should NOT trigger UP008
306+
307+
class Base:
308+
def __init__(self, foo):
309+
self.foo = foo
310+
311+
312+
class Outer(Base):
313+
def __init__(self, foo):
314+
super().__init__(foo) # Should not trigger UP008
315+
316+
class Inner(Base):
317+
def __init__(self, foo):
318+
super(Outer.Inner, self).__init__(foo) # UP008: matches enclosing class chain
319+
320+
321+
# super() first arg is an attribute that only matches on the last segment,
322+
# but refers to a different class (Outer.Inner, not __class__ which is Inner).
323+
class Inner(Outer.Inner):
324+
def __init__(self, foo):
325+
super(Outer.Inner, self).__init__(foo) # Should NOT trigger UP008
326+
327+
# super() first arg has an unrelated module prefix
328+
class Outer2:
329+
class Inner2(Base):
330+
def __init__(self, foo):
331+
super(some_module.Outer2.Inner2, self).__init__(foo) # Should NOT trigger UP008
332+
333+
# 3-level deep nesting: super(A.B.C, self) should trigger UP008
334+
class A:
335+
class B:
336+
class C(Base):
337+
def __init__(self, foo):
338+
super(A.B.C, self).__init__(foo) # UP008: matches full chain
339+
340+
# Mismatched middle segment: Wrong.Inner doesn't match Outer3.Inner
341+
class Outer3:
342+
class Inner(Base):
343+
def __init__(self, foo):
344+
super(Wrong.Inner, self).__init__(foo) # Should NOT trigger UP008
345+

crates/ruff_linter/src/rules/pyupgrade/rules/super_call_with_parameters.rs

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ pub(crate) fn super_call_with_parameters(checker: &Checker, call: &ast::ExprCall
8383
}
8484

8585
let mut parents = checker.semantic().current_statements();
86-
8786
// For a `super` invocation to be unnecessary, the first argument needs to match
8887
// the enclosing class, and the second argument needs to match the first
8988
// argument to the enclosing function.
@@ -123,28 +122,51 @@ pub(crate) fn super_call_with_parameters(checker: &Checker, call: &ast::ExprCall
123122
return;
124123
};
125124

126-
let (
127-
Expr::Name(ast::ExprName {
128-
id: first_arg_id, ..
129-
}),
130-
Expr::Name(ast::ExprName {
131-
id: second_arg_id, ..
132-
}),
133-
) = (first_arg, second_arg)
125+
let Expr::Name(ast::ExprName {
126+
id: second_arg_id, ..
127+
}) = second_arg
134128
else {
135129
return;
136130
};
137131

138-
// The `super(__class__, self)` and `super(ParentClass, self)` patterns are redundant in Python 3
139-
// when the first argument refers to the implicit `__class__` cell or to the enclosing class.
140-
// Avoid triggering if a local variable shadows either name.
141-
if !(((first_arg_id == "__class__") || (first_arg_id == parent_name.as_str()))
142-
&& !checker.semantic().current_scope().has(first_arg_id)
143-
&& second_arg_id == parent_arg.name().as_str())
144-
{
132+
if second_arg_id != parent_arg.name().as_str() {
145133
return;
146134
}
147135

136+
// Verify the first argument matches the enclosing class chain.
137+
// For `super(__class__, self)` or `super(ClassName, self)`, just check the immediate class.
138+
// For `super(Outer.Inner, self)`, verify each segment matches the enclosing class nesting.
139+
match first_arg {
140+
Expr::Name(ast::ExprName { id, .. }) => {
141+
if !((id == "__class__" || id == parent_name.as_str())
142+
&& !checker.semantic().current_scope().has(id))
143+
{
144+
return;
145+
}
146+
}
147+
Expr::Attribute(_) => {
148+
let chain = collect_attribute_chain(first_arg);
149+
// The innermost name must match the immediately enclosing class.
150+
if chain.last() != Some(&parent_name.as_str()) {
151+
return;
152+
}
153+
// Each preceding name must match the next enclosing class.
154+
for name in chain.iter().rev().skip(1) {
155+
let Some(Stmt::ClassDef(ast::StmtClassDef {
156+
name: enclosing_name,
157+
..
158+
})) = parents.find(|stmt| stmt.is_class_def_stmt())
159+
else {
160+
return;
161+
};
162+
if *name != enclosing_name.as_str() {
163+
return;
164+
}
165+
}
166+
}
167+
_ => return,
168+
}
169+
148170
drop(parents);
149171

150172
// If the class is an `@dataclass` with `slots=True`, calling `super()` without arguments raises
@@ -197,6 +219,29 @@ pub(crate) fn super_call_with_parameters(checker: &Checker, call: &ast::ExprCall
197219
}
198220
}
199221

222+
/// Collects the chain of names from an attribute expression.
223+
///
224+
/// For example, `A.B.C` returns `["A", "B", "C"]`.
225+
fn collect_attribute_chain(expr: &Expr) -> Vec<&str> {
226+
let mut chain = Vec::new();
227+
let mut current = expr;
228+
loop {
229+
match current {
230+
Expr::Attribute(ast::ExprAttribute { value, attr, .. }) => {
231+
chain.push(attr.id.as_str());
232+
current = value;
233+
}
234+
Expr::Name(ast::ExprName { id, .. }) => {
235+
chain.push(id.as_str());
236+
break;
237+
}
238+
_ => return Vec::new(),
239+
}
240+
}
241+
chain.reverse();
242+
chain
243+
}
244+
200245
/// Returns `true` if a call is an argumented `super` invocation.
201246
fn is_super_call_with_arguments(call: &ast::ExprCall, checker: &Checker) -> bool {
202247
checker.semantic().match_builtin_expr(&call.func, "super") && !call.arguments.is_empty()

crates/ruff_linter/src/rules/pyupgrade/snapshots/ruff_linter__rules__pyupgrade__tests__UP008.py.snap

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,3 +476,41 @@ help: Remove `super()` parameters
476476
210 |
477477
211 |
478478
212 | # Must be ignored
479+
480+
UP008 [*] Use `super()` instead of `super(__class__, self)`
481+
--> UP008.py:318:18
482+
|
483+
316 | class Inner(Base):
484+
317 | def __init__(self, foo):
485+
318 | super(Outer.Inner, self).__init__(foo) # UP008: matches enclosing class chain
486+
| ^^^^^^^^^^^^^^^^^^^
487+
|
488+
help: Remove `super()` parameters
489+
315 |
490+
316 | class Inner(Base):
491+
317 | def __init__(self, foo):
492+
- super(Outer.Inner, self).__init__(foo) # UP008: matches enclosing class chain
493+
318 + super().__init__(foo) # UP008: matches enclosing class chain
494+
319 |
495+
320 |
496+
321 | # super() first arg is an attribute that only matches on the last segment,
497+
498+
UP008 [*] Use `super()` instead of `super(__class__, self)`
499+
--> UP008.py:338:22
500+
|
501+
336 | class C(Base):
502+
337 | def __init__(self, foo):
503+
338 | super(A.B.C, self).__init__(foo) # UP008: matches full chain
504+
| ^^^^^^^^^^^^^
505+
339 |
506+
340 | # Mismatched middle segment: Wrong.Inner doesn't match Outer3.Inner
507+
|
508+
help: Remove `super()` parameters
509+
335 | class B:
510+
336 | class C(Base):
511+
337 | def __init__(self, foo):
512+
- super(A.B.C, self).__init__(foo) # UP008: matches full chain
513+
338 + super().__init__(foo) # UP008: matches full chain
514+
339 |
515+
340 | # Mismatched middle segment: Wrong.Inner doesn't match Outer3.Inner
516+
341 | class Outer3:

0 commit comments

Comments
 (0)