Bug: "colors" can not be overruled from within a secondary ruleset

[jrfnl]

Given the following primary ruleset saved as phpcs.xml.dist:

<?xml version="1.0"?>
<ruleset name="Test">
    <file>.</file>
    <arg name="colors"/>
    <rule ref="PSR12"/>
</ruleset>

... and this secondary ruleset saved as phpcs.xml:

<?xml version="1.0"?>
<ruleset name="Test">
    <rule ref="phpcs.xml.dist"/>
    <arg name="no-colors"/>
</ruleset>

The no-colors setting has no effect.

Overruling the setting from the command-line using phpcs --no-colors does work.

[jrfnl]

Along the same lines, an <arg value="n"/> also can't seem to be overruled from a secondary ruleset using <arg value="w"/>. Again, overruling from the command-line using -w does work.

[jrfnl]

Loosely related to #2041

[gsherwood]

The config has been specifically designed to not allow this style of overriding, so the primary rulesets have the final say, not the rulesets they are including.

This is how I think it should be. If my project has a config that says I want colours, I want colours. If including a 3rd-party ruleset is able to turn off that setting, there is no point in me even having the ability to set it in my project's ruleset.

The CLI overrides were added in because they should have the absolute final say as the developer is individually setting those.

[jrfnl]

so the primary rulesets have the final say, not the rulesets they are including.

Maybe I used the wrong terminology, but in the example I posted in the original issue report, the phpcs.xml file includes the phpcs.xml.dist file.

However, instead of the setting from the phpcs.xml file being used, the setting in the included "3rd party" phpcs.xml.dist ruleset prevails.

[gsherwood]

However, instead of the setting from the phpcs.xml file being used, the setting in the included "3rd party" phpcs.xml.dist ruleset prevails.

Well that's not how it is supposed to work at all. I'll have a look at it when I get a chance.

[jrfnl]

Appreciated!

Edit: easy way to test/reproduce the issue - just add the above phpcs.xml file to the root of this project ;-)

[gsherwood]

I can't replicate this issue. Arg colors is set first (checked using -vv) and no-colors is set second, but is ignored (although debug output doesn't show this). My report output still has colour in it.

The code is quite simple too:

PHP_CodeSniffer/src/Config.php

Lines 705 to 720 in 0b44f1c

	case 'colors':
	if (isset(self::$overriddenDefaults['colors']) === true) {
	break;
	}
	$this->colors = true;
	self::$overriddenDefaults['colors'] = true;
	break;
	case 'no-colors':
	if (isset(self::$overriddenDefaults['colors']) === true) {
	break;
	}
	$this->colors = false;
	self::$overriddenDefaults['colors'] = true;
	break;

If colours are turned on first (as they should be in this case) a var is set so that turning off colours is ignored. I added some debug code in to ensure it is working as intended, and it is for me.

Are you maybe seeing things process in a different order to me?

[jrfnl]

I can't replicate this issue. Arg colors is set first (checked using -vv) and no-colors is set second, but is ignored (although debug output doesn't show this). My report output still has colour in it.

Sounds like you have replicated the issue: no-colors is set second, but is ignored.

[gsherwood]

Sounds like you have replicated the issue: no-colors is set second, but is ignored.

Oh, I get you now. Sorry.

Is this the issue: #2197

Edit: What I meant was - is this is the solution? Line by line parsing should fix this, but it's targeting 4.0

[jrfnl]

It's similar and definitely closely related.

In my view, it would be most logical for all included standards/rulesets to be processed first in the order in which they are found in a custom ruleset. And then for the top-level configuration to be processed after that, i.e.:

<ruleset name="Test">
    <arg name="no-colors"/>
    <config name="testVersion" value="7.1-"/>
    <rule ref="PHPCompatibility"/>
    <rule ref="phpcs.xml.dist"/>
</ruleset>

So, given the above example, the most logical processing order IMO would be:

1. Read in PHPCompatibility and set whatever it says.
2. Read in phpcs.xml.dist and set whatever it says, including potentially overruling things set in the PHPCompatibility ruleset.
3. Overrule colors with no-colors (if colors would have been set).
4. Set testVersion to 7.1.-

Leaving this for 4.0 makes sense.

As a temporary solution for now, I've just created a Windows .bat file with all the custom settings I can't overrule via the ruleset.

[gsherwood]

I'll close this issue in favour of the other one. It's linked now anyway.

I prototyped the line-by-line change for the other issue, but didn't commit it due to the potential BC break, so it's an easy win. But if you have another idea or how to fix this processing issue, post over there and we'll figure out a good way.

[jrfnl]

FYI: the reason why this is such a pain (aside from the issues with overruling the testVersion for PHPCompatibility as reported before), is that what with all the PRs I send in, I regularly run the PHPCS native ruleset over the PHPCS code and as colors is set in the phpcs.xml.dist file, the report on Windows is completely unreadable unless I pass --no-colors on the command-line.

Being able to just set it in a phpcs.xml file which includes the phpcs.xml.dist file would make life so much easier 😄

[jrfnl]

We're currently discussing a completely different solution to this problem in the livestream for PHPCS 4.0 (see PHPCSStandards/PHP_CodeSniffer#924) or join the livestream at: https://whereby.com/phpcs

The new solution can be seen here: PHPCSStandards/PHP_CodeSniffer#1005