From 3c6019af4d4648a18f30826bfc263349ea78b467 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 27 Jul 2024 22:50:46 +0200 Subject: [PATCH] Improve return type of tokenizer --- UPGRADE.md | 9 ++--- src/Rules/AbstractFixableRule.php | 4 +-- src/Rules/AbstractRule.php | 4 +-- src/Rules/FixableRuleInterface.php | 6 +--- src/Rules/RuleInterface.php | 5 +-- src/Runner/Fixer.php | 4 +-- src/Runner/Linter.php | 6 ++-- src/Token/Tokenizer.php | 17 ++++------ src/Token/TokenizerInterface.php | 5 +-- src/Token/Tokens.php | 44 +++++++++++++++++++++++++ tests/Runner/FixerTest.php | 5 ++- tests/Runner/LinterTest.php | 12 +++---- tests/Token/Tokenizer/TokenizerTest.php | 21 ++++++++---- tests/Token/TokensTest.php | 27 +++++++++++++++ 14 files changed, 114 insertions(+), 55 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 8440c0eb..b0e013c9 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -54,14 +54,14 @@ If you never implemented a custom rule, nothing else changed. Otherwise, ... ```diff - public function lintFile(array $tokens, Report $report, array $ignoredViolations = []): void; -+ public function lintFile(Tokens $tokens, Report $report, array $ignoredViolations = []): void; ++ public function lintFile(Tokens $tokens, Report $report): void; ``` ### FixableRuleInterface ```diff - public function fixFile(array $tokens, FixerInterface $fixer, array $ignoredViolations = []): void; -+ public function fixFile(Tokens $tokens, FixerInterface $fixer, array $ignoredViolations = []): void; ++ public function fixFile(Tokens $tokens, FixerInterface $fixer): void; ``` ### TokenizerInterface @@ -71,10 +71,7 @@ If you never implemented a custom rule, nothing else changed. Otherwise, ... - * @return array{list, list} - */ - public function tokenize(Source $source): array; -+ /** -+ * @return array{Tokens, list} -+ */ -+ public function tokenize(Source $source): array; ++ public function tokenize(Source $source): Tokens; ``` ### Token diff --git a/src/Rules/AbstractFixableRule.php b/src/Rules/AbstractFixableRule.php index a41fffbe..ea7dc298 100644 --- a/src/Rules/AbstractFixableRule.php +++ b/src/Rules/AbstractFixableRule.php @@ -22,9 +22,9 @@ protected function init( $this->fixer = $fixer; } - public function fixFile(Tokens $tokens, FixerInterface $fixer, array $ignoredViolations = []): void + public function fixFile(Tokens $tokens, FixerInterface $fixer): void { - $this->init(null, $ignoredViolations, $fixer); + $this->init(null, $tokens->getIgnoredViolations(), $fixer); foreach (array_keys($tokens->toArray()) as $index) { $this->process($index, $tokens); diff --git a/src/Rules/AbstractRule.php b/src/Rules/AbstractRule.php index 7562b753..1d2c2af5 100644 --- a/src/Rules/AbstractRule.php +++ b/src/Rules/AbstractRule.php @@ -14,9 +14,9 @@ abstract class AbstractRule implements RuleInterface { use RuleTrait; - public function lintFile(Tokens $tokens, Report $report, array $ignoredViolations = []): void + public function lintFile(Tokens $tokens, Report $report): void { - $this->init($report, $ignoredViolations); + $this->init($report, $tokens->getIgnoredViolations()); foreach (array_keys($tokens->toArray()) as $index) { $this->process($index, $tokens); diff --git a/src/Rules/FixableRuleInterface.php b/src/Rules/FixableRuleInterface.php index 21b3ed46..6f69f646 100644 --- a/src/Rules/FixableRuleInterface.php +++ b/src/Rules/FixableRuleInterface.php @@ -4,14 +4,10 @@ namespace TwigCsFixer\Rules; -use TwigCsFixer\Report\ViolationId; use TwigCsFixer\Runner\FixerInterface; use TwigCsFixer\Token\Tokens; interface FixableRuleInterface { - /** - * @param list $ignoredViolations - */ - public function fixFile(Tokens $tokens, FixerInterface $fixer, array $ignoredViolations = []): void; + public function fixFile(Tokens $tokens, FixerInterface $fixer): void; } diff --git a/src/Rules/RuleInterface.php b/src/Rules/RuleInterface.php index 7965d701..18cd3593 100644 --- a/src/Rules/RuleInterface.php +++ b/src/Rules/RuleInterface.php @@ -5,15 +5,12 @@ namespace TwigCsFixer\Rules; use TwigCsFixer\Report\Report; -use TwigCsFixer\Report\ViolationId; use TwigCsFixer\Token\Tokens; interface RuleInterface { /** * Messages will be added to the given `$report` object. - * - * @param list $ignoredViolations */ - public function lintFile(Tokens $tokens, Report $report, array $ignoredViolations = []): void; + public function lintFile(Tokens $tokens, Report $report): void; } diff --git a/src/Runner/Fixer.php b/src/Runner/Fixer.php index 1a59366f..3921391f 100644 --- a/src/Runner/Fixer.php +++ b/src/Runner/Fixer.php @@ -85,14 +85,14 @@ public function fixFile(string $content, Ruleset $ruleset): string $this->inConflict = false; $twigSource = new Source($content, 'TwigCsFixer'); - [$stream, $ignoredViolations] = $this->tokenizer->tokenize($twigSource); + $stream = $this->tokenizer->tokenize($twigSource); $this->startFile($stream); $rules = $ruleset->getRules(); foreach ($rules as $rule) { if ($rule instanceof FixableRuleInterface) { - $rule->fixFile($stream, $this, $ignoredViolations); + $rule->fixFile($stream, $this); } } diff --git a/src/Runner/Linter.php b/src/Runner/Linter.php index 3663182d..8ea219de 100644 --- a/src/Runner/Linter.php +++ b/src/Runner/Linter.php @@ -102,7 +102,7 @@ public function run(iterable $files, Ruleset $ruleset, ?FixerInterface $fixer = $this->setErrorHandler($report, $filePath); try { $twigSource = new Source($content, $filePath); - [$stream, $ignoredViolations] = $this->tokenizer->tokenize($twigSource); + $stream = $this->tokenizer->tokenize($twigSource); } catch (CannotTokenizeException $exception) { $violation = new Violation( Violation::LEVEL_FATAL, @@ -116,7 +116,7 @@ public function run(iterable $files, Ruleset $ruleset, ?FixerInterface $fixer = restore_error_handler(); foreach ($rules as $rule) { - $rule->lintFile($stream, $report, $ignoredViolations); + $rule->lintFile($stream, $report); } if ([] !== $nodeVisitorRules) { @@ -126,7 +126,7 @@ public function run(iterable $files, Ruleset $ruleset, ?FixerInterface $fixer = } foreach ($nodeVisitorRules as $nodeVisitor) { - $nodeVisitor->setReport($report, $ignoredViolations); + $nodeVisitor->setReport($report, $stream->getIgnoredViolations()); } $traverser->traverse($node); diff --git a/src/Token/Tokenizer.php b/src/Token/Tokenizer.php index 932fdd4f..69e048a3 100644 --- a/src/Token/Tokenizer.php +++ b/src/Token/Tokenizer.php @@ -60,11 +60,6 @@ final class Tokenizer implements TokenizerInterface private ?Token $lastNonEmptyToken = null; - /** - * @var list - */ - private array $ignoredViolations = []; - /** * @var list */ @@ -94,11 +89,9 @@ public function __construct(Environment $env) } /** - * @return array{Tokens, list} - * * @throws CannotTokenizeException */ - public function tokenize(Source $source): array + public function tokenize(Source $source): Tokens { $this->resetState($source); $this->pushState(self::STATE_DATA); @@ -160,7 +153,9 @@ public function tokenize(Source $source): array $this->pushToken(Token::EOF_TYPE); - return [$this->tokens, $this->ignoredViolations]; + $this->tokens->setReadOnly(); + + return $this->tokens; } private function resetState(Source $source): void @@ -764,7 +759,7 @@ private function processIgnoredViolations(): void }; if ('' === $ignoredViolations) { - $this->ignoredViolations[] = ViolationId::fromString($ignoredViolations, $line); + $this->tokens->addIgnoredViolation(ViolationId::fromString($ignoredViolations, $line)); return; } @@ -774,7 +769,7 @@ private function processIgnoredViolations(): void if ('' === $ignoredViolation) { continue; } - $this->ignoredViolations[] = ViolationId::fromString($ignoredViolation, $line); + $this->tokens->addIgnoredViolation(ViolationId::fromString($ignoredViolation, $line)); } } } diff --git a/src/Token/TokenizerInterface.php b/src/Token/TokenizerInterface.php index 78b79350..857aec1a 100644 --- a/src/Token/TokenizerInterface.php +++ b/src/Token/TokenizerInterface.php @@ -6,14 +6,11 @@ use Twig\Source; use TwigCsFixer\Exception\CannotTokenizeException; -use TwigCsFixer\Report\ViolationId; interface TokenizerInterface { /** - * @return array{Tokens, list} - * * @throws CannotTokenizeException */ - public function tokenize(Source $source): array; + public function tokenize(Source $source): Tokens; } diff --git a/src/Token/Tokens.php b/src/Token/Tokens.php index b6246459..17ddfa5d 100644 --- a/src/Token/Tokens.php +++ b/src/Token/Tokens.php @@ -4,6 +4,8 @@ namespace TwigCsFixer\Token; +use TwigCsFixer\Report\ViolationId; + final class Tokens { /** @@ -21,6 +23,13 @@ final class Tokens */ private array $indexes = []; + /** + * @var list + */ + private array $ignoredViolations = []; + + private bool $readOnly = false; + /** * @param array $tokens */ @@ -31,8 +40,24 @@ public function __construct(array $tokens = []) } } + public function setReadOnly(): self + { + $this->readOnly = true; + + return $this; + } + + public function isReadOnly(): bool + { + return $this->readOnly; + } + public function add(Token $token): self { + if ($this->readOnly) { + throw new \LogicException('Cannot add token because the tokens are in read-only mode.'); + } + $this->tokens[] = $token; $this->indexes[spl_object_id($token)] = $this->tokenCount; ++$this->tokenCount; @@ -100,4 +125,23 @@ public function findPrevious(int|string|array $type, int $start, int $end = 0, b return false; } + + public function addIgnoredViolation(ViolationId $violationId): self + { + if ($this->readOnly) { + throw new \LogicException('Cannot add ignored violation because the tokens are in read-only mode.'); + } + + $this->ignoredViolations[] = $violationId; + + return $this; + } + + /** + * @return list + */ + public function getIgnoredViolations(): array + { + return $this->ignoredViolations; + } } diff --git a/tests/Runner/FixerTest.php b/tests/Runner/FixerTest.php index 7edc17f4..cfe9ac4b 100644 --- a/tests/Runner/FixerTest.php +++ b/tests/Runner/FixerTest.php @@ -36,12 +36,11 @@ public function testInvalidFile(): void public function testValidFile(): void { $tokenizer = $this->createMock(TokenizerInterface::class); - $tokenizer->expects(static::once())->method('tokenize')->willReturn([ + $tokenizer->expects(static::once())->method('tokenize')->willReturn( new Tokens([ new Token(Token::EOF_TYPE, 0, 0, 'TwigCsFixer'), ]), - [], - ]); + ); $ruleset = new Ruleset(); $fixer = new Fixer($tokenizer); diff --git a/tests/Runner/LinterTest.php b/tests/Runner/LinterTest.php index e6f95018..be751b08 100644 --- a/tests/Runner/LinterTest.php +++ b/tests/Runner/LinterTest.php @@ -72,14 +72,14 @@ public function testUntokenizableFilesAreReported(): void $call = 0; $tokenizer->method('tokenize')->willReturnCallback( - static function () use (&$call): array { + static function () use (&$call): Tokens { /** @psalm-suppress RedundantCondition https://github.com/vimeo/psalm/issues/10513 */ if (0 === $call) { ++$call; throw CannotTokenizeException::unknownError(); } - return [[], []]; + return new Tokens(); } ); $ruleset = new Ruleset(); @@ -117,11 +117,11 @@ public function testUserDeprecationAreReported(): void $env = new StubbedEnvironment(); $tokenizer = self::createStub(TokenizerInterface::class); - $tokenizer->method('tokenize')->willReturnCallback(static function (): array { + $tokenizer->method('tokenize')->willReturnCallback(static function (): Tokens { @trigger_error('Default'); trigger_error('User Deprecation', \E_USER_DEPRECATED); - return [[], []]; + return new Tokens(); }); $ruleset = new Ruleset(); @@ -330,7 +330,7 @@ public function testNodeVisitorWithInvalidFiles(): void $env = self::createStub(Environment::class); $env->method('tokenize')->willThrowException(new SyntaxError('Error.')); $tokenizer = self::createStub(TokenizerInterface::class); - $tokenizer->method('tokenize')->willReturn([new Tokens(), []]); + $tokenizer->method('tokenize')->willReturn(new Tokens()); $linter = new Linter($env, $tokenizer); $report = $linter->run([new \SplFileInfo($filePath), new \SplFileInfo($filePath2)], $ruleset); @@ -360,7 +360,7 @@ public function testNodeVisitorWithBuggyFixer(): void $env = new StubbedEnvironment(); $tokenizer = static::createStub(TokenizerInterface::class); - $tokenizer->method('tokenize')->willReturn([new Tokens(), []]); + $tokenizer->method('tokenize')->willReturn(new Tokens()); $linter = new Linter($env, $tokenizer); $fixer = static::createStub(FixerInterface::class); diff --git a/tests/Token/Tokenizer/TokenizerTest.php b/tests/Token/Tokenizer/TokenizerTest.php index bf360630..d2edf51c 100644 --- a/tests/Token/Tokenizer/TokenizerTest.php +++ b/tests/Token/Tokenizer/TokenizerTest.php @@ -33,9 +33,10 @@ public function testTokenize(): void new Token(Token::EOL_TYPE, 1, 16, $filePath, \PHP_EOL), new Token(Token::EOF_TYPE, 2, 1, $filePath), ], - $result[0]->toArray() + $result->toArray() ); - static::assertEquals([], $result[1]); + static::assertEquals([], $result->getIgnoredViolations()); + static::assertTrue($result->isReadOnly()); } public function testTokenizeMixedEOL(): void @@ -55,9 +56,10 @@ public function testTokenizeMixedEOL(): void new Token(Token::EOL_TYPE, 4, 1, 'path', "\n"), new Token(Token::EOF_TYPE, 5, 1, 'path'), ], - $result[0]->toArray() + $result->toArray() ); - static::assertEquals([], $result[1]); + static::assertEquals([], $result->getIgnoredViolations()); + static::assertTrue($result->isReadOnly()); } public function testTokenizeWithCustomOperators(): void @@ -70,6 +72,7 @@ public function testTokenizeWithCustomOperators(): void $tokenizer = new Tokenizer($env); $source = new Source($content, $filePath); + $result = $tokenizer->tokenize($source); static::assertEquals( [ new Token(Token::VAR_START_TYPE, 1, 1, $filePath, '{{'), @@ -89,8 +92,9 @@ public function testTokenizeWithCustomOperators(): void new Token(Token::EOL_TYPE, 1, 36, $filePath, \PHP_EOL), new Token(Token::EOF_TYPE, 2, 1, $filePath), ], - $tokenizer->tokenize($source)[0]->toArray() + $result->toArray() ); + static::assertTrue($result->isReadOnly()); } public function testTokenizeIgnoredViolations(): void @@ -103,6 +107,7 @@ public function testTokenizeIgnoredViolations(): void $tokenizer = new Tokenizer($env); $source = new Source($content, $filePath); + $result = $tokenizer->tokenize($source); static::assertEquals( [ 'Foo.Bar', @@ -114,9 +119,10 @@ public function testTokenizeIgnoredViolations(): void ], array_map( static fn (ViolationId $validationId) => $validationId->toString(), - $tokenizer->tokenize($source)[1] + $result->getIgnoredViolations() ) ); + static::assertTrue($result->isReadOnly()); } /** @@ -133,7 +139,7 @@ public function testTokenizeTypes(string $filePath, array $expectedTokenTypes): $tokenizer = new Tokenizer($env); $source = new Source($content, $filePath); - $tokens = $tokenizer->tokenize($source)[0]; + $tokens = $tokenizer->tokenize($source); $tokenValues = array_map(static fn (Token $token): string => $token->getValue(), $tokens->toArray()); @@ -144,6 +150,7 @@ public function testTokenizeTypes(string $filePath, array $expectedTokenTypes): $tokenTypes = array_map(static fn (Token $token): int|string => $token->getType(), $tokens->toArray()); static::assertSame($expectedTokenTypes, $tokenTypes); + static::assertTrue($tokens->isReadOnly()); } /** diff --git a/tests/Token/TokensTest.php b/tests/Token/TokensTest.php index 522d5bee..7b7b771b 100644 --- a/tests/Token/TokensTest.php +++ b/tests/Token/TokensTest.php @@ -5,6 +5,7 @@ namespace TwigCsFixer\Tests\Token; use PHPUnit\Framework\TestCase; +use TwigCsFixer\Report\ViolationId; use TwigCsFixer\Token\Token; use TwigCsFixer\Token\Tokens; @@ -23,6 +24,32 @@ public function testTokens(): void static::assertSame($token2, $tokens->get(1)); static::assertSame(0, $tokens->getIndex($token1)); static::assertSame(1, $tokens->getIndex($token2)); + + static::assertFalse($tokens->isReadOnly()); + $tokens->setReadOnly(); + static::assertTrue($tokens->isReadOnly()); + } + + public function testAddToken(): void + { + $tokens = new Tokens(); + $tokens->add(new Token(Token::TEXT_TYPE, 1, 1, 'file.twig')); + $tokens->setReadOnly(); + + $this->expectException(\LogicException::class); + + $tokens->add(new Token(Token::TEXT_TYPE, 1, 1, 'file.twig')); + } + + public function testAddIgnoredViolation(): void + { + $tokens = new Tokens(); + $tokens->addIgnoredViolation(new ViolationId()); + $tokens->setReadOnly(); + + $this->expectException(\LogicException::class); + + $tokens->addIgnoredViolation(new ViolationId()); } public function testInvalidGet(): void