Bug fix: select sniffs would not run

[jrfnl]

Ok, complex one to explain again, but the short of it is that PHPCS does the file name to sniff name translation when autoloading sniffs.

Now for sniffs which extend other sniffs and/or abstracts, when the first sniff which extends such an abstract is requested, the PHPCS autoloader would kick in and then the Composer autoloader would also kick in for the extends, with the end result being that the PHPCS autoloader would return the name of the abstract class to the originating call, which in effect meant that the sniff could never be referenced/included by name as PHPCS would not know which file went with that name.

As the PHPCS autoloader can handle loading any files within the "standard" directory, loading those files should be left to the PHPCS autoloader so it can do the file to sniffname translation correctly.

Removing the autoload directive for --no-dev installs fixes the issue.

To reproduce the issue:

• Clone the SlevomatCS repo at master
• Run composer install --no-dev in the root of the SlevomatCS clone.
• Clone PHPCS
• Register the PHP_CodeSniffer/bin directory to the system path.
• Run phpcs --config-set installed_paths path/to/slevomatCS_clone
• Go to a directory with some files which can be used for testing.
• Run . --standard=slevomatcodingstandard --extensions=php --ignore=/vendor/* --sniffs=SlevomatCodingStandard.Namespaces.FullyQualifiedGlobalConstants

The response will be:

ERROR: No sniffs were registered

Run "phpcs --help" for usage information

• Now, switch to this PR branch.
• Run composer install --no-dev in the root of the SlevomatCS clone.
• Go to a directory with some files which can be used for testing.
• Run . --standard=slevomatcodingstandard --extensions=php --ignore=/vendor/* --sniffs=SlevomatCodingStandard.Namespaces.FullyQualifiedGlobalConstants

PHPCS will run as expected.

While the test instructions are for a stand-alone install, I expect this same issue exists when the standard is installed via Composer as a project dependency, though I haven't tested that.

Also, while the test instructions mention one particular sniff the problem affects several sniffs.

Note that when the sniff is included in a ruleset <rule ref="..."/> and PHPCS can not find it, PHPCS will just silently ignore the sniff, so it would just seem as if there are no violations, while in actual fact the sniff is not being run.

[kukulich]

Thanks.

[jrfnl]

You're welcome.

[grossmannmartin]

With this PR merged is impossible to use for example Helper classes in own sniffs. Is it intentional? Or the classes were never meant to be used outside the coding-standard package? /cc @kukulich

[kukulich]

Hmm, currently some helpers are marked as @internal and some not. @jrfnl is it possible to use autoload because of helpers?

[enumag]

This is a massive break. :-( #983

[kukulich]

@enumag Yes, that's why I'm trying to find the solution.

[enumag]

@kukulich It's not only about helpers, ECS needs to autoload the sniffs too.

[jrfnl]

With this PR merged is impossible to use for example Helper classes in own sniffs. Is it intentional? Or the classes were never meant to be used outside the coding-standard package?

Actually, that is not true. As long as the SlevomatCodingStandard is registered as one of the installed standards with the PHP_CodeSniffer instance used to run your own sniffs (for instance, by using the DealerDirect Composer PHPCS plugin), the PHPCS autoloader should kick in automatically and handle loading the files correctly.

So instead of relying on the Composer autoload, you'd need to rely on the PHPCS autoload.

If the files are used outside of a PHPCS context, that would, of course, be a different thing, then yes, this would be a problem.

Either way, there is still something else wrong with the autoloading and I haven't figured out yet whether it is in PHPCS itself or to do with this package, so if this gets reverted for now, I can live with that as - even though it is a step in the right direction - it's still not the final solution.

---

To illustrate/demonstrate the problem I'm (still) running into:

1. Have PHPCS installed stand-alone, be it PEAR, git clone or via the phar.
2. Register the SlevomatCodingStandard (in my case, also a git clone) with PHPCS.
3. Run the below ruleset over the below file:

<?xml version="1.0"?>
<ruleset name="Demo">
    <autoload>/path/to/slevomat/autoload-bootstrap.php</autoload>

    <rule ref="SlevomatCodingStandard.Namespaces.FullyQualifiedGlobalFunctions"/>
    <rule ref="SlevomatCodingStandard.Namespaces.FullyQualifiedGlobalConstants"/>

</ruleset>

<?php

namespace My\Package;

class Foo
{
	public function bar()
	{
		$foo = filter_input(INPUT_POST, 'foo', FILTER_SANITIZE_ENCODED);
	}
}

You'd expect three errors:

• 1 for the unqualified function call to filter_input();
• 2 for the unqualified use of the two PHP native global constants.

Instead the FullyQualifiedGlobalFunctions sniff does not kick in and only the FullyQualifiedGlobalConstants reports the expected errors.

If you change the ruleset to use the reverse order for the sniffs, i.e.:

<?xml version="1.0"?>
<ruleset name="Demo">
    <autoload>./../../slevomat/autoload-bootstrap.php</autoload>

    <rule ref="SlevomatCodingStandard.Namespaces.FullyQualifiedGlobalConstants"/>
    <rule ref="SlevomatCodingStandard.Namespaces.FullyQualifiedGlobalFunctions"/>

</ruleset>

You'll only get the error about filter_input() and won't get the error about the global constants.

[kukulich]

@jrfnl

Either way, there is still something else wrong with the autoloading and I haven't figured out yet whether it is in PHPCS itself or to do with this package, so if this gets reverted for now, I can live with that as - even though it is a step in the right direction - it's still not the final solution

Yes, I expect that the revert is temporary solution.