Print deprecation warning on types and aliases #15962
Conversation
We could already deprecate a type and have the documentation report the deprecation. We now also print a deprecation warning during compilation when we try to access the type, when calling a class method on the type (no need to deprecate every method) but also when extending a type during a class (or struct) definition. There are more cases that haven't been implemented, yet (include, extend, instance methods, ...). We can also now deprecate an alias, which prints a deprecation warning and documents the deprecation.
|
There's a deprecation to take care of. Trying to compile crystal or the std specs, we get the following deprecation (6 times): We don't use the types (we use |
|
Unless we can detect that a class is used or not, and in that case skip checking its superclass for deprecation, maybe we could deprecate each specific ABI and check the superclass if the class itself ain't deprecated 🤔 |
|
Perhaps subclasses should implicitly inherit the deprecation of their parent class? I suppose there could be some use cases where you don't want that, though. I.e. only deprecate the parent class, not the children, but the inheritance relationship needs to be kept until the deprecated parent is actually removed. Maybe that could be configured with an annotation property? But I'd leave that for later. Implicitly deprecating all children seems like a more useful default behaviour. |
|
A deprecated use site referring to another deprecated entity should not emit a warning. A deprecated class inheriting from another deprecated one is fine, the same way methods behave (#13513). I would leave the annotation on both parents and subclasses. |
This avoids warnings about the superclass just because we required LLVM and that required the deprecated abi classes.
I did just that: don't check superclass for deprecation if the class is deprecated, and annotated the LLVM::ABI::* types with a deprecation. No more warnings until we use them. |
| @@ -140,6 +140,13 @@ module Crystal | |||
| def transform(node : ClassDef) | |||
| super | |||
|
|
|||
| # check superclass for deprecation if the node isn't deprecated | |||
| if (type = node.type.lookup_type?(node.name)) && !type.annotation(@program.deprecated_annotation) | |||
There was a problem hiding this comment.
BTW: node.type is NilType ... I must lookup the type by its name (actually a Path) to get the actual type for the ClassDef node 🙃
There's probably a reason, but it escapes me.
Co-authored-by: Johannes Müller <[email protected]>
It documents each individual class as deprecated, not just the LLVM::ABI superclass. This will also avoid warnings about the superclass after crystal-lang#15962 just because we required LLVM and that in turn required the deprecated abi classes.
|
I added a couple specs for the namespace and instance methods (unaffected). We should detect |
|
And another one for referencing a deprecated type, such as |
It documents each individual class as deprecated, not just the `LLVM::ABI` superclass. This will also avoid warnings about the superclass after #15962 just because we required LLVM and that in turn required the deprecated abi classes.
|
Use in generic arguments does not yet trigger a deprecation warning in this branch (#11043 (comment)). |
|
Argh, Generics. That eluded me. Let's say a follow-up, along with |
|
They're both actually pretty simple given the prep work you did in this PR. I added some commits directly to this branch. |
|
Oh, thanks a ton @straight-shoota 🙇 |
straight-shoota
added
topic:compiler:semantic
topic:lang:annotations
and removed
topic:compiler
labels
We can already deprecate a type and have the documentation report the deprecation, but there was no warning when trying to access the type as reported in #11043.
This PR enables a few use cases to also print a deprecation warning during compilation: when calling a class method on the type (no need to deprecate every method) which includes the constructors, when directly referencing a type (e.g.
p Foo) and when extending a type in a class definition. There are more cases, but these should already cover a bunch.This will be useful for #15805 for example:
This PR also allows to deprecate an alias, which prints a deprecation warning and documents the deprecation. For example after renaming the contexts in #15936 and create deprecated aliases for the legacy names (instead of being fully silent).