PHP_CodeSniffer
PHP_CodeSniffer copied to clipboard
`// phpcs:set` is not limited to the file in which it is used
I may be missing something, but AFAICS and in contrast to the other //phpcs: annotations, using // phpcs:set Standard.Category.Sniff PropertyName value is not file-based, but changes the sniff property for the rest of the PHPCS run.
IMHO this is unreliable as the effect of this depends on:
- the order in which the files are sniffed which may change without notice;
- will not work as expected when used in combination with the
paralleloption.
While in practice, this feature is mostly used in PHPCS unit tests, it is a useful feature as there is no other way to set a property for a limited group of files.
Would it be possible to make the effect of the // phpcs:set annotation be file-based ?
I imagine, that this could be done by saving the original value of the property when the first // phpcs:set for a certain property in a file is encountered into an array like $propertyOverrides[$sniffName][$propertyName] = $originalValue; and resetting the property to that original value once the last token of a file has been reached (or something along those lines).
Example use-case:
Say I have a sniff which checks variables in the global namespace or within a function scope when a global statement is encountered.
Now , say a project has view files which are included from within a function in another file. For those files it would be useful to be able to set a property FileType view which would only apply to that file.
That way, for that file, the "global" namespace could be treated as scoped and the sniff would only check the variables and throw errors when a global statement is encountered.
Using phpcs:disable for that file or <exclude-pattern> the file from within a custom ruleset, is not an option as that would disable the sniff completely.
But setting phpcs:set at the top of the file currently changes the behaviour of the sniff for that file and all files scanned after it, which is not the intention either.
Would it be possible to make the effect of the // phpcs:set annotation be file-based ?
That's probably a good idea.
Would be great if this could be implemented, literally every other linter has it and there are so many use cases especially with unused variables for it.
Hello!
So many times I use PHP templates with the include keyword. It would be great to have a PHPCS directive that allows to define "global" variables in the templates, so that the linter does not complain every time I use them because they are undefined... just as you can do with global keywork on ESLint (link).
Thank you, Guido
Yes, it would be really very helpful if this proposal were to be implemented. Maybe an additional parameter for phpcs:set or a new keyword like e.g. phpcs:setlocal ?
@jrfnl
Since yesterday I have been working with a workaround to solve this problem. This will probably not work with parallel, but that is irrelevant for me personally, because I can't use this option anyway. See the following comment:
https://github.com/sirbrillig/phpcs-variable-analysis/issues/45#issuecomment-871523011.