-
-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ Native handling of sniff deprecations #281
Merged
Merged
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
1 task
Unless anyone leaves a comment/feedback on this PR, I will merge it tomorrow. |
As per 164, this commit introduces a new feature to PHPCS: native handling of sniff deprecations. ### Background There are quite a few sniffs slated for removal in PHPCS 4.0 - 45 so far, to be exact - and save for two sniffs which have received a deprecation mention in the changelogs, this has only been announced in issues (squizlabs/PHP_CodeSniffer 2448 + squizlabs/PHP_CodeSniffer 2471) in the Squizlabs repo and should therefore only be considered "known" to a very small group of people. Increasing awareness of the upcoming sniff removals should allow for a smoother upgrade experience to PHPCS 4.0 for end-users. Aside from use by PHPCS itself, this feature can also be used by external standards to signal sniff deprecations to _their_ end-users. All in all, this feature should hopefully improve the end-user experience. ### Policy for use of this feature in PHP_CodeSniffer itself As per the notes in 188, the intention is for PHPCS itself to use this feature as follows: * Soft deprecate (changelog notice and `@deprecated` tag in sniff) sniffs during the lifetime of a major. * Hard deprecate sniffs, i.e. implement the `DeprecatedSniff` interface, in the last minor before the next major release. External standards are, of course, free to apply a different deprecation policy. ### Implementation details This commit introduces: #### A new `PHP_CodeSniffer\Sniffs\DeprecatedSniff` interface This interface enforces the implementation of three new methods: * `getDeprecationVersion(): string` - the return value should be a non-empty string with the version in which the sniff was deprecated. * `getRemovalVersion(): string` - the return value should be a non-empty string with the version in which the sniff will be removed. If the removal version is not yet known, it is recommended to set this to: "a future version". * `getDeprecationMessage(): string` - the return value allows for an arbitrary message to be added, such as a replacement recommendation. If no additional information needs to be conveyed to end-users, an empty string can be returned. Custom messages are allowed to contain new lines. #### Changes to the `Ruleset` class to allow for showing the deprecation notices The Ruleset class contains two new methods: * `hasSniffDeprecations(): bool` to allow for checking whether a loading ruleset includes deprecated sniffs. * `showSniffDeprecations(): void` to display the applicable deprecation notices. The implementation for showing the sniff deprecation notices is as follows: * No notices will be shown when PHPCS is running with the `-q` flag (quite mode). * No notices will be shown when PHPCS was giving the `-e` flag to "explain" a standard (list all sniffs). * No notices will be shown when PHPCS was asked to display documentation using the `--generator=...` CLI argument. * No notices will be shown when PHPCS is asked for information which doesn't involve loading a ruleset, such as running `phpcs` with the any of the following flags: `-i` (listing installed standards), `--version`, `--help`, `--config-show`, `--config-set`, `--config-delete`. * Only deprecation notices will be shown for _active_ sniffs. This means that when `--exclude=...` is used and deprecated sniffs are excluded, no notices will be shown for the excluded sniffs. It also means that when `--sniffs=...` is used, deprecation notices will only be shown for those sniffs selected (if deprecated). * The deprecation notices have no impact on the PHPCS (or PHPCBF) run itself. - Deprecated sniffs will still be loaded and run. - Properties can be set on deprecated sniffs. - The exit codes of runs are not affected by whether or not a ruleset contains deprecated sniffs. * As things are, deprecation notices will show both for `phpcs` as well as `phpcbf` runs. I did considered silencing them by default for `phpcbf`. For now, however, I've decided against this as the whole point of showing deprecation notices is to increase awareness of upcoming changes and as a subsection of the PHPCS users only run `phpcbf` and rarely `phpcs`, silencing the notices for `phpcbf` could be counter-productive. Additional implementation notes: * Any user set `--report-width` will be respected. This includes when the `report-width` is set to `auto`. * New lines in custom messages will be normalized to be suitable for the OS of the end-user. * The output will have select colourization if `--colors` is turned on. * The deprecation notices will not be included in generated reports saved to file using `--report-file=...` (and variants thereof). * If the interface is implemented incorrectly, a `PHP_CodeSniffer\Exceptions\RuntimeException` will be thrown. The intention is to add return type declarations to the interface in the future. The complete new feature is extensively covered by tests. Related issues: 188, 276 Fixes 164
This commit makes a small adjustment to the output of the `-e` (explain) command. Deprecated sniffs will now be marked with a trailing `*` asterix and if the standard contains deprecated sniffs, a line will show at the end of the output to explain that the `*` means that a sniff is deprecated. Includes a test documenting and safeguarding this behaviour.
jrfnl
force-pushed
the
feature/164-handle-sniff-deprecation-natively
branch
from
January 25, 2024 15:51
014bc9c
to
3fb0d95
Compare
Rebased without changes to be sure. Merging once the build passes. |
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.
Background
There are quite a few sniffs slated for removal in PHPCS 4.0 - 45 so far, to be exact - and save for two sniffs which have received a deprecation mention in the changelogs, this has only been announced in issues (squizlabs/PHP_CodeSniffer#2448 + squizlabs/PHP_CodeSniffer#2471) in the Squizlabs repo and should therefore only be considered "known" to a very small group of people.
Increasing awareness of the upcoming sniff removals should allow for a smoother upgrade experience to PHPCS 4.0 for end-users.
Aside from use by PHPCS itself, this feature can also be used by external standards to signal sniff deprecations to their end-users.
All in all, this feature should hopefully improve the end-user experience.
Policy for use of this feature in PHP_CodeSniffer itself
As per the notes in #188, the intention is for PHPCS itself to use this feature as follows:
@deprecated
tag in sniff) sniffs during the lifetime of a major.DeprecatedSniff
interface, in (what is expected to be) the last minor before the next major release.External standards are, of course, free to apply a different deprecation policy.
Description
PHPUnit config: make sure unexpected output fails the tests
✨ Native handling of sniff deprecations
As per #164, this commit introduces a new feature to PHPCS: native handling of sniff deprecations.
A new
PHP_CodeSniffer\Sniffs\DeprecatedSniff
interfaceThis interface enforces the implementation of three new methods:
getDeprecationVersion(): string
- the return value should be a non-empty string with the version in which the sniff was deprecated.getRemovalVersion(): string
- the return value should be a non-empty string with the version in which the sniff will be removed.If the removal version is not yet known, it is recommended to set this to: "a future version".
getDeprecationMessage(): string
- the return value allows for an arbitrary message to be added, such as a replacement recommendation. If no additional information needs to be conveyed to end-users, an empty string can be returned.Custom messages are allowed to contain new lines.
Changes to the
Ruleset
class to allow for showing the deprecation noticesThe Ruleset class contains two new methods:
hasSniffDeprecations(): bool
to allow for checking whether a loading ruleset includes deprecated sniffs.showSniffDeprecations(): void
to display the applicable deprecation notices.The implementation for showing the sniff deprecation notices is as follows:
-q
flag (quite mode).-e
flag to "explain" a standard (list all sniffs).--generator=...
CLI argument.phpcs
with the any of the following flags:-i
(listing installed standards),--version
,--help
,--config-show
,--config-set
,--config-delete
.This means that when
--exclude=...
is used and deprecated sniffs are excluded, no notices will be shown for the excluded sniffs.It also means that when
--sniffs=...
is used, deprecation notices will only be shown for those sniffs selected (if deprecated).phpcs
as well asphpcbf
runs.I did considered silencing them by default for
phpcbf
. For now, however, I've decided against this as the whole point of showing deprecation notices is to increase awareness of upcoming changes and as a subsection of the PHPCS users only runphpcbf
and rarelyphpcs
, silencing the notices forphpcbf
could be counter-productive.Additional implementation notes:
--report-width
will be respected. This includes when thereport-width
is set toauto
.--colors
is turned on.--report-file=...
(and variants thereof).PHP_CodeSniffer\Exceptions\RuntimeException
will be thrown.The intention is to add return type declarations to the interface in the future.
The complete new feature is extensively covered by tests.
Explain: mark deprecated sniffs as such
This commit makes a small adjustment to the output of the
-e
(explain) command.Deprecated sniffs will now be marked with a trailing
*
asterix and if the standard contains deprecated sniffs, a line will show at the end of the output to explain that the*
means that a sniff is deprecated.Includes a test documenting and safeguarding this behaviour.
Suggested changelog entry
PHP_CodeSniffer\Sniffs\DeprecatedSniff
interface to allow for marking a sniff as deprecated.Related issues/external references
Related issues: #188, #276
Fixes #164
Types of changes