[pylint] Implement redefined-slots-in-subclass (W0244)
#9640
Conversation
|
There was a problem hiding this comment.
Hy @tsugumi-sys. Thanks for working on this rule. It seems that the code currently fails to compile. Would you mind taking a look why that is?
|
@MichaReiser |
crates/ruff_linter/resources/test/fixtures/pylint/redefined_slots_in_subclass.py
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pylint/rules/redefined_slots_in_subclass.rs
Show resolved
Hide resolved
dylwil3
force-pushed
the
impl-redefined-slots-in-subclass-W0244
branch
from
35bb539 to
6f83375
Compare
|
@tsugumi-sys I've got your branch up-to-date with main (with as few changes as possible). I also moved the proposed rule to preview. Would you be interested in revisiting this PR and addressing dhruv's comments? I'd be happy to help if you run into any trouble. Let me know either way, and thanks for implementing this rule! |
|
@dylwil3 |
No need to apologize! Just wanted to check in, not scold 😄 Glad to hear you're interested in revisiting. Feel free to ping me with any questions or when you're ready for a review, and thanks again! |
dylwil3
force-pushed
the
impl-redefined-slots-in-subclass-W0244
branch
from
ffad4b1 to
d527a5b
Compare
|
Hey @tsugumi-sys , since there weren't too many changes to make I went ahead and made them. Assuming @dhruvmanila agrees that this fixes the previous concerns, we can get this merged in. Thanks so much for your contribution! |
|
@dylwil3 |
| // This function checks every super class | ||
| // but we want every _strict_ super class, hence: | ||
| if class_def.name == super_class.name { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Unrelated to this PR, this is interesting as it seems like a confusing behavior given the name of the function (any_super_class).
There was a problem hiding this comment.
It's confusing but I think technically correct since any class is a superclass of itself
>>> class Foo:...
...
>>> issubclass(Foo,Foo)
TrueI like your idea of implementing iter_super_class. Then we can leave any_super_class as is, but use iter_super_class with a skip to get this behavior.
crates/ruff_linter/src/rules/pylint/rules/redefined_slots_in_subclass.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/pylint/rules/redefined_slots_in_subclass.rs
Outdated
Show resolved
Hide resolved
|
@tsugumi-sys no worries at all! you did the bulk of the work, so not much left on my end. thank you! |
* main: [red-knot] Inline `SubclassOfType::as_instance_type_of_metaclass()` (#15556) [`flake8-comprehensions`] strip parentheses around generators in `unnecessary-generator-set` (`C401`) (#15553) [`pylint`] Implement `redefined-slots-in-subclass` (`W0244`) (#9640) [`flake8-bugbear`] Do not raise error if keyword argument is present and target-python version is less or equals than 3.9 (`B903`) (#15549) [red-knot] `type[T]` is disjoint from `type[S]` if the metaclass of `T` is disjoint from the metaclass of `S` (#15547) [red-knot] Pure instance variables declared in class body (#15515) Update snapshots of #15507 with new annotated snipetts rendering (#15546) [`pylint`] Do not report methods with only one `EM101`-compatible `raise` (`PLR6301`) (#15507) Fix unstable f-string formatting for expressions containing a trailing comma (#15545) Support `knot.toml` files in project discovery (#15505) Add support for configuring knot in `pyproject.toml` files (#15493) Fix bracket spacing for single-element tuples in f-string expressions (#15537) [`flake8-simplify`] Do not emit diagnostics for expressions inside string type annotations (`SIM222`, `SIM223`) (#15405) [`flake8-pytest-style`] Do not emit diagnostics for empty `for` loops (`PT012`, `PT031`) (#15542)
…-in-subclass` (`W0244`) (#15559) 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 .
Summary
Test Plan