From 581eef94514cabdaa5d95ae24b81540924ff65e6 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 29 Jan 2025 03:57:04 +0100 Subject: [PATCH 1/3] :sparkles: New `MessageCollector` utility class This _internal-use only_ `PHP_CodeSniffer\Util\MessageCollector` class can be used to collect various type of "error" messages, including notices, warnings and deprecations for display at a later point in time. This class is intended to be used by various steps in the PHPCS process flow to improve the error handling and prevent the "one error hiding another" pattern. Design choices: * This class does not have a constructor and has no awareness of the applicable `Config` or other aspects of a PHPCS run. If notices should be displayed conditionally, like only when not in `-q` quite mode, this should be handled by the class _using_ the `MessageCollector`. * While duplicate error messages are not a great user experience, this class allows for them. It is the responsibility of the class _using_ the `MessageCollector` to ensure that the messages cached are informative for the end-user. * The class provides a number of (`public`) class constants to indicate the error level when adding messages. It is _strongly_ recommended to only ever use these class constants for the error levels. The default error level for messages will be `NOTICE`. Note: yes, these class constants should basically be an Enum, however, enums are a PHP 8.1+ feature, so can't be used at this time. * The class provides two (`public`) "orderby" class constants to indicate the display order of the messages. It is _strongly_ recommended to only ever use these class constants for the display order. By default, messages will be ordered by severity (with "order received" being the secondary ordering for messages of the same severity). Note: again, yes, these class constants should basically be an Enum. * When display of the messages is requested and the message cache includes messages with severity `ERROR`, the class will throw a RuntimeException, which will cause the PHPCS run to exit with a non-zero exit code (currently `3`). * In all other cases, the messages will be displayed, but the PHPCS run will continue without affecting the exit code. * It was suggested by fredden to consider implementing [PSR-3: Logger Interface](https://www.php-fig.org/psr/psr-3/) for the `MessageCollector`. While we discussed this, I don't consider this necessary at this time, though as the class has no BC-promise, this can be reconsidered in the future. Arguments against implementing PSR-3 at this time: - Placeholder handling is required in the PSR-3 implementation to allow for logging to different contexts and using appropriate variable escaping based on the context. For PHPCS, messages will only ever be displayed on the command-line, so there is no need to take escaping for different contexts into account. - PSR-3 expects handling of eight different severity levels - emergency, alert, critical, error, warning, notice, info, debug -. For PHPCS, the `emergency`, `alert` and `critical` levels, as defined in PSR-3, are redundant as these will never occur. Along the same line, PHPCS does not log `info`. However, we do want to be able to display deprecation notices, but that is a severity level not supported under PSR-3. The new functionality is fully covered by tests. Potential future scope: * Add a constructor and inject the `Config` class to allow for colorizing the messages/message prefixes if color support is enabled. This would be most effective after issue 448 has been addressed first. * Add a new `CRITICAL` level for errors which prohibit the current task from finishing. When an error with such level would be received, it would cause the class to display all messages collected up to that point straight away via an exception. * If the conditional displaying of the messages, like only when not in `-q` mode, would lead to undue duplicate code, potentially move display conditions handling into the `MessageCollector` class. This would also need the aforementioned constructor with `Config` injection. --- src/Util/MessageCollector.php | 310 ++++++++++ .../MessageCollector/MessageCollectorTest.php | 539 ++++++++++++++++++ 2 files changed, 849 insertions(+) create mode 100644 src/Util/MessageCollector.php create mode 100644 tests/Core/Util/MessageCollector/MessageCollectorTest.php diff --git a/src/Util/MessageCollector.php b/src/Util/MessageCollector.php new file mode 100644 index 0000000000..2b6ca8f902 --- /dev/null +++ b/src/Util/MessageCollector.php @@ -0,0 +1,310 @@ + + * @copyright 2025 PHPCSStandards and contributors + * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Util; + +use InvalidArgumentException; +use PHP_CodeSniffer\Exceptions\RuntimeException; + +final class MessageCollector +{ + + /** + * Indicator for a (blocking) error. + * + * @var int + */ + const ERROR = 1; + + /** + * Indicator for a warning. + * + * @var int + */ + const WARNING = 2; + + /** + * Indicator for a notice. + * + * @var int + */ + const NOTICE = 4; + + /** + * Indicator for a deprecation notice. + * + * @var int + */ + const DEPRECATED = 8; + + /** + * Indicator for ordering the messages based on severity first, order received second. + * + * @var string + */ + const ORDERBY_SEVERITY = 'severity'; + + /** + * Indicator for ordering the messages based on the order in which they were received. + * + * @var string + */ + const ORDERBY_RECEIVED = 'received'; + + /** + * Collected messages. + * + * @var array> The value for each array entry is an associative array + * which holds two keys: + * - 'message' string The message text. + * - 'type' int The type of the message based on the + * above declared error level constants. + */ + private $cache = []; + + + /** + * Add a new message. + * + * @param string $message The message text. + * @param int $type The type of message. Should be one of the following constants: + * MessageCollector::ERROR, MessageCollector::WARNING, MessageCollector::NOTICE + * or MessageCollector::DEPRECATED. + * Defaults to MessageCollector::NOTICE. + * + * @return void + * + * @throws \InvalidArgumentException If the message text is not a string. + * @throws \InvalidArgumentException If the message type is not one of the accepted types. + */ + public function add($message, $type=self::NOTICE) + { + if (is_string($message) === false) { + throw new InvalidArgumentException('The $message should be of type string. Received: '.gettype($message).'.'); + } + + if ($type !== self::ERROR + && $type !== self::WARNING + && $type !== self::NOTICE + && $type !== self::DEPRECATED + ) { + throw new InvalidArgumentException('The message $type should be one of the predefined MessageCollector constants. Received: '.$type.'.'); + } + + $this->cache[] = [ + 'message' => $message, + 'type' => $type, + ]; + + }//end add() + + + /** + * Determine whether or not the currently cached errors include blocking errors. + * + * @return bool + */ + public function containsBlockingErrors() + { + $seenTypes = $this->arrayColumn($this->cache, 'type'); + $typeFrequency = array_count_values($seenTypes); + return isset($typeFrequency[self::ERROR]); + + }//end containsBlockingErrors() + + + /** + * Display the cached messages. + * + * Displaying the messages will also clear the message cache. + * + * @param string $order Optional. The order in which to display the messages. + * Should be one of the following constants: MessageCollector::ORDERBY_SEVERITY, + * MessageCollector::ORDERBY_RECEIVED. + * Defaults to MessageCollector::ORDERBY_SEVERITY. + * + * @return void + * + * @throws \PHP_CodeSniffer\Exceptions\RuntimeException When there are blocking errors. + */ + public function display($order=self::ORDERBY_SEVERITY) + { + if ($this->cache === []) { + return; + } + + $blocking = $this->containsBlockingErrors(); + $messageInfo = $this->prefixAll($this->cache); + $this->clearCache(); + + if ($order === self::ORDERBY_RECEIVED) { + $messages = $this->arrayColumn($messageInfo, 'message'); + } else { + $messages = $this->sortBySeverity($messageInfo); + } + + $allMessages = implode(PHP_EOL, $messages).PHP_EOL.PHP_EOL; + + if ($blocking === true) { + throw new RuntimeException($allMessages); + } else { + echo $allMessages; + } + + }//end display() + + + /** + * Label all messages based on their type. + * + * @param array> $messages A multi-dimensional array of messages with their severity. + * + * @return array> + */ + private function prefixAll($messages) + { + foreach ($messages as $i => $details) { + $messages[$i]['message'] = $this->prefix($details['message'], $details['type']); + } + + return $messages; + + }//end prefixAll() + + + /** + * Add a message type prefix to a message. + * + * @param string $message The message text. + * @param int $type The type of message. + * + * @return string + */ + private function prefix($message, $type) + { + switch ($type) { + case self::ERROR: + $message = 'ERROR: '.$message; + break; + + case self::WARNING: + $message = 'WARNING: '.$message; + break; + + case self::DEPRECATED: + $message = 'DEPRECATED: '.$message; + break; + + default: + $message = 'NOTICE: '.$message; + break; + } + + return $message; + + }//end prefix() + + + /** + * Sort an array of messages by severity. + * + * @param array> $messages A multi-dimensional array of messages with their severity. + * + * @return array A single dimensional array of only messages, sorted by severity. + */ + private function sortBySeverity($messages) + { + if (count($messages) === 1) { + return [$messages[0]['message']]; + } + + $errors = []; + $warnings = []; + $notices = []; + $deprecations = []; + + foreach ($messages as $details) { + switch ($details['type']) { + case self::ERROR: + $errors[] = $details['message']; + break; + + case self::WARNING: + $warnings[] = $details['message']; + break; + + case self::DEPRECATED: + $deprecations[] = $details['message']; + break; + + default: + $notices[] = $details['message']; + break; + } + } + + return array_merge($errors, $warnings, $notices, $deprecations); + + }//end sortBySeverity() + + + /** + * Clear the message cache. + * + * @return void + */ + private function clearCache() + { + $this->cache = []; + + }//end clearCache() + + + /** + * Return the values from a single column in the input array. + * + * Polyfill for the PHP 5.5+ native array_column() function (for the functionality needed here). + * + * @param array> $input A multi-dimensional array from which to pull a column of values. + * @param string $columnKey The name of the column of values to return. + * + * @link https://www.php.net/function.array-column + * + * @return array + */ + private function arrayColumn(array $input, $columnKey) + { + if (function_exists('array_column') === true) { + // PHP 5.5+. + return array_column($input, $columnKey); + } + + // PHP 5.4. + $callback = function ($row) use ($columnKey) { + return $row[$columnKey]; + }; + + return array_map($callback, $input); + + }//end arrayColumn() + + +}//end class diff --git a/tests/Core/Util/MessageCollector/MessageCollectorTest.php b/tests/Core/Util/MessageCollector/MessageCollectorTest.php new file mode 100644 index 0000000000..78e8961a58 --- /dev/null +++ b/tests/Core/Util/MessageCollector/MessageCollectorTest.php @@ -0,0 +1,539 @@ + + * @copyright 2025 PHPCSStandards and contributors + * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Tests\Core\Util\MessageCollector; + +use PHP_CodeSniffer\Util\MessageCollector; +use PHPUnit\Framework\TestCase; + +/** + * Tests the message caching and display functionality. + * + * @covers \PHP_CodeSniffer\Util\MessageCollector + */ +final class MessageCollectorTest extends TestCase +{ + + + /** + * Verify that non-string "messages" are rejected with an exception. + * + * @param mixed $message The invalid "message" to add. + * + * @dataProvider dataAddingNonStringMessageResultsInException + * + * @return void + */ + public function testAddingNonStringMessageResultsInException($message) + { + $exception = 'InvalidArgumentException'; + $exceptionMsg = 'The $message should be of type string. Received: '; + if (method_exists($this, 'expectException') === true) { + // PHPUnit 5+. + $this->expectException($exception); + $this->expectExceptionMessage($exceptionMsg); + } else { + // PHPUnit 4. + $this->setExpectedException($exception, $exceptionMsg); + } + + $msgCollector = new MessageCollector(); + $msgCollector->add($message); + + }//end testAddingNonStringMessageResultsInException() + + + /** + * Data provider. + * + * @see testAddingNonStringMessageResultsInException() + * + * @return array> + */ + public function dataAddingNonStringMessageResultsInException() + { + return [ + 'null' => [null], + 'boolean' => [true], + 'integer' => [10], + 'array' => [['something' => 'incorrect']], + ]; + + }//end dataAddingNonStringMessageResultsInException() + + + /** + * Verify that passing a message type which is not one of the predefined types is rejected with an exception. + * + * @param mixed $type The invalid "type" to pass. + * + * @dataProvider dataAddingMessageWithUnsupportedMessageTypeResultsInException + * + * @return void + */ + public function testAddingMessageWithUnsupportedMessageTypeResultsInException($type) + { + $exception = 'InvalidArgumentException'; + $exceptionMsg = 'The message $type should be one of the predefined MessageCollector constants. Received: '; + if (method_exists($this, 'expectException') === true) { + // PHPUnit 5+. + $this->expectException($exception); + $this->expectExceptionMessage($exceptionMsg); + } else { + // PHPUnit 4. + $this->setExpectedException($exception, $exceptionMsg); + } + + $msgCollector = new MessageCollector(); + $msgCollector->add('Message', $type); + + }//end testAddingMessageWithUnsupportedMessageTypeResultsInException() + + + /** + * Data provider. + * + * @see testAddingMessageWithUnsupportedMessageTypeResultsInException() + * + * @return array> + */ + public function dataAddingMessageWithUnsupportedMessageTypeResultsInException() + { + return [ + 'null' => [null], + 'boolean' => [true], + 'string' => ['DEPRECATED'], + 'integer which doesn\'t match any of the message type constants: -235' => [-235], + 'integer which doesn\'t match any of the message type constants: 0' => [0], + 'integer which doesn\'t match any of the message type constants: 3' => [3], + 'integer which doesn\'t match any of the message type constants: 6' => [6], + 'integer which doesn\'t match any of the message type constants: 123' => [123], + 'integer which doesn\'t match any of the message type constants: PHP_INT_MAX' => [PHP_INT_MAX], + ]; + + }//end dataAddingMessageWithUnsupportedMessageTypeResultsInException() + + + /** + * Verify that the `containsBlockingErrors()` method correctly identifies whether the collected messages + * include messages which are blocking (errors), or only include non-blocking (warnings, notices, + * deprecations) messages. + * + * @param array $messages The messages to display. + * Key is the message, value is the error level. + * @param bool $expected The expected function output. + * + * @dataProvider dataContainsBlockingErrors + * + * @return void + */ + public function testContainsBlockingErrors($messages, $expected) + { + $msgCollector = new MessageCollector(); + $this->createErrorCache($msgCollector, $messages); + + $this->assertSame($expected, $msgCollector->containsBlockingErrors()); + + }//end testContainsBlockingErrors() + + + /** + * Data provider. + * + * @see testContainsBlockingErrors() + * + * @return array|bool>> + */ + public function dataContainsBlockingErrors() + { + return [ + 'No messages' => [ + 'messages' => [], + 'expected' => false, + ], + 'Only non-blocking messages' => [ + 'messages' => [ + 'First message' => MessageCollector::WARNING, + 'Second message' => MessageCollector::NOTICE, + 'Third message' => MessageCollector::DEPRECATED, + ], + 'expected' => false, + ], + 'Only blocking messages' => [ + 'messages' => [ + 'First message' => MessageCollector::ERROR, + 'Second message' => MessageCollector::ERROR, + 'Third message' => MessageCollector::ERROR, + ], + 'expected' => true, + ], + 'Mix of blocking and non-blocking messages' => [ + 'messages' => [ + 'First message' => MessageCollector::DEPRECATED, + 'Second message' => MessageCollector::ERROR, + 'Third message' => MessageCollector::WARNING, + ], + 'expected' => true, + ], + ]; + + }//end dataContainsBlockingErrors() + + + /** + * Test displaying non-blocking messages only. + * + * Verifies that: + * - Each message is prefixed with the appropriate prefix. + * - The default message order is observed. + * - The messages use the appropriate OS-specific EOL character. + * + * @param array $messages The messages to display. + * Key is the message, value is the error level. + * @param string $expected The expected exception message. + * + * @dataProvider dataDisplayingNonBlockingMessages + * + * @return void + */ + public function testDisplayingNonBlockingMessages($messages, $expected) + { + $msgCollector = new MessageCollector(); + $this->createErrorCache($msgCollector, $messages); + + $this->expectOutputString($expected); + + $msgCollector->display(); + + }//end testDisplayingNonBlockingMessages() + + + /** + * Data provider. + * + * @see testDisplayingNonBlockingMessages() + * + * @return array|string>> + */ + public function dataDisplayingNonBlockingMessages() + { + // phpcs:disable Squiz.Strings.ConcatenationSpacing.PaddingFound -- Test readability is more important. + return [ + 'No messages' => [ + 'messages' => [], + 'expected' => '', + ], + 'One warning' => [ + 'messages' => [ + 'This is a warning' => MessageCollector::WARNING, + ], + 'expected' => 'WARNING: This is a warning'.PHP_EOL.PHP_EOL, + ], + 'One notice' => [ + 'messages' => [ + 'This is a notice' => MessageCollector::NOTICE, + ], + 'expected' => 'NOTICE: This is a notice'.PHP_EOL.PHP_EOL, + ], + 'One deprecation' => [ + 'messages' => [ + 'This is a deprecation' => MessageCollector::DEPRECATED, + ], + 'expected' => 'DEPRECATED: This is a deprecation'.PHP_EOL.PHP_EOL, + ], + 'Multiple warnings' => [ + 'messages' => [ + 'First warning' => MessageCollector::WARNING, + 'Second warning' => MessageCollector::WARNING, + ], + 'expected' => 'WARNING: First warning'.PHP_EOL + .'WARNING: Second warning'.PHP_EOL.PHP_EOL, + ], + 'Multiple notices' => [ + 'messages' => [ + 'First notice' => MessageCollector::NOTICE, + 'Second notice' => MessageCollector::NOTICE, + 'Third notice' => MessageCollector::NOTICE, + ], + 'expected' => 'NOTICE: First notice'.PHP_EOL + .'NOTICE: Second notice'.PHP_EOL + .'NOTICE: Third notice'.PHP_EOL.PHP_EOL, + ], + 'Multiple deprecations' => [ + 'messages' => [ + 'First deprecation' => MessageCollector::DEPRECATED, + 'Second deprecation' => MessageCollector::DEPRECATED, + ], + 'expected' => 'DEPRECATED: First deprecation'.PHP_EOL + .'DEPRECATED: Second deprecation'.PHP_EOL.PHP_EOL, + ], + 'All together now' => [ + 'messages' => [ + 'First deprecation' => MessageCollector::DEPRECATED, + 'Second warning' => MessageCollector::WARNING, + 'Third notice' => MessageCollector::NOTICE, + 'Fourth warning' => MessageCollector::WARNING, + ], + 'expected' => 'WARNING: Second warning'.PHP_EOL + .'WARNING: Fourth warning'.PHP_EOL + .'NOTICE: Third notice'.PHP_EOL + .'DEPRECATED: First deprecation'.PHP_EOL.PHP_EOL, + ], + ]; + // phpcs:enable + + }//end dataDisplayingNonBlockingMessages() + + + /** + * Test displaying message collections containing blocking messages. + * + * Verifies that: + * - Each message is prefixed with the appropriate prefix. + * - The default message order is observed. + * - The messages use the appropriate OS-specific EOL character. + * + * @param array $messages The messages to display. + * Key is the message, value is the error level. + * @param string $expected The expected exception message. + * + * @dataProvider dataDisplayingBlockingErrors + * + * @return void + */ + public function testDisplayingBlockingErrors($messages, $expected) + { + $exception = 'PHP_CodeSniffer\Exceptions\RuntimeException'; + if (method_exists($this, 'expectException') === true) { + // PHPUnit 5+. + $this->expectException($exception); + $this->expectExceptionMessage($expected); + } else { + // PHPUnit 4. + $this->setExpectedException($exception, $expected); + } + + $msgCollector = new MessageCollector(); + $this->createErrorCache($msgCollector, $messages); + $msgCollector->display(); + + }//end testDisplayingBlockingErrors() + + + /** + * Data provider. + * + * @see testDisplayingBlockingErrors() + * + * @return array|string>> + */ + public function dataDisplayingBlockingErrors() + { + // phpcs:disable Squiz.Strings.ConcatenationSpacing.PaddingFound -- Test readability is more important. + return [ + 'Single error' => [ + 'messages' => [ + 'This is an error' => MessageCollector::ERROR, + ], + 'expected' => 'ERROR: This is an error'.PHP_EOL.PHP_EOL, + ], + 'Multiple errors' => [ + 'messages' => [ + 'First error' => MessageCollector::ERROR, + 'Second error' => MessageCollector::ERROR, + ], + 'expected' => 'ERROR: First error'.PHP_EOL + .'ERROR: Second error'.PHP_EOL.PHP_EOL, + ], + 'Errors mixed with non-blocking messages' => [ + 'messages' => [ + 'First deprecation' => MessageCollector::DEPRECATED, + 'Second warning' => MessageCollector::WARNING, + 'Third notice' => MessageCollector::NOTICE, + 'Fourth error' => MessageCollector::ERROR, + 'Fifth warning' => MessageCollector::WARNING, + 'Sixth error' => MessageCollector::ERROR, + 'Seventh deprecation' => MessageCollector::DEPRECATED, + ], + 'expected' => 'ERROR: Fourth error'.PHP_EOL + .'ERROR: Sixth error'.PHP_EOL + .'WARNING: Second warning'.PHP_EOL + .'WARNING: Fifth warning'.PHP_EOL + .'NOTICE: Third notice'.PHP_EOL + .'DEPRECATED: First deprecation'.PHP_EOL + .'DEPRECATED: Seventh deprecation'.PHP_EOL.PHP_EOL, + ], + ]; + // phpcs:enable + + }//end dataDisplayingBlockingErrors() + + + /** + * Verify and safeguard that adding the same message twice is accepted when messages have different error levels. + * + * Note: have multiple messages with the exact same text is not great for conveying information + * to the end-user, but that's not the concern of the MessageCollector class. + * + * @return void + */ + public function testNonUniqueMessagesWithDifferentErrorLevelAreAccepted() + { + $message = 'Trying to add the same message twice'; + $msgCollector = new MessageCollector(); + $msgCollector->add($message, MessageCollector::NOTICE); + $msgCollector->add($message, MessageCollector::WARNING); + + $expected = 'WARNING: Trying to add the same message twice'.PHP_EOL; + $expected .= 'NOTICE: Trying to add the same message twice'.PHP_EOL.PHP_EOL; + $this->expectOutputString($expected); + + $msgCollector->display(); + + }//end testNonUniqueMessagesWithDifferentErrorLevelAreAccepted() + + + /** + * Verify and safeguard that adding the same message twice is accepted when messages have the same error level. + * + * Note: have multiple messages with the exact same text is not great for conveying information + * to the end-user, but that's not the concern of the MessageCollector class. + * + * @return void + */ + public function testNonUniqueMessagesWithSameErrorLevelAreAccepted() + { + $message = 'Trying to add the same message twice'; + $msgCollector = new MessageCollector(); + $msgCollector->add($message, MessageCollector::NOTICE); + $msgCollector->add($message, MessageCollector::NOTICE); + + $expected = 'NOTICE: Trying to add the same message twice'.PHP_EOL; + $expected .= 'NOTICE: Trying to add the same message twice'.PHP_EOL.PHP_EOL; + $this->expectOutputString($expected); + + $msgCollector->display(); + + }//end testNonUniqueMessagesWithSameErrorLevelAreAccepted() + + + /** + * Ensure that the message cache is cleared when the collected messages are displayed. + * + * @return void + */ + public function testCallingDisplayTwiceWillNotShowMessagesTwice() + { + $messages = [ + 'First notice' => MessageCollector::NOTICE, + 'Second deprecation' => MessageCollector::DEPRECATED, + 'Third notice' => MessageCollector::NOTICE, + 'Fourth warning' => MessageCollector::WARNING, + ]; + + $msgCollector = new MessageCollector(); + $this->createErrorCache($msgCollector, $messages); + + $expected = 'WARNING: Fourth warning'.PHP_EOL; + $expected .= 'NOTICE: First notice'.PHP_EOL; + $expected .= 'NOTICE: Third notice'.PHP_EOL; + $expected .= 'DEPRECATED: Second deprecation'.PHP_EOL.PHP_EOL; + $this->expectOutputString($expected); + + $msgCollector->display(); + $msgCollector->display(); + + }//end testCallingDisplayTwiceWillNotShowMessagesTwice() + + + /** + * Test that messages are ordered correctly. + * + * @param string $order The display order. + * @param string $expected The expected output. + * + * @dataProvider dataDisplayOrderHandling + * + * @return void + */ + public function testDisplayOrderHandling($order, $expected) + { + $messages = [ + 'First notice' => MessageCollector::NOTICE, + 'Second deprecation' => MessageCollector::DEPRECATED, + 'Third notice' => MessageCollector::NOTICE, + 'Fourth warning' => MessageCollector::WARNING, + ]; + + $msgCollector = new MessageCollector(); + $this->createErrorCache($msgCollector, $messages); + + $this->expectOutputString($expected); + + $msgCollector->display($order); + + }//end testDisplayOrderHandling() + + + /** + * Data provider. + * + * @see testDisplayOrderHandling() + * + * @return array> + */ + public function dataDisplayOrderHandling() + { + $expectedForSeverity = 'WARNING: Fourth warning'.PHP_EOL; + $expectedForSeverity .= 'NOTICE: First notice'.PHP_EOL; + $expectedForSeverity .= 'NOTICE: Third notice'.PHP_EOL; + $expectedForSeverity .= 'DEPRECATED: Second deprecation'.PHP_EOL.PHP_EOL; + + $expectedForReceived = 'NOTICE: First notice'.PHP_EOL; + $expectedForReceived .= 'DEPRECATED: Second deprecation'.PHP_EOL; + $expectedForReceived .= 'NOTICE: Third notice'.PHP_EOL; + $expectedForReceived .= 'WARNING: Fourth warning'.PHP_EOL.PHP_EOL; + + return [ + 'Order by severity' => [ + 'order' => MessageCollector::ORDERBY_SEVERITY, + 'expected' => $expectedForSeverity, + ], + 'Order by received' => [ + 'order' => MessageCollector::ORDERBY_RECEIVED, + 'expected' => $expectedForReceived, + ], + 'Invalid order defaults to severity' => [ + 'order' => 'unknown order', + 'expected' => $expectedForSeverity, + ], + ]; + + }//end dataDisplayOrderHandling() + + + /** + * Test helper. + * + * @param \PHP_CodeSniffer\Util\MessageCollector $collector The error cache object. + * @param array $messages The error messages to add to the cache. + * Key is the message, value is the error level. + * + * @return void + */ + private function createErrorCache(MessageCollector $collector, $messages) + { + foreach ($messages as $msg => $type) { + $collector->add($msg, $type); + } + + }//end createErrorCache() + + +}//end class From c1db8c839558a7c2f61f311b1c099388e06b3203 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 29 Jan 2025 13:41:17 +0100 Subject: [PATCH 2/3] Ruleset: wire in `MessageCollector` This commit adds a mechanism to handle errors encountered while loading a ruleset in a more user-friendly manner. * Errors and notices aimed at end-users and ruleset maintainers will be collected while processing the ruleset. * Only once the complete processing of the ruleset is finished, will all errors/notices be displayed. This prevents "one error hiding behind another". * If there are only notices (deprecations/notices/warnings), these will not display when running `-e` (explain), `-q` (quiet mode) or `--generator=...` (documentation). * Errors will always display and will hard exit out of the run with a non-zero exit code. This implementation should be seen as an interim - "good enough for now" - solution, which can be iterated on in the future. I can imagine a more code-base wide solution at some later point in time, but that should not block this initial improvement. As the current implementation doesn't change the public API (the only new methods are `private`), it should be feasible to transform the current solution to whatever form a future solution will take without breaking changes. Includes tests covering the new functionality. --- src/Ruleset.php | 57 +++- .../Ruleset/DisplayCachedMessagesTest.php | 306 ++++++++++++++++++ 2 files changed, 358 insertions(+), 5 deletions(-) create mode 100644 tests/Core/Ruleset/DisplayCachedMessagesTest.php diff --git a/src/Ruleset.php b/src/Ruleset.php index f71620c477..9f8aaf9a3a 100644 --- a/src/Ruleset.php +++ b/src/Ruleset.php @@ -14,6 +14,7 @@ use PHP_CodeSniffer\Exceptions\RuntimeException; use PHP_CodeSniffer\Sniffs\DeprecatedSniff; use PHP_CodeSniffer\Util\Common; +use PHP_CodeSniffer\Util\MessageCollector; use PHP_CodeSniffer\Util\Standards; use RecursiveDirectoryIterator; use RecursiveIteratorIterator; @@ -131,6 +132,20 @@ class Ruleset */ private $deprecatedSniffs = []; + /** + * Message collector object. + * + * User-facing messages should be collected via this object for display once the ruleset processing has finished. + * + * The following type of errors should *NOT* be collected, but should still throw their own `RuntimeException`: + * - Errors which could cause other (uncollectable) errors further into the ruleset processing, like a missing autoload file. + * - Errors which are directly aimed at and only intended for sniff developers or integrators + * (in contrast to ruleset maintainers or end-users). + * + * @var \PHP_CodeSniffer\Util\MessageCollector + */ + private $msgCache; + /** * Initialise the ruleset that the run will use. @@ -138,14 +153,15 @@ class Ruleset * @param \PHP_CodeSniffer\Config $config The config data for the run. * * @return void - * @throws \PHP_CodeSniffer\Exceptions\RuntimeException If no sniffs were registered. + * @throws \PHP_CodeSniffer\Exceptions\RuntimeException If blocking errors were encountered when processing the ruleset. */ public function __construct(Config $config) { - $this->config = $config; - $restrictions = $config->sniffs; - $exclusions = $config->exclude; - $sniffs = []; + $this->config = $config; + $restrictions = $config->sniffs; + $exclusions = $config->exclude; + $sniffs = []; + $this->msgCache = new MessageCollector(); $standardPaths = []; foreach ($config->standards as $standard) { @@ -242,6 +258,8 @@ public function __construct(Config $config) throw new RuntimeException('ERROR: No sniffs were registered'); } + $this->displayCachedMessages(); + }//end __construct() @@ -461,6 +479,35 @@ public function showSniffDeprecations() }//end showSniffDeprecations() + /** + * Print any notices encountered while processing the ruleset(s). + * + * Note: these messages aren't shown at the time they are encountered to avoid "one error hiding behind another". + * This way the (end-)user gets to see all of them in one go. + * + * @return void + * + * @throws \PHP_CodeSniffer\Exceptions\RuntimeException If blocking errors were encountered. + */ + private function displayCachedMessages() + { + // Don't show deprecations/notices/warnings in quiet mode, in explain mode + // or when the documentation is being shown. + // Documentation and explain will call the Ruleset multiple times which + // would lead to duplicate display of the messages. + if ($this->msgCache->containsBlockingErrors() === false + && ($this->config->quiet === true + || $this->config->explain === true + || $this->config->generator !== null) + ) { + return; + } + + $this->msgCache->display(); + + }//end displayCachedMessages() + + /** * Processes a single ruleset and returns a list of the sniffs it represents. * diff --git a/tests/Core/Ruleset/DisplayCachedMessagesTest.php b/tests/Core/Ruleset/DisplayCachedMessagesTest.php new file mode 100644 index 0000000000..b8ac52af61 --- /dev/null +++ b/tests/Core/Ruleset/DisplayCachedMessagesTest.php @@ -0,0 +1,306 @@ + + * @copyright 2024 PHPCSStandards and contributors + * @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence + */ + +namespace PHP_CodeSniffer\Tests\Core\Ruleset; + +use PHP_CodeSniffer\Ruleset; +use PHP_CodeSniffer\Tests\ConfigDouble; +use PHP_CodeSniffer\Tests\Core\Ruleset\AbstractRulesetTestCase; +use PHP_CodeSniffer\Util\MessageCollector; +use ReflectionMethod; +use ReflectionProperty; + +/** + * Test error handling for the Ruleset. + * + * Note: this is purely a unit test of the `displayCachedMessages()` method. + * The errors themselves are mocked. + * + * @covers \PHP_CodeSniffer\Ruleset::displayCachedMessages + */ +final class DisplayCachedMessagesTest extends AbstractRulesetTestCase +{ + + + /** + * Test that no exception nor output is generated when there are no cached messsages. + * + * @return void + */ + public function testDisplayCachedMessagesStaysSilentWithoutErrors() + { + $ruleset = $this->getPlainRuleset(); + + $this->expectOutputString(''); + + $this->invokeDisplayCachedMessages($ruleset); + + }//end testDisplayCachedMessagesStaysSilentWithoutErrors() + + + /** + * Verify that blocking errors encountered while loading the ruleset(s) result in an exception being thrown. + * + * @param array $messages The messages encountered. + * @param string $expected The expected function output to screen (via an internally handled exception). + * + * @dataProvider dataBlockingErrorsAreDisplayedViaAnException + * + * @return void + */ + public function testBlockingErrorsAreDisplayedViaAnException($messages, $expected) + { + $ruleset = $this->getPlainRuleset(); + $this->mockCachedMessages($ruleset, $messages); + + $this->expectRuntimeExceptionMessage($expected); + + $this->invokeDisplayCachedMessages($ruleset); + + }//end testBlockingErrorsAreDisplayedViaAnException() + + + /** + * Data provider. + * + * @see testBlockingErrorsAreDisplayedViaAnException() + * + * @return array>> + */ + public static function dataBlockingErrorsAreDisplayedViaAnException() + { + return [ + 'One error' => [ + 'messages' => ['This is a serious blocking issue' => MessageCollector::ERROR], + 'expected' => 'ERROR: This is a serious blocking issue'.PHP_EOL.PHP_EOL, + ], + 'Multiple blocking errors' => [ + 'messages' => [ + 'This is a serious blocking issue' => MessageCollector::ERROR, + 'And here is another one' => MessageCollector::ERROR, + 'OMG, why do you think that would work ?' => MessageCollector::ERROR, + ], + // phpcs:disable Squiz.Strings.ConcatenationSpacing.PaddingFound -- Test readability is more important. + 'expected' => 'ERROR: This is a serious blocking issue'.PHP_EOL + . 'ERROR: And here is another one'.PHP_EOL + . 'ERROR: OMG, why do you think that would work ?'.PHP_EOL.PHP_EOL, + // phpcs:enable + ], + 'Mix of blocking and non-blocking errors' => [ + 'messages' => [ + 'This is a serious blocking issue' => MessageCollector::ERROR, + 'Something something deprecated and will be removed in v x.x.x' => MessageCollector::DEPRECATED, + 'Careful, this may not be correct' => MessageCollector::NOTICE, + ], + // phpcs:disable Squiz.Strings.ConcatenationSpacing.PaddingFound -- Test readability is more important. + 'expected' => 'ERROR: This is a serious blocking issue'.PHP_EOL + . 'NOTICE: Careful, this may not be correct'.PHP_EOL + . 'DEPRECATED: Something something deprecated and will be removed in v x.x.x'.PHP_EOL.PHP_EOL, + // phpcs:enable + ], + ]; + + }//end dataBlockingErrorsAreDisplayedViaAnException() + + + /** + * Test display of non-blocking messages encountered while loading the ruleset(s). + * + * @param array $messages The messages encountered. + * @param string $expected The expected function output to screen. + * + * @dataProvider dataNonBlockingErrorsGenerateOutput + * + * @return void + */ + public function testNonBlockingErrorsGenerateOutput($messages, $expected) + { + $ruleset = $this->getPlainRuleset(); + $this->mockCachedMessages($ruleset, $messages); + + $this->expectOutputString($expected); + + $this->invokeDisplayCachedMessages($ruleset); + + }//end testNonBlockingErrorsGenerateOutput() + + + /** + * Data provider. + * + * @see testNonBlockingErrorsGenerateOutput() + * + * @return array>> + */ + public static function dataNonBlockingErrorsGenerateOutput() + { + return [ + 'One deprecation' => [ + 'messages' => ['My deprecation message' => MessageCollector::DEPRECATED], + 'expected' => 'DEPRECATED: My deprecation message'.PHP_EOL.PHP_EOL, + ], + 'One notice' => [ + 'messages' => ['My notice message' => MessageCollector::NOTICE], + 'expected' => 'NOTICE: My notice message'.PHP_EOL.PHP_EOL, + ], + 'One warning' => [ + 'messages' => ['My warning message' => MessageCollector::WARNING], + 'expected' => 'WARNING: My warning message'.PHP_EOL.PHP_EOL, + ], + 'Multiple non-blocking errors' => [ + 'messages' => [ + 'Something something deprecated and will be removed in v x.x.x' => MessageCollector::DEPRECATED, + 'Something is not supported and support may be removed' => MessageCollector::WARNING, + 'Some other deprecation notice' => MessageCollector::DEPRECATED, + 'Careful, this may not be correct' => MessageCollector::NOTICE, + ], + // phpcs:disable Squiz.Strings.ConcatenationSpacing.PaddingFound -- Test readability is more important. + 'expected' => 'WARNING: Something is not supported and support may be removed'.PHP_EOL + .'NOTICE: Careful, this may not be correct'.PHP_EOL + .'DEPRECATED: Something something deprecated and will be removed in v x.x.x'.PHP_EOL + .'DEPRECATED: Some other deprecation notice'.PHP_EOL.PHP_EOL, + // phpcs:enable + ], + ]; + + }//end dataNonBlockingErrorsGenerateOutput() + + + /** + * Test that blocking errors will always show, independently of specific command-line options being used. + * + * @param array $configArgs Arguments to pass to the Config. + * + * @dataProvider dataSelectiveDisplayOfMessages + * + * @return void + */ + public function testBlockingErrorsAlwaysShow($configArgs) + { + $config = new ConfigDouble($configArgs); + $ruleset = new Ruleset($config); + + $message = 'Some serious error'; + $errors = [$message => MessageCollector::ERROR]; + $this->mockCachedMessages($ruleset, $errors); + + $this->expectRuntimeExceptionMessage('ERROR: '.$message.PHP_EOL); + + $this->invokeDisplayCachedMessages($ruleset); + + }//end testBlockingErrorsAlwaysShow() + + + /** + * Test that non-blocking messsages will not show when specific command-line options are being used. + * + * @param array $configArgs Arguments to pass to the Config. + * + * @dataProvider dataSelectiveDisplayOfMessages + * + * @return void + */ + public function testNonBlockingErrorsDoNotShowUnderSpecificCircumstances($configArgs) + { + $config = new ConfigDouble($configArgs); + $ruleset = new Ruleset($config); + $this->mockCachedMessages($ruleset, ['Deprecation notice' => MessageCollector::DEPRECATED]); + + $this->expectOutputString(''); + + $this->invokeDisplayCachedMessages($ruleset); + + }//end testNonBlockingErrorsDoNotShowUnderSpecificCircumstances() + + + /** + * Data provider. + * + * @see testBlockingErrorsAlwaysShow() + * @see testNonBlockingErrorsDoNotShow() + * + * @return array>> + */ + public static function dataSelectiveDisplayOfMessages() + { + return [ + 'Explain mode' => [ + 'configArgs' => ['-e'], + ], + 'Quiet mode' => [ + 'configArgs' => ['-q'], + ], + 'Documentation is requested' => [ + 'configArgs' => ['--generator=text'], + ], + ]; + + }//end dataSelectiveDisplayOfMessages() + + + /** + * Test Helper. + * + * @return \PHP_CodeSniffer\Ruleset + */ + private function getPlainRuleset() + { + static $ruleset; + + if (isset($ruleset) === false) { + $config = new ConfigDouble(); + $ruleset = new Ruleset($config); + } + + return $ruleset; + + }//end getPlainRuleset() + + + /** + * Add mock messages to the message cache. + * + * @param \PHP_CodeSniffer\Ruleset $ruleset The ruleset object. + * @param array $messages The messages to add to the message cache. + * + * @return void + */ + private function mockCachedMessages(Ruleset $ruleset, $messages) + { + $reflProperty = new ReflectionProperty($ruleset, 'msgCache'); + $reflProperty->setAccessible(true); + + $msgCache = $reflProperty->getValue($ruleset); + foreach ($messages as $msg => $type) { + $msgCache->add($msg, $type); + } + + $reflProperty->setAccessible(false); + + }//end mockCachedMessages() + + + /** + * Invoke the display of the cached messages. + * + * @param \PHP_CodeSniffer\Ruleset $ruleset The ruleset object. + * + * @return void + */ + private function invokeDisplayCachedMessages(Ruleset $ruleset) + { + $reflMethod = new ReflectionMethod($ruleset, 'displayCachedMessages'); + $reflMethod->setAccessible(true); + $reflMethod->invoke($ruleset); + $reflMethod->setAccessible(false); + + }//end invokeDisplayCachedMessages() + + +}//end class From 45baf5ceda49cb6df60ba6661211709ade20ed36 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 30 Jan 2025 00:12:52 +0100 Subject: [PATCH 3/3] 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). --- src/Ruleset.php | 51 ++++++++++--------- tests/Core/Ruleset/ConstructorTest.php | 2 +- .../ExpandRulesetReferenceHomePathTest.php | 3 +- .../Ruleset/ExpandRulesetReferenceTest.php | 3 +- ...ulateTokenListenersRegisterNoArrayTest.xml | 2 + .../Ruleset/PopulateTokenListenersTest.php | 7 ++- .../Ruleset/ProcessRuleInvalidTypeTest.php | 4 +- tests/Core/Ruleset/SetSniffPropertyTest.php | 4 +- 8 files changed, 45 insertions(+), 31 deletions(-) diff --git a/src/Ruleset.php b/src/Ruleset.php index 9f8aaf9a3a..cf3923092d 100644 --- 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 index 8ba13bb8e7..eaea6c239e 100644 --- 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 index 91202523cc..c16da4315c 100644 --- 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 index bb4d3bad4c..284bdf7fb8 100644 --- 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 index 5ae8568f8b..52a406a699 100644 --- a/tests/Core/Ruleset/PopulateTokenListenersRegisterNoArrayTest.xml +++ b/tests/Core/Ruleset/PopulateTokenListenersRegisterNoArrayTest.xml @@ -5,4 +5,6 @@ + + diff --git a/tests/Core/Ruleset/PopulateTokenListenersTest.php b/tests/Core/Ruleset/PopulateTokenListenersTest.php index ce922f55b9..6f79c10da1 100644 --- 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 index da2fa35205..d43958c77b 100644 --- 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 index 48aaba4fc4..b9d9ac1f29 100644 --- 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.