mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-04 10:48:32 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[pylint] Include name of base class in message for redefined-slots-in-subclass (W0244) (#15559)

Browse source 
In the following situation:

```python
class Grandparent:
  __slots__ = "a"

class Parent(Grandparent): ...

class Child(Parent):
  __slots__ = "a"
```

the message for `W0244` now specifies that `a` is overwriting a slot
from `Grandparent`.

To implement this, we introduce a helper function `iter_super_classes`
which does a breadth-first traversal of the superclasses of a given
class (as long as they are defined in the same file, due to the usual
limitations of the semantic model).

Note: Python does not allow conflicting slots definitions under multiple
inheritance. Unless I'm misunderstanding something, I believe It follows
that the subposet of superclasses of a given class that redefine a given
slot is in fact totally ordered. There is therefore a unique _nearest_
superclass whose slot is being overwritten. So, you know, in case anyone
was super worried about that... you can just chill.

This is a followup to #9640 .
This commit is contained in:
Dylan 2025-01-18 09:50:27 -06:00 • committed by GitHub
parent fb15da5694
commit 4caeeb8d98
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 79 additions and 62 deletions
Download patch file Download diff file Expand all files Collapse all files

41
crates/ruff_linter/src/rules/pylint/rules/redefined_slots_in_subclass.rs
View file

@ -1,6 +1,6 @@
use std::hash::Hash;
use ruff_python_semantic::{analyze::class::any_super_class, SemanticModel};
use ruff_python_semantic::{analyze::class::iter_super_class, SemanticModel};
use rustc_hash::FxHashSet;
use ruff_diagnostics::{Diagnostic, Violation};
@ -39,14 +39,15 @@ use crate::checkers::ast::Checker;
/// ```
#[derive(ViolationMetadata)]
pub(crate) struct RedefinedSlotsInSubclass {
name: String,
base: String,
slot_name: String,
}
impl Violation for RedefinedSlotsInSubclass {
#[derive_message_formats]
fn message(&self) -> String {
let RedefinedSlotsInSubclass { name } = self;
format!("Redefined slot '{name}' in subclass")
let RedefinedSlotsInSubclass { base, slot_name } = self;
format!("Slot `{slot_name}` redefined from base class `{base}`")
}
}
@ -68,15 +69,7 @@ pub(crate) fn redefined_slots_in_subclass(checker: &mut Checker, class_def: &ast
let semantic = checker.semantic();
let mut diagnostics: Vec<_> = class_slots
.iter()
.filter(|&slot| contained_in_super_slots(class_def, semantic, slot))
.map(|slot| {
Diagnostic::new(
RedefinedSlotsInSubclass {
name: slot.name.to_string(),
},
slot.range(),
)
})
.filter_map(|slot| check_super_slots(class_def, semantic, slot))
.collect();
checker.diagnostics.append(&mut diagnostics);
}
@ -111,18 +104,24 @@ impl Ranged for Slot<'_> {
}
}
fn contained_in_super_slots(
fn check_super_slots(
class_def: &ast::StmtClassDef,
semantic: &SemanticModel,
slot: &Slot,
) -> bool {
any_super_class(class_def, semantic, &|super_class| {
// This function checks every super class
// but we want every _strict_ super class, hence:
if class_def.name == super_class.name {
return false;
) -> Option<Diagnostic> {
iter_super_class(class_def, semantic)
.skip(1)
.find_map(&|super_class: &ast::StmtClassDef| {
if slots_members(&super_class.body).contains(slot) {
return Some(Diagnostic::new(
RedefinedSlotsInSubclass {
base: super_class.name.to_string(),
slot_name: slot.name.to_string(),
},
slot.range(),
));
}
slots_members(&super_class.body).contains(slot)
None
})
}

8
crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW0244_redefined_slots_in_subclass.py.snap
View file

@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
redefined_slots_in_subclass.py:6:18: PLW0244 Redefined slot 'a' in subclass
redefined_slots_in_subclass.py:6:18: PLW0244 Slot `a` redefined from base class `Base`
|
5 | class Subclass(Base):
6 | __slots__ = ("a", "d") # [redefined-slots-in-subclass]
@ -10,7 +10,7 @@ redefined_slots_in_subclass.py:6:18: PLW0244 Redefined slot 'a' in subclass
8 | class Grandparent:
|
redefined_slots_in_subclass.py:17:23: PLW0244 Redefined slot 'a' in subclass
redefined_slots_in_subclass.py:17:23: PLW0244 Slot `a` redefined from base class `Grandparent`
|
16 | class Child(Parent):
17 | __slots__ = ("c", "a")
@ -19,14 +19,14 @@ redefined_slots_in_subclass.py:17:23: PLW0244 Redefined slot 'a' in subclass
19 | class AnotherBase:
|
redefined_slots_in_subclass.py:23:18: PLW0244 Redefined slot 'a' in subclass
redefined_slots_in_subclass.py:23:18: PLW0244 Slot `a` redefined from base class `AnotherBase`
|
22 | class AnotherChild(AnotherBase):
23 | __slots__ = ["a","b","e","f"]
| ^^^ PLW0244
|
redefined_slots_in_subclass.py:23:22: PLW0244 Redefined slot 'b' in subclass
redefined_slots_in_subclass.py:23:22: PLW0244 Slot `b` redefined from base class `AnotherBase`
|
22 | class AnotherChild(AnotherBase):
23 | __slots__ = ["a","b","e","f"]

88
crates/ruff_python_semantic/src/analyze/class.rs
View file

@ -1,3 +1,5 @@
use std::collections::VecDeque;
use rustc_hash::FxHashSet;
use crate::analyze::typing;
@ -70,46 +72,62 @@ pub fn any_base_class(
inner(class_def, semantic, func, &mut FxHashSet::default())
}
/// Return `true` if any base class matches an [`ast::StmtClassDef`] predicate.
/// Returns an iterator over all base classes, beginning with the
/// given class.
///
/// The traversal of the class hierarchy is breadth-first, since
/// this graph tends to have small width but could be rather deep.
pub fn iter_super_class<'stmt>(
class_def: &'stmt ast::StmtClassDef,
semantic: &'stmt SemanticModel,
) -> impl Iterator<Item = &'stmt ast::StmtClassDef> {
struct SuperClassIterator<'stmt> {
semantic: &'stmt SemanticModel<'stmt>,
to_visit: VecDeque<&'stmt ast::StmtClassDef>,
seen: FxHashSet<BindingId>,
}
impl<'a> Iterator for SuperClassIterator<'a> {
type Item = &'a ast::StmtClassDef;
fn next(&mut self) -> Option<Self::Item> {
let current = self.to_visit.pop_front()?;
// Add all base classes to the to_visit list
for expr in current.bases() {
if let Some(id) = self.semantic.lookup_attribute(map_subscript(expr)) {
if self.seen.insert(id) {
let binding = self.semantic.binding(id);
if let Some(base_class) = binding
.kind
.as_class_definition()
.map(|id| &self.semantic.scopes[*id])
.and_then(|scope| scope.kind.as_class())
{
self.to_visit.push_back(base_class);
}
}
}
}
Some(current)
}
}
SuperClassIterator {
semantic,
to_visit: VecDeque::from([class_def]),
seen: FxHashSet::default(),
}
}
/// Return `true` if any base class, including the given class,
/// matches an [`ast::StmtClassDef`] predicate.
pub fn any_super_class(
class_def: &ast::StmtClassDef,
semantic: &SemanticModel,
func: &dyn Fn(&ast::StmtClassDef) -> bool,
) -> bool {
fn inner(
class_def: &ast::StmtClassDef,
semantic: &SemanticModel,
func: &dyn Fn(&ast::StmtClassDef) -> bool,
seen: &mut FxHashSet<BindingId>,
) -> bool {
// If the function itself matches the pattern, then this does too.
if func(class_def) {
return true;
}
// Otherwise, check every base class.
class_def.bases().iter().any(|expr| {
// If the base class extends a class that matches the pattern, then this does too.
if let Some(id) = semantic.lookup_attribute(map_subscript(expr)) {
if seen.insert(id) {
let binding = semantic.binding(id);
if let Some(base_class) = binding
.kind
.as_class_definition()
.map(|id| &semantic.scopes[*id])
.and_then(|scope| scope.kind.as_class())
{
if inner(base_class, semantic, func, seen) {
return true;
}
}
}
}
false
})
}
inner(class_def, semantic, func, &mut FxHashSet::default())
iter_super_class(class_def, semantic).any(func)
}
#[derive(Clone, Debug)]