From aa8ff077136579dc00390ebac16725a91c5bee37 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 13 Oct 2023 15:48:01 -0400 Subject: [PATCH] Use bindings --- .../checkers/ast/analyze/deferred_scopes.rs | 2 +- .../src/rules/pylint/rules/no_self_use.rs | 59 ++++++++----------- 2 files changed, 26 insertions(+), 35 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs index 493186ec444ec4..e3b18d0685a274 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -306,7 +306,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { if scope.kind.is_function() { if checker.enabled(Rule::NoSelfUse) { - pylint::rules::no_self_use(checker, scope, &mut diagnostics); + pylint::rules::no_self_use(checker, scope_id, scope, &mut diagnostics); } } } diff --git a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs index 9cb1b4d9277a4d..a5c6d253c68635 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs @@ -1,12 +1,10 @@ -use ast::visitor::{self, Visitor}; -use ast::Expr; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::{from_qualified_name, CallPath}; use ruff_python_ast::{self as ast, ParameterWithDefault}; use ruff_python_semantic::{ analyze::{function_type, visibility}, - Scope, ScopeKind, + Scope, ScopeId, ScopeKind, }; use ruff_text_size::Ranged; @@ -47,7 +45,12 @@ impl Violation for NoSelfUse { } /// PLR6301 -pub(crate) fn no_self_use(checker: &Checker, scope: &Scope, diagnostics: &mut Vec) { +pub(crate) fn no_self_use( + checker: &Checker, + scope_id: ScopeId, + scope: &Scope, + diagnostics: &mut Vec, +) { let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else { return; }; @@ -107,19 +110,27 @@ pub(crate) fn no_self_use(checker: &Checker, scope: &Scope, diagnostics: &mut Ve return; }; - // Traverse the method's body looking for `super()` calls - let mut call_visitor = CallVisitor { is_super: false }; - call_visitor.visit_body(body); - - if call_visitor.is_super { + if parameter.name.as_str() != "self" { return; } - if parameter.name.as_str() == "self" - && scope - .get("self") - .map(|binding_id| checker.semantic().binding(binding_id)) - .is_some_and(|binding| binding.kind.is_argument() && !binding.is_used()) + // If the method contains a `super` reference, then it should be considered to use self + // implicitly. + if let Some(binding_id) = checker.semantic().global_scope().get("super") { + if checker + .semantic() + .binding(binding_id) + .references() + .any(|id| checker.semantic().reference(id).scope_id() == scope_id) + { + return; + } + } + + if scope + .get("self") + .map(|binding_id| checker.semantic().binding(binding_id)) + .is_some_and(|binding| binding.kind.is_argument() && !binding.is_used()) { diagnostics.push(Diagnostic::new( NoSelfUse { @@ -129,23 +140,3 @@ pub(crate) fn no_self_use(checker: &Checker, scope: &Scope, diagnostics: &mut Ve )); } } - -struct CallVisitor { - is_super: bool, -} - -impl<'a> Visitor<'a> for CallVisitor { - fn visit_expr(&mut self, expr: &'a Expr) { - match expr { - Expr::Call(ast::ExprCall { func, .. }) => match func.as_ref() { - Expr::Name(expr_name) => { - self.is_super = expr_name.id == "super"; - } - _ => { - visitor::walk_expr(self, expr); - } - }, - _ => visitor::walk_expr(self, expr), - } - } -}