Sniff properties not set when referencing a sniff using relative paths or non-native slashes

[yguedidi]

When using this syntax:

<rule ref="./path/to/sniff/class.php">
    <properties>
        <property name="foo" value="bar" />
    </properties>
</rule>

The sniff will the value from the class, not the one configured.

[jrfnl]

I've just tried to reproduce this and found something which I believe is relevant:

• When using an absolute path to the sniff using the platform native slashes (Windows: \), the ruleset value is correctly set.
• When using an absolute path to the sniff using non-platform native slashes (i.e. /), the ruleset value is not set and the default value from the sniff is used.
• When using a relative path to the sniff, the ruleset value is not set and the default value from the sniff is used.

	<!-- Works. -->
	<rule ref="TestCode.Test.Test">
	    <properties>
	        <property name="test" value="ruleset value" />
	    </properties>
	</rule>

	<!-- Works too. -->
	<rule ref="D:\path\to\TestCode\TestCode\Sniffs\Test\TestSniff.php">
	    <properties>
	        <property name="test" value="ruleset value" />
	    </properties>
	</rule>

	<!-- Uh oh, this fails. -->
	<rule ref="D:/path/to/TestCode/TestCode/Sniffs/Test/TestSniff.php">
	    <properties>
	        <property name="test" value="ruleset value" />
	    </properties>
	</rule>

	<!-- This fails too. -->
	<rule ref="./../../TestCode/TestCode/Sniffs/Test/TestSniff.php">
	    <properties>
	        <property name="test" value="ruleset value" />
	    </properties>
	</rule>

So this looks to be related to the path resolution when including sniffs like this.

[jrfnl]

@yguedidi Could you confirm you are running PHPCS on Windows ? And would you be able to test PR #2501 which is intended to fix this issue ?

[yguedidi]

@jrfnl I'm not on Windows, I'm running on Linux. I'll test the PR ASAP, thank you

[jrfnl]

Keeping my fingers crossed 🤞

[gsherwood]

There is obviously a Windows issue in there if you've been able to replicate one, but I'm also keen to know if this change has any impact on the original issue given it targets Windows slashes.

[jrfnl]

I'm also keen to know if this change has any impact on the original issue given it targets Windows slashes.

@gsherwood So was I, which is why I added unit tests for various ways of including sniffs to the fix to try and verify (and make sure I wasn't breaking anything else).

Based on the unit tests (run on Windows + Linux via Travis), the original case has also been fixed by the fix now pulled, but verification by the Op would still be helpful.

[jrfnl]

The technical explanation of the change I made is as follows:

• ./path/to/sniff/class.php contains 2 .s, so the original explode() returns three elements, the first one of which is empty.
• Because it contains three elements, it is seen as a sniffcode Stnd.Cat.Sniff and processed as is and the properties are added to the file, not to the sniff and will therefore not be recognized in the sniff.
• The fix adds a second check to the condition, which removes empty elements and if the count is not the same as before, will do the same additional processing as for inclusion of whole standards/categories, where it translates the paths to the individual sniffs to sniff codes.
• That additional processing has also received an additional fix for Windows vs *nix slashes to get round the Windows specific part of the problem, which I ran into.

[gsherwood]

@yguedidi Did you ever manage to test the fix? I'm going to look into this soon, but it would be great to have your feedback if you did manage to try it out.