From f11f3960622dcba83f923ad87835b62e1edcb1d3 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 20 Jan 2024 01:27:11 +0100 Subject: [PATCH] Improve file errors --- src/Rules/AbstractRule.php | 70 ++++++++++++++++--- src/Rules/File/DirectoryNameRule.php | 2 +- src/Rules/File/FileNameRule.php | 2 +- .../DirectoryName/DirectoryNameRuleTest.php | 4 +- .../Rules/File/FileName/FileNameRuleTest.php | 4 +- tests/Rules/RuleTest.php | 8 ++- 6 files changed, 71 insertions(+), 19 deletions(-) diff --git a/src/Rules/AbstractRule.php b/src/Rules/AbstractRule.php index c8b658e8..45b58a9c 100644 --- a/src/Rules/AbstractRule.php +++ b/src/Rules/AbstractRule.php @@ -124,16 +124,57 @@ protected function findPrevious(int|string|array $type, array $tokens, int $star protected function addWarning(string $message, Token $token, ?string $messageId = null): bool { - return $this->addMessage(Violation::LEVEL_WARNING, $message, $token, $messageId); + return $this->addMessage( + Violation::LEVEL_WARNING, + $message, + $token->getFilename(), + $token->getLine(), + $token->getPosition(), + $messageId, + ); + } + + protected function addFileWarning(string $message, Token $token, ?string $messageId = null): bool + { + return $this->addMessage( + Violation::LEVEL_WARNING, + $message, + $token->getFilename(), + null, + null, + $messageId, + ); } protected function addError(string $message, Token $token, ?string $messageId = null): bool { - return $this->addMessage(Violation::LEVEL_ERROR, $message, $token, $messageId); + return $this->addMessage( + Violation::LEVEL_ERROR, + $message, + $token->getFilename(), + $token->getLine(), + $token->getPosition(), + $messageId, + ); } - protected function addFixableWarning(string $message, Token $token, ?string $messageId = null): ?FixerInterface + protected function addFileError(string $message, Token $token, ?string $messageId = null): bool { + return $this->addMessage( + Violation::LEVEL_ERROR, + $message, + $token->getFilename(), + null, + null, + $messageId, + ); + } + + protected function addFixableWarning( + string $message, + Token $token, + ?string $messageId = null + ): ?FixerInterface { $added = $this->addWarning($message, $token, $messageId); if (!$added) { return null; @@ -142,8 +183,11 @@ protected function addFixableWarning(string $message, Token $token, ?string $mes return $this->fixer; } - protected function addFixableError(string $message, Token $token, ?string $messageId = null): ?FixerInterface - { + protected function addFixableError( + string $message, + Token $token, + ?string $messageId = null + ): ?FixerInterface { $added = $this->addError($message, $token, $messageId); if (!$added) { return null; @@ -152,13 +196,19 @@ protected function addFixableError(string $message, Token $token, ?string $messa return $this->fixer; } - private function addMessage(int $messageType, string $message, Token $token, ?string $messageId = null): bool - { + private function addMessage( + int $messageType, + string $message, + string $fileName, + ?int $line = null, + ?int $position = null, + ?string $messageId = null + ): bool { $id = new ViolationId( $this->getShortName(), $messageId ?? ucfirst(strtolower(Violation::getLevelAsString($messageType))), - $token->getLine(), - $token->getPosition(), + $line, + $position, ); foreach ($this->ignoredViolations as $ignoredViolation) { if ($ignoredViolation->match($id)) { @@ -171,7 +221,7 @@ private function addMessage(int $messageType, string $message, Token $token, ?st $violation = new Violation( $messageType, $message, - $token->getFilename(), + $fileName, $this->getName(), $id, ); diff --git a/src/Rules/File/DirectoryNameRule.php b/src/Rules/File/DirectoryNameRule.php index 8d2dbff0..b63129f0 100644 --- a/src/Rules/File/DirectoryNameRule.php +++ b/src/Rules/File/DirectoryNameRule.php @@ -61,7 +61,7 @@ protected function process(int $tokenPosition, array $tokens): void }; if ($expected !== $directory) { - $this->addError( + $this->addFileError( sprintf('The directory name must use %s ; expected %s.', $this->case, $expected), $token, ); diff --git a/src/Rules/File/FileNameRule.php b/src/Rules/File/FileNameRule.php index 0af0c015..9092016e 100644 --- a/src/Rules/File/FileNameRule.php +++ b/src/Rules/File/FileNameRule.php @@ -64,7 +64,7 @@ protected function process(int $tokenPosition, array $tokens): void }; if ($expected !== $fileName) { - $this->addError( + $this->addFileError( sprintf('The file name must use %s ; expected %s.', $this->case, $expected), $token, ); diff --git a/tests/Rules/File/DirectoryName/DirectoryNameRuleTest.php b/tests/Rules/File/DirectoryName/DirectoryNameRuleTest.php index f8e8a3f0..b4f8f8dc 100644 --- a/tests/Rules/File/DirectoryName/DirectoryNameRuleTest.php +++ b/tests/Rules/File/DirectoryName/DirectoryNameRuleTest.php @@ -52,7 +52,7 @@ public function testRuleInvalidTemplatesDirectory(): void { $this->checkRule( new DirectoryNameRule(baseDirectory: 'templates'), - ['DirectoryName.Error:1:1'], + ['DirectoryName.Error'], __DIR__.'/templates/directoryNameRuleTest/DirectoryNameRuleTest.twig' ); } @@ -64,7 +64,7 @@ public function testRulePascalCase(): void public function testRuleInvalidDirectory(): void { - $this->checkRule(new DirectoryNameRule(baseDirectory: 'File'), ['DirectoryName.Error:1:1']); + $this->checkRule(new DirectoryNameRule(baseDirectory: 'File'), ['DirectoryName.Error']); } public function testRuleKebabCase(): void diff --git a/tests/Rules/File/FileName/FileNameRuleTest.php b/tests/Rules/File/FileName/FileNameRuleTest.php index 1c6a55f5..050017ba 100644 --- a/tests/Rules/File/FileName/FileNameRuleTest.php +++ b/tests/Rules/File/FileName/FileNameRuleTest.php @@ -37,7 +37,7 @@ public function testConfiguration(): void public function testRule(): void { $this->checkRule(new FileNameRule(), [ - 'FileName.Error:1:1', + 'FileName.Error', ]); } @@ -69,7 +69,7 @@ public function testRuleValidFileWithDot(): void public function testRuleBaseDir(): void { $this->checkRule(new FileNameRule(baseDirectory: 'File'), [ - 'FileName.Error:1:1', + 'FileName.Error', ]); } diff --git a/tests/Rules/RuleTest.php b/tests/Rules/RuleTest.php index dfdf2b48..669484b4 100644 --- a/tests/Rules/RuleTest.php +++ b/tests/Rules/RuleTest.php @@ -29,7 +29,9 @@ protected function process(int $tokenPosition, array $tokens): void if (0 === $tokenPosition) { $this->addWarning('Fake Warning', $token); - $this->addError('Fake Error', $token); + $this->addFileWarning('Fake File Warning', $token); + $this->addError('Fake File Error', $token); + $this->addFileError('Fake Error', $token); $this->addFixableWarning('Fake fixable warning', $token); $this->addFixableError('Fake fixable error', $token); } @@ -38,8 +40,8 @@ protected function process(int $tokenPosition, array $tokens): void $rule->lintFile([new Token(Token::EOF_TYPE, 0, 0, 'fakeFile.html.twig')], $report); - static::assertSame(2, $report->getTotalWarnings()); - static::assertSame(2, $report->getTotalErrors()); + static::assertSame(3, $report->getTotalWarnings()); + static::assertSame(3, $report->getTotalErrors()); } public function testRuleName(): void