Skip to content

Commit db72ff9

Browse files
committed
feat(UPO10): reworked FunctionScopeNameFinder to AnyExpressionFinder
renamed FunctionScopeNameFinder to AnyExpressionFinder moved AnyExpressionFinder to super_call_with_parameters.rs updated test cases and snapshots removed duplicated parents Now AnyExpressionFinder finds any expression by given conditions
1 parent 650f48f commit db72ff9

File tree

5 files changed

+374
-460
lines changed

5 files changed

+374
-460
lines changed

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

Lines changed: 23 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -144,143 +144,82 @@ def method3(self):
144144

145145
# See: https://github.com/astral-sh/ruff/issues/19357
146146
# Must be detected
147-
class ParentD1:
147+
class ParentD:
148148
def f(self):
149-
print("!")
149+
print("D")
150150

151-
class ChildD1(ParentD1):
151+
class ChildD1(ParentD):
152152
def f(self):
153153
if False: __class__ # Python injects __class__ into scope
154154
builtins.super(ChildD1, self).f()
155155

156-
ChildD1().f()
157-
158-
class ParentD2:
159-
def f(self):
160-
print("!")
161-
162-
class ChildD2(ParentD2):
156+
class ChildD2(ParentD):
163157
def f(self):
164158
if False: super # Python injects __class__ into scope
165159
builtins.super(ChildD2, self).f()
166160

167-
ChildD2().f()
168-
169-
class ParentD3:
170-
def f(self):
171-
print("!")
172-
173-
class ChildD3(ParentD3):
161+
class ChildD3(ParentD):
174162
def f(self):
175163
builtins.super(ChildD3, self).f()
176164
super # Python injects __class__ into scope
177165

178-
ChildD3().f()
179-
180166
import builtins as builtins_alias
181-
182-
class ParentD4:
183-
def f(self):
184-
print("!")
185-
186-
class ChildD4(ParentD4):
167+
class ChildD4(ParentD):
187168
def f(self):
188169
builtins_alias.super(ChildD4, self).f()
189170
super # Python injects __class__ into scope
190171

191-
ChildD4().f()
192-
193-
class ParentD5:
194-
def f(self):
195-
print("!")
196-
197-
class ChildD5(ParentD5):
172+
class ChildD5(ParentD):
198173
def f(self):
199174
super = 1
200175
super # Python injects __class__ into scope
201176
builtins.super(ChildD5, self).f()
202177

203-
ChildD5().f()
204-
205-
class ParentD6:
206-
def f(self):
207-
print("!")
208-
209-
class ChildD6(ParentD6):
178+
class ChildD6(ParentD):
210179
def f(self):
211180
super: "Any"
212181
__class__ # Python injects __class__ into scope
213182
builtins.super(ChildD6, self).f()
214183

215-
ChildD6().f()
216-
217-
class ParentD7:
218-
def f(self):
219-
print("!")
220-
221-
class ChildD7(ParentD7):
184+
class ChildD7(ParentD):
222185
def f(self):
223186
def x():
224187
__class__ # Python injects __class__ into scope
225188
builtins.super(ChildD7, self).f()
226189

227-
ChildD7().f()
228-
229-
class ParentD8:
230-
def f(self):
231-
print("!")
232-
233-
class ChildD8(ParentD8):
190+
class ChildD8(ParentD):
234191
def f(self):
235192
def x():
236193
super = 1
237194
super # Python injects __class__ into scope
238195
builtins.super(ChildD8, self).f()
239196

240-
ChildD8().f()
241-
242-
class ParentD9:
243-
def f(self):
244-
print("!")
245-
246-
class ChildD9(ParentD9):
197+
class ChildD9(ParentD):
247198
def f(self):
248199
def x():
249200
__class__ = 1
250201
__class__ # Python injects __class__ into scope
251202
builtins.super(ChildD9, self).f()
252203

253-
ChildD9().f()
254-
255-
class ParentD10:
256-
def f(self):
257-
print("!")
258-
259-
class ChildD10(ParentD10):
204+
class ChildD10(ParentD):
260205
def f(self):
261206
def x():
262207
__class__ = 1
263208
super # Python injects __class__ into scope
264209
builtins.super(ChildD10, self).f()
265210

266-
ChildD10().f()
267211

268212
# Must be ignored
269-
class ParentI1:
213+
class ParentI:
270214
def f(self):
271-
print("!")
215+
print("I")
272216

273-
class ChildI1(ParentI1):
217+
class ChildI1(ParentI):
274218
def f(self):
275219
builtins.super(ChildI1, self).f() # no __class__ in the local scope
276220

277-
ChildI1().f()
278-
279-
class ParentI2:
280-
def f(self):
281-
print("!")
282221

283-
class ChildI2(ParentI2):
222+
class ChildI2(ParentI):
284223
def b(self):
285224
x = __class__
286225
if False: super
@@ -289,90 +228,46 @@ def f(self):
289228
self.b()
290229
builtins.super(ChildI2, self).f() # no __class__ in the local scope
291230

292-
ChildI2().f()
293-
294-
class ParentI3:
295-
def f(self):
296-
print("!")
297-
298-
class ChildI3(ParentI3):
231+
class ChildI3(ParentI):
299232
def f(self):
300233
if False: super
301234
def x(_):
302235
builtins.super(ChildI3, self).f() # no __class__ in the local scope
303236
x(None)
304237

305-
ChildI3().f()
306-
307-
class ParentI4:
308-
def f(self):
309-
print("!")
310-
311-
class ChildI4(ParentI4):
238+
class ChildI4(ParentI):
312239
def f(self):
313240
super: "str"
314241
builtins.super(ChildI4, self).f() # no __class__ in the local scope
315242

316-
ChildI4().f()
317-
318-
class ParentI5:
319-
def f(self):
320-
print("!")
321-
322-
class ChildI5(ParentI5):
243+
class ChildI5(ParentI):
323244
def f(self):
324245
super = 1
325246
__class__ = 3
326247
builtins.super(ChildI5, self).f() # no __class__ in the local scope
327248

328-
ChildI5().f()
329-
330-
class ParentI6:
331-
def f(self):
332-
print("!")
333-
334-
class ChildI6(ParentI6):
249+
class ChildI6(ParentI):
335250
def f(self):
336251
__class__ = None
337252
__class__
338253
builtins.super(ChildI6, self).f() # no __class__ in the local scope
339254

340-
ChildI6().f()
341-
342-
class ParentI7:
343-
def f(self):
344-
print("!")
345-
346-
class ChildI7(ParentI7):
255+
class ChildI7(ParentI):
347256
def f(self):
348257
__class__ = None
349258
super
350259
builtins.super(ChildI7, self).f()
351260

352-
ChildI7().f()
353-
354-
class ParentI8:
355-
def f(self):
356-
print("!")
357-
358-
class ChildI8(ParentI8):
261+
class ChildI8(ParentI):
359262
def f(self):
360263
__class__: "Any"
361264
super
362265
builtins.super(ChildI8, self).f()
363266

364-
ChildI8().f()
365-
366-
class ParentI9:
367-
def f(self):
368-
print("!")
369-
370-
class ChildI9(ParentI9):
267+
class ChildI9(ParentI):
371268
def f(self):
372269
class A:
373270
def foo(self):
374271
if False: super
375272
if False: __class__
376273
builtins.super(ChildI9, self).f()
377-
378-
ChildI9().f()

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

Lines changed: 67 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use ruff_diagnostics::Applicability;
22
use ruff_macros::{ViolationMetadata, derive_message_formats};
3-
use ruff_python_ast::helpers::FunctionScopeNameFinder;
4-
use ruff_python_ast::visitor::Visitor;
3+
use ruff_python_ast::visitor::{Visitor, walk_expr, walk_stmt};
54
use ruff_python_ast::{self as ast, Expr, Stmt};
65
use ruff_python_semantic::SemanticModel;
76
use ruff_text_size::{Ranged, TextSize};
@@ -97,13 +96,12 @@ pub(crate) fn super_call_with_parameters(checker: &Checker, call: &ast::ExprCall
9796
};
9897

9998
// Find the enclosing function definition (if any).
100-
let Some(func_stmt) = parents.find(|stmt| stmt.is_function_def_stmt()) else {
101-
return;
102-
};
103-
let Stmt::FunctionDef(ast::StmtFunctionDef {
104-
parameters: parent_parameters,
105-
..
106-
}) = func_stmt
99+
let Some(
100+
func_stmt @ Stmt::FunctionDef(ast::StmtFunctionDef {
101+
parameters: parent_parameters,
102+
..
103+
}),
104+
) = parents.find(|stmt| stmt.is_function_def_stmt())
107105
else {
108106
return;
109107
};
@@ -205,24 +203,30 @@ pub(crate) fn super_call_with_parameters(checker: &Checker, call: &ast::ExprCall
205203
fn is_super_call_with_arguments(call: &ast::ExprCall, checker: &Checker) -> bool {
206204
checker.semantic().match_builtin_expr(&call.func, "super") && !call.arguments.is_empty()
207205
}
206+
208207
/// Returns `true` if the function contains load references to `__class__` or `super` without
209208
/// local binding.
210209
///
211210
/// This indicates that the function relies on the implicit `__class__` cell variable created by
212211
/// Python when `super()` is called without arguments, making it unsafe to remove `super()` parameters.
213212
fn has_local_dunder_class_var_ref(semantic: &SemanticModel, func_stmt: &Stmt) -> bool {
214-
let mut finder = FunctionScopeNameFinder::default();
213+
if semantic.current_scope().has("__class__") {
214+
return false;
215+
}
216+
217+
let mut finder = AnyExpressionFinder::new(vec![
218+
|expr: &Expr| {
219+
expr.as_name_expr()
220+
.is_some_and(|name| name.id.as_str() == "super" && name.ctx.is_load())
221+
},
222+
|expr: &Expr| {
223+
expr.as_name_expr()
224+
.is_some_and(|name| name.id.as_str() == "__class__" && name.ctx.is_load())
225+
},
226+
]);
215227
finder.visit_stmt(func_stmt);
216-
let has_load_super = finder
217-
.names
218-
.iter()
219-
.any(|expr| expr.id.as_str() == "super" && expr.ctx.is_load());
220-
let has_load_dunder_class = finder
221-
.names
222-
.iter()
223-
.any(|expr| expr.id.as_str() == "__class__" && expr.ctx.is_load());
224-
let has_dunder_class_binding = semantic.current_scope().has("__class__");
225-
(has_load_super || has_load_dunder_class) && !has_dunder_class_binding
228+
229+
finder.has_expression()
226230
}
227231

228232
/// Returns `true` if the call is to the built-in `builtins.super` function.
@@ -231,3 +235,46 @@ fn is_builtins_super(semantic: &SemanticModel, call: &ast::ExprCall) -> bool {
231235
.resolve_qualified_name(&call.func)
232236
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["builtins", "super"]))
233237
}
238+
239+
/// A [`Visitor`] that searches for [`Expr`] matching any of the provided conditions
240+
/// , excluding nested class definitions.
241+
#[derive(Debug)]
242+
struct AnyExpressionFinder<'a> {
243+
result_expression: Vec<&'a Expr>,
244+
conditions: Vec<fn(&Expr) -> bool>,
245+
}
246+
247+
impl AnyExpressionFinder<'_> {
248+
pub(crate) fn new(conditions: Vec<fn(&Expr) -> bool>) -> Self {
249+
AnyExpressionFinder {
250+
result_expression: Vec::with_capacity(1),
251+
conditions,
252+
}
253+
}
254+
pub(crate) fn has_expression(&self) -> bool {
255+
!self.result_expression.is_empty()
256+
}
257+
}
258+
259+
impl<'a> Visitor<'a> for AnyExpressionFinder<'a> {
260+
fn visit_stmt(&mut self, stmt: &'a Stmt) {
261+
match stmt {
262+
Stmt::ClassDef(_) => {}
263+
_ => {
264+
if self.result_expression.is_empty() {
265+
walk_stmt(self, stmt);
266+
}
267+
}
268+
}
269+
}
270+
271+
fn visit_expr(&mut self, expr: &'a Expr) {
272+
for condition in &self.conditions {
273+
if condition(expr) {
274+
self.result_expression.insert(0, expr);
275+
return;
276+
}
277+
}
278+
walk_expr(self, expr);
279+
}
280+
}

0 commit comments

Comments
 (0)