diff --git a/src/Config.php b/src/Config.php index d550f85e68..1c8d8dbdd1 100644 --- a/src/Config.php +++ b/src/Config.php @@ -104,6 +104,23 @@ class Config */ const DEFAULT_REPORT_WIDTH = 80; + /** + * Translation table for config settings which can be changed via multiple CLI flags. + * + * If the flag name matches the setting name, there is no need to add it to this translation table. + * Similarly, if there is only one flag which can change a setting, there is no need to include + * it in this table, even if the flag name and the setting name don't match. + * + * @var array Key is the CLI flag name, value the corresponding config setting name. + */ + public const CLI_FLAGS_TO_SETTING_NAME = [ + 'n' => 'warningSeverity', + 'w' => 'warningSeverity', + 'warning-severity' => 'warningSeverity', + 'no-colors' => 'colors', + 'no-cache' => 'cache', + ]; + /** * An array of settings that PHPCS and PHPCBF accept. * diff --git a/src/Ruleset.php b/src/Ruleset.php index 2c243c0c9e..3b23442a0f 100644 --- a/src/Ruleset.php +++ b/src/Ruleset.php @@ -131,6 +131,14 @@ class Ruleset */ private $configDirectivesApplied = []; + /** + * The `` directives which have been applied. + * + * @var array Key is the name of the setting. Value is the ruleset depth + * at which this setting was applied (if not overruled from the CLI). + */ + private $cliSettingsApplied = []; + /** * An array of the names of sniffs which have been marked as deprecated. * @@ -613,6 +621,92 @@ public function processRuleset($rulesetPath, $depth=0) } }//end foreach + // Process custom command line arguments. + // Processing rules: + // - Highest level ruleset take precedence. + // - If the same CLI flag is being set from two rulesets at the same level, *first* one "wins". + $cliArgs = []; + foreach ($ruleset->{'arg'} as $arg) { + if ($this->shouldProcessElement($arg) === false) { + continue; + } + + // "Long" CLI argument. Arg is in the format ``. + if (isset($arg['name']) === true) { + $name = (string) $arg['name']; + $cliSettingName = $name; + if (isset($this->config::CLI_FLAGS_TO_SETTING_NAME[$name]) === true) { + $cliSettingName = $this->config::CLI_FLAGS_TO_SETTING_NAME[$name]; + } + + if (isset($this->cliSettingsApplied[$cliSettingName]) === true + && $this->cliSettingsApplied[$cliSettingName] < $depth + ) { + // Ignore this CLI flag. A higher level ruleset has already set a value for this setting. + if (PHP_CODESNIFFER_VERBOSITY > 1) { + $statusMessage = str_repeat("\t", $depth); + $statusMessage .= "\t=> ignoring command line arg --".$name; + if (isset($arg['value']) === true) { + $statusMessage .= '='.(string) $arg['value']; + } + + echo $statusMessage.' => already changed by a higher level ruleset '.PHP_EOL; + } + + continue; + } + + // Remember which settings we've seen. + $this->cliSettingsApplied[$cliSettingName] = $depth; + + $argString = '--'.$name; + if (isset($arg['value']) === true) { + $argString .= '='.(string) $arg['value']; + } + } else { + // "Short" CLI flag. Arg is in the format `` and + // value can contain multiple flags, like ``. + $cleanedValue = ''; + $cliFlagsAsArray = str_split((string) $arg['value']); + foreach ($cliFlagsAsArray as $flag) { + $cliSettingName = $flag; + if (isset($this->config::CLI_FLAGS_TO_SETTING_NAME[$flag]) === true) { + $cliSettingName = $this->config::CLI_FLAGS_TO_SETTING_NAME[$flag]; + } + + if (isset($this->cliSettingsApplied[$cliSettingName]) === true + && $this->cliSettingsApplied[$cliSettingName] < $depth + ) { + // Ignore this CLI flag. A higher level ruleset has already set a value for this setting. + if (PHP_CODESNIFFER_VERBOSITY > 1) { + echo str_repeat("\t", $depth); + echo "\t=> ignoring command line flag -".$flag.' => already changed by a higher level ruleset '.PHP_EOL; + } + + continue; + } + + // Remember which settings we've seen. + $cleanedValue .= $flag; + $this->cliSettingsApplied[$cliSettingName] = $depth; + }//end foreach + + if ($cleanedValue === '') { + // No flags found which should be applied. + continue; + } + + $argString = '-'.$cleanedValue; + }//end if + + $cliArgs[] = $argString; + + if (PHP_CODESNIFFER_VERBOSITY > 1) { + echo str_repeat("\t", $depth); + echo "\t=> set command line value $argString".PHP_EOL; + } + }//end foreach + foreach ($ruleset->rule as $rule) { if (isset($rule['ref']) === false || $this->shouldProcessElement($rule) === false @@ -706,30 +800,6 @@ public function processRuleset($rulesetPath, $depth=0) $this->processRule($rule, $newSniffs, $depth); }//end foreach - // Process custom command line arguments. - $cliArgs = []; - foreach ($ruleset->{'arg'} as $arg) { - if ($this->shouldProcessElement($arg) === false) { - continue; - } - - if (isset($arg['name']) === true) { - $argString = '--'.(string) $arg['name']; - if (isset($arg['value']) === true) { - $argString .= '='.(string) $arg['value']; - } - } else { - $argString = '-'.(string) $arg['value']; - } - - $cliArgs[] = $argString; - - if (PHP_CODESNIFFER_VERBOSITY > 1) { - echo str_repeat("\t", $depth); - echo "\t=> set command line value $argString".PHP_EOL; - } - }//end foreach - // Set custom php ini values as CLI args. foreach ($ruleset->{'ini'} as $arg) { if ($this->shouldProcessElement($arg) === false) { diff --git a/tests/Core/Ruleset/Fixtures/ProcessRulesetCliArgsTest/.phpcs.xml b/tests/Core/Ruleset/Fixtures/ProcessRulesetCliArgsTest/.phpcs.xml new file mode 100644 index 0000000000..3e2f6e1066 --- /dev/null +++ b/tests/Core/Ruleset/Fixtures/ProcessRulesetCliArgsTest/.phpcs.xml @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/Core/Ruleset/Fixtures/ProcessRulesetCliArgsTest/config/phpcs/include-overrules.xml b/tests/Core/Ruleset/Fixtures/ProcessRulesetCliArgsTest/config/phpcs/include-overrules.xml new file mode 100644 index 0000000000..000d3d0415 --- /dev/null +++ b/tests/Core/Ruleset/Fixtures/ProcessRulesetCliArgsTest/config/phpcs/include-overrules.xml @@ -0,0 +1,15 @@ + + + + + + + + + + + + + diff --git a/tests/Core/Ruleset/Fixtures/ProcessRulesetCliArgsTest/phpcs.xml.dist b/tests/Core/Ruleset/Fixtures/ProcessRulesetCliArgsTest/phpcs.xml.dist new file mode 100644 index 0000000000..0a6b7e76b2 --- /dev/null +++ b/tests/Core/Ruleset/Fixtures/ProcessRulesetCliArgsTest/phpcs.xml.dist @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/Core/Ruleset/Fixtures/ProcessRulesetCliArgsTest/vendor/OrgName/ExtStandard/bootstrap.php b/tests/Core/Ruleset/Fixtures/ProcessRulesetCliArgsTest/vendor/OrgName/ExtStandard/bootstrap.php new file mode 100644 index 0000000000..ae64a5c679 --- /dev/null +++ b/tests/Core/Ruleset/Fixtures/ProcessRulesetCliArgsTest/vendor/OrgName/ExtStandard/bootstrap.php @@ -0,0 +1,3 @@ + + + + + + + + + + + + + + + diff --git a/tests/Core/Ruleset/ProcessRulesetCliArgsTest.php b/tests/Core/Ruleset/ProcessRulesetCliArgsTest.php new file mode 100644 index 0000000000..0231c5d84a --- /dev/null +++ b/tests/Core/Ruleset/ProcessRulesetCliArgsTest.php @@ -0,0 +1,225 @@ +__destruct(); + } + + }//end tearDownAfterClass() + + + /** + * Verify the CLI arguments passed from within rulesets 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 ini, 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 ini directives in a ruleset file is deliberately irrelevant. + * + * @param string $name The name of the PHP ini setting we're checking. + * @param mixed $expected The expected value for that ini setting. + * + * @dataProvider dataCliArgs + * + * @return void + */ + public function testCliArgs($name, $expected) + { + $this->assertSame($expected, self::$config->{$name}); + + }//end testCliArgs() + + + /** + * Data provider. + * + * @return array> + */ + public static function dataCliArgs() + { + return [ + 'Issue squiz#2395: "no-colors" set in parent before includes; "colors" set in child; parent should win' => [ + 'name' => 'colors', + 'expected' => false, + ], + 'Issue squiz#2395: "w" set in parent after includes; "n" set in child; warningSeverity in grandchild; parent should win' => [ + 'name' => 'warningSeverity', + 'expected' => 5, + ], + 'Issue squiz#2395: "q" in child A before includes; "p" set in grandchild A, child A should win' => [ + 'name' => 'showProgress', + 'expected' => false, + ], + + 'Issue squiz#2597: "extensions" set in parent before includes; also set in child; parent should win' => [ + 'name' => 'extensions', + 'expected' => [ + 'inc' => 'PHP', + 'install' => 'PHP', + 'module' => 'PHP', + 'php' => 'PHP', + 'profile' => 'PHP', + 'test' => 'PHP', + 'theme' => 'PHP', + ], + ], + 'Issue squiz#2602: "tab-width" set in parent after includes; also set in both children; parent should win' => [ + 'name' => 'tabWidth', + 'expected' => 3, + ], + + // Verify overrule order for various other miscellaneous settings. + 'Flag set in child A before includes; also set in grandchild A; child A should win' => [ + 'name' => 'parallel', + 'expected' => 10, + ], + 'Flag set in parent before includes; also set in grandchild A; parent should win' => [ + 'name' => 'reports', + 'expected' => [ + 'full' => null, + 'summary' => null, + 'source' => null, + ], + ], + 'Flag set in child A after includes; also set in child B; child A should win' => [ + 'name' => 'reportWidth', + 'expected' => 120, + ], + ]; + + }//end dataCliArgs() + + + /** + * Verify that path resolution for CLI flags passing paths is done based on the location of the ruleset setting the flag. + * + * @param string $name The name of the Config setting we're checking. + * @param string|array $expected The expected value for that setting. + * + * @dataProvider dataCliArgsWithPaths + * + * @return void + */ + public function testCliArgsWithPaths($name, $expected) + { + // Normalize slashes everywhere to allow the tests to pass on all OS-es. + $expected = $this->normalizeSlashes($expected); + $actual = $this->normalizeSlashes(self::$config->{$name}); + + $this->assertSame($expected, $actual); + + }//end testCliArgsWithPaths() + + + /** + * Data provider. + * + * @return array>> + */ + public static function dataCliArgsWithPaths() + { + return [ + 'Paths should be resolved based on the ruleset location: basepath' => [ + 'name' => 'basepath', + 'expected' => self::FIXTURE_DIR, + ], + 'Paths should be resolved based on the ruleset location: reportFile' => [ + 'name' => 'reportFile', + 'expected' => self::FIXTURE_DIR.'/config/phpcs/report.txt', + ], + 'Paths should be resolved based on the ruleset location: bootstrap' => [ + 'name' => 'bootstrap', + 'expected' => [ + self::FIXTURE_DIR.'/vendor/OrgName/ExtStandard/bootstrap.php', + ], + ], + ]; + + }//end dataCliArgsWithPaths() + + + /** + * Test helper to allow for path comparisons to succeed, independently of the OS on which the tests are run. + * + * @param string|array $path Path(s) to normalize. + * + * @return string|array + */ + private function normalizeSlashes($path) + { + if (is_array($path) === true) { + foreach ($path as $k => $v) { + $path[$k] = $this->normalizeSlashes($v); + } + + return $path; + } + + return str_replace(DIRECTORY_SEPARATOR, '/', $path); + + }//end normalizeSlashes() + + +}//end class