Skip to content
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

PSR1/SideEffects: improve recognition of disable/enable annotations #54

Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Nov 9, 2023

Description

Recreation of upstream PR squizlabs/PHP_CodeSniffer#3772:

As it was, the sniff would respect PHPCS disable annotations, but only when either unqualified or qualified up to a sniff name. It would not respect a disable annotation using a sniffname with errorcode, i.e. PSR1.Files.SideEffects.FoundWithSymbols.

While the FoundWithSymbols errorcode is the only error code for this sniff and using // phpcs:disable PSR1.Files.SideEffects.FoundWithSymbols is effectively the same as using // phpcs:disable PSR1.Files.SideEffects, I do believe end-users should not need to be aware of whether or not a sniff has multiple error codes when using disable annotations.

This updates the sniff to also respect disable annotations which include the error code.

Includes unit test.

Fixes squizlabs/PHP_CodeSniffer#3386

Suggested changelog entry

PSR1/SideEffects: improved recognition of disable/enable annotations

@jrfnl jrfnl added the Type: bug label Nov 9, 2023
@jrfnl jrfnl added this to the 3.8.0 milestone Nov 9, 2023
@jrfnl jrfnl force-pushed the feature/3386-psr1-sideffects-allow-for-ignore-with-errorcode branch from ac183ed to e8cda77 Compare November 11, 2023 03:35
@jrfnl jrfnl force-pushed the feature/3386-psr1-sideffects-allow-for-ignore-with-errorcode branch from e8cda77 to 7552c17 Compare December 4, 2023 13:43
As it was, the sniff would respect PHPCS disable annotations, but only when either unqualified or qualified up to a sniff name.
It would not respect a disable annotation using a sniffname with errorcode, i.e. `PSR1.Files.SideEffects.FoundWithSymbols`.

While the `FoundWithSymbols` errorcode is the only error code for this sniff and using `// phpcs:disable PSR1.Files.SideEffects.FoundWithSymbols` is effectively the same as using `// phpcs:disable PSR1.Files.SideEffects`, I do believe end-users should not need to be aware of whether or not a sniff has multiple error codes when using disable annotations.

This updates the sniff to also respect disable annotations which include the error code.

Includes unit test.

Fixes 3386
@jrfnl jrfnl force-pushed the feature/3386-psr1-sideffects-allow-for-ignore-with-errorcode branch from 7552c17 to 5df13b3 Compare December 4, 2023 13:45
@jrfnl jrfnl merged commit 16db52c into master Dec 4, 2023
63 checks passed
@jrfnl jrfnl deleted the feature/3386-psr1-sideffects-allow-for-ignore-with-errorcode branch December 4, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PSR1.Files.SideEffects.FoundWithSymbols not being ignored
1 participant