Multiple Config loading does not apply rulesets the same way

[baptistepillot]

I have goe a ruleset.xml file with this line :

<arg name="tab-width" value="4"/>

If il load the Config twice, they will not both have this rule : the first yes, the second not :

$config1 = new Config();
$ruleset = new Ruleset($config);
$ruleset->populateTokenListeners();
// the tabWidth configuration is set to 4 : everything is ok
// here Config::$overriddenDefaults['tab-width'] is set to tue
$config2 = new Config();
$ruleset = new Ruleset($config);
$ruleset->populateTokenListeners();
// with $overriddenDefaults['tab-width'] set to true, the tabWidth configuration becomes 0 : BAD

I patched it with this one, but it is a dirty patch to ensure my Config object initializes always the same way :

// (...)
$property = new ReflectionProperty(Config::class, 'overriddenDefaults');
$property->setAccessible(true);
$property->setValue([]);
$property->setAccessible(false);
$config2 = new Config();
// everything will be alright now

[jrfnl]

Related #2197 ?

[gsherwood]

$overriddenDefaults is a static member, which is why this problem is occurring for you. PHPCS is not designed to support this use case at the moment.

I'm pretty sure I could change $overriddenDefaults to be a non-static member var, as long as Config::setConfigData can also be changed to not being a static method. It is only called statically from Ruleset and itself, so this change is pretty easy.

But it obviously changes the PHPCS API, so I can't make this change before 4.0

[baptistepillot]

Ok, I will be patient, it works with my patch so it is not blocking.
Thanks a lot for your work.

[gsherwood]

I've now made this change in the 4.0 branch and can confirm your original code snippet works as expected.

[jrfnl]

I wonder whether this change will break sniff unit tests which test sniffs with different config settings. Also see #2899.

[gsherwood]

I wonder whether this change will break sniff unit tests which test sniffs with different config settings

The tests use a single config object, so they should still be working ok. Plus, they can (and do, for PHPCS tests at least) modify the Config object directly.

If the tests used multiple Config instances, they should be suffering from the problem described here - that setting the config values doesn't do anything the second time because PHPCS has already recorded that the value has been overridden.

So I think that this change would allows more flexibility when it comes to setting config values during testing, although I don't have a use case for it yet.