Skip to content

Commit cae50d8

Browse files
jrfnljrfnl
committed
Ruleset: use MessageCollector for pre-existing errors
Notes: **For the "invalid type" message** Includes updating the message for the "invalid type" message to mention the reference for which the `type` was (incorrectly) being changed. This should make it more straight forward for ruleset maintainers to find the problem in their ruleset. It also makes the message more unique, as this message could occur in multiple places in a ruleset and there was no indication of that in the message previously. Potential future scope for "invalid type" message It could be considered to downgrade this message from an `ERROR` to a `NOTICE` as an invalid type is not blocking for running the sniffs, though this could lead to results not being as expected if, for instance, the `-n` flag is being used, which is why I've not changed this at this time. **For the "register() method must return an array" error Includes some new assertions which won't run until the test suite supports PHPUnit 10+ (PHPCS 4.0). These tests belong with this commit though, so adding them now anyway. **For the "setting non-existent property" error Includes minor adjustment to the error message (removal of "Ruleset invalid" and punctuation).
1 parent 051cc15 commit cae50d8

8 files changed

+44
-30
lines changed

‎src/Ruleset.php‎

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -209,11 +209,11 @@ public function __construct(Config $config)
209209

210210
if (defined('PHP_CODESNIFFER_IN_TESTS') === true && empty($restrictions) === false) {
211211
// In unit tests, only register the sniffs that the test wants and not the entire standard.
212-
try {
213-
foreach ($restrictions as $restriction) {
214-
$sniffs = array_merge($sniffs, $this->expandRulesetReference($restriction, dirname($standard)));
215-
}
216-
} catch (RuntimeException $e) {
212+
foreach ($restrictions as $restriction) {
213+
$sniffs = array_merge($sniffs, $this->expandRulesetReference($restriction, dirname($standard)));
214+
}
215+
216+
if (empty($sniffs) === true) {
217217
// Sniff reference could not be expanded, which probably means this
218218
// is an installed standard. Let the unit test system take care of
219219
// setting the correct sniff for testing.
@@ -262,7 +262,7 @@ public function __construct(Config $config)
262262
}
263263

264264
if ($numSniffs === 0) {
265-
throw new RuntimeException('ERROR: No sniffs were registered');
265+
$this->msgCache->add('No sniffs were registered.', MessageCollector::ERROR);
266266
}
267267

268268
$this->displayCachedMessages();
@@ -1015,8 +1015,8 @@ private function expandRulesetReference($ref, $rulesetDir, $depth=0)
10151015
}
10161016
} else {
10171017
if (is_file($ref) === false) {
1018-
$error = "ERROR: Referenced sniff \"$ref\" does not exist";
1019-
throw new RuntimeException($error);
1018+
$this->msgCache->add("Referenced sniff \"$ref\" does not exist.", MessageCollector::ERROR);
1019+
return [];
10201020
}
10211021

10221022
if (substr($ref, -9) === 'Sniff.php') {
@@ -1103,17 +1103,18 @@ private function processRule($rule, $newSniffs, $depth=0)
11031103

11041104
$type = strtolower((string) $rule->type);
11051105
if ($type !== 'error' && $type !== 'warning') {
1106-
throw new RuntimeException("ERROR: Message type \"$type\" is invalid; must be \"error\" or \"warning\"");
1107-
}
1106+
$message = "Message type \"$type\" for \"$code\" is invalid; must be \"error\" or \"warning\".";
1107+
$this->msgCache->add($message, MessageCollector::ERROR);
1108+
} else {
1109+
$this->ruleset[$code]['type'] = $type;
1110+
if (PHP_CODESNIFFER_VERBOSITY > 1) {
1111+
$statusMessage = '=> message type set to '.(string) $rule->type;
1112+
if ($code !== $ref) {
1113+
$statusMessage .= " for $code";
1114+
}
11081115

1109-
$this->ruleset[$code]['type'] = $type;
1110-
if (PHP_CODESNIFFER_VERBOSITY > 1) {
1111-
$statusMessage = '=> message type set to '.(string) $rule->type;
1112-
if ($code !== $ref) {
1113-
$statusMessage .= " for $code";
1116+
Common::printStatusMessage($statusMessage, ($depth + 2));
11141117
}
1115-
1116-
Common::printStatusMessage($statusMessage, ($depth + 2));
11171118
}
11181119
}//end if
11191120

@@ -1407,8 +1408,12 @@ public function populateTokenListeners()
14071408

14081409
$tokens = $this->sniffs[$sniffClass]->register();
14091410
if (is_array($tokens) === false) {
1410-
$msg = "ERROR: Sniff $sniffClass register() method must return an array";
1411-
throw new RuntimeException($msg);
1411+
$msg = "The sniff {$sniffClass}::register() method must return an array.";
1412+
$this->msgCache->add($msg, MessageCollector::ERROR);
1413+
1414+
// Unregister the sniff.
1415+
unset($this->sniffs[$sniffClass], $this->sniffCodes[$sniffCode], $this->deprecatedSniffs[$sniffCode]);
1416+
continue;
14121417
}
14131418

14141419
$ignorePatterns = [];
@@ -1489,9 +1494,9 @@ public function setSniffProperty($sniffClass, $name, $settings)
14891494

14901495
if ($isSettable === false) {
14911496
if ($settings['scope'] === 'sniff') {
1492-
$notice = "ERROR: Ruleset invalid. Property \"$propertyName\" does not exist on sniff ";
1493-
$notice .= array_search($sniffClass, $this->sniffCodes, true);
1494-
throw new RuntimeException($notice);
1497+
$notice = "Property \"$propertyName\" does not exist on sniff ";
1498+
$notice .= array_search($sniffClass, $this->sniffCodes, true).'.';
1499+
$this->msgCache->add($notice, MessageCollector::ERROR);
14951500
}
14961501

14971502
return;

‎tests/Core/Ruleset/ConstructorTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ public function testNoSniffsRegisteredException()
281281
$standard = __DIR__.'/ConstructorNoSniffsTest.xml';
282282
$config = new ConfigDouble(["--standard=$standard"]);
283283

284-
$message = 'ERROR: No sniffs were registered';
284+
$message = 'ERROR: No sniffs were registered.'.PHP_EOL.PHP_EOL;
285285
$this->expectRuntimeExceptionMessage($message);
286286

287287
new Ruleset($config);

‎tests/Core/Ruleset/ExpandRulesetReferenceHomePathTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ public function testHomePathRefGetsExpandedAndThrowsExceptionWhenPathIsInvalid()
109109
$standard = __DIR__.'/ExpandRulesetReferenceHomePathFailTest.xml';
110110
$config = new ConfigDouble(["--standard=$standard"]);
111111

112-
$exceptionMessage = 'ERROR: Referenced sniff "~/src/MyStandard/Sniffs/DoesntExist/" does not exist';
112+
$exceptionMessage = 'ERROR: Referenced sniff "~/src/MyStandard/Sniffs/DoesntExist/" does not exist.'.PHP_EOL;
113+
$exceptionMessage .= 'ERROR: No sniffs were registered.'.PHP_EOL.PHP_EOL;
113114
$this->expectRuntimeExceptionMessage($exceptionMessage);
114115

115116
new Ruleset($config);

‎tests/Core/Ruleset/ExpandRulesetReferenceTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ public function testUnresolvableReferenceThrowsException($standard, $replacement
6161
$standard = __DIR__.'/'.$standard;
6262
$config = new ConfigDouble(["--standard=$standard"]);
6363

64-
$exceptionMessage = 'ERROR: Referenced sniff "%s" does not exist';
64+
$exceptionMessage = 'ERROR: Referenced sniff "%s" does not exist.'.PHP_EOL;
65+
$exceptionMessage .= 'ERROR: No sniffs were registered.'.PHP_EOL.PHP_EOL;
6566
$this->expectRuntimeExceptionMessage(sprintf($exceptionMessage, $replacement));
6667

6768
new Ruleset($config);

‎tests/Core/Ruleset/PopulateTokenListenersRegisterNoArrayTest.xml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,6 @@
55

66
<rule ref="TestStandard.InvalidSniffs.RegisterNoArray"/>
77

8+
<!-- Prevent a "no sniff were registered" error. -->
9+
<rule ref="Generic.PHP.BacktickOperator"/>
810
</ruleset>

‎tests/Core/Ruleset/PopulateTokenListenersTest.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,16 @@ public function testSniffWhereRegisterDoesNotReturnAnArrayThrowsException()
6262
$config = new ConfigDouble(["--standard=$standard"]);
6363

6464
$sniffClass = 'Fixtures\\TestStandard\\Sniffs\\InvalidSniffs\\RegisterNoArraySniff';
65-
$message = "ERROR: Sniff $sniffClass register() method must return an array";
65+
$message = "ERROR: The sniff {$sniffClass}::register() method must return an array.".PHP_EOL.PHP_EOL;
6666
$this->expectRuntimeExceptionMessage($message);
6767

6868
new Ruleset($config);
6969

70+
// Verify that the sniff has not been registered/has been unregistered.
71+
// These assertions will only take effect for PHPUnit 10+.
72+
$this->assertArrayNotHasKey($sniffClass, self::$ruleset->sniffs, "Sniff class $sniffClass is listed in registered sniffs");
73+
$this->assertArrayNotHasKey('TestStandard.InvalidSniffs.RegisterNoArray', self::$ruleset->sniffCodes, 'Sniff code is registered');
74+
7075
}//end testSniffWhereRegisterDoesNotReturnAnArrayThrowsException()
7176

7277

‎tests/Core/Ruleset/ProcessRuleInvalidTypeTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ final class ProcessRuleInvalidTypeTest extends AbstractRulesetTestCase
2323

2424

2525
/**
26-
* Test displaying an informative error message when an invalid type is given.
26+
* Test displaying an error when an invalid type is given.
2727
*
2828
* @return void
2929
*/
@@ -32,7 +32,7 @@ public function testInvalidTypeHandling()
3232
$standard = __DIR__.'/ProcessRuleInvalidTypeTest.xml';
3333
$config = new ConfigDouble(["--standard=$standard"]);
3434

35-
$message = 'ERROR: Message type "notice" is invalid; must be "error" or "warning"';
35+
$message = 'ERROR: Message type "notice" for "Generic.Files.ByteOrderMark" is invalid; must be "error" or "warning".'.PHP_EOL.PHP_EOL;
3636
$this->expectRuntimeExceptionMessage($message);
3737

3838
new Ruleset($config);

‎tests/Core/Ruleset/SetSniffPropertyTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ public function testSetPropertyAppliesPropertyToMultipleSniffsInCategory()
135135
*/
136136
public function testSetPropertyThrowsErrorOnInvalidProperty()
137137
{
138-
$exceptionMsg = 'ERROR: Ruleset invalid. Property "indentation" does not exist on sniff Generic.Arrays.ArrayIndent';
138+
$exceptionMsg = 'ERROR: Property "indentation" does not exist on sniff Generic.Arrays.ArrayIndent.'.PHP_EOL.PHP_EOL;
139139
$this->expectRuntimeExceptionMessage($exceptionMsg);
140140

141141
// Set up the ruleset.
@@ -155,7 +155,7 @@ public function testSetPropertyThrowsErrorOnInvalidProperty()
155155
*/
156156
public function testSetPropertyThrowsErrorWhenPropertyOnlyAllowedViaAttribute()
157157
{
158-
$exceptionMsg = 'ERROR: Ruleset invalid. Property "arbitrarystring" does not exist on sniff TestStandard.SetProperty.NotAllowedViaAttribute';
158+
$exceptionMsg = 'ERROR: Property "arbitrarystring" does not exist on sniff TestStandard.SetProperty.NotAllowedViaAttribute.'.PHP_EOL.PHP_EOL;
159159
$this->expectRuntimeExceptionMessage($exceptionMsg);
160160

161161
// Set up the ruleset.

0 commit comments

Comments
 (0)