diff --git a/src/Config.php b/src/Config.php index 2bcc78e6ec..19c0da5007 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=' @@ -1277,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 array + * @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. * diff --git a/tests/Core/Config/SniffsExcludeArgsTest.php b/tests/Core/Config/SniffsExcludeArgsTest.php index 4eedb624ea..59aabecc19 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 @@ -23,16 +24,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 + * @dataProvider dataInvalid */ - 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+. @@ -52,9 +68,9 @@ public function testInvalid($argument, $value, $message) * Data provider for testInvalid(). * * @see self::testInvalid() - * @return array> + * @return array|string|null>> */ - public static function dataInvalidSniffs() + public static function dataInvalid() { $arguments = [ 'sniffs', @@ -62,69 +78,143 @@ public static function dataInvalidSniffs() ]; $data = []; - $messageTemplate = 'ERROR: The specified sniff code "%s" is invalid'.PHP_EOL.PHP_EOL; - foreach ($arguments as $argument) { - // An empty string is not a valid sniff. - $data[$argument.'; empty string'] = [ - 'argument' => $argument, - 'value' => '', - 'message' => sprintf($messageTemplate, ''), + // 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, - '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', + ]; + + // 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; - }//end dataInvalidSniffs() + }//end 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 $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 dataValidSniffs + * @dataProvider dataValid */ - 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() @@ -133,9 +223,9 @@ public function testValid($argument, $value) * Data provider for testValid(). * * @see self::testValid() - * @return array> + * @return array|string>> */ - public static function dataValidSniffs() + public static function dataValid() { $arguments = [ 'sniffs', @@ -147,16 +237,48 @@ public static function dataValidSniffs() $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.'; 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', + ], + ]; + + // 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; - }//end dataValidSniffs() + }//end dataValid() /**