[red-knot] Ban direct instantiations of Protocol classes
#17597
Conversation
AlexWaygood
force-pushed
the
alex/ban-protocol-instantiation
branch
from
1fa7488 to
ef3a4b7
Compare
|
| let db = context.db(); | ||
| let class_name = protocol.name(db); | ||
| let mut diagnostic = builder.into_diagnostic(format_args!( | ||
| "Cannot instantiate abstract class `{class_name}`", |
There was a problem hiding this comment.
Are you sure that it's clearest to use the term "abstract" for this? I can see why it's not unreasonable, but both mypy and pyright just say "Cannot instantiate protocol class 'MyProtocol'". It seems like introducing the term "abstract" might be unnecessary complexity, and evoke only-sort-of-related things like ABCMeta? It just forces us to explain below that inheriting Protocol makes a class "implicitly abstract", which seems unnecessary when we could just as well say "it inherits Protocol, making it a protocol class, and protocol classes can't be instantiated"
There was a problem hiding this comment.
Hmm, fair point. In my mental model, protocol classes are banned from being instantiated for exactly the same reason that ABCs with abstract methods are banned from being instantiated: both classes represent abstract interfaces to some extent (with the former really just being an extension of the latter in terms of how far you take that idea). But using the term "abstract" probably doesn't help our users, who won't necessary have the same mental model!
* main: [red-knot] fix collapsing literal and its negation to object (#17605) [red-knot] Add more tests for protocols (#17603) [red-knot] Ban direct instantiations of `Protocol` classes (#17597) [`pyupgrade`] Preserve parenthesis when fixing native literals containing newlines (`UP018`) (#17220) [`airflow`] fix typos (`AIR302`, `AIR312`) (#17574) [red-knot] Special case `@abstractmethod` for function type (#17591) [red-knot] Emit diagnostics for isinstance() and issubclass() calls where a non-runtime-checkable protocol is the second argument (#17561) [red-knot] Infer the members of a protocol class (#17556) [red-knot] Add `FunctionType::to_overloaded` (#17585) [red-knot] Add mdtests for `global` statement (#17563) [syntax-errors] Make duplicate parameter names a semantic error (#17131)
2 participants
Summary
Protocol classes are abstract and thus cannot be directly instantiated (although concrete subclasses of protocols can be!). This PR adds logic to detect attempts to instantiate them, and report such attempts as diagnostics.
I also factored out some common code from
diagnostics.rsintoclass.rs.Test Plan
Existing mdtests updated, and snapshots added.
Local screenshot to show what it looks like in the terminal: