Sniff exclusions/restrictions dont work with custom sniffs unless they use the PHP_CodeSniffer NS

[jrfnl]

When running PHPCS with a custom standard, the command-line --sniffs and --exclude options no longer work.

This is caused by the sniffNames being prefixed with PHPCS's own php_codesniffer\standards\ namespace which custom standard should not use in the first place.

This PR fixes this by only prefixing the sniffname with the PHPCS native namespace when it is a sniff from one of the build-in standards.

[gsherwood]

I vaguely remember wanting to come back to this code...

I don't like the idea of keeping a hard-coded array of included standards though. It feels very high-level yet used for something really specific.

I'd prefer either being able to gather these from a helper function by looking at children of src/Standards or just checking with is_dir() directly when required. Thoughts?

[jrfnl]

I don't like the idea of keeping a hard-coded array of included standards though. It feels very high-level yet used for something really specific.

I agree, but wasn't sure what you'd prefer and considering the included standards haven't changed since 2012 and it is rare for a new standard to be added, it seemed the simplest & most efficient way to make the fix.
Oh and by efficient, I mean: no overhead in calling functions for something which is a known fixed array.

Let me add a second commit with a function to check it instead.

[jrfnl]

Rebased & added the second commit. Feel free to squash the commits on merge.

[gsherwood]

I was just doing some testing of this and I don't think this change is actually going to fix the problem as it enforces that the namespace be exactly STANDARD\Sniffs\CATEGORY and so doesn't allow for prefixing it with project information.

I'll take a deeper look into this and see if I can figure out where things are going wrong.

[gsherwood]

I should have kept looking before typing...

I just need to change the way the restriction is checked so it is looking at the end of the namespace instead of the entire thing. Bit more work, but it should work.

[jrfnl]

I just need to change the way the restriction is checked so it is looking at the end of the namespace instead of the entire thing.

That would be in another part of the code though, wouldn't it be ? and is an additional change which may be needed.

Would you like me to add that to this PR or are you already fixing this yourself ?

[gsherwood]

I've just committed a fix for this issue that was working in my testing. It basically enforces the rule that sniff namespaces must end with STANDARD\Sniffs\CATEGORY but can be prefixed with whatever you want. I had to clean the actual NS before comparison when checking restrictions and inside the unit tests.

Interested to know if this works for your case as well.

[jrfnl]

Sorry, but my first test run fails miserably with the following error:

Warning: strrpos(): Offset is greater than the length of haystack string in PHP_CodeSniffer\src\Util\Common.php on line 439

Call Stack:
    0.0030     124568   1. {main}() PHP_CodeSniffer\bin\phpcs:0
    0.0100     288712   2. PHP_CodeSniffer\Runner->runPHPCS() PHP_CodeSniffer\bin\phpcs:18
    0.3820     725976   3. PHP_CodeSniffer\Runner->init() PHP_CodeSniffer\src\Runner.php:67
    0.3940     988816   4. PHP_CodeSniffer\Ruleset->__construct() PHPCS\PHP_CodeSniffer\src\Runner.php:273
    0.8100    1007616   5. PHP_CodeSniffer\Ruleset->registerSniffs() PHP_CodeSniffer\src\Ruleset.php:200
    0.8490    1618744   6. PHP_CodeSniffer\Util\Common::cleanSniffClass() PHP_CodeSniffer\src\Ruleset.php:1063
    0.8490    1618952   7. strrpos() PHP_CodeSniffer\src\Util\Common.php:439

[gsherwood]

I think I need the repo you are testing with so I can replicate your problems. Is that possible?

[jrfnl]

Just a guess (haven't tested it), but this may fix it:

$end     = (strlen($newName) - (strrpos($newName, '\sniffs\\') + 1));

(= brackets around the strrpos + 1)

[gsherwood]

Just a guess (haven't tested it), but this may fix it:

I don't think so. I think the problem here is that the sniff namespaces are not in the allowed format. As in, they don't even include \Sniffs\ anywhere. That would get it returning false and stuffing up the offset. That's something I should be checking for anyway.

But that's just a guess based off some basic testing.

[jrfnl]

I think the problem here is that the sniff namespaces are not in the allowed format.

Sorry, but no, the namespace declaration which I'm using in this case is namespace PHPCompatibility\Sniffs\PHP;

[jrfnl]

Also - the original code would be cutting off too soon - see some test code I was just trying out: https://3v4l.org/0f83v

[gsherwood]

Thanks. I completely forgot the case where nothing needs cleaning.

[jrfnl]

No worries, keeping my fingers crossed for a reliable fix ;-)

[jrfnl]

Just tested with the latest commit and it seems to work fine now.

[DanielSiepmann]

When will this be available in a new release? Once all 3.0.1 tasks are done?

[gsherwood]

When will this be available in a new release? Once all 3.0.1 tasks are done?

Yep.