From 291ef9856acfc2442bee5a145e3cfeadf26d8dc9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 5 Feb 2023 18:02:09 -0500 Subject: [PATCH] Remove unnecessary `super_args.rs` (#2594) --- crates/ruff/src/ast/helpers.rs | 9 -- crates/ruff/src/rules/pyupgrade/rules/mod.rs | 2 - .../src/rules/pyupgrade/rules/super_args.rs | 80 ------------------ .../rules/super_call_with_parameters.rs | 82 +++++++++++++++++-- 4 files changed, 73 insertions(+), 100 deletions(-) delete mode 100644 crates/ruff/src/rules/pyupgrade/rules/super_args.rs diff --git a/crates/ruff/src/ast/helpers.rs b/crates/ruff/src/ast/helpers.rs index a8c4d88596..1a0af0bf57 100644 --- a/crates/ruff/src/ast/helpers.rs +++ b/crates/ruff/src/ast/helpers.rs @@ -592,15 +592,6 @@ pub fn has_comments_in(range: Range, locator: &Locator) -> bool { false } -/// Returns `true` if a call is an argumented `super` invocation. -pub fn is_super_call_with_arguments(func: &Expr, args: &[Expr]) -> bool { - if let ExprKind::Name { id, .. } = &func.node { - id == "super" && !args.is_empty() - } else { - false - } -} - /// Return `true` if the body uses `locals()`, `globals()`, `vars()`, `eval()`. pub fn uses_magic_variable_access(checker: &Checker, body: &[Stmt]) -> bool { any_over_body(body, &|expr| { diff --git a/crates/ruff/src/rules/pyupgrade/rules/mod.rs b/crates/ruff/src/rules/pyupgrade/rules/mod.rs index 9bb6adeed5..ff7fb5cecc 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/mod.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/mod.rs @@ -29,7 +29,6 @@ pub(crate) use rewrite_mock_import::{ }; pub(crate) use rewrite_unicode_literal::{rewrite_unicode_literal, RewriteUnicodeLiteral}; pub(crate) use rewrite_yield_from::{rewrite_yield_from, RewriteYieldFrom}; -pub(crate) use super_args::super_args; pub(crate) use super_call_with_parameters::{super_call_with_parameters, SuperCallWithParameters}; pub(crate) use type_of_primitive::{type_of_primitive, TypeOfPrimitive}; pub(crate) use typing_text_str_alias::{typing_text_str_alias, TypingTextStrAlias}; @@ -68,7 +67,6 @@ mod rewrite_c_element_tree; mod rewrite_mock_import; mod rewrite_unicode_literal; mod rewrite_yield_from; -mod super_args; mod super_call_with_parameters; mod type_of_primitive; mod typing_text_str_alias; diff --git a/crates/ruff/src/rules/pyupgrade/rules/super_args.rs b/crates/ruff/src/rules/pyupgrade/rules/super_args.rs deleted file mode 100644 index e1505cd7fe..0000000000 --- a/crates/ruff/src/rules/pyupgrade/rules/super_args.rs +++ /dev/null @@ -1,80 +0,0 @@ -use crate::ast::helpers; -use crate::ast::types::{Range, Scope, ScopeKind}; - -use crate::registry::Diagnostic; - -use crate::rules::pyupgrade::rules::SuperCallWithParameters; - -use rustpython_ast::{ArgData, Expr, ExprKind, Stmt, StmtKind}; - -/// UP008 -pub fn super_args( - scope: &Scope, - parents: &[&Stmt], - expr: &Expr, - func: &Expr, - args: &[Expr], -) -> Option { - if !helpers::is_super_call_with_arguments(func, args) { - return None; - } - - // Check: are we in a Function scope? - if !matches!(scope.kind, ScopeKind::Function { .. }) { - return None; - } - - 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. - let [first_arg, second_arg] = args else { - return None; - }; - - // Find the enclosing function definition (if any). - let Some(StmtKind::FunctionDef { - args: parent_args, .. - }) = parents - .find(|stmt| matches!(stmt.node, StmtKind::FunctionDef { .. })) - .map(|stmt| &stmt.node) else { - return None; - }; - - // Extract the name of the first argument to the enclosing function. - let Some(ArgData { - arg: parent_arg, .. - }) = parent_args.args.first().map(|expr| &expr.node) else { - return None; - }; - - // Find the enclosing class definition (if any). - let Some(StmtKind::ClassDef { - name: parent_name, .. - }) = parents - .find(|stmt| matches!(stmt.node, StmtKind::ClassDef { .. })) - .map(|stmt| &stmt.node) else { - return None; - }; - - let ( - ExprKind::Name { - id: first_arg_id, .. - }, - ExprKind::Name { - id: second_arg_id, .. - }, - ) = (&first_arg.node, &second_arg.node) else { - return None; - }; - - if first_arg_id == parent_name && second_arg_id == parent_arg { - return Some(Diagnostic::new( - SuperCallWithParameters, - Range::from_located(expr), - )); - } - - None -} diff --git a/crates/ruff/src/rules/pyupgrade/rules/super_call_with_parameters.rs b/crates/ruff/src/rules/pyupgrade/rules/super_call_with_parameters.rs index 1010a85a97..716cdd427f 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/super_call_with_parameters.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/super_call_with_parameters.rs @@ -1,11 +1,13 @@ -use crate::define_violation; -use crate::violation::AlwaysAutofixableViolation; -use ruff_macros::derive_message_formats; -use rustpython_ast::{Expr, Stmt}; +use rustpython_ast::{ArgData, Expr, ExprKind, Stmt, StmtKind}; -use super::super::fixes; -use crate::ast::helpers; +use ruff_macros::derive_message_formats; + +use crate::ast::types::{Range, ScopeKind}; use crate::checkers::ast::Checker; +use crate::define_violation; +use crate::registry::Diagnostic; +use crate::rules::pyupgrade::fixes; +use crate::violation::AlwaysAutofixableViolation; define_violation!( pub struct SuperCallWithParameters; @@ -21,11 +23,20 @@ impl AlwaysAutofixableViolation for SuperCallWithParameters { } } +/// Returns `true` if a call is an argumented `super` invocation. +fn is_super_call_with_arguments(func: &Expr, args: &[Expr]) -> bool { + if let ExprKind::Name { id, .. } = &func.node { + id == "super" && !args.is_empty() + } else { + false + } +} + /// UP008 pub fn super_call_with_parameters(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { // Only bother going through the super check at all if we're in a `super` call. - // (We check this in `check_super_args` too, so this is just an optimization.) - if !helpers::is_super_call_with_arguments(func, args) { + // (We check this in `super_args` too, so this is just an optimization.) + if !is_super_call_with_arguments(func, args) { return; } let scope = checker.current_scope(); @@ -34,9 +45,62 @@ pub fn super_call_with_parameters(checker: &mut Checker, expr: &Expr, func: &Exp .iter() .map(std::convert::Into::into) .collect(); - let Some(mut diagnostic) = super::super_args(scope, &parents, expr, func, args) else { + + // Check: are we in a Function scope? + if !matches!(scope.kind, ScopeKind::Function { .. }) { + return; + } + + 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. + let [first_arg, second_arg] = args else { return; }; + + // Find the enclosing function definition (if any). + let Some(StmtKind::FunctionDef { + args: parent_args, .. + }) = parents + .find(|stmt| matches!(stmt.node, StmtKind::FunctionDef { .. })) + .map(|stmt| &stmt.node) else { + return; + }; + + // Extract the name of the first argument to the enclosing function. + let Some(ArgData { + arg: parent_arg, .. + }) = parent_args.args.first().map(|expr| &expr.node) else { + return; + }; + + // Find the enclosing class definition (if any). + let Some(StmtKind::ClassDef { + name: parent_name, .. + }) = parents + .find(|stmt| matches!(stmt.node, StmtKind::ClassDef { .. })) + .map(|stmt| &stmt.node) else { + return; + }; + + let ( + ExprKind::Name { + id: first_arg_id, .. + }, + ExprKind::Name { + id: second_arg_id, .. + }, + ) = (&first_arg.node, &second_arg.node) else { + return; + }; + + if !(first_arg_id == parent_name && second_arg_id == parent_arg) { + return; + } + + let mut diagnostic = Diagnostic::new(SuperCallWithParameters, Range::from_located(expr)); if checker.patch(diagnostic.kind.rule()) { if let Some(fix) = fixes::remove_super_arguments(checker.locator, checker.stylist, expr) { diagnostic.amend(fix);