Skip to content

Commit

Permalink
Fix false positive in PLR6301 (#7933)
Browse files Browse the repository at this point in the history
## Summary

Don't report a diagnostic if the method contains a `super()` call.

Closes #6961

## Test Plan

`cargo test`
  • Loading branch information
LaBatata101 committed Oct 14, 2023
1 parent bd06cbe commit e261eb7
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 32 deletions.
23 changes: 23 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pylint/no_self_use.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import abc

from typing_extensions import override


class Person:
def developer_greeting(self, name): # [no-self-use]
Expand Down Expand Up @@ -60,3 +62,24 @@ class Prop:
@property
def count(self):
return 24


class A:
def foo(self):
...


class B(A):
@override
def foo(self):
...

def foobar(self):
super()

def bar(self):
super().foo()

def baz(self):
if super().foo():
...
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
36 changes: 29 additions & 7 deletions crates/ruff_linter/src/rules/pylint/rules/no_self_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ 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 @@ -45,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 @@ -105,11 +110,28 @@ pub(crate) fn no_self_use(checker: &Checker, scope: &Scope, diagnostics: &mut Ve
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 parameter.name.as_str() != "self" {
return;
}

// 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") {
let binding = checker.semantic().binding(binding_id);
if binding.kind.is_builtin() {
if binding
.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 Down
Original file line number Diff line number Diff line change
@@ -1,39 +1,30 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
no_self_use.py:5:28: PLR6301 Method `developer_greeting` could be a function or static method
no_self_use.py:7:28: PLR6301 Method `developer_greeting` could be a function or static method
|
4 | class Person:
5 | def developer_greeting(self, name): # [no-self-use]
6 | class Person:
7 | def developer_greeting(self, name): # [no-self-use]
| ^^^^ PLR6301
6 | print(f"Greetings {name}!")
8 | print(f"Greetings {name}!")
|

no_self_use.py:8:20: PLR6301 Method `greeting_1` could be a function or static method
|
6 | print(f"Greetings {name}!")
7 |
8 | def greeting_1(self): # [no-self-use]
| ^^^^ PLR6301
9 | print("Hello!")
|

no_self_use.py:11:20: PLR6301 Method `greeting_2` could be a function or static method
no_self_use.py:10:20: PLR6301 Method `greeting_1` could be a function or static method
|
9 | print("Hello!")
10 |
11 | def greeting_2(self): # [no-self-use]
8 | print(f"Greetings {name}!")
9 |
10 | def greeting_1(self): # [no-self-use]
| ^^^^ PLR6301
12 | print("Hi!")
11 | print("Hello!")
|

no_self_use.py:55:25: PLR6301 Method `abstract_method` could be a function or static method
no_self_use.py:13:20: PLR6301 Method `greeting_2` could be a function or static method
|
53 | class Sub(Base):
54 | @override
55 | def abstract_method(self):
| ^^^^ PLR6301
56 | print("concrete method")
11 | print("Hello!")
12 |
13 | def greeting_2(self): # [no-self-use]
| ^^^^ PLR6301
14 | print("Hi!")
|


0 comments on commit e261eb7

Please sign in to comment.