Ruleset: use MessageCollector for pre-existing errors

jrfnl · jrfnl · commit 45baf5ceda49 · 2025-03-12T23:28:47.000+01:00
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).
diff --git a/src/Ruleset.php b/src/Ruleset.php
@@ -202,11 +202,11 @@ public function __construct(Config $config)
 
             if (defined('PHP_CODESNIFFER_IN_TESTS') === true && empty($restrictions) === false) {
                 // In unit tests, only register the sniffs that the test wants and not the entire standard.
-                try {
-                    foreach ($restrictions as $restriction) {
-                        $sniffs = array_merge($sniffs, $this->expandRulesetReference($restriction, dirname($standard)));
-                    }
-                } catch (RuntimeException $e) {
+                foreach ($restrictions as $restriction) {
+                    $sniffs = array_merge($sniffs, $this->expandRulesetReference($restriction, dirname($standard)));
+                }
+
+                if (empty($sniffs) === true) {
                     // Sniff reference could not be expanded, which probably means this
                     // is an installed standard. Let the unit test system take care of
                     // setting the correct sniff for testing.
@@ -255,7 +255,7 @@ public function __construct(Config $config)
         }
 
         if ($numSniffs === 0) {
-            throw new RuntimeException('ERROR: No sniffs were registered');
+            $this->msgCache->add('No sniffs were registered.', MessageCollector::ERROR);
         }
 
         $this->displayCachedMessages();
@@ -1040,8 +1040,8 @@ private function expandRulesetReference($ref, $rulesetDir, $depth=0)
             }
         } else {
             if (is_file($ref) === false) {
-                $error = "ERROR: Referenced sniff \"$ref\" does not exist";
-                throw new RuntimeException($error);
+                $this->msgCache->add("Referenced sniff \"$ref\" does not exist.", MessageCollector::ERROR);
+                return [];
             }
 
             if (substr($ref, -9) === 'Sniff.php') {
@@ -1130,18 +1130,19 @@ private function processRule($rule, $newSniffs, $depth=0)
 
                 $type = strtolower((string) $rule->type);
                 if ($type !== 'error' && $type !== 'warning') {
-                    throw new RuntimeException("ERROR: Message type \"$type\" is invalid; must be \"error\" or \"warning\"");
-                }
+                    $message = "Message type \"$type\" for \"$code\" is invalid; must be \"error\" or \"warning\".";
+                    $this->msgCache->add($message, MessageCollector::ERROR);
+                } else {
+                    $this->ruleset[$code]['type'] = $type;
+                    if (PHP_CODESNIFFER_VERBOSITY > 1) {
+                        echo str_repeat("\t", $depth);
+                        echo "\t\t=> message type set to ".(string) $rule->type;
+                        if ($code !== $ref) {
+                            echo " for $code";
+                        }
 
-                $this->ruleset[$code]['type'] = $type;
-                if (PHP_CODESNIFFER_VERBOSITY > 1) {
-                    echo str_repeat("\t", $depth);
-                    echo "\t\t=> message type set to ".(string) $rule->type;
-                    if ($code !== $ref) {
-                        echo " for $code";
+                        echo PHP_EOL;
                     }
-
-                    echo PHP_EOL;
                 }
             }//end if
 
@@ -1459,8 +1460,12 @@ public function populateTokenListeners()
 
             $tokens = $this->sniffs[$sniffClass]->register();
             if (is_array($tokens) === false) {
-                $msg = "ERROR: Sniff $sniffClass register() method must return an array";
-                throw new RuntimeException($msg);
+                $msg = "The sniff {$sniffClass}::register() method must return an array.";
+                $this->msgCache->add($msg, MessageCollector::ERROR);
+
+                // Unregister the sniff.
+                unset($this->sniffs[$sniffClass], $this->sniffCodes[$sniffCode], $this->deprecatedSniffs[$sniffCode]);
+                continue;
             }
 
             $ignorePatterns = [];
@@ -1570,9 +1575,9 @@ public function setSniffProperty($sniffClass, $name, $settings)
 
         if ($isSettable === false) {
             if ($settings['scope'] === 'sniff') {
-                $notice  = "ERROR: Ruleset invalid. Property \"$propertyName\" does not exist on sniff ";
-                $notice .= array_search($sniffClass, $this->sniffCodes, true);
-                throw new RuntimeException($notice);
+                $notice  = "Property \"$propertyName\" does not exist on sniff ";
+                $notice .= array_search($sniffClass, $this->sniffCodes, true).'.';
+                $this->msgCache->add($notice, MessageCollector::ERROR);
             }
 
             return;
diff --git a/tests/Core/Ruleset/ConstructorTest.php b/tests/Core/Ruleset/ConstructorTest.php
@@ -282,7 +282,7 @@ public function testNoSniffsRegisteredException()
         $standard = __DIR__.'/ConstructorNoSniffsTest.xml';
         $config   = new ConfigDouble(["--standard=$standard"]);
 
-        $message = 'ERROR: No sniffs were registered';
+        $message = 'ERROR: No sniffs were registered.'.PHP_EOL.PHP_EOL;
         $this->expectRuntimeExceptionMessage($message);
 
         new Ruleset($config);
diff --git a/tests/Core/Ruleset/ExpandRulesetReferenceHomePathTest.php b/tests/Core/Ruleset/ExpandRulesetReferenceHomePathTest.php
@@ -109,7 +109,8 @@ public function testHomePathRefGetsExpandedAndThrowsExceptionWhenPathIsInvalid()
         $standard = __DIR__.'/ExpandRulesetReferenceHomePathFailTest.xml';
         $config   = new ConfigDouble(["--standard=$standard"]);
 
-        $exceptionMessage = 'ERROR: Referenced sniff "~/src/MyStandard/Sniffs/DoesntExist/" does not exist';
+        $exceptionMessage  = 'ERROR: Referenced sniff "~/src/MyStandard/Sniffs/DoesntExist/" does not exist.'.PHP_EOL;
+        $exceptionMessage .= 'ERROR: No sniffs were registered.'.PHP_EOL.PHP_EOL;
         $this->expectRuntimeExceptionMessage($exceptionMessage);
 
         new Ruleset($config);
diff --git a/tests/Core/Ruleset/ExpandRulesetReferenceTest.php b/tests/Core/Ruleset/ExpandRulesetReferenceTest.php
@@ -61,7 +61,8 @@ public function testUnresolvableReferenceThrowsException($standard, $replacement
         $standard = __DIR__.'/'.$standard;
         $config   = new ConfigDouble(["--standard=$standard"]);
 
-        $exceptionMessage = 'ERROR: Referenced sniff "%s" does not exist';
+        $exceptionMessage  = 'ERROR: Referenced sniff "%s" does not exist.'.PHP_EOL;
+        $exceptionMessage .= 'ERROR: No sniffs were registered.'.PHP_EOL.PHP_EOL;
         $this->expectRuntimeExceptionMessage(sprintf($exceptionMessage, $replacement));
 
         new Ruleset($config);
diff --git a/tests/Core/Ruleset/PopulateTokenListenersRegisterNoArrayTest.xml b/tests/Core/Ruleset/PopulateTokenListenersRegisterNoArrayTest.xml
@@ -5,4 +5,6 @@
 
     <rule ref="TestStandard.InvalidSniffs.RegisterNoArray"/>
 
+    <!-- Prevent a "no sniff were registered" error. -->
+    <rule ref="Generic.PHP.BacktickOperator"/>
 </ruleset>
diff --git a/tests/Core/Ruleset/PopulateTokenListenersTest.php b/tests/Core/Ruleset/PopulateTokenListenersTest.php
@@ -62,11 +62,16 @@ public function testSniffWhereRegisterDoesNotReturnAnArrayThrowsException()
         $config   = new ConfigDouble(["--standard=$standard"]);
 
         $sniffClass = 'Fixtures\\TestStandard\\Sniffs\\InvalidSniffs\\RegisterNoArraySniff';
-        $message    = "ERROR: Sniff $sniffClass register() method must return an array";
+        $message    = "ERROR: The sniff {$sniffClass}::register() method must return an array.".PHP_EOL.PHP_EOL;
         $this->expectRuntimeExceptionMessage($message);
 
         new Ruleset($config);
 
+        // Verify that the sniff has not been registered/has been unregistered.
+        // These assertions will only take effect for PHPUnit 10+.
+        $this->assertArrayNotHasKey($sniffClass, self::$ruleset->sniffs, "Sniff class $sniffClass is listed in registered sniffs");
+        $this->assertArrayNotHasKey('TestStandard.InvalidSniffs.RegisterNoArray', self::$ruleset->sniffCodes, 'Sniff code is registered');
+
     }//end testSniffWhereRegisterDoesNotReturnAnArrayThrowsException()
 
 
diff --git a/tests/Core/Ruleset/ProcessRuleInvalidTypeTest.php b/tests/Core/Ruleset/ProcessRuleInvalidTypeTest.php
@@ -23,7 +23,7 @@ final class ProcessRuleInvalidTypeTest extends AbstractRulesetTestCase
 
 
     /**
-     * Test displaying an informative error message when an invalid type is given.
+     * Test displaying an error when an invalid type is given.
      *
      * @return void
      */
@@ -32,7 +32,7 @@ public function testInvalidTypeHandling()
         $standard = __DIR__.'/ProcessRuleInvalidTypeTest.xml';
         $config   = new ConfigDouble(["--standard=$standard"]);
 
-        $message = 'ERROR: Message type "notice" is invalid; must be "error" or "warning"';
+        $message = 'ERROR: Message type "notice" for "Generic.Files.ByteOrderMark" is invalid; must be "error" or "warning".'.PHP_EOL.PHP_EOL;
         $this->expectRuntimeExceptionMessage($message);
 
         new Ruleset($config);
diff --git a/tests/Core/Ruleset/SetSniffPropertyTest.php b/tests/Core/Ruleset/SetSniffPropertyTest.php
@@ -135,7 +135,7 @@ public function testSetPropertyAppliesPropertyToMultipleSniffsInCategory()
      */
     public function testSetPropertyThrowsErrorOnInvalidProperty()
     {
-        $exceptionMsg = 'ERROR: Ruleset invalid. Property "indentation" does not exist on sniff Generic.Arrays.ArrayIndent';
+        $exceptionMsg = 'ERROR: Property "indentation" does not exist on sniff Generic.Arrays.ArrayIndent.'.PHP_EOL.PHP_EOL;
         $this->expectRuntimeExceptionMessage($exceptionMsg);
 
         // Set up the ruleset.
@@ -155,7 +155,7 @@ public function testSetPropertyThrowsErrorOnInvalidProperty()
      */
     public function testSetPropertyThrowsErrorWhenPropertyOnlyAllowedViaAttribute()
     {
-        $exceptionMsg = 'ERROR: Ruleset invalid. Property "arbitrarystring" does not exist on sniff TestStandard.SetProperty.NotAllowedViaAttribute';
+        $exceptionMsg = 'ERROR: Property "arbitrarystring" does not exist on sniff TestStandard.SetProperty.NotAllowedViaAttribute.'.PHP_EOL.PHP_EOL;
         $this->expectRuntimeExceptionMessage($exceptionMsg);
 
         // Set up the ruleset.
