Sniffs not loaded when one-standard directories are being registered in installed_paths
#1581
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
If any external standard or a custom ruleset refers to a complete standard and this standard is registered in the
installed_paths
config as a one-standard directory, aReference sniff ... does not exist
error will be thrown and no sniffs will be run.Bug introduced in #1450 (by me, sorry!).
The reason for this is that the
getInstalledStandardPath()
method expects a string, but will in actual fact receive aSimpleXMLElement
object when a complete standard is included by another standard. This was previously not documented in the function doc block as a possibility.While in the original code, the
$standards
name was only used as a concatenated string - which triggered theSimpleXML.toString()
method -, the fix in #1450 introduced a strict comparison without thetoString()
conversion, causing the bug.$standards
$standards
While debugging this, I also noticed that this method is called for every single ref in the ruleset and would go through the whole loop before returning
null
, while forrefs
which refer to a custom xml ruleset or to a sniff or sniff category,null
can be returned a lot earlier and for paths which don't resolve to a ruleset, part of the loop can be skipped.Testing the fix:
Using the WordPress Coding Standards as an example to demonstrate what I mean, set
installed_paths
like so:phpcs --config-set installed_paths /path/to/WordPressCS/WordPress,/path/to/WordPressCS/WordPress-Core,/path/to/WordPressCS/WordPress-Docs,/path/to/WordPressCS/WordPress-Extra,/path/to/WordPressCS/WordPress-VIP
Custom ruleset
test.xml
:Then run PHPCS against any file:
phpcs ./test-file.php --standard=test.xml