[pylint] Include name of base class in message for redefined-slots-in-subclass (W0244)
#15559
+75
−59
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
dhruvmanila
reviewed
Jan 18, 2025
Comment on lines
-71
to
+72
| .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| contained_in_super_slots(class_def, semantic, slot)) |
There was a problem hiding this comment.
I think we should change the function name as it's now returning a Diagnostic and not a boolean. Maybe check_super_slots or something similar?
| 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 `Base` |
There was a problem hiding this comment.
nit: "Slot a redefined from base class Base"
Comment on lines
+78
to
+82
| /// The traversal of the class hierarchy is breadth-first. | ||
| pub fn iter_super_class<'stmt>( | ||
| class_def: &'stmt ast::StmtClassDef, | ||
| semantic: &'stmt SemanticModel, | ||
| ) -> impl Iterator<Item = &'stmt ast::StmtClassDef> { |
There was a problem hiding this comment.
What's the reason for doing breadth-first? I think the original implementation did depth-first traversal.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the following situation:
the message for
W0244now specifies thatais overwriting a slot fromGrandparent.To implement this, we introduce a helper function
iter_super_classeswhich 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 .