Only flag super calls in class-function scopes (#323)

resources/test/fixtures/SPR001.py

@@ -20,3 +20,46 @@ class Child(Parent):
             Child,
             self,
         ).method()  # wrong
+
+
+class BaseClass:
+    def f(self):
+        print("f")
+
+
+def defined_outside(self):
+    super(MyClass, self).f()  # CANNOT use super()
+
+
+class MyClass(BaseClass):
+    def normal(self):
+        super(MyClass, self).f()  # can use super()
+        super().f()
+
+    def different_argument(self, other):
+        super(MyClass, other).f()  # CANNOT use super()
+
+    def comprehension_scope(self):
+        [super(MyClass, self).f() for x in [1]]  # CANNOT use super()
+
+    def inner_functions(self):
+        def outer_argument():
+            super(MyClass, self).f()  # CANNOT use super()
+
+        def inner_argument(self):
+            super(MyClass, self).f()  # can use super()
+            super().f()
+
+        outer_argument()
+        inner_argument(self)
+
+    def inner_class(self):
+        class InnerClass:
+            super(MyClass, self).f()  # CANNOT use super()
+
+            def method(inner_self):
+                super(MyClass, self).f()  # CANNOT use super()
+
+        InnerClass().method()
+
+    defined_outside = defined_outside

src/ast/checks.rs

@@ -3,8 +3,8 @@ use std::collections::BTreeSet;
 use itertools::izip;
 use regex::Regex;
 use rustpython_parser::ast::{
-    Arg, Arguments, Cmpop, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Keyword,
+    Arg, ArgData, Arguments, Cmpop, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind,
-    Location, Stmt, StmtKind, Unaryop,
+    Keyword, Location, Stmt, StmtKind, Unaryop,
 };
 
 use crate::ast::operations::SourceCodeLocator;
@@ -693,29 +693,79 @@ pub fn check_builtin_shadowing(
     }
 }
 
+/// Returns `true` if a call is an argumented `super` invocation.
+pub fn is_super_call_with_arguments(func: &Expr, args: &Vec<Expr>) -> bool {
+    // Check: is this a `super` call?
+    if let ExprKind::Name { id, .. } = &func.node {
+        id == "super" && !args.is_empty()
+    } else {
+        false
+    }
+}
+
 // flake8-super
 /// Check that `super()` has no args
 pub fn check_super_args(
+    scope: &Scope,
+    parents: &[&Stmt],
     expr: &Expr,
     func: &Expr,
     args: &Vec<Expr>,
-    locator: &mut SourceCodeLocator,
-    autofix: &fixer::Mode,
 ) -> Option<Check> {
-    if let ExprKind::Name { id, .. } = &func.node {
+    if !is_super_call_with_arguments(func, args) {
-        if id == "super" && !args.is_empty() {
+        return None;
-            let mut check = Check::new(
+    }
-                CheckKind::SuperCallWithParameters,
+
-                Range::from_located(expr),
+    // Check: are we in a Function scope?
-            );
+    if !matches!(scope.kind, ScopeKind::Function { .. }) {
-            if matches!(autofix, fixer::Mode::Generate | fixer::Mode::Apply) {
+        return None;
-                if let Some(fix) = fixes::remove_super_arguments(locator, expr) {
+    }
-                    check.amend(fix);
+
+    let mut parents = parents.iter().rev();
+
+    // For a `super` invocation to be unnecessary, the first argument needs to match the enclosing
+    // class, and the second argument needs to match the first argument to the enclosing function.
+    if let [first_arg, second_arg] = args.as_slice() {
+        // Find the enclosing function definition (if any).
+        if let Some(StmtKind::FunctionDef {
+            args: parent_args, ..
+        }) = parents
+            .find(|stmt| matches!(stmt.node, StmtKind::FunctionDef { .. }))
+            .map(|stmt| &stmt.node)
+        {
+            // Extract the name of the first argument to the enclosing function.
+            if let Some(ArgData {
+                arg: parent_arg, ..
+            }) = parent_args.args.first().map(|expr| &expr.node)
+            {
+                // Find the enclosing class definition (if any).
+                if let Some(StmtKind::ClassDef {
+                    name: parent_name, ..
+                }) = parents
+                    .find(|stmt| matches!(stmt.node, StmtKind::ClassDef { .. }))
+                    .map(|stmt| &stmt.node)
+                {
+                    if let (
+                        ExprKind::Name {
+                            id: first_arg_id, ..
+                        },
+                        ExprKind::Name {
+                            id: second_arg_id, ..
+                        },
+                    ) = (&first_arg.node, &second_arg.node)
+                    {
+                        if first_arg_id == parent_name && second_arg_id == parent_arg {
+                            return Some(Check::new(
+                                CheckKind::SuperCallWithParameters,
+                                Range::from_located(expr),
+                            ));
+                        }
+                    }
                 }
             }
-            return Some(check);
         }
     }
+
     None
 }
 

src/check_ast.rs

@@ -755,10 +755,28 @@ where
 
                 // flake8-super
                 if self.settings.select.contains(&CheckCode::SPR001) {
-                    if let Some(check) =
+                    // Only bother going through the super check at all if we're in a `super` call.
-                        checks::check_super_args(expr, func, args, &mut self.locator, self.autofix)
+                    // (We check this in `check_super_args` too, so this is just an optimization.)
-                    {
+                    if checks::is_super_call_with_arguments(func, args) {
-                        self.checks.push(check)
+                        let scope = &mut self.scopes
+                            [*(self.scope_stack.last().expect("No current scope found."))];
+                        let parents: Vec<&Stmt> = self
+                            .parent_stack
+                            .iter()
+                            .map(|index| self.parents[*index])
+                            .collect();
+                        if let Some(mut check) =
+                            checks::check_super_args(scope, &parents, expr, func, args)
+                        {
+                            if matches!(self.autofix, fixer::Mode::Generate | fixer::Mode::Apply) {
+                                if let Some(fix) =
+                                    fixes::remove_super_arguments(&mut self.locator, expr)
+                                {
+                                    check.amend(fix);
+                                }
+                            }
+                            self.checks.push(check)
+                        }
                     }
                 }
 

src/snapshots/ruff__linter__tests__spr001.snap

@@ -50,4 +50,36 @@ expression: checks
       row: 22
       column: 10
     applied: false
+- kind: SuperCallWithParameters
+  location:
+    row: 36
+    column: 9
+  end_location:
+    row: 36
+    column: 29
+  fix:
+    content: super()
+    location:
+      row: 36
+      column: 9
+    end_location:
+      row: 36
+      column: 29
+    applied: false
+- kind: SuperCallWithParameters
+  location:
+    row: 50
+    column: 13
+  end_location:
+    row: 50
+    column: 33
+  fix:
+    content: super()
+    location:
+      row: 50
+      column: 13
+    end_location:
+      row: 50
+      column: 33
+    applied: false