Skip to content

Commit

Permalink
Runner: don't prefix Ruleset error notices
Browse files Browse the repository at this point in the history
As things are, the `Runner` class catches any exception thrown by the Ruleset, enhances the message with an `ERROR` prefix, adds new lines and short help usage info and then re-throws this as a `DeepExitException`.

In an upcoming PR, a new `MsgCollector` class will be introduced, which will allow for both errors, as well as warnings, notices and deprecations.

This means that the `ERROR` prefix won't always be appropriate anymore.

This commit moves the `ERROR` prefix addition from the `Runner` class to the individual messages for the Exceptions being thrown in the `Ruleset` class.

Additionally, it changes the "trailing new lines" handling as the `MsgCollector` may also add new lines at the end of a message, as messages will not necessarily always be displayed via an exception (think: non-blocking deprecation notices).
So to prevent a duplicate set of new lines, any new lines which are included in the exception are trimmed off before adding the new lines desired for the Exception display.

_This is a preliminary step before introducing the `MsgCollector` to the `Ruleset` class._
  • Loading branch information
jrfnl committed Feb 23, 2025
1 parent 6472698 commit 14de190
Show file tree
Hide file tree
Showing 11 changed files with 25 additions and 25 deletions.
16 changes: 8 additions & 8 deletions src/Ruleset.php
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ public function __construct(Config $config)
}

if ($numSniffs === 0) {
throw new RuntimeException('No sniffs were registered');
throw new RuntimeException('ERROR: No sniffs were registered');
}

}//end __construct()
Expand Down Expand Up @@ -366,7 +366,7 @@ public function showSniffDeprecations()

$messages = [];
$messageTemplate = 'This sniff has been deprecated since %s and will be removed in %s. %s';
$errorTemplate = 'The %s::%s() method must return a %sstring, received %s';
$errorTemplate = 'ERROR: The %s::%s() method must return a %sstring, received %s';

foreach ($this->deprecatedSniffs as $sniffCode => $className) {
if (isset($this->sniffs[$className]) === false) {
Expand Down Expand Up @@ -486,7 +486,7 @@ public function processRuleset($rulesetPath, $depth=0)
libxml_use_internal_errors(true);
$ruleset = simplexml_load_string(file_get_contents($rulesetPath));
if ($ruleset === false) {
$errorMsg = "Ruleset $rulesetPath is not valid".PHP_EOL;
$errorMsg = "ERROR: Ruleset $rulesetPath is not valid".PHP_EOL;
$errors = libxml_get_errors();
foreach ($errors as $error) {
$errorMsg .= '- On line '.$error->line.', column '.$error->column.': '.$error->message;
Expand Down Expand Up @@ -530,7 +530,7 @@ public function processRuleset($rulesetPath, $depth=0)
if ($relativePath !== false && is_file($relativePath) === true) {
$autoloadPath = $relativePath;
} else if (is_file($autoloadPath) === false) {
throw new RuntimeException('The specified autoload file "'.$autoload.'" does not exist');
throw new RuntimeException('ERROR: The specified autoload file "'.$autoload.'" does not exist');
}

include_once $autoloadPath;
Expand Down Expand Up @@ -993,7 +993,7 @@ private function expandRulesetReference($ref, $rulesetDir, $depth=0)
}
} else {
if (is_file($ref) === false) {
$error = "Referenced sniff \"$ref\" does not exist";
$error = "ERROR: Referenced sniff \"$ref\" does not exist";
throw new RuntimeException($error);
}

Expand Down Expand Up @@ -1083,7 +1083,7 @@ private function processRule($rule, $newSniffs, $depth=0)

$type = strtolower((string) $rule->type);
if ($type !== 'error' && $type !== 'warning') {
throw new RuntimeException("Message type \"$type\" is invalid; must be \"error\" or \"warning\"");
throw new RuntimeException("ERROR: Message type \"$type\" is invalid; must be \"error\" or \"warning\"");
}

$this->ruleset[$code]['type'] = $type;
Expand Down Expand Up @@ -1412,7 +1412,7 @@ public function populateTokenListeners()

$tokens = $this->sniffs[$sniffClass]->register();
if (is_array($tokens) === false) {
$msg = "Sniff $sniffClass register() method must return an array";
$msg = "ERROR: Sniff $sniffClass register() method must return an array";
throw new RuntimeException($msg);
}

Expand Down Expand Up @@ -1523,7 +1523,7 @@ public function setSniffProperty($sniffClass, $name, $settings)

if ($isSettable === false) {
if ($settings['scope'] === 'sniff') {
$notice = "Ruleset invalid. Property \"$propertyName\" does not exist on sniff ";
$notice = "ERROR: Ruleset invalid. Property \"$propertyName\" does not exist on sniff ";
$notice .= array_search($sniffClass, $this->sniffCodes, true);
throw new RuntimeException($notice);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Runner.php
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ public function init()
$this->ruleset->showSniffDeprecations();
}
} catch (RuntimeException $e) {
$error = 'ERROR: '.$e->getMessage().PHP_EOL.PHP_EOL;
$error = rtrim($e->getMessage(), "\r\n").PHP_EOL.PHP_EOL;
$error .= $this->config->printShortUsage(true);
throw new DeepExitException($error, 3);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Core/Ruleset/ConstructorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ public function testNoSniffsRegisteredException()
$standard = __DIR__.'/ConstructorNoSniffsTest.xml';
$config = new ConfigDouble(["--standard=$standard"]);

$message = 'No sniffs were registered';
$message = 'ERROR: No sniffs were registered';
$this->expectRuntimeExceptionMessage($message);

new Ruleset($config);
Expand Down
2 changes: 1 addition & 1 deletion tests/Core/Ruleset/ExpandRulesetReferenceHomePathTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public function testHomePathRefGetsExpandedAndThrowsExceptionWhenPathIsInvalid()
$standard = __DIR__.'/ExpandRulesetReferenceHomePathFailTest.xml';
$config = new ConfigDouble(["--standard=$standard"]);

$exceptionMessage = 'Referenced sniff "~/src/MyStandard/Sniffs/DoesntExist/" does not exist';
$exceptionMessage = 'ERROR: Referenced sniff "~/src/MyStandard/Sniffs/DoesntExist/" does not exist';
$this->expectRuntimeExceptionMessage($exceptionMessage);

new Ruleset($config);
Expand Down
2 changes: 1 addition & 1 deletion tests/Core/Ruleset/ExpandRulesetReferenceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function testUnresolvableReferenceThrowsException($standard, $replacement
$standard = __DIR__.'/'.$standard;
$config = new ConfigDouble(["--standard=$standard"]);

$exceptionMessage = 'Referenced sniff "%s" does not exist';
$exceptionMessage = 'ERROR: Referenced sniff "%s" does not exist';
$this->expectRuntimeExceptionMessage(sprintf($exceptionMessage, $replacement));

new Ruleset($config);
Expand Down
2 changes: 1 addition & 1 deletion tests/Core/Ruleset/PopulateTokenListenersTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public function testSniffWhereRegisterDoesNotReturnAnArrayThrowsException()
$config = new ConfigDouble(["--standard=$standard"]);

$sniffClass = 'Fixtures\\TestStandard\\Sniffs\\InvalidSniffs\\RegisterNoArraySniff';
$message = "Sniff $sniffClass register() method must return an array";
$message = "ERROR: Sniff $sniffClass register() method must return an array";
$this->expectRuntimeExceptionMessage($message);

new Ruleset($config);
Expand Down
2 changes: 1 addition & 1 deletion tests/Core/Ruleset/ProcessRuleInvalidTypeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public function testInvalidTypeHandling()
$standard = __DIR__.'/ProcessRuleInvalidTypeTest.xml';
$config = new ConfigDouble(["--standard=$standard"]);

$message = 'Message type "notice" is invalid; must be "error" or "warning"';
$message = 'ERROR: Message type "notice" is invalid; must be "error" or "warning"';
$this->expectRuntimeExceptionMessage($message);

new Ruleset($config);
Expand Down
2 changes: 1 addition & 1 deletion tests/Core/Ruleset/ProcessRulesetAutoloadTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public function testShouldProcessAutoloadCbfonly()
*/
public function testFileNotFoundException()
{
$exceptionMsg = 'The specified autoload file "./tests/Core/Ruleset/Fixtures/ThisFileDoesNotExist.php" does not exist';
$exceptionMsg = 'ERROR: The specified autoload file "./tests/Core/Ruleset/Fixtures/ThisFileDoesNotExist.php" does not exist';
$this->expectRuntimeExceptionMessage($exceptionMsg);

// Set up the ruleset.
Expand Down
6 changes: 3 additions & 3 deletions tests/Core/Ruleset/ProcessRulesetBrokenRulesetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public function testBrokenRulesetEmptyFile()
$standard = __DIR__.'/ProcessRulesetBrokenRulesetEmptyFileTest.xml';
$config = new ConfigDouble(["--standard=$standard"]);

$regex = '`^Ruleset \S+ProcessRulesetBrokenRulesetEmptyFileTest\.xml is not valid\R';
$regex = '`^ERROR: Ruleset \S+ProcessRulesetBrokenRulesetEmptyFileTest\.xml is not valid\R';
$regex .= '(- On line 1, column 1: Document is empty\R)?$`';

$this->expectRuntimeExceptionRegex($regex);
Expand All @@ -59,7 +59,7 @@ public function testBrokenRulesetSingleError()
$standard = __DIR__.'/ProcessRulesetBrokenRulesetSingleErrorTest.xml';
$config = new ConfigDouble(["--standard=$standard"]);

$regex = '`^Ruleset \S+ProcessRulesetBrokenRulesetSingleErrorTest\.xml is not valid\R';
$regex = '`^ERROR: Ruleset \S+ProcessRulesetBrokenRulesetSingleErrorTest\.xml is not valid\R';
$regex .= '- On line 3, column 1: Premature end of data in tag ruleset line 2\R$`';

$this->expectRuntimeExceptionRegex($regex);
Expand All @@ -79,7 +79,7 @@ public function testBrokenRulesetMultiError()
$standard = __DIR__.'/ProcessRulesetBrokenRulesetMultiErrorTest.xml';
$config = new ConfigDouble(["--standard=$standard"]);

$regex = '`^Ruleset \S+ProcessRulesetBrokenRulesetMultiErrorTest\.xml is not valid\R';
$regex = '`^ERROR: Ruleset \S+ProcessRulesetBrokenRulesetMultiErrorTest\.xml is not valid\R';
$regex .= '- On line 8, column 12: Opening and ending tag mismatch: property line 7 and rule\R';
$regex .= '- On line 10, column 11: Opening and ending tag mismatch: properties line 5 and ruleset\R';
$regex .= '(- On line 11, column 1: Premature end of data in tag rule line 4\R)?$`';
Expand Down
4 changes: 2 additions & 2 deletions tests/Core/Ruleset/SetSniffPropertyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public function testSetPropertyAppliesPropertyToMultipleSniffsInCategory()
*/
public function testSetPropertyThrowsErrorOnInvalidProperty()
{
$exceptionMsg = 'Ruleset invalid. Property "indentation" does not exist on sniff Generic.Arrays.ArrayIndent';
$exceptionMsg = 'ERROR: Ruleset invalid. Property "indentation" does not exist on sniff Generic.Arrays.ArrayIndent';
$this->expectRuntimeExceptionMessage($exceptionMsg);

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

// Set up the ruleset.
Expand Down
10 changes: 5 additions & 5 deletions tests/Core/Ruleset/ShowSniffDeprecationsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -514,23 +514,23 @@ public static function dataExceptionIsThrownOnIncorrectlyImplementedInterface()
return [
'getDeprecationVersion() does not return a string' => [
'standard' => 'ShowSniffDeprecationsInvalidDeprecationVersionTest.xml',
'exceptionMessage' => 'The Fixtures\TestStandard\Sniffs\DeprecatedInvalid\InvalidDeprecationVersionSniff::getDeprecationVersion() method must return a non-empty string, received double',
'exceptionMessage' => 'ERROR: The Fixtures\TestStandard\Sniffs\DeprecatedInvalid\InvalidDeprecationVersionSniff::getDeprecationVersion() method must return a non-empty string, received double',
],
'getRemovalVersion() does not return a string' => [
'standard' => 'ShowSniffDeprecationsInvalidRemovalVersionTest.xml',
'exceptionMessage' => 'The Fixtures\TestStandard\Sniffs\DeprecatedInvalid\InvalidRemovalVersionSniff::getRemovalVersion() method must return a non-empty string, received array',
'exceptionMessage' => 'ERROR: The Fixtures\TestStandard\Sniffs\DeprecatedInvalid\InvalidRemovalVersionSniff::getRemovalVersion() method must return a non-empty string, received array',
],
'getDeprecationMessage() does not return a string' => [
'standard' => 'ShowSniffDeprecationsInvalidDeprecationMessageTest.xml',
'exceptionMessage' => 'The Fixtures\TestStandard\Sniffs\DeprecatedInvalid\InvalidDeprecationMessageSniff::getDeprecationMessage() method must return a string, received object',
'exceptionMessage' => 'ERROR: The Fixtures\TestStandard\Sniffs\DeprecatedInvalid\InvalidDeprecationMessageSniff::getDeprecationMessage() method must return a string, received object',
],
'getDeprecationVersion() returns an empty string' => [
'standard' => 'ShowSniffDeprecationsEmptyDeprecationVersionTest.xml',
'exceptionMessage' => 'The Fixtures\TestStandard\Sniffs\DeprecatedInvalid\EmptyDeprecationVersionSniff::getDeprecationVersion() method must return a non-empty string, received ""',
'exceptionMessage' => 'ERROR: The Fixtures\TestStandard\Sniffs\DeprecatedInvalid\EmptyDeprecationVersionSniff::getDeprecationVersion() method must return a non-empty string, received ""',
],
'getRemovalVersion() returns an empty string' => [
'standard' => 'ShowSniffDeprecationsEmptyRemovalVersionTest.xml',
'exceptionMessage' => 'The Fixtures\TestStandard\Sniffs\DeprecatedInvalid\EmptyRemovalVersionSniff::getRemovalVersion() method must return a non-empty string, received ""',
'exceptionMessage' => 'ERROR: The Fixtures\TestStandard\Sniffs\DeprecatedInvalid\EmptyRemovalVersionSniff::getRemovalVersion() method must return a non-empty string, received ""',
],
];

Expand Down

0 comments on commit 14de190

Please sign in to comment.