Skip to content

Commit 4d5b0c4

Browse files
freddenfredden
committed
Improve error message for an invalid sniff code
1 parent 0855bf2 commit 4d5b0c4

File tree

2 files changed

+191
-53
lines changed

2 files changed

+191
-53
lines changed

‎src/Config.php‎

Lines changed: 78 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -885,32 +885,14 @@ public function processLongArgument($arg, $pos)
885885
break;
886886
}
887887

888-
$sniffs = explode(',', substr($arg, 7));
889-
foreach ($sniffs as $sniff) {
890-
if (substr_count($sniff, '.') !== 2) {
891-
$error = 'ERROR: The specified sniff code "'.$sniff.'" is invalid'.PHP_EOL.PHP_EOL;
892-
$error .= $this->printShortUsage(true);
893-
throw new DeepExitException($error, 3);
894-
}
895-
}
896-
897-
$this->sniffs = $sniffs;
888+
$this->sniffs = $this->parseSniffCodes(substr($arg, 7), 'sniffs');
898889
self::$overriddenDefaults['sniffs'] = true;
899890
} else if (substr($arg, 0, 8) === 'exclude=') {
900891
if (isset(self::$overriddenDefaults['exclude']) === true) {
901892
break;
902893
}
903894

904-
$sniffs = explode(',', substr($arg, 8));
905-
foreach ($sniffs as $sniff) {
906-
if (substr_count($sniff, '.') !== 2) {
907-
$error = 'ERROR: The specified sniff code "'.$sniff.'" is invalid'.PHP_EOL.PHP_EOL;
908-
$error .= $this->printShortUsage(true);
909-
throw new DeepExitException($error, 3);
910-
}
911-
}
912-
913-
$this->exclude = $sniffs;
895+
$this->exclude = $this->parseSniffCodes(substr($arg, 8), 'exclude');
914896
self::$overriddenDefaults['exclude'] = true;
915897
} else if (defined('PHP_CODESNIFFER_IN_TESTS') === false
916898
&& substr($arg, 0, 6) === 'cache='
@@ -1658,4 +1640,80 @@ public function printConfigData($data)
16581640
}//end printConfigData()
16591641

16601642

1643+
/**
1644+
* Parse supplied string into a list of sniff codes.
1645+
*
1646+
* @param string $input Comma-separated string of sniff codes.
1647+
* @param string $argument The name of the argument which is being processed.
1648+
*
1649+
* @return string[]
1650+
* @throws DeepExitException
1651+
*/
1652+
private function parseSniffCodes($input, $argument)
1653+
{
1654+
$errors = [];
1655+
$sniffs = [];
1656+
1657+
$possibleSniffs = explode(',', $input);
1658+
$possibleSniffs = array_unique($possibleSniffs);
1659+
1660+
foreach ($possibleSniffs as $sniff) {
1661+
$sniff = trim($sniff);
1662+
1663+
if ($sniff === '') {
1664+
// Empty values can be safely ignored.
1665+
continue;
1666+
}
1667+
1668+
if (preg_match('{[^A-Za-z0-9.]}', $sniff) === 1) {
1669+
$errors[] = 'Unsupported character detected: '.$sniff;
1670+
continue;
1671+
}
1672+
1673+
$partCount = substr_count($sniff, '.');
1674+
if ($partCount === 2) {
1675+
// Correct number of parts.
1676+
$sniffs[] = $sniff;
1677+
continue;
1678+
}
1679+
1680+
if ($partCount === 0) {
1681+
$errors[] = 'Standard codes are not supported: '.$sniff;
1682+
} else if ($partCount === 1) {
1683+
$errors[] = 'Category codes are not supported: '.$sniff;
1684+
} else if ($partCount === 3) {
1685+
$errors[] = 'Message codes are not supported: '.$sniff;
1686+
} else {
1687+
$errors[] = 'Too many parts: '.$sniff;
1688+
}
1689+
1690+
if ($partCount > 2) {
1691+
$parts = explode('.', $sniff, 4);
1692+
$sniffs[] = $parts[0].'.'.$parts[1].'.'.$parts[2];
1693+
}
1694+
}//end foreach
1695+
1696+
if ($errors !== []) {
1697+
$error = 'ERROR: The --'.$argument.' option only supports sniff codes.'.PHP_EOL;
1698+
$error .= 'Sniff codes are in the form "Standard.Category.Sniff"'.PHP_EOL;
1699+
$error .= PHP_EOL;
1700+
$error .= 'The following problems were detected:'.PHP_EOL;
1701+
$error .= '* '.implode(PHP_EOL.'* ', $errors).PHP_EOL;
1702+
1703+
if ($sniffs !== []) {
1704+
$sniffs = array_unique($sniffs);
1705+
$error .= PHP_EOL;
1706+
$error .= 'Perhaps try --'.$argument.'="'.implode(',', $sniffs).'" instead.'.PHP_EOL;
1707+
}
1708+
1709+
$error .= PHP_EOL;
1710+
$error .= $this->printShortUsage(true);
1711+
throw new DeepExitException(ltrim($error), 3);
1712+
}
1713+
1714+
return $sniffs;
1715+
1716+
}//end parseSniffCodes()
1717+
1718+
16611719
}//end class

‎tests/Core/Config/SniffsExcludeArgsTest.php‎

Lines changed: 113 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,31 @@ final class SniffsExcludeArgsTest extends TestCase
2323
/**
2424
* Ensure that the expected error message is returned for invalid arguments.
2525
*
26-
* @param string $argument 'sniffs' or 'exclude'.
27-
* @param string $value List of sniffs to include / exclude.
28-
* @param string $message Expected error message text.
26+
* @param string $argument 'sniffs' or 'exclude'.
27+
* @param string $value List of sniffs to include / exclude.
28+
* @param array<string, string> $errors Sniff code and associated help text.
29+
* @param string|null $suggestion Help text shown to end user with correct syntax for argument.
2930
*
3031
* @return void
3132
* @dataProvider dataInvalidSniffs
3233
*/
33-
public function testInvalid($argument, $value, $message)
34+
public function testInvalid($argument, $value, $errors, $suggestion)
3435
{
3536
$exception = 'PHP_CodeSniffer\Exceptions\DeepExitException';
37+
$message = 'ERROR: The --'.$argument.' option only supports sniff codes.'.PHP_EOL;
38+
$message .= 'Sniff codes are in the form "Standard.Category.Sniff"'.PHP_EOL;
39+
$message .= PHP_EOL;
40+
$message .= 'The following problems were detected:'.PHP_EOL;
41+
$message .= '* '.implode(PHP_EOL.'* ', $errors).PHP_EOL;
42+
43+
if ($suggestion !== null) {
44+
$message .= PHP_EOL;
45+
$message .= "Perhaps try --$argument=\"$suggestion\" instead.".PHP_EOL;
46+
}
47+
48+
$message .= PHP_EOL;
49+
$message .= 'Run "phpcs --help" for usage information'.PHP_EOL;
50+
$message .= PHP_EOL;
3651

3752
if (method_exists($this, 'expectException') === true) {
3853
// PHPUnit 5+.
@@ -62,47 +77,76 @@ public static function dataInvalidSniffs()
6277
];
6378
$data = [];
6479

65-
$messageTemplate = 'ERROR: The specified sniff code "%s" is invalid'.PHP_EOL.PHP_EOL;
80+
$messageTemplate = 'ERROR: The specified sniff code "%s" is invalid'.PHP_EOL;
6681

6782
foreach ($arguments as $argument) {
68-
// An empty string is not a valid sniff.
69-
$data[$argument.'; empty string'] = [
70-
'argument' => $argument,
71-
'value' => '',
72-
'message' => sprintf($messageTemplate, ''),
73-
];
74-
7583
// A standard is not a valid sniff.
7684
$data[$argument.'; standard'] = [
77-
'argument' => $argument,
78-
'value' => 'Standard',
79-
'message' => sprintf($messageTemplate, 'Standard'),
85+
'argument' => $argument,
86+
'value' => 'Standard',
87+
'errors' => [
88+
'Standard codes are not supported: Standard',
89+
],
90+
'suggestion' => null,
8091
];
8192

8293
// A category is not a valid sniff.
8394
$data[$argument.'; category'] = [
84-
'argument' => $argument,
85-
'value' => 'Standard.Category',
86-
'message' => sprintf($messageTemplate, 'Standard.Category'),
95+
'argument' => $argument,
96+
'value' => 'Standard.Category',
97+
'errors' => [
98+
'Category codes are not supported: Standard.Category',
99+
],
100+
'suggestion' => null,
87101
];
88102

89103
// An error-code is not a valid sniff.
90104
$data[$argument.'; error-code'] = [
91-
'argument' => $argument,
92-
'value' => 'Standard.Category',
93-
'message' => sprintf($messageTemplate, 'Standard.Category'),
105+
'argument' => $argument,
106+
'value' => 'Standard.Category.Sniff.Code',
107+
'errors' => [
108+
'Message codes are not supported: Standard.Category.Sniff.Code',
109+
],
110+
'suggestion' => 'Standard.Category.Sniff',
111+
];
112+
113+
// Too many dots.
114+
$data[$argument.'; too many dots'] = [
115+
'argument' => $argument,
116+
'value' => 'Standard.Category.Sniff.Code.Extra',
117+
'errors' => [
118+
'Too many parts: Standard.Category.Sniff.Code.Extra',
119+
],
120+
'suggestion' => 'Standard.Category.Sniff',
94121
];
95122

96-
// Only the first error is reported.
123+
// All errors are reported in one go.
97124
$data[$argument.'; two errors'] = [
98-
'argument' => $argument,
99-
'value' => 'StandardOne,StandardTwo',
100-
'message' => sprintf($messageTemplate, 'StandardOne'),
125+
'argument' => $argument,
126+
'value' => 'StandardOne,StandardTwo',
127+
'errors' => [
128+
'Standard codes are not supported: StandardOne',
129+
'Standard codes are not supported: StandardTwo',
130+
],
131+
'suggestion' => null,
101132
];
133+
134+
// Order of valid/invalid does not impact error reporting.
102135
$data[$argument.'; valid followed by invalid'] = [
103-
'argument' => $argument,
104-
'value' => 'StandardOne.Category.Sniff,StandardTwo.Category',
105-
'message' => sprintf($messageTemplate, 'StandardTwo.Category'),
136+
'argument' => $argument,
137+
'value' => 'StandardOne.Category.Sniff,StandardTwo.Category',
138+
'errors' => [
139+
'Category codes are not supported: StandardTwo.Category',
140+
],
141+
'suggestion' => 'StandardOne.Category.Sniff',
142+
];
143+
$data[$argument.'; invalid followed by valid'] = [
144+
'argument' => $argument,
145+
'value' => 'StandardOne.Category,StandardTwo.Category.Sniff',
146+
'errors' => [
147+
'Category codes are not supported: StandardOne.Category',
148+
],
149+
'suggestion' => 'StandardTwo.Category.Sniff',
106150
];
107151
}//end foreach
108152

@@ -114,17 +158,18 @@ public static function dataInvalidSniffs()
114158
/**
115159
* Ensure that the valid data does not throw an exception, and the value is stored.
116160
*
117-
* @param string $argument 'sniffs' or 'exclude'.
118-
* @param string $value List of sniffs to include or exclude.
161+
* @param string $argument 'sniffs' or 'exclude'.
162+
* @param string $value List of sniffs to include or exclude.
163+
* @param string[] $result Expected sniffs to be set on the Config object.
119164
*
120165
* @return void
121166
* @dataProvider dataValidSniffs
122167
*/
123-
public function testValid($argument, $value)
168+
public function testValid($argument, $value, $result)
124169
{
125170
$config = new ConfigDouble(["--$argument=$value"]);
126171

127-
$this->assertSame(explode(',', $value), $config->$argument);
172+
$this->assertSame($result, $config->$argument);
128173

129174
}//end testValid()
130175

@@ -144,15 +189,50 @@ public static function dataValidSniffs()
144189
$data = [];
145190

146191
foreach ($arguments as $argument) {
192+
$data[$argument.'; empty string'] = [
193+
'argument' => $argument,
194+
'value' => '',
195+
'result' => [],
196+
];
147197
$data[$argument.'; one valid sniff'] = [
148198
'argument' => $argument,
149199
'value' => 'Standard.Category.Sniff',
200+
'result' => ['Standard.Category.Sniff'],
150201
];
151202
$data[$argument.'; two valid sniffs'] = [
152203
'argument' => $argument,
153204
'value' => 'StandardOne.Category.Sniff,StandardTwo.Category.Sniff',
205+
'result' => [
206+
'StandardOne.Category.Sniff',
207+
'StandardTwo.Category.Sniff',
208+
],
154209
];
155-
}
210+
211+
// Rogue commas are quietly ignored.
212+
$data[$argument.'; one comma alone'] = [
213+
'argument' => $argument,
214+
'value' => ',',
215+
'result' => [],
216+
];
217+
$data[$argument.'; two commas alone'] = [
218+
'argument' => $argument,
219+
'value' => ',,',
220+
'result' => [],
221+
];
222+
$data[$argument.'; trailing comma'] = [
223+
'argument' => $argument,
224+
'value' => 'Standard.Category.Sniff,',
225+
'result' => ['Standard.Category.Sniff'],
226+
];
227+
$data[$argument.'; double comma between sniffs'] = [
228+
'argument' => $argument,
229+
'value' => 'StandardOne.Category.Sniff,,StandardTwo.Category.Sniff',
230+
'result' => [
231+
'StandardOne.Category.Sniff',
232+
'StandardTwo.Category.Sniff',
233+
],
234+
];
235+
}//end foreach
156236

157237
return $data;
158238

0 commit comments

Comments
 (0)