From 4d5b0c4a3fad21b23b5a1db7a7501d1424ddeca2 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 16 Feb 2024 18:52:14 +0000 Subject: [PATCH 01/22] Improve error message for an invalid sniff code --- src/Config.php | 98 ++++++++++--- tests/Core/Config/SniffsExcludeArgsTest.php | 146 +++++++++++++++----- 2 files changed, 191 insertions(+), 53 deletions(-) diff --git a/src/Config.php b/src/Config.php index 4e4c68f083..c39779465e 100644 --- a/src/Config.php +++ b/src/Config.php @@ -885,32 +885,14 @@ public function processLongArgument($arg, $pos) break; } - $sniffs = explode(',', substr($arg, 7)); - foreach ($sniffs as $sniff) { - if (substr_count($sniff, '.') !== 2) { - $error = 'ERROR: The specified sniff code "'.$sniff.'" is invalid'.PHP_EOL.PHP_EOL; - $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); - } - } - - $this->sniffs = $sniffs; + $this->sniffs = $this->parseSniffCodes(substr($arg, 7), 'sniffs'); self::$overriddenDefaults['sniffs'] = true; } else if (substr($arg, 0, 8) === 'exclude=') { if (isset(self::$overriddenDefaults['exclude']) === true) { break; } - $sniffs = explode(',', substr($arg, 8)); - foreach ($sniffs as $sniff) { - if (substr_count($sniff, '.') !== 2) { - $error = 'ERROR: The specified sniff code "'.$sniff.'" is invalid'.PHP_EOL.PHP_EOL; - $error .= $this->printShortUsage(true); - throw new DeepExitException($error, 3); - } - } - - $this->exclude = $sniffs; + $this->exclude = $this->parseSniffCodes(substr($arg, 8), 'exclude'); self::$overriddenDefaults['exclude'] = true; } else if (defined('PHP_CODESNIFFER_IN_TESTS') === false && substr($arg, 0, 6) === 'cache=' @@ -1658,4 +1640,80 @@ public function printConfigData($data) }//end printConfigData() + /** + * Parse supplied string into a list of sniff codes. + * + * @param string $input Comma-separated string of sniff codes. + * @param string $argument The name of the argument which is being processed. + * + * @return string[] + * @throws DeepExitException + */ + private function parseSniffCodes($input, $argument) + { + $errors = []; + $sniffs = []; + + $possibleSniffs = explode(',', $input); + $possibleSniffs = array_unique($possibleSniffs); + + foreach ($possibleSniffs as $sniff) { + $sniff = trim($sniff); + + if ($sniff === '') { + // Empty values can be safely ignored. + continue; + } + + if (preg_match('{[^A-Za-z0-9.]}', $sniff) === 1) { + $errors[] = 'Unsupported character detected: '.$sniff; + continue; + } + + $partCount = substr_count($sniff, '.'); + if ($partCount === 2) { + // Correct number of parts. + $sniffs[] = $sniff; + continue; + } + + if ($partCount === 0) { + $errors[] = 'Standard codes are not supported: '.$sniff; + } else if ($partCount === 1) { + $errors[] = 'Category codes are not supported: '.$sniff; + } else if ($partCount === 3) { + $errors[] = 'Message codes are not supported: '.$sniff; + } else { + $errors[] = 'Too many parts: '.$sniff; + } + + if ($partCount > 2) { + $parts = explode('.', $sniff, 4); + $sniffs[] = $parts[0].'.'.$parts[1].'.'.$parts[2]; + } + }//end foreach + + if ($errors !== []) { + $error = 'ERROR: The --'.$argument.' option only supports sniff codes.'.PHP_EOL; + $error .= 'Sniff codes are in the form "Standard.Category.Sniff"'.PHP_EOL; + $error .= PHP_EOL; + $error .= 'The following problems were detected:'.PHP_EOL; + $error .= '* '.implode(PHP_EOL.'* ', $errors).PHP_EOL; + + if ($sniffs !== []) { + $sniffs = array_unique($sniffs); + $error .= PHP_EOL; + $error .= 'Perhaps try --'.$argument.'="'.implode(',', $sniffs).'" instead.'.PHP_EOL; + } + + $error .= PHP_EOL; + $error .= $this->printShortUsage(true); + throw new DeepExitException(ltrim($error), 3); + } + + return $sniffs; + + }//end parseSniffCodes() + + }//end class diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index 4eedb624ea..cd4bcbf475 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -23,16 +23,31 @@ final class SniffsExcludeArgsTest extends TestCase /** * Ensure that the expected error message is returned for invalid arguments. * - * @param string $argument 'sniffs' or 'exclude'. - * @param string $value List of sniffs to include / exclude. - * @param string $message Expected error message text. + * @param string $argument 'sniffs' or 'exclude'. + * @param string $value List of sniffs to include / exclude. + * @param array $errors Sniff code and associated help text. + * @param string|null $suggestion Help text shown to end user with correct syntax for argument. * * @return void * @dataProvider dataInvalidSniffs */ - public function testInvalid($argument, $value, $message) + public function testInvalid($argument, $value, $errors, $suggestion) { $exception = 'PHP_CodeSniffer\Exceptions\DeepExitException'; + $message = 'ERROR: The --'.$argument.' option only supports sniff codes.'.PHP_EOL; + $message .= 'Sniff codes are in the form "Standard.Category.Sniff"'.PHP_EOL; + $message .= PHP_EOL; + $message .= 'The following problems were detected:'.PHP_EOL; + $message .= '* '.implode(PHP_EOL.'* ', $errors).PHP_EOL; + + if ($suggestion !== null) { + $message .= PHP_EOL; + $message .= "Perhaps try --$argument=\"$suggestion\" instead.".PHP_EOL; + } + + $message .= PHP_EOL; + $message .= 'Run "phpcs --help" for usage information'.PHP_EOL; + $message .= PHP_EOL; if (method_exists($this, 'expectException') === true) { // PHPUnit 5+. @@ -62,47 +77,76 @@ public static function dataInvalidSniffs() ]; $data = []; - $messageTemplate = 'ERROR: The specified sniff code "%s" is invalid'.PHP_EOL.PHP_EOL; + $messageTemplate = 'ERROR: The specified sniff code "%s" is invalid'.PHP_EOL; foreach ($arguments as $argument) { - // An empty string is not a valid sniff. - $data[$argument.'; empty string'] = [ - 'argument' => $argument, - 'value' => '', - 'message' => sprintf($messageTemplate, ''), - ]; - // A standard is not a valid sniff. $data[$argument.'; standard'] = [ - 'argument' => $argument, - 'value' => 'Standard', - 'message' => sprintf($messageTemplate, 'Standard'), + 'argument' => $argument, + 'value' => 'Standard', + 'errors' => [ + 'Standard codes are not supported: Standard', + ], + 'suggestion' => null, ]; // A category is not a valid sniff. $data[$argument.'; category'] = [ - 'argument' => $argument, - 'value' => 'Standard.Category', - 'message' => sprintf($messageTemplate, 'Standard.Category'), + 'argument' => $argument, + 'value' => 'Standard.Category', + 'errors' => [ + 'Category codes are not supported: Standard.Category', + ], + 'suggestion' => null, ]; // An error-code is not a valid sniff. $data[$argument.'; error-code'] = [ - 'argument' => $argument, - 'value' => 'Standard.Category', - 'message' => sprintf($messageTemplate, 'Standard.Category'), + 'argument' => $argument, + 'value' => 'Standard.Category.Sniff.Code', + 'errors' => [ + 'Message codes are not supported: Standard.Category.Sniff.Code', + ], + 'suggestion' => 'Standard.Category.Sniff', + ]; + + // Too many dots. + $data[$argument.'; too many dots'] = [ + 'argument' => $argument, + 'value' => 'Standard.Category.Sniff.Code.Extra', + 'errors' => [ + 'Too many parts: Standard.Category.Sniff.Code.Extra', + ], + 'suggestion' => 'Standard.Category.Sniff', ]; - // Only the first error is reported. + // All errors are reported in one go. $data[$argument.'; two errors'] = [ - 'argument' => $argument, - 'value' => 'StandardOne,StandardTwo', - 'message' => sprintf($messageTemplate, 'StandardOne'), + 'argument' => $argument, + 'value' => 'StandardOne,StandardTwo', + 'errors' => [ + 'Standard codes are not supported: StandardOne', + 'Standard codes are not supported: StandardTwo', + ], + 'suggestion' => null, ]; + + // Order of valid/invalid does not impact error reporting. $data[$argument.'; valid followed by invalid'] = [ - 'argument' => $argument, - 'value' => 'StandardOne.Category.Sniff,StandardTwo.Category', - 'message' => sprintf($messageTemplate, 'StandardTwo.Category'), + 'argument' => $argument, + 'value' => 'StandardOne.Category.Sniff,StandardTwo.Category', + 'errors' => [ + 'Category codes are not supported: StandardTwo.Category', + ], + 'suggestion' => 'StandardOne.Category.Sniff', + ]; + $data[$argument.'; invalid followed by valid'] = [ + 'argument' => $argument, + 'value' => 'StandardOne.Category,StandardTwo.Category.Sniff', + 'errors' => [ + 'Category codes are not supported: StandardOne.Category', + ], + 'suggestion' => 'StandardTwo.Category.Sniff', ]; }//end foreach @@ -114,17 +158,18 @@ public static function dataInvalidSniffs() /** * Ensure that the valid data does not throw an exception, and the value is stored. * - * @param string $argument 'sniffs' or 'exclude'. - * @param string $value List of sniffs to include or exclude. + * @param string $argument 'sniffs' or 'exclude'. + * @param string $value List of sniffs to include or exclude. + * @param string[] $result Expected sniffs to be set on the Config object. * * @return void * @dataProvider dataValidSniffs */ - public function testValid($argument, $value) + public function testValid($argument, $value, $result) { $config = new ConfigDouble(["--$argument=$value"]); - $this->assertSame(explode(',', $value), $config->$argument); + $this->assertSame($result, $config->$argument); }//end testValid() @@ -144,15 +189,50 @@ public static function dataValidSniffs() $data = []; foreach ($arguments as $argument) { + $data[$argument.'; empty string'] = [ + 'argument' => $argument, + 'value' => '', + 'result' => [], + ]; $data[$argument.'; one valid sniff'] = [ 'argument' => $argument, 'value' => 'Standard.Category.Sniff', + 'result' => ['Standard.Category.Sniff'], ]; $data[$argument.'; two valid sniffs'] = [ 'argument' => $argument, 'value' => 'StandardOne.Category.Sniff,StandardTwo.Category.Sniff', + 'result' => [ + 'StandardOne.Category.Sniff', + 'StandardTwo.Category.Sniff', + ], ]; - } + + // Rogue commas are quietly ignored. + $data[$argument.'; one comma alone'] = [ + 'argument' => $argument, + 'value' => ',', + 'result' => [], + ]; + $data[$argument.'; two commas alone'] = [ + 'argument' => $argument, + 'value' => ',,', + 'result' => [], + ]; + $data[$argument.'; trailing comma'] = [ + 'argument' => $argument, + 'value' => 'Standard.Category.Sniff,', + 'result' => ['Standard.Category.Sniff'], + ]; + $data[$argument.'; double comma between sniffs'] = [ + 'argument' => $argument, + 'value' => 'StandardOne.Category.Sniff,,StandardTwo.Category.Sniff', + 'result' => [ + 'StandardOne.Category.Sniff', + 'StandardTwo.Category.Sniff', + ], + ]; + }//end foreach return $data; From 8dd0102ce8c2e846a8dae03955ca62c86c9ddb6a Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 18:30:39 +0000 Subject: [PATCH 02/22] Update src/Config.php Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com> --- src/Config.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Config.php b/src/Config.php index c39779465e..780beb19c8 100644 --- a/src/Config.php +++ b/src/Config.php @@ -1641,7 +1641,7 @@ public function printConfigData($data) /** - * Parse supplied string into a list of sniff codes. + * Parse supplied string into a list of validated sniff codes. * * @param string $input Comma-separated string of sniff codes. * @param string $argument The name of the argument which is being processed. From 995522f2cb9cf34231e36a40ac89f163c765e941 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 18:33:11 +0000 Subject: [PATCH 03/22] Update src/Config.php Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com> --- src/Config.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Config.php b/src/Config.php index 780beb19c8..3587bf65f6 100644 --- a/src/Config.php +++ b/src/Config.php @@ -1647,7 +1647,7 @@ public function printConfigData($data) * @param string $argument The name of the argument which is being processed. * * @return string[] - * @throws DeepExitException + * @throws DeepExitException When any of the provided codes are not valid as sniff codes. */ private function parseSniffCodes($input, $argument) { From 792cf6653b5593cb3afcfed647bd4ad3b0117afe Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:00:24 +0000 Subject: [PATCH 04/22] Report all errors, even if duplicates --- src/Config.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Config.php b/src/Config.php index 3587bf65f6..5da5b5d1be 100644 --- a/src/Config.php +++ b/src/Config.php @@ -1655,7 +1655,6 @@ private function parseSniffCodes($input, $argument) $sniffs = []; $possibleSniffs = explode(',', $input); - $possibleSniffs = array_unique($possibleSniffs); foreach ($possibleSniffs as $sniff) { $sniff = trim($sniff); From 4ed5733a90c80314de71e0ffd94c6411595f3cdf Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:00:58 +0000 Subject: [PATCH 05/22] Filter out empty values sooner --- src/Config.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Config.php b/src/Config.php index 5da5b5d1be..f8526016d4 100644 --- a/src/Config.php +++ b/src/Config.php @@ -1654,16 +1654,11 @@ private function parseSniffCodes($input, $argument) $errors = []; $sniffs = []; - $possibleSniffs = explode(',', $input); + $possibleSniffs = array_filter(explode(',', $input)); foreach ($possibleSniffs as $sniff) { $sniff = trim($sniff); - if ($sniff === '') { - // Empty values can be safely ignored. - continue; - } - if (preg_match('{[^A-Za-z0-9.]}', $sniff) === 1) { $errors[] = 'Unsupported character detected: '.$sniff; continue; From 9850de46c54686d36e4e3e032a60607c3fcd8f97 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:01:27 +0000 Subject: [PATCH 06/22] Complain if nothing (or only commas) specified --- src/Config.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Config.php b/src/Config.php index f8526016d4..94bf48695c 100644 --- a/src/Config.php +++ b/src/Config.php @@ -1656,6 +1656,10 @@ private function parseSniffCodes($input, $argument) $possibleSniffs = array_filter(explode(',', $input)); + if ($possibleSniffs === []) { + $errors[] = 'No codes specified / empty argument'; + } + foreach ($possibleSniffs as $sniff) { $sniff = trim($sniff); From 95d59550c9d1b4c86345dafab79f879a53572a09 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:02:55 +0000 Subject: [PATCH 07/22] Ignore case when removing duplicates from final list --- src/Config.php | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Config.php b/src/Config.php index 94bf48695c..ce0a854973 100644 --- a/src/Config.php +++ b/src/Config.php @@ -1691,6 +1691,21 @@ private function parseSniffCodes($input, $argument) } }//end foreach + $sniffs = array_reduce($sniffs, static function ($carry, $item) { + $lower = strtolower($item); + + foreach ($carry as $found) { + if ($lower === strtolower($found)) { + // This sniff is already in our list. + return $carry; + } + } + + $carry[] = $item; + + return $carry; + }, []); + if ($errors !== []) { $error = 'ERROR: The --'.$argument.' option only supports sniff codes.'.PHP_EOL; $error .= 'Sniff codes are in the form "Standard.Category.Sniff"'.PHP_EOL; @@ -1699,7 +1714,6 @@ private function parseSniffCodes($input, $argument) $error .= '* '.implode(PHP_EOL.'* ', $errors).PHP_EOL; if ($sniffs !== []) { - $sniffs = array_unique($sniffs); $error .= PHP_EOL; $error .= 'Perhaps try --'.$argument.'="'.implode(',', $sniffs).'" instead.'.PHP_EOL; } From 5c48598b131afa21ddbb09d84a4c7ff94bfb7216 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:03:22 +0000 Subject: [PATCH 08/22] Add missing `@covers` annotation --- tests/Core/Config/SniffsExcludeArgsTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index cd4bcbf475..c76d4a603d 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -14,6 +14,7 @@ /** * Tests for the \PHP_CodeSniffer\Config --sniffs and --exclude arguments. * + * @covers \PHP_CodeSniffer\Config::parseSniffCodes * @covers \PHP_CodeSniffer\Config::processLongArgument */ final class SniffsExcludeArgsTest extends TestCase From d36d44ac8caea38bf5f3323ac6966fe72328012b Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:06:38 +0000 Subject: [PATCH 09/22] Update src/Config.php Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com> --- src/Config.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Config.php b/src/Config.php index ce0a854973..a0de835a6b 100644 --- a/src/Config.php +++ b/src/Config.php @@ -1708,7 +1708,7 @@ private function parseSniffCodes($input, $argument) if ($errors !== []) { $error = 'ERROR: The --'.$argument.' option only supports sniff codes.'.PHP_EOL; - $error .= 'Sniff codes are in the form "Standard.Category.Sniff"'.PHP_EOL; + $error .= 'Sniff codes are in the form "Standard.Category.Sniff".'.PHP_EOL; $error .= PHP_EOL; $error .= 'The following problems were detected:'.PHP_EOL; $error .= '* '.implode(PHP_EOL.'* ', $errors).PHP_EOL; From c2aec40c6e5ea18a608ddd2b28b6823d9c042578 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:09:50 +0000 Subject: [PATCH 10/22] Remove invalid assumption regarding valid characters --- src/Config.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Config.php b/src/Config.php index a0de835a6b..5dea43d5f4 100644 --- a/src/Config.php +++ b/src/Config.php @@ -1663,11 +1663,6 @@ private function parseSniffCodes($input, $argument) foreach ($possibleSniffs as $sniff) { $sniff = trim($sniff); - if (preg_match('{[^A-Za-z0-9.]}', $sniff) === 1) { - $errors[] = 'Unsupported character detected: '.$sniff; - continue; - } - $partCount = substr_count($sniff, '.'); if ($partCount === 2) { // Correct number of parts. From d5f5fe0d8cd7080bc7813a0a47ebaf5ead8228c1 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:16:36 +0000 Subject: [PATCH 11/22] Update test to reflect change via GitHub --- tests/Core/Config/SniffsExcludeArgsTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index c76d4a603d..7d243a6fdb 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -36,7 +36,7 @@ public function testInvalid($argument, $value, $errors, $suggestion) { $exception = 'PHP_CodeSniffer\Exceptions\DeepExitException'; $message = 'ERROR: The --'.$argument.' option only supports sniff codes.'.PHP_EOL; - $message .= 'Sniff codes are in the form "Standard.Category.Sniff"'.PHP_EOL; + $message .= 'Sniff codes are in the form "Standard.Category.Sniff".'.PHP_EOL; $message .= PHP_EOL; $message .= 'The following problems were detected:'.PHP_EOL; $message .= '* '.implode(PHP_EOL.'* ', $errors).PHP_EOL; From 994bb1c6de18e0d1ecd5e26b871086ef52e2bd32 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:18:11 +0000 Subject: [PATCH 12/22] Update test: treat empty values as errors --- tests/Core/Config/SniffsExcludeArgsTest.php | 41 +++++++++++++-------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index 7d243a6fdb..920bf4b2b5 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -81,6 +81,32 @@ public static function dataInvalidSniffs() $messageTemplate = 'ERROR: The specified sniff code "%s" is invalid'.PHP_EOL; foreach ($arguments as $argument) { + // Empty values are errors. + $data[$argument.'; empty string'] = [ + 'argument' => $argument, + 'value' => '', + 'errors' => [ + 'No codes specified / empty argument', + ], + 'suggestion' => null, + ]; + $data[$argument.'; one comma alone'] = [ + 'argument' => $argument, + 'value' => ',', + 'errors' => [ + 'No codes specified / empty argument', + ], + 'suggestion' => null, + ]; + $data[$argument.'; two commas alone'] = [ + 'argument' => $argument, + 'value' => ',,', + 'errors' => [ + 'No codes specified / empty argument', + ], + 'suggestion' => null, + ]; + // A standard is not a valid sniff. $data[$argument.'; standard'] = [ 'argument' => $argument, @@ -190,11 +216,6 @@ public static function dataValidSniffs() $data = []; foreach ($arguments as $argument) { - $data[$argument.'; empty string'] = [ - 'argument' => $argument, - 'value' => '', - 'result' => [], - ]; $data[$argument.'; one valid sniff'] = [ 'argument' => $argument, 'value' => 'Standard.Category.Sniff', @@ -210,16 +231,6 @@ public static function dataValidSniffs() ]; // Rogue commas are quietly ignored. - $data[$argument.'; one comma alone'] = [ - 'argument' => $argument, - 'value' => ',', - 'result' => [], - ]; - $data[$argument.'; two commas alone'] = [ - 'argument' => $argument, - 'value' => ',,', - 'result' => [], - ]; $data[$argument.'; trailing comma'] = [ 'argument' => $argument, 'value' => 'Standard.Category.Sniff,', From 0dcd4efde718a81d9679e6cb62d600cfc5d70841 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:19:53 +0000 Subject: [PATCH 13/22] Remove unused variable --- tests/Core/Config/SniffsExcludeArgsTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index 920bf4b2b5..a863dbea37 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -78,8 +78,6 @@ public static function dataInvalidSniffs() ]; $data = []; - $messageTemplate = 'ERROR: The specified sniff code "%s" is invalid'.PHP_EOL; - foreach ($arguments as $argument) { // Empty values are errors. $data[$argument.'; empty string'] = [ From bccdffc7e9c099d9f35e8e0865a1cdf0b2efdaf9 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:22:18 +0000 Subject: [PATCH 14/22] Update docblocks to match reality --- tests/Core/Config/SniffsExcludeArgsTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index a863dbea37..ff83950733 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -68,7 +68,7 @@ public function testInvalid($argument, $value, $errors, $suggestion) * Data provider for testInvalid(). * * @see self::testInvalid() - * @return array> + * @return array> */ public static function dataInvalidSniffs() { @@ -203,7 +203,7 @@ public function testValid($argument, $value, $result) * Data provider for testValid(). * * @see self::testValid() - * @return array> + * @return array */ public static function dataValidSniffs() { From 3421e004d5112d2a7dacdc416b5777494bc6a080 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:23:32 +0000 Subject: [PATCH 15/22] Apply whitespace changes from phpcbf --- src/Config.php | 26 ++++++++++++--------- tests/Core/Config/SniffsExcludeArgsTest.php | 20 ++++++++-------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/Config.php b/src/Config.php index db05bbe7a0..5de6350bd8 100644 --- a/src/Config.php +++ b/src/Config.php @@ -1686,20 +1686,24 @@ private function parseSniffCodes($input, $argument) } }//end foreach - $sniffs = array_reduce($sniffs, static function ($carry, $item) { - $lower = strtolower($item); - - foreach ($carry as $found) { - if ($lower === strtolower($found)) { - // This sniff is already in our list. - return $carry; + $sniffs = array_reduce( + $sniffs, + static function ($carry, $item) { + $lower = strtolower($item); + + foreach ($carry as $found) { + if ($lower === strtolower($found)) { + // This sniff is already in our list. + return $carry; + } } - } - $carry[] = $item; + $carry[] = $item; - return $carry; - }, []); + return $carry; + }, + [] + ); if ($errors !== []) { $error = 'ERROR: The --'.$argument.' option only supports sniff codes.'.PHP_EOL; diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index ff83950733..bd0440f9ed 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -81,25 +81,25 @@ public static function dataInvalidSniffs() foreach ($arguments as $argument) { // Empty values are errors. $data[$argument.'; empty string'] = [ - 'argument' => $argument, - 'value' => '', - 'errors' => [ + 'argument' => $argument, + 'value' => '', + 'errors' => [ 'No codes specified / empty argument', ], 'suggestion' => null, ]; $data[$argument.'; one comma alone'] = [ - 'argument' => $argument, - 'value' => ',', - 'errors' => [ + 'argument' => $argument, + 'value' => ',', + 'errors' => [ 'No codes specified / empty argument', ], 'suggestion' => null, ]; $data[$argument.'; two commas alone'] = [ - 'argument' => $argument, - 'value' => ',,', - 'errors' => [ + 'argument' => $argument, + 'value' => ',,', + 'errors' => [ 'No codes specified / empty argument', ], 'suggestion' => null, @@ -229,7 +229,7 @@ public static function dataValidSniffs() ]; // Rogue commas are quietly ignored. - $data[$argument.'; trailing comma'] = [ + $data[$argument.'; trailing comma'] = [ 'argument' => $argument, 'value' => 'Standard.Category.Sniff,', 'result' => ['Standard.Category.Sniff'], From 99753e879b452eb7b85125a0fb013755d45a1da0 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:32:17 +0000 Subject: [PATCH 16/22] Add more tests --- tests/Core/Config/SniffsExcludeArgsTest.php | 32 +++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index bd0440f9ed..8b4dcda1b7 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -173,6 +173,26 @@ public static function dataInvalidSniffs() ], 'suggestion' => 'StandardTwo.Category.Sniff', ]; + + // Different cases are reported individually (in duplicate), but suggestions are reduced. + $data[$argument.'; case mismatch - different errors'] = [ + 'argument' => $argument, + 'value' => 'Standard.Category.Sniff.Code,sTANDARD.cATEGORY.sNIFF.cODE.eXTRA', + 'errors' => [ + 'Message codes are not supported: Standard.Category.Sniff.Code', + 'Too many parts: sTANDARD.cATEGORY.sNIFF.cODE.eXTRA', + ], + 'suggestion' => 'Standard.Category.Sniff', + ]; + $data[$argument.'; case mismatch - same error'] = [ + 'argument' => $argument, + 'value' => 'sTANDARD.cATEGORY.sNIFF.cODE,Standard.Category.Sniff.Code', + 'errors' => [ + 'Message codes are not supported: sTANDARD.cATEGORY.sNIFF.cODE', + 'Message codes are not supported: Standard.Category.Sniff.Code', + ], + 'suggestion' => 'sTANDARD.cATEGORY.sNIFF', + ]; }//end foreach return $data; @@ -242,6 +262,18 @@ public static function dataValidSniffs() 'StandardTwo.Category.Sniff', ], ]; + + // Duplicates are reduced silently. + $data[$argument.'; one valid sniff twice'] = [ + 'argument' => $argument, + 'value' => 'Standard.Category.Sniff,Standard.Category.Sniff', + 'result' => ['Standard.Category.Sniff'], + ]; + $data[$argument.'; one valid sniff in different cases'] = [ + 'argument' => $argument, + 'value' => 'Standard.Category.Sniff, standard.category.sniff, STANDARD.CATEGORY.SNIFF', + 'result' => ['Standard.Category.Sniff'], + ]; }//end foreach return $data; From d1bfb0a2c1638cdafa4dec40fc1f5e7f54f55aa1 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Fri, 10 Jan 2025 19:43:04 +0000 Subject: [PATCH 17/22] Apply whitespace changes from phpcbf --- tests/Core/Config/SniffsExcludeArgsTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index 8b4dcda1b7..4ea286c95e 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -184,7 +184,7 @@ public static function dataInvalidSniffs() ], 'suggestion' => 'Standard.Category.Sniff', ]; - $data[$argument.'; case mismatch - same error'] = [ + $data[$argument.'; case mismatch - same error'] = [ 'argument' => $argument, 'value' => 'sTANDARD.cATEGORY.sNIFF.cODE,Standard.Category.Sniff.Code', 'errors' => [ @@ -264,12 +264,12 @@ public static function dataValidSniffs() ]; // Duplicates are reduced silently. - $data[$argument.'; one valid sniff twice'] = [ + $data[$argument.'; one valid sniff twice'] = [ 'argument' => $argument, 'value' => 'Standard.Category.Sniff,Standard.Category.Sniff', 'result' => ['Standard.Category.Sniff'], ]; - $data[$argument.'; one valid sniff in different cases'] = [ + $data[$argument.'; one valid sniff in different cases'] = [ 'argument' => $argument, 'value' => 'Standard.Category.Sniff, standard.category.sniff, STANDARD.CATEGORY.SNIFF', 'result' => ['Standard.Category.Sniff'], From b270da2d315ab0aada7cab69efbd56b928190287 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Thu, 23 Jan 2025 22:13:30 +0000 Subject: [PATCH 18/22] Move method further up the file This class is (too) long. --- src/Config.php | 174 ++++++++++++++++++++++++------------------------- 1 file changed, 87 insertions(+), 87 deletions(-) diff --git a/src/Config.php b/src/Config.php index 5de6350bd8..78e9f227ee 100644 --- a/src/Config.php +++ b/src/Config.php @@ -1259,6 +1259,93 @@ public function processLongArgument($arg, $pos) }//end processLongArgument() + /** + * Parse supplied string into a list of validated sniff codes. + * + * @param string $input Comma-separated string of sniff codes. + * @param string $argument The name of the argument which is being processed. + * + * @return string[] + * @throws DeepExitException When any of the provided codes are not valid as sniff codes. + */ + private function parseSniffCodes($input, $argument) + { + $errors = []; + $sniffs = []; + + $possibleSniffs = array_filter(explode(',', $input)); + + if ($possibleSniffs === []) { + $errors[] = 'No codes specified / empty argument'; + } + + foreach ($possibleSniffs as $sniff) { + $sniff = trim($sniff); + + $partCount = substr_count($sniff, '.'); + if ($partCount === 2) { + // Correct number of parts. + $sniffs[] = $sniff; + continue; + } + + if ($partCount === 0) { + $errors[] = 'Standard codes are not supported: '.$sniff; + } else if ($partCount === 1) { + $errors[] = 'Category codes are not supported: '.$sniff; + } else if ($partCount === 3) { + $errors[] = 'Message codes are not supported: '.$sniff; + } else { + $errors[] = 'Too many parts: '.$sniff; + } + + if ($partCount > 2) { + $parts = explode('.', $sniff, 4); + $sniffs[] = $parts[0].'.'.$parts[1].'.'.$parts[2]; + } + }//end foreach + + $sniffs = array_reduce( + $sniffs, + static function ($carry, $item) { + $lower = strtolower($item); + + foreach ($carry as $found) { + if ($lower === strtolower($found)) { + // This sniff is already in our list. + return $carry; + } + } + + $carry[] = $item; + + return $carry; + }, + [] + ); + + if ($errors !== []) { + $error = 'ERROR: The --'.$argument.' option only supports sniff codes.'.PHP_EOL; + $error .= 'Sniff codes are in the form "Standard.Category.Sniff".'.PHP_EOL; + $error .= PHP_EOL; + $error .= 'The following problems were detected:'.PHP_EOL; + $error .= '* '.implode(PHP_EOL.'* ', $errors).PHP_EOL; + + if ($sniffs !== []) { + $error .= PHP_EOL; + $error .= 'Perhaps try --'.$argument.'="'.implode(',', $sniffs).'" instead.'.PHP_EOL; + } + + $error .= PHP_EOL; + $error .= $this->printShortUsage(true); + throw new DeepExitException(ltrim($error), 3); + } + + return $sniffs; + + }//end parseSniffCodes() + + /** * Processes an unknown command line argument. * @@ -1640,91 +1727,4 @@ public function printConfigData($data) }//end printConfigData() - /** - * Parse supplied string into a list of validated sniff codes. - * - * @param string $input Comma-separated string of sniff codes. - * @param string $argument The name of the argument which is being processed. - * - * @return string[] - * @throws DeepExitException When any of the provided codes are not valid as sniff codes. - */ - private function parseSniffCodes($input, $argument) - { - $errors = []; - $sniffs = []; - - $possibleSniffs = array_filter(explode(',', $input)); - - if ($possibleSniffs === []) { - $errors[] = 'No codes specified / empty argument'; - } - - foreach ($possibleSniffs as $sniff) { - $sniff = trim($sniff); - - $partCount = substr_count($sniff, '.'); - if ($partCount === 2) { - // Correct number of parts. - $sniffs[] = $sniff; - continue; - } - - if ($partCount === 0) { - $errors[] = 'Standard codes are not supported: '.$sniff; - } else if ($partCount === 1) { - $errors[] = 'Category codes are not supported: '.$sniff; - } else if ($partCount === 3) { - $errors[] = 'Message codes are not supported: '.$sniff; - } else { - $errors[] = 'Too many parts: '.$sniff; - } - - if ($partCount > 2) { - $parts = explode('.', $sniff, 4); - $sniffs[] = $parts[0].'.'.$parts[1].'.'.$parts[2]; - } - }//end foreach - - $sniffs = array_reduce( - $sniffs, - static function ($carry, $item) { - $lower = strtolower($item); - - foreach ($carry as $found) { - if ($lower === strtolower($found)) { - // This sniff is already in our list. - return $carry; - } - } - - $carry[] = $item; - - return $carry; - }, - [] - ); - - if ($errors !== []) { - $error = 'ERROR: The --'.$argument.' option only supports sniff codes.'.PHP_EOL; - $error .= 'Sniff codes are in the form "Standard.Category.Sniff".'.PHP_EOL; - $error .= PHP_EOL; - $error .= 'The following problems were detected:'.PHP_EOL; - $error .= '* '.implode(PHP_EOL.'* ', $errors).PHP_EOL; - - if ($sniffs !== []) { - $error .= PHP_EOL; - $error .= 'Perhaps try --'.$argument.'="'.implode(',', $sniffs).'" instead.'.PHP_EOL; - } - - $error .= PHP_EOL; - $error .= $this->printShortUsage(true); - throw new DeepExitException(ltrim($error), 3); - } - - return $sniffs; - - }//end parseSniffCodes() - - }//end class From d0e1cf8b84e65a0374fb392c361f029408cf8a8c Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Thu, 23 Jan 2025 22:30:27 +0000 Subject: [PATCH 19/22] Correct param type for $errors --- tests/Core/Config/SniffsExcludeArgsTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index 4ea286c95e..cd7e0edaa5 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -24,10 +24,10 @@ final class SniffsExcludeArgsTest extends TestCase /** * Ensure that the expected error message is returned for invalid arguments. * - * @param string $argument 'sniffs' or 'exclude'. - * @param string $value List of sniffs to include / exclude. - * @param array $errors Sniff code and associated help text. - * @param string|null $suggestion Help text shown to end user with correct syntax for argument. + * @param string $argument 'sniffs' or 'exclude'. + * @param string $value List of sniffs to include / exclude. + * @param array $errors Sniff code and associated help text. + * @param string|null $suggestion Help text shown to end user with correct syntax for argument. * * @return void * @dataProvider dataInvalidSniffs From b127df4bd150df3b3f97bc65d3d25c7ac3de5492 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Thu, 23 Jan 2025 22:30:48 +0000 Subject: [PATCH 20/22] Use phpstan-style return types --- tests/Core/Config/SniffsExcludeArgsTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index cd7e0edaa5..451887e0c5 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -68,7 +68,7 @@ public function testInvalid($argument, $value, $errors, $suggestion) * Data provider for testInvalid(). * * @see self::testInvalid() - * @return array> + * @return non-empty-array */ public static function dataInvalidSniffs() { @@ -223,7 +223,7 @@ public function testValid($argument, $value, $result) * Data provider for testValid(). * * @see self::testValid() - * @return array + * @return non-empty-array */ public static function dataValidSniffs() { From 5b5208b081649023855dd9fc2152bacb9e602085 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Thu, 23 Jan 2025 22:34:58 +0000 Subject: [PATCH 21/22] Remove word 'sniffs' from method names --- tests/Core/Config/SniffsExcludeArgsTest.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index 451887e0c5..c88e6934c4 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -30,7 +30,7 @@ final class SniffsExcludeArgsTest extends TestCase * @param string|null $suggestion Help text shown to end user with correct syntax for argument. * * @return void - * @dataProvider dataInvalidSniffs + * @dataProvider dataInvalid */ public function testInvalid($argument, $value, $errors, $suggestion) { @@ -70,7 +70,7 @@ public function testInvalid($argument, $value, $errors, $suggestion) * @see self::testInvalid() * @return non-empty-array */ - public static function dataInvalidSniffs() + public static function dataInvalid() { $arguments = [ 'sniffs', @@ -197,7 +197,7 @@ public static function dataInvalidSniffs() return $data; - }//end dataInvalidSniffs() + }//end dataInvalid() /** @@ -208,7 +208,7 @@ public static function dataInvalidSniffs() * @param string[] $result Expected sniffs to be set on the Config object. * * @return void - * @dataProvider dataValidSniffs + * @dataProvider dataValid */ public function testValid($argument, $value, $result) { @@ -225,7 +225,7 @@ public function testValid($argument, $value, $result) * @see self::testValid() * @return non-empty-array */ - public static function dataValidSniffs() + public static function dataValid() { $arguments = [ 'sniffs', @@ -278,7 +278,7 @@ public static function dataValidSniffs() return $data; - }//end dataValidSniffs() + }//end dataValid() /** From f87a5e1858d2464a37b02396e1471dcb54d0ccd1 Mon Sep 17 00:00:00 2001 From: Dan Wallis Date: Tue, 11 Feb 2025 19:45:01 +0000 Subject: [PATCH 22/22] Use a different syntax for special comments --- src/Config.php | 2 +- tests/Core/Config/SniffsExcludeArgsTest.php | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Config.php b/src/Config.php index 78e9f227ee..19c0da5007 100644 --- a/src/Config.php +++ b/src/Config.php @@ -1265,7 +1265,7 @@ public function processLongArgument($arg, $pos) * @param string $input Comma-separated string of sniff codes. * @param string $argument The name of the argument which is being processed. * - * @return string[] + * @return array * @throws DeepExitException When any of the provided codes are not valid as sniff codes. */ private function parseSniffCodes($input, $argument) diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index c88e6934c4..59aabecc19 100644 --- a/tests/Core/Config/SniffsExcludeArgsTest.php +++ b/tests/Core/Config/SniffsExcludeArgsTest.php @@ -68,7 +68,7 @@ public function testInvalid($argument, $value, $errors, $suggestion) * Data provider for testInvalid(). * * @see self::testInvalid() - * @return non-empty-array + * @return array|string|null>> */ public static function dataInvalid() { @@ -203,9 +203,9 @@ public static function dataInvalid() /** * Ensure that the valid data does not throw an exception, and the value is stored. * - * @param string $argument 'sniffs' or 'exclude'. - * @param string $value List of sniffs to include or exclude. - * @param string[] $result Expected sniffs to be set on the Config object. + * @param string $argument 'sniffs' or 'exclude'. + * @param string $value List of sniffs to include or exclude. + * @param array $result Expected sniffs to be set on the Config object. * * @return void * @dataProvider dataValid @@ -223,7 +223,7 @@ public function testValid($argument, $value, $result) * Data provider for testValid(). * * @see self::testValid() - * @return non-empty-array + * @return array|string>> */ public static function dataValid() {