diff --git a/src/Ruleset.php b/src/Ruleset.php index 34795889b6..2c243c0c9e 100644 --- a/src/Ruleset.php +++ b/src/Ruleset.php @@ -123,6 +123,14 @@ class Ruleset */ private $config = null; + /** + * The `` directives which have been applied. + * + * @var array Key is the name of the config. Value is the ruleset depth + * at which this config was applied (if not overruled from the CLI). + */ + private $configDirectivesApplied = []; + /** * An array of the names of sniffs which have been marked as deprecated. * @@ -574,17 +582,36 @@ public function processRuleset($rulesetPath, $depth=0) }//end foreach // Process custom sniff config settings. + // Processing rules: + // - Highest level ruleset take precedence. + // - If the same config is being set from two rulesets at the same level, *last* one "wins". foreach ($ruleset->{'config'} as $config) { if ($this->shouldProcessElement($config) === false) { continue; } - $this->config->setConfigData((string) $config['name'], (string) $config['value'], true); - if (PHP_CODESNIFFER_VERBOSITY > 1) { + $name = (string) $config['name']; + + if (isset($this->configDirectivesApplied[$name]) === true + && $this->configDirectivesApplied[$name] < $depth + ) { + // Ignore this config. A higher level ruleset has already set a value for this directive. + if (PHP_CODESNIFFER_VERBOSITY > 1) { + echo str_repeat("\t", $depth); + echo "\t=> ignoring config value ".$name.': '.(string) $config['value'].' => already changed by a higher level ruleset '.PHP_EOL; + } + + continue; + } + + $this->configDirectivesApplied[$name] = $depth; + + $applied = $this->config->setConfigData($name, (string) $config['value'], true); + if ($applied === true && PHP_CODESNIFFER_VERBOSITY > 1) { echo str_repeat("\t", $depth); - echo "\t=> set config value ".(string) $config['name'].': '.(string) $config['value'].PHP_EOL; + echo "\t=> set config value ".$name.': '.(string) $config['value'].PHP_EOL; } - } + }//end foreach foreach ($ruleset->rule as $rule) { if (isset($rule['ref']) === false diff --git a/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesATest.xml b/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesATest.xml new file mode 100644 index 0000000000..d97a9e7e40 --- /dev/null +++ b/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesATest.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + + + + diff --git a/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesBTest.xml b/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesBTest.xml new file mode 100644 index 0000000000..cac6030ea1 --- /dev/null +++ b/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesBTest.xml @@ -0,0 +1,17 @@ + + + + + + + + + + diff --git a/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesIncludeATest.xml b/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesIncludeATest.xml new file mode 100644 index 0000000000..b336d3b96c --- /dev/null +++ b/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesIncludeATest.xml @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesIncludeBTest.xml b/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesIncludeBTest.xml new file mode 100644 index 0000000000..39a1e3f059 --- /dev/null +++ b/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesIncludeBTest.xml @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + + + + diff --git a/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesSubIncludeATest.xml b/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesSubIncludeATest.xml new file mode 100644 index 0000000000..4a606dca8c --- /dev/null +++ b/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesSubIncludeATest.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + diff --git a/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesSubIncludeBTest.xml b/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesSubIncludeBTest.xml new file mode 100644 index 0000000000..d03b92c1a0 --- /dev/null +++ b/tests/Core/Ruleset/Fixtures/ProcessRulesetConfigDirectivesTest/ProcessRulesetConfigDirectivesSubIncludeBTest.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + diff --git a/tests/Core/Ruleset/ProcessRulesetConfigDirectivesTest.php b/tests/Core/Ruleset/ProcessRulesetConfigDirectivesTest.php new file mode 100644 index 0000000000..b98ec63112 --- /dev/null +++ b/tests/Core/Ruleset/ProcessRulesetConfigDirectivesTest.php @@ -0,0 +1,153 @@ +__destruct(); + } + + }//end tearDownAfterClass() + + + /** + * Verify the config directives are set based on the nesting level of the ruleset. + * + * - Highest level ruleset (root) should win over lower level (included) ruleset. + * - When two rulesets at the same "level" both set the same config, last included ruleset should win. + * - But if no higher level ruleset sets the value, the values from lowel levels should be applied. + * - The order of includes versus config directives in a ruleset file is deliberately irrelevant. + * + * @param string $name The name of the config setting we're checking. + * @param string $expected The expected value for that config setting. + * + * @dataProvider dataConfigDirectives + * + * @return void + */ + public function testConfigDirectives($name, $expected) + { + $this->assertSame($expected, self::$config->getConfigData($name)); + + }//end testConfigDirectives() + + + /** + * Data provider. + * + * @return array> + */ + public static function dataConfigDirectives() + { + return [ + 'Config set in parent A before includes; all includes try to overrule; parent A should win' => [ + 'name' => 'expectValueFromParentATestSetBeforeIncludes', + 'expected' => 'parent A', + ], + 'Config set in parent A after includes; all includes try to overrule; parent A should win' => [ + 'name' => 'expectValueFromParentATestSetAfterIncludes', + 'expected' => 'parent A', + ], + 'Config set in both parent A and parent B; B is included last via CLI; parent B should win' => [ + 'name' => 'expectValueFromParentBTest', + 'expected' => 'parent B', + ], + 'Config set in parent B; also set in non-direct grandchild B; parent B should win' => [ + 'name' => 'expectValueFromParentBTestAlsoInGrandChildB', + 'expected' => 'parent B', + ], + 'Config set in child A before includes; also set in grandchild A; child A should win' => [ + 'name' => 'expectValueFromChildATestSetBeforeIncludes', + 'expected' => 'child A', + ], + 'Config set in child A after includes; also set in grandchild B; child A should win' => [ + 'name' => 'expectValueFromChildATestSetAfterIncludes', + 'expected' => 'child A', + ], + 'Config set in both child A and child B; B is included last via ruleset includes; child B should win' => [ + 'name' => 'expectValueFromChildBTestInChildAAndChildB', + 'expected' => 'child B', + ], + 'Config set in child B; also set in non-direct grandchild A, child B should win' => [ + 'name' => 'expectValueFromChildBTestAlsoInGrandChildA', + 'expected' => 'child B', + ], + 'Config set in child B - no overrules' => [ + 'name' => 'expectValueFromChildBTest', + 'expected' => 'child B', + ], + 'Config set in grandchild A ruleset - no overrules' => [ + 'name' => 'expectValueFromGrandChildATest', + 'expected' => 'grandchild A', + ], + 'Config set in grandchild B ruleset - no overrules' => [ + 'name' => 'expectValueFromGrandChildBTest', + 'expected' => 'grandchild B', + ], + ]; + + }//end dataConfigDirectives() + + +}//end class