mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-03 10:23:11 +00:00
Ignore some common builtin overrides on standard library subclasses (#6074)
## Summary If a user subclasses `threading.Event`, e.g. with: ```python from threading import Event class CustomEvent(Event): def set(self) -> None: ... ``` They no control over the method name (`set`). This PR allows `threading.Event#set` and `logging.Filter#filter` overrides, and avoids flagging A003 in such cases. Ideally, we'd avoid flagging all overridden methods, but... that's a lot more difficult, and this is at least _better_ than what we do now. Closes https://github.com/astral-sh/ruff/issues/6057. Closes https://github.com/astral-sh/ruff/issues/5956.
This commit is contained in:
parent
c996b614fe
commit
5f63b8bfb8
4 changed files with 88 additions and 0 deletions
|
@ -17,3 +17,25 @@ from typing import TypedDict
|
|||
|
||||
class MyClass(TypedDict):
|
||||
id: int
|
||||
|
||||
|
||||
from threading import Event
|
||||
|
||||
|
||||
class CustomEvent(Event):
|
||||
def set(self) -> None:
|
||||
...
|
||||
|
||||
def str(self) -> None:
|
||||
...
|
||||
|
||||
|
||||
from logging import Filter, LogRecord
|
||||
|
||||
|
||||
class CustomFilter(Filter):
|
||||
def filter(self, record: LogRecord) -> bool:
|
||||
...
|
||||
|
||||
def str(self) -> None:
|
||||
...
|
||||
|
|
|
@ -4,6 +4,7 @@ use rustpython_parser::ast;
|
|||
use ruff_diagnostics::Diagnostic;
|
||||
use ruff_diagnostics::Violation;
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_semantic::SemanticModel;
|
||||
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::rules::flake8_builtins::helpers::shadows_builtin;
|
||||
|
@ -80,6 +81,12 @@ pub(crate) fn builtin_attribute_shadowing(
|
|||
return;
|
||||
}
|
||||
|
||||
// Ignore some standard-library methods. Ideally, we'd ignore all overridden methods, since
|
||||
// those should be flagged on the superclass, but that's more difficult.
|
||||
if is_standard_library_override(name, class_def, checker.semantic()) {
|
||||
return;
|
||||
}
|
||||
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
BuiltinAttributeShadowing {
|
||||
name: name.to_string(),
|
||||
|
@ -88,3 +95,26 @@ pub(crate) fn builtin_attribute_shadowing(
|
|||
));
|
||||
}
|
||||
}
|
||||
|
||||
/// Return `true` if an attribute appears to be an override of a standard-library method.
|
||||
fn is_standard_library_override(
|
||||
name: &str,
|
||||
class_def: &ast::StmtClassDef,
|
||||
model: &SemanticModel,
|
||||
) -> bool {
|
||||
match name {
|
||||
// Ex) `Event#set`
|
||||
"set" => class_def.bases.iter().any(|base| {
|
||||
model.resolve_call_path(base).map_or(false, |call_path| {
|
||||
matches!(call_path.as_slice(), ["threading", "Event"])
|
||||
})
|
||||
}),
|
||||
// Ex) `Filter#filter`
|
||||
"filter" => class_def.bases.iter().any(|base| {
|
||||
model.resolve_call_path(base).map_or(false, |call_path| {
|
||||
matches!(call_path.as_slice(), ["logging", "Filter"])
|
||||
})
|
||||
}),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
|
|
@ -38,4 +38,22 @@ A003.py:11:9: A003 Class attribute `str` is shadowing a Python builtin
|
|||
12 | pass
|
||||
|
|
||||
|
||||
A003.py:29:9: A003 Class attribute `str` is shadowing a Python builtin
|
||||
|
|
||||
27 | ...
|
||||
28 |
|
||||
29 | def str(self) -> None:
|
||||
| ^^^ A003
|
||||
30 | ...
|
||||
|
|
||||
|
||||
A003.py:40:9: A003 Class attribute `str` is shadowing a Python builtin
|
||||
|
|
||||
38 | ...
|
||||
39 |
|
||||
40 | def str(self) -> None:
|
||||
| ^^^ A003
|
||||
41 | ...
|
||||
|
|
||||
|
||||
|
||||
|
|
|
@ -19,4 +19,22 @@ A003.py:11:9: A003 Class attribute `str` is shadowing a Python builtin
|
|||
12 | pass
|
||||
|
|
||||
|
||||
A003.py:29:9: A003 Class attribute `str` is shadowing a Python builtin
|
||||
|
|
||||
27 | ...
|
||||
28 |
|
||||
29 | def str(self) -> None:
|
||||
| ^^^ A003
|
||||
30 | ...
|
||||
|
|
||||
|
||||
A003.py:40:9: A003 Class attribute `str` is shadowing a Python builtin
|
||||
|
|
||||
38 | ...
|
||||
39 |
|
||||
40 | def str(self) -> None:
|
||||
| ^^^ A003
|
||||
41 | ...
|
||||
|
|
||||
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue