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

Using the '--sniffs' argument has a problem with case sensitivity #1656

Conversation

Fischer-Bjoern
Copy link

When using the '--sniffs' argument with phpcs you have to enter the the sniffs case sensitive for the sniff to work.
However when using any other casing (like all lowercase) of the same letters, phpcs will not fail.
Instead it will find the sniff and run it, but it will never report any errors.
If different letters are used, phpcs obviously complains directly that it can't find the sniff.

This is since the matching of errors to sniffs is done with in_array(), which is case sensitive. But the loading of sniffs uses strtolower() in various places.

So this patch makes the '--sniffs' argument fully case sensitive.

@gsherwood gsherwood added this to the 3.1.1 milestone Sep 19, 2017
@jrfnl
Copy link
Contributor

jrfnl commented Sep 22, 2017

I've run into this a number of times as well. While I would personally prefer the solution to be for PHPCS to handle this in a case-insensitive manner (fault tolerant), any solution which will not make PHPCS silently fail on this is an improvement.

gsherwood added a commit that referenced this pull request Sep 25, 2017
@gsherwood
Copy link
Member

The intention was to make this string case insensitive, but I obviously stuffed that up during the file rewrite. I've made a change to the File class so that sniff error codes are compared in a case insensitive way.

I think there is still a fair bit of work left to make rules inside ruleset.xml files case insensitive, but that would need to be left for a feature release if it is deemed worth doing. Rulesets are a bit different because they are fairly static, but CLI args tend to be written quickly and benefit from case insensitive checks where possible.

Thanks for letting me know about this, even though I didn't end up using the PR.

@Fischer-Bjoern
Copy link
Author

Sure. What matters is that the problem is fixed.

@Fischer-Bjoern Fischer-Bjoern deleted the fix-sniff-case-senitivity branch October 11, 2017 07:41
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.

None yet

3 participants