Skip to content

Improve error message for an invalid sniff code #344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Feb 12, 2025
Merged

Improve error message for an invalid sniff code #344

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
4d5b0c4
Improve error message for an invalid sniff code
fredden Feb 16, 2024
8dd0102
Update src/Config.php
fredden Jan 10, 2025
995522f
Update src/Config.php
fredden Jan 10, 2025
792cf66
Report all errors, even if duplicates
fredden Jan 10, 2025
4ed5733
Filter out empty values sooner
fredden Jan 10, 2025
9850de4
Complain if nothing (or only commas) specified
fredden Jan 10, 2025
95d5955
Ignore case when removing duplicates from final list
fredden Jan 10, 2025
5c48598
Add missing `@covers` annotation
fredden Jan 10, 2025
d36d44a
Update src/Config.php
fredden Jan 10, 2025
c2aec40
Remove invalid assumption regarding valid characters
fredden Jan 10, 2025
d5f5fe0
Update test to reflect change via GitHub
fredden Jan 10, 2025
994bb1c
Update test: treat empty values as errors
fredden Jan 10, 2025
af962dc
Merge remote-tracking branch 'upstream/master' into feature/improve-s…
fredden Jan 10, 2025
0dcd4ef
Remove unused variable
fredden Jan 10, 2025
bccdffc
Update docblocks to match reality
fredden Jan 10, 2025
3421e00
Apply whitespace changes from phpcbf
fredden Jan 10, 2025
99753e8
Add more tests
fredden Jan 10, 2025
d1bfb0a
Apply whitespace changes from phpcbf
fredden Jan 10, 2025
b270da2
Move method further up the file
fredden Jan 23, 2025
d0e1cf8
Correct param type for $errors
fredden Jan 23, 2025
b127df4
Use phpstan-style return types
fredden Jan 23, 2025
5b5208b
Remove word 'sniffs' from method names
fredden Jan 23, 2025
f87a5e1
Use a different syntax for special comments
fredden Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 78 additions & 20 deletions src/Config.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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='
Expand Down Expand Up @@ -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
146 changes: 113 additions & 33 deletions tests/Core/Config/SniffsExcludeArgsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> $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+.
Expand Down Expand Up @@ -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

Expand All @@ -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()

Expand All @@ -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;

Expand Down
Loading