Description: ------------ Hi, as commented in #20202 we have moved to CS 1.5.2 recently. And we have detected a possible regression (and perhaps a bug too) when processing "complex" ruleset combinations. For your consideration. First, let me show our standards and directories structure. 1) We run the CS from a (Moodle) plugin, aimed to help developers to follow our guidelines/avoid some typical mistakes. You can see its directory structure here: https://github.com/stronk7/moodle-local_codechecker - Under "pear" is where CS 1.5.2 upstream resides. - "moodle" is where our standard and own Sniffs are available. - "phpcompatibility" is another standard that we incorporate to the "moodle" one via ruleset. 2) We always run the CS by passing the "moodle" standard (or its "moodle/ruleset.xml") definition. You can see it here: https://github.com/stronk7/moodle- local_codechecker/blob/master/moodle/ruleset.xml All it does is, basically, pick some Sniffs from other (upstream) standards and, in the very last lines of the file, we incorporate the "phpcompatibility" standard and exclude one of the sniffs from it. And here is where we come to problems, namely: Problem A) That exclude used to work before CS 1.5.2. Now it has stopped working and it's not excluded anymore (and no message is given about that by executing CS with -vv. Problem B) I've tried changing from current: <exclude name="../phpcompatibility/Sniffs/PHP/DefaultTimezoneRequired Sniff.php"/> to: <exclude name="PHPCompatibility.PHP.DefaultTimeZoneRequired"/> And that one is failing badly with error: PHP Fatal error: Uncaught exception 'PHP_CodeSniffer_Exception' with message 'Referenced sniff "PHPCompatibility.PHP.DefaultTimeZoneRequired" does not exist' in /opt/local/lib/php53/pear/PHP/CodeSniffer.php:847 Stack trace: #0 /opt/local/lib/php53/pear/PHP/CodeSniffer.php(609): PHP_CodeSniffer- >_expandRulesetReference(Object(SimpleXMLElement), '/Users/stronk7/...', 1) #1 /opt/local/lib/php53/pear/PHP/CodeSniffer.php(435): PHP_CodeSniffer->processRuleset('moodle/ruleset....') #2 /opt/local/lib/php53/pear/PHP/CodeSniffer/CLI.php(614): PHP_CodeSniffer->process(Array, Array, Array, false) #3 /opt/local/bin/phpcs(37): PHP_CodeSniffer_CLI->process() #4 {main} Looking into code I've arrived to this exact line: https://github.com/stronk7/moodle- local_codechecker/blob/master/pear/PHP/CodeSniffer.php#L600 where it expects FOUR parts in the Sniff code. Obviously, I've changed it to THREE and then the exclude using the sniff code has started to work. In other places where we have moved from class names to sniff codes... we have changed always to a 3-part code and it's working ok that way. For example, in our own unit tests: https://github.com/stronk7/moodle- local_codechecker/commit/9b2a305288fd29c6550d363136254d 89ab8e6d5d So... is there any reason for that code looking for 4 parts? Or is it a real bug and should be 3 instead? And those are the 2 problems we are experiencing since the upgrade. 1) excludes by filename stopped working and 2) excludes by sniff code failing badly. For your consideration, ciao :-) Test script: --------------- runnning phpcs with the moodle standard above reproduces the 2 problems commented. Expected result: ---------------- Both excludes by filename and by sniff code should work. Actual result: -------------- Excludes by filename are ignored silently and excludes by sniff code don't work (they expect 4 parts)

For cross-reference, we are handling the problem in our tracker @ https://tracker.moodle.org/browse/MDLSITE-2800

Line 600 is just showing that you can also exclude an individual sniff code using an exclude tag. In this case, it is the same as setting the severity of an error message to 0. The problem actually comes from the fact that the standard you are excluding from is called PHPCompatibility but the directory you include it in is all lowercase. I have committed a fix for this here: https://github.com/squizlabs/PHP_CodeSniffer/commit/13eeba855694f6cfc6fe4b798 b29f1500dc61eb3 And you are correct that the way to exclude the sniff is to use the code. Relative paths never worked very well in the 1.4 branch and the only reason that exclude rule would have worked is to where you were runnings the check from. It should be portable, which is what the code aims to do. So to fix, change your exclude rule to: <exclude name="PHPCompatibility.PHP.DefaultTimeZoneRequired"/> Then either get the latest code from the git repo's master branch, or rename your phpcompatibility directory to PHPCompatibility. I've tested the latest code with a clone of your standard repo and it is processing properly for me, but let me know if it doesn't work for you.

Hi Greg, thanks for your promptly answer and fix. I'll try it immediately. To be honest, I'm not sure that is going to fix my problem, because the rule was already finding the "../phpcompatibility" standard perfectly (I'm a Mac), but the problem was exclusively with the exclude within it. Also, I tried with, exactly, the code you propose above, but it was dying badly, and then is when I changed line #600 to look for 3 parts, not 4. In any case... all these are suppositions... I'm going to update from upstream/master and try all the combinations. Again, thanks!

Aha, so it seems that you were ok and I was wrong, after all! I've tried with your patch applied and can see how the code: PHPCompatibility.PHP.DefaultTimeZoneRequired is now being properly excluded (running with -vv). But, curiously, the "Default timezone is required since PHP 5.4" message (calculated by that Sniff) continues being shown, so it's not effectively being excluded. And if I change it to "PHPCompatibility.PHP.DefaultTimezoneRequired" (lowered "Z") then it's properly excluded. Bloody camelcase files and classes, lol. Ok, I'm going to update to master (I just applied your fix over 1.5.2 here) to see if it finally works both with "Z" and "z" :-)

Aha (again), just to confirm that, with current master head, the exclusion works ok here if I lowercase the "Z", but if does not work if I keep it upper. Obviously it's a problem in the phpcompatibility standard, because classname does not match filename and I'm going to suggest it to be fixed there. Although perhaps, it could be interesting to have them matching ok in phpcs. For your consideration. I still don't understand which is the case for the FOUR-PARTs case in your exclude code, but that's another story. Going to update our code to your upstream version and fix the exclude to use that lowered "Z" to get it working again here. Again thanks, and ciao :-)