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

Commenting/PhpcsAnnotation breaks PHPCS native ignore annotation handling #208

Open
jrfnl opened this issue Nov 10, 2024 · 2 comments
Open
Labels
bug Something isn't working

Comments

@jrfnl
Copy link

jrfnl commented Nov 10, 2024

The sniff overrides the token content of PHPCS ignore annotations to allow for fixing these but doesn't revert that change correctly once the sniff is done, which means that this change will affect all sniffs and breaks the PHPCS native ignore annotation handling.

The issue is straight-forward to reproduce.

Given the below file:

<?php
/**
 * @phpcs:ignoreFile
 */

Class Foo {}

When running this command:

phpcs -ps ./webimpress-breaks-ignores.php --report=full,source --standard=WebImpressCodingStandard

I'd expect to see no errors. Instead I get 6.

@michalbundyra michalbundyra added the bug Something isn't working label Nov 19, 2024
@michalbundyra
Copy link
Member

@jrfnl Nice catch, thanks. I was looking into it, and I cannot find a solution. Looks like from the very beginning this sniff was not working properly. As I see all errors are cleared in the File class when ignoreFile annotation is found, and I guess it is not possible to report just error from this sniff and ignore everything else?
I bet I would need to deprecate this sniff and kill it, or... just keep as is with the warning about this unexpected behaviour.
Will you have a second to advise what you think? Thanks 🙏

@jrfnl
Copy link
Author

jrfnl commented Nov 25, 2024

@michalbundyra Just by looking at this sniff alone, all I can think of is to undo the token override after the if block within the while loop.

Should probably be done by storing the original value and restoring it (and getting rid of the overrideToken method).

But if that doesn't work or doesn't work reliably enough, then yes, killing this sniff would be better as this hacking of the PHPCS native File object is problematic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants