[ty] Add a subdiagnostic if invalid-return-type is emitted on a method with an empty body on a non-protocol subclass of a protocol class by AlexWaygood · Pull Request #18243 · astral-sh/ruff (original) (raw)
Conversation
Summary
Fixes astral-sh/ty#473.
If invalid-return-type is emitted on a method with an empty body on a non-protocol subclass of a protocol class, the user probably meant for the non-protocol class to actually be a protocol class too. Add a nice subdiagnostic explaining why we do not infer the concrete subclass as being a protocol class.
Test Plan
snapshots
…hod with an empty body on a non-protocol subclass of a protocol class
mypy_primer results
No ecosystem changes detected ✅
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you!
| /// Returns `true` if the current scope is the function body scope of a method of a protocol |
| /// (that is, a class which directly inherits `typing.Protocol`.) |
| fn in_protocol_class(&self) -> bool { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that there were two different uses of this method, and both now become significantly more verbose, would it really be a problem to keep this method as short-hand, even though it's body would now be a one-liner?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In one of the two use cases (the one where we now want to retain the enclosing class context even if the class is not a protocol class), I think it would be a mistake to use this method, because you'd end up doing a lot of work twice in quick succession. So I feel like it's sort-of a footgun to keep it?
This file contains hidden or 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
[ Show hidden characters]({{ revealButtonHref }})
Labels
Related to reporting of diagnostics.
Multi-file analysis & type inference
2 participants
