Fix false positive in PLR6301 (#7933)

## Summary

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

Closes #6961

## Test Plan

`cargo test`
This commit is contained in:
Victor Hugo Gomes 2023-10-14 15:55:38 -03:00 committed by GitHub
parent bd06cbe0c5
commit e261eb7461
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 68 additions and 32 deletions

View file

@ -1,5 +1,7 @@
import abc
from typing_extensions import override
class Person:
def developer_greeting(self, name): # [no-self-use]
@ -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():
...

View file

@ -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);
}
}
}

View file

@ -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;
@ -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;
};
@ -105,8 +110,25 @@ pub(crate) fn no_self_use(checker: &Checker, scope: &Scope, diagnostics: &mut Ve
return;
};
if parameter.name.as_str() == "self"
&& scope
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())

View file

@ -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
no_self_use.py:10: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]
8 | print(f"Greetings {name}!")
9 |
10 | def greeting_1(self): # [no-self-use]
| ^^^^ PLR6301
9 | print("Hello!")
11 | print("Hello!")
|
no_self_use.py:11:20: PLR6301 Method `greeting_2` could be a function or static method
no_self_use.py:13:20: PLR6301 Method `greeting_2` could be a function or static method
|
9 | print("Hello!")
10 |
11 | def greeting_2(self): # [no-self-use]
11 | print("Hello!")
12 |
13 | def greeting_2(self): # [no-self-use]
| ^^^^ PLR6301
12 | print("Hi!")
|
no_self_use.py:55:25: PLR6301 Method `abstract_method` could be a function or static method
|
53 | class Sub(Base):
54 | @override
55 | def abstract_method(self):
| ^^^^ PLR6301
56 | print("concrete method")
14 | print("Hi!")
|