PHPCS 3.x: Autoloading not working with arbitrary namespace prefix

[jrfnl]

In the upgrade guide, it says:

Note: It doesn't matter what namespace you use for your sniffs as long as the last part of the namespace is in the format StandardName\Sniffs\Category

I'm currently trying to upgrade the WordPress Coding Standards library and am using the following namespace / class declaration for one of the sniffs:

namespace WordPressCS\WordPress\Sniffs\Arrays;

use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff;

class ArrayAssignmentRestrictionsSniff extends AbstractArrayAssignmentRestrictionsSniff {
    // sniff code
}

The file AbstractArrayAssignmentRestrictionsSniff class is as expected located in /path/to/WPCS/WordPress/AbstractArrayAssignmentRestrictionsSniff.php and the /path/to/WPCS location is registered with installed_paths.

This results in a Fatal Error:

Fatal error: Class 'WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff' not found in /path/to/WPCS/WordPress/Sniffs/Arrays/ArrayAssignmentRestrictionsSniff.php on line 28

Call Stack:
    0.0010     124488   1. {main}() \path\to\PHP_CodeSniffer\bin\phpcs:0
    0.0090     291840   2. PHP_CodeSniffer\Runner->runPHPCS() \path\to\PHP_CodeSniffer\bin\phpcs:18
    0.3030     730056   3. PHP_CodeSniffer\Runner->init() \path\to\PHP_CodeSniffer\src\Runner.php:68
    0.3140     994536   4. PHP_CodeSniffer\Ruleset->__construct() \path\to\PHP_CodeSniffer\src\Runner.php:280
    3.3092    1046176   5. PHP_CodeSniffer\Ruleset->registerSniffs() \path\to\PHP_CodeSniffer\src\Ruleset.php:201
    3.3102    1052392   6. PHP_CodeSniffer\Autoload::loadFile() \path\to\PHP_CodeSniffer\src\Ruleset.php:1069
    3.3142    1077680   7. include('/path/to/WPCS/WordPress/Sniffs/Arrays/ArrayAssignmentRestrictionsSniff.php') \path\to\PHP_CodeSniffer\autoload.php:171

Some light debugging shows me that the autoloader is trying to find the file in the following locations:

"\path\to\PHP_CodeSniffer\src\Standards\WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff.php"
"\path\to\WPCS\WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff.php"
"\path\to/PHPCompatibility/WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff.php"
"\path\to/yoastcs\WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff.php"

This would imply that - unless you use Composer and force all users of the standard to use Composer - you cannot use prefixed namespaces.

Or is there a way to "hook in" an extra autoload file which I haven't found out about yet ? The bootstrap option cannot be used as it is run too late (after the autoloading of the sniffs).

Basically - for the purpose of the autoloading -, I would expect the namespace prefix WordPressCS to be ignored, or, to be stripped off the sniff based namespace if the prefixed file cannot be loaded, in order to "try again".

While I could use a non-prefixed namespace - after all WP currently does not use namespaces (yet) -, it would be kind of irresponsible to do so IMHO and considering how large the userbase is of the standard, changing the name of the standard would also cause a massive BC break.

Bright ideas or solutions very welcome ;-)

[jrfnl]

FYI: I've put a work-in-progress branch online if you need it: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/tree/WIP/issue-718-862-phpcs-3.x-compatibility

[gsherwood]

I could change the autoloader to detect that the class name looks like a sniff, but the namespace you've used for this abstract sniff wouldn't match the require format of StandardName\Sniffs\Category. The namespace you've used is just StandardName.

So what the autoloader would need to do is keep chopping off parts of the namespace and looking to see if any file matches somewhere in the installed_paths. That doesn't sound like a great idea because it is very possible that it will autoload files that are not actually sniffs or sniff-related helpers.

Here are some options:

1. Put the abstract sniff into a category and change the namespace to match. Then have the autoloader changed to detect that format and locate the file. PHPCS should be ignoring abstract classes when registering sniff for checking, so it shouldn't actually run this sniff.
2. Put the abstract sniff into a Sniffs directory and have the autoloader changed to assume that a class like Standard/Sniffs/SomethingSniff is a helper sniff. This adds a new forced convention to PHPCS for where abstract sniffs are allowed to live and still be autoloaded by PHPCS.
3. Add a feature to PHPCS so you can include your own autoloader for cases where you aren't using composer. Note that I don't use composer for anything and so never enforce its use for PHPCS.
4. Include the files manually rather than relying on autoload.
5. Change your namespace to begin with WordPress, as you had with class names in the 2.x version.

Right now, the prefixing of the namespace works because PHPCS goes and loads all the sniffs in a standard and then uses a util function to look at that special namespace format and figure out the sniff details from it. So if one sniff extends another, PHPCS already knows about them because it loaded all the sniffs files up manually. But it doesn't go through and load files outside the Sniffs directory, so it has to discover those based on a fully qualified class name only.

The naming convention in the doc is for sniff files, and so that is what is enforced. There is no naming convention for helpers, so PHPCS can't autoload them. That means adding a convention for helpers is possible (option 2) but I'm not really sold on it. Option 4 sounds ok, but I'm not sure how that would work with multiple custom standards referencing each other. You'd probably need to hard-code the autoloader option into the ruleset of the standard to make it work, which might be fine.

Got an opinion about those options, or other options?

[gmponos]

This would imply that - unless you use Composer and force all users of the standard to use Composer - you cannot use prefixed namespaces.

I might have misunderstood this but I don't see the problem about it.

Most of the frameworks and packages I see for PHP in github are using the autoload of composer. We live in that age (thank god) that Jordi made the autoloading pretty simple. Codesniffer has made a huge step moving into that age and using this autoloading.

after all WP currently does not use namespaces (yet).

So IMHO this is a WP issue and libraries/framework should not try to be backward compatible for something that the age has moved on.

[gsherwood]

So IMHO this is a WP issue and libraries/framework should not try to be backward compatible for something that the age has moved on.

This is not my position and I am committed to getting this working. I'd hate for PHPCS to only be usable with projects that use composer, or to force custom coding standards to be installed via composer for them to work.

[gmponos]

I believe that in order to achieve this you will need to create your own custom autoloading mechanism combined with the one of composer just in order to be independent from that autoload mechanism (that I think most of the PHP community uses). I don't see the reason, but anyway...

[gsherwood]

I believe that in order to achieve this you will need to create your own custom autoloading mechanism combined with the one of composer just in order to be independent from that autoload mechanism

That's something I've was working on the entire 2 years 3.0 was in development. You can check out the history of the autoloader if you'd like to see how it came together: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/autoload.php

This is just an extension of that already released system.

[jrfnl]

Hi @gsherwood Thanks for getting back to me.

Got an opinion about those options, or other options?

I'm trying to create a version which is compatible with PHPCS 2.x and 3.x - though have found since that 2.x can't handle prefixed namespaces either, but that's another matter -. I am trying to balance the needs of both.

After everything I've tried so far, option 3 sounds like it would be most flexible.

• Option 1 feels very counter-intuitive as the abstracts are unrelated to categories.
• Option 2 would - as you say - add a new forced convention. While I'd not be unhappy about this, I'm not sure how others would feel about this and introducing this new convention after PHPCS 3.x has already been released feels a bit late.
• Option 3 sounds most flexible if the autoloader could be registered after the PHPCS autoloader, but before any actual autoloading would be done. that way the PHPCS autoloader would still take priority and sniffs would still be registered correctly.
• Option 4 feels like a step back in time what with autoloading having been around for a while now. Also, in PHPCS 2.x manual requires where only needed when using a sniff from another standard as the basis for a sniff, not for loading a base sniff in your "own" standard.
• Option 5 - while I may have to end up with that to stay compatible with PHPCS 2.x, it just feels very very dirty to do so. The WP Coding Standards !== WordPress. Namespacing the standard as WordPress feels wrong and could cause massive problems in the future if/when WP itself would start using namespaces.

Option 4 sounds ok, but I'm not sure how that would work with multiple custom standards referencing each other. You'd probably need to hard-code the autoloader option into the ruleset of the standard to make it work, which might be fine.

I believe you are talking about option 3 here ?
If so, then yes, adding the autoloader as an option in the ruleset - maybe even with an additional "autoload priority" to order autoloaders between custom standards ? - and then registering the additional autoloaders to SPL before doing any actual autoloading sounds like the best solution.
That may also solve an issue I have with trying to make the unit tests run on PHPCS 2.x as well as PHPCS 3.x.

[gsherwood]

I believe you are talking about option 3 here ?

Sorry, yes I was.

I think I can have an autoload file (defined in a ruleset.xml file) loaded up before the rest of the ruleset is processed. I'm fairly sure I wont be able to define any sort of order because I need to process a ruleset in order to figure out what other rulesets it requires, so loading order would be dependant on how the ruleset is constructed. I don't know if that's a problem, but I don't think it would be a problem in your specific case, so it might be a good first step.

I'll take a look into it and report back.

[jrfnl]

Sounds great! I really appreciate your help with this!

[gsherwood]

I built the autoload include point, but I'm wondering if it's actually easier to just have the ruleset tell PHPCS what the NS prefix is instead of having to write your own autoloader for just a couple of files.

When PHPCS adds the search path based on the standard it includes, it could tell the autoloader what portion of the NS to strip. So for the WP standard, you'd include something like <namespace>WordPressCS\WordPress</namespace> and the autoloader now knows where to start looking.

I could also do both.

[gsherwood]

I've pushed up a change to allow a namespace to be set for a custom coding standard. You put this into the ruleset.xml for any standard that has custom sniffs and has a prefixed namespace. In your case, you'd put this into the main WordPress ruleset.xml file:

<?xml version="1.0"?>
<ruleset name="WordPress" namespace="WordPressCS\WordPress">
	<description>WordPress Coding Standards</description>
        ...
</ruleset>

When I do this, the helper files are loading correctly, but I still get errors about PHPUnit_Framework_TestCase not being found. I assume this is the PHPUnit issue you were talking about, so I'm not sure if I need to do anything to get that working.

[gsherwood]

Note that I haven't changed the upgrade guide yet. I won't do that until we get this issue sorted and confirm the best way to do things.

[jrfnl]

Hi @gsherwood Oh wow, both actually sound great! I'm on the road (well, in the air) today, but will have a play with it as soon as I can and get back to you.

I still get errors about PHPUnit_Framework_TestCase not being found

That's strange, I thought I'd fixed those (and pushed the fix) already.

[gsherwood]

I went ahead and added the autoload feature to rulesets even though I don't think it is currently needed in this case. It's not much code, so doesn't really matter if it doesn't get used right now.

[jrfnl]

@gsherwood Thank you very much for that. Much appreciated. Will try to have a play with it next week once I'm back from the conference I'm currently at. And will - of course - report back ;-)

[gsherwood]

Closing as this was released a couple of versions back. Autoload problems can now be handled in new issues.