Add deprecation subcategories, test deprecations properly including in perldiag and perlderprecation.pod #20916
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
… category as parameter we should have a deprecation category per type of deprecation
Currently we seem to lack a way to have a subcategory under deprecated.
It seems reasonable to me that people might want to disable a specific
subcategory warning while leaving the rest in place. This patch allows
that. Note that both
no warnings "deprecated";
and
no warnings "deprecated::smartmatch";
work to disable the warning. Deprecated warnings shouldn't be "all or
nothing", they should be specific and targetted.
Instead of using a generic warnings category switch to fine grained control.
This category is only used in the regex engine, we should be able to disable it specifically, as it seems like we will never actually remove demove support for the things it warns about.
…deprecation category This category is about use of apostrophe as a package separator, eg for things like "Test::More::isn't()".
Some delimiters are considered deprecated because in the future they will be used as part of a paired delimiter. This adds a new category for these cases.
This category applies to attempts to goto the internals of a block construct.
This also fixes the version_downgrade to show the correct version that version downgrades will be removed in.
This makes it easier to see the order the categories were added in.
Also do not generate PERL_ARGS style macros for macros.
…cros the "when" parameter is expected to be a version string of the form "5.\d+", with no minor version.
This was missed when smartmatch was deprecated.
It was incorrectly documented as going away in 5.40
Multiple deprecation types that are not scheduled for removal in a specific version were not listed. Also now that we have deprecation subcategories we should specify them in the docs.
demerphq
force-pushed
the
yves/deprecation_subcategories
branch
from
70c1072
to
0b2570f
Compare
|
@leonerd you said someone objected to this, but nobody has comment on this PR, nor on the thread on p5p. Can you ask them to comment here please? |
Based on the PSC "PSC #101, 2023-03-17" comment above, and the fact noone has raised any objections, I am merging this. |
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.
Currently we use a single warning category for all deprecations. This is dangerous in that it allows a new deprecation to sneak in when the user only wanted to allow a different deprecation, and IMO counterproductive as it means there is no way to know /which/ deprecation someone is silencing in their code and give them good feedback when the feature is no longer supported.
Having only one category essentially says "if you want to risk using a deprecation then you have to risk not knowing about any deprecation", but this does not make sense. Why should I have to take maximal risk when I want to take a known risk?It makes even less sense when you consider we have at least three categories which are long standing and still not scheduled for removal. If a user wants to silence one of these long standing deprecation types they essentially must risk not being informed about any other category at all. That seems unhelpful and unnecessarily risky.
By having all deprecations be subcategorized in regen/warnings.pl we also get some built in support for tracking the version that a deprecation was added, which can then be used to test that our various files provide sufficient dev cycles before they are removed.
It also makes it more obvious that we have unscheduled deprecations that have been around for a very long time, and which will likely always be present (or so it seems).
This PR adds support for deprecation subcategories, it converts all existing deprecations to use subcategories, it fixes the API's of all deprecation related functionality that I could find to require a category, and it ensures that diag.t knows about these functions so it can test the diagnostics produced, it also validates that perldeprecation.pod metions all existing deprecation categories and that they are documented with sufficient version cycles before they are removed.
I understand that the PSC has "decided" that it is a bad idea to have deprecation subcategories. I believe that this decision is wrong, and should not have been made without wider discusion. This patch demonstrates why it is a good idea to have these subcategories. It facilitates testing and documenting the categories, it allows our users to manage their risk at a more granular level, and it allows us quick access as to which deprecations are "live" at any given point in time. The counter arguments that I have seen expressed, for instance here: #20855 (comment) and #20855 do not seem very compelling compared to the advantages enumerated here, and demonstrated by this PR.