Skip to content

Commit

Permalink
Use bindings
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Oct 13, 2023
1 parent d361a83 commit aa8ff07
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
59 changes: 25 additions & 34 deletions crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -47,7 +45,12 @@ impl Violation for NoSelfUse {
}

/// PLR6301
pub(crate) fn no_self_use(checker: &Checker, scope: &Scope, diagnostics: &mut Vec<Diagnostic>) {
pub(crate) fn no_self_use(
checker: &Checker,
scope_id: ScopeId,
scope: &Scope,
diagnostics: &mut Vec<Diagnostic>,
) {
let Some(parent) = &checker.semantic().first_non_type_parent_scope(scope) else {
return;
};
Expand Down Expand Up @@ -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 {
Expand All @@ -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),
}
}
}

0 comments on commit aa8ff07

Please sign in to comment.