From a56013d00be4eaf5c2f3bf1b1e71cb550b797bdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9goire=20Paris?= Date: Mon, 28 Nov 2022 22:49:39 +0100 Subject: [PATCH] Replace object-like array with class The new class is templatable, which should enable us to specify that the ORM Lexer is an Lexer, and in the future, define an enum called TokenType in the ORM, and switch to Lexer. --- UPGRADE.md | 14 ++ composer.json | 3 +- docs/en/dql-parser.rst | 2 +- docs/en/simple-parser-example.rst | 4 +- lib/Doctrine/Common/Lexer/AbstractLexer.php | 42 +++--- lib/Doctrine/Common/Lexer/Token.php | 124 ++++++++++++++++++ .../Common/Lexer/AbstractLexerTest.php | 100 ++++---------- tests/Doctrine/Common/Lexer/ConcreteLexer.php | 1 + tests/Doctrine/Common/Lexer/MutableLexer.php | 1 + tests/Doctrine/Common/Lexer/TokenTest.php | 53 ++++++++ 10 files changed, 241 insertions(+), 103 deletions(-) create mode 100644 UPGRADE.md create mode 100644 lib/Doctrine/Common/Lexer/Token.php create mode 100644 tests/Doctrine/Common/Lexer/TokenTest.php diff --git a/UPGRADE.md b/UPGRADE.md new file mode 100644 index 0000000..42b85b3 --- /dev/null +++ b/UPGRADE.md @@ -0,0 +1,14 @@ +Note about upgrading: Doctrine uses static and runtime mechanisms to raise +awareness about deprecated code. + +- Use of `@deprecated` docblock that is detected by IDEs (like PHPStorm) or + Static Analysis tools (like Psalm, phpstan) +- Use of our low-overhead runtime deprecation API, details: + https://github.com/doctrine/deprecations/ + +# Upgrade to 2.0.0 + +`AbstractLexer::glimpse()` and `AbstractLexer::peek()` now return +instances of `Doctrine\Common\Lexer\Token`, which is an array-like class +Using it as an array is deprecated in favor of using properties of that class. +Using `count()` on it is deprecated with no replacement. diff --git a/composer.json b/composer.json index b8b9052..919c9d6 100644 --- a/composer.json +++ b/composer.json @@ -26,7 +26,8 @@ ], "homepage": "https://www.doctrine-project.org/projects/lexer.html", "require": { - "php": "^7.1 || ^8.0" + "php": "^7.1 || ^8.0", + "doctrine/deprecations": "^1.0" }, "require-dev": { "doctrine/coding-standard": "^9 || ^10", diff --git a/docs/en/dql-parser.rst b/docs/en/dql-parser.rst index cc29603..82fc429 100644 --- a/docs/en/dql-parser.rst +++ b/docs/en/dql-parser.rst @@ -254,7 +254,7 @@ Lexer implementation: { $this->lexer->moveNext(); - switch ($this->lexer->lookahead['type']) { + switch ($this->lexer->lookahead->type) { case Lexer::T_SELECT: $statement = $this->SelectStatement(); break; diff --git a/docs/en/simple-parser-example.rst b/docs/en/simple-parser-example.rst index bd4d7ab..23becdb 100644 --- a/docs/en/simple-parser-example.rst +++ b/docs/en/simple-parser-example.rst @@ -70,8 +70,8 @@ Use ``CharacterTypeLexer`` to extract an array of upper case characters: $this->lexer->moveNext(); - if ($this->lexer->token['type'] === CharacterTypeLexer::T_UPPER) { - $upperCaseChars[] = $this->lexer->token['value']; + if ($this->lexer->token->isA(CharacterTypeLexer::T_UPPER)) { + $upperCaseChars[] = $this->lexer->token->value; } } diff --git a/lib/Doctrine/Common/Lexer/AbstractLexer.php b/lib/Doctrine/Common/Lexer/AbstractLexer.php index 7e8a11d..44ea16b 100644 --- a/lib/Doctrine/Common/Lexer/AbstractLexer.php +++ b/lib/Doctrine/Common/Lexer/AbstractLexer.php @@ -7,7 +7,6 @@ use ReflectionClass; use function implode; -use function in_array; use function preg_split; use function sprintf; use function substr; @@ -19,7 +18,7 @@ /** * Base class for writing simple lexers, i.e. for creating small DSLs. * - * @psalm-type Token = array{value: int|string, type:string|int|null, position:int} + * @template T of string|int */ abstract class AbstractLexer { @@ -33,14 +32,7 @@ abstract class AbstractLexer /** * Array of scanned tokens. * - * Each token is an associative array containing three items: - * - 'value' : the string value of the token in the input string - * - 'type' : the type of the token (identifier, numeric, string, input - * parameter, none) - * - 'position' : the position of the token in the input string - * - * @var mixed[][] - * @psalm-var list + * @var list> */ private $tokens = []; @@ -62,7 +54,7 @@ abstract class AbstractLexer * The next token in the input. * * @var mixed[]|null - * @psalm-var Token|null + * @psalm-var Token|null */ public $lookahead; @@ -70,7 +62,7 @@ abstract class AbstractLexer * The last matched/seen token. * * @var mixed[]|null - * @psalm-var Token|null + * @psalm-var Token|null */ public $token; @@ -150,25 +142,25 @@ public function getInputUntilPosition($position) /** * Checks whether a given token matches the current lookahead. * - * @param int|string $type + * @param T $type * * @return bool */ public function isNextToken($type) { - return $this->lookahead !== null && $this->lookahead['type'] === $type; + return $this->lookahead !== null && $this->lookahead->isA($type); } /** * Checks whether any of the given tokens matches the current lookahead. * - * @param list $types + * @param list $types * * @return bool */ public function isNextTokenAny(array $types) { - return $this->lookahead !== null && in_array($this->lookahead['type'], $types, true); + return $this->lookahead !== null && $this->lookahead->isA(...$types); } /** @@ -195,7 +187,7 @@ public function moveNext() */ public function skipUntil($type) { - while ($this->lookahead !== null && $this->lookahead['type'] !== $type) { + while ($this->lookahead !== null && ! $this->lookahead->isA($type)) { $this->moveNext(); } } @@ -217,7 +209,7 @@ public function isA($value, $token) * Moves the lookahead token forward. * * @return mixed[]|null The next token or NULL if there are no more tokens ahead. - * @psalm-return Token|null + * @psalm-return Token|null */ public function peek() { @@ -232,7 +224,7 @@ public function peek() * Peeks at the next token, returns it and immediately resets the peek. * * @return mixed[]|null The next token or NULL if there are no more tokens ahead. - * @psalm-return Token|null + * @psalm-return Token|null */ public function glimpse() { @@ -272,11 +264,11 @@ protected function scan($input) // Must remain before 'value' assignment since it can change content $type = $this->getType($match[0]); - $this->tokens[] = [ - 'value' => $match[0], - 'type' => $type, - 'position' => $match[1], - ]; + $this->tokens[] = new Token( + $match[0], + $type, + $match[1] + ); } } @@ -331,7 +323,7 @@ abstract protected function getNonCatchablePatterns(); * * @param string $value * - * @return int|string|null + * @return T|null */ abstract protected function getType(&$value); } diff --git a/lib/Doctrine/Common/Lexer/Token.php b/lib/Doctrine/Common/Lexer/Token.php new file mode 100644 index 0000000..101b34d --- /dev/null +++ b/lib/Doctrine/Common/Lexer/Token.php @@ -0,0 +1,124 @@ + + */ +final class Token implements ArrayAccess +{ + /** + * The string value of the token in the input string + * + * @readonly + * @var string|int + */ + public $value; + + /** + * The type of the token (identifier, numeric, string, input parameter, none) + * + * @readonly + * @var T|null + */ + public $type; + + /** + * The position of the token in the input string + * + * @readonly + * @var int + */ + public $position; + + /** + * @param string|int $value + * @param T|null $type + */ + public function __construct($value, $type, int $position) + { + $this->value = $value; + $this->type = $type; + $this->position = $position; + } + + /** @param T ...$types */ + public function isA(...$types): bool + { + return in_array($this->type, $types, true); + } + + /** + * @deprecated Use the value, type or position property instead + * {@inheritDoc} + */ + public function offsetExists($offset): bool + { + Deprecation::trigger( + 'doctrine/lexer', + 'https://github.com/doctrine/lexer/pull/79', + 'Accessing %s properties via ArrayAccess is deprecated, use the value, type or position property instead', + self::class + ); + + return in_array($offset, ['value', 'type', 'position'], true); + } + + /** + * @deprecated Use the value, type or position property instead + * {@inheritDoc} + */ + #[ReturnTypeWillChange] + public function offsetGet($offset) + { + Deprecation::trigger( + 'doctrine/lexer', + 'https://github.com/doctrine/lexer/pull/79', + 'Accessing %s properties via ArrayAccess is deprecated, use the value, type or position property instead', + self::class + ); + + return $this->$offset; + } + + /** + * @deprecated no replacement planned + * {@inheritDoc} + */ + public function offsetSet($offset, $value): void + { + Deprecation::trigger( + 'doctrine/lexer', + 'https://github.com/doctrine/lexer/pull/79', + 'Setting %s properties via ArrayAccess is deprecated', + self::class + ); + + $this->$offset = $value; + } + + /** + * @deprecated no replacement planned + * {@inheritDoc} + */ + public function offsetUnset($offset): void + { + Deprecation::trigger( + 'doctrine/lexer', + 'https://github.com/doctrine/lexer/pull/79', + 'Setting %s properties via ArrayAccess is deprecated', + self::class + ); + + $this->$offset = null; + } +} diff --git a/tests/Doctrine/Common/Lexer/AbstractLexerTest.php b/tests/Doctrine/Common/Lexer/AbstractLexerTest.php index 803a42e..2a84391 100644 --- a/tests/Doctrine/Common/Lexer/AbstractLexerTest.php +++ b/tests/Doctrine/Common/Lexer/AbstractLexerTest.php @@ -4,6 +4,7 @@ namespace Doctrine\Tests\Common\Lexer; +use Doctrine\Common\Lexer\Token; use PHPUnit\Framework\TestCase; use function array_map; @@ -12,9 +13,6 @@ use const LC_ALL; -/** - * @psalm-type ValidToken = array{value: string|int, type: string|int, position: int} - */ class AbstractLexerTest extends TestCase { /** @var ConcreteLexer */ @@ -31,7 +29,7 @@ public function tearDown(): void } /** - * @psalm-return list}> + * @psalm-return list>}> */ public function dataProvider(): array { @@ -39,21 +37,9 @@ public function dataProvider(): array [ 'price=10', [ - [ - 'value' => 'price', - 'type' => 'string', - 'position' => 0, - ], - [ - 'value' => '=', - 'type' => 'operator', - 'position' => 5, - ], - [ - 'value' => 10, - 'type' => 'int', - 'position' => 6, - ], + new Token('price', 'string', 0), + new Token('=', 'operator', 5), + new Token(10, 'int', 6), ], ], ]; @@ -62,21 +48,9 @@ public function dataProvider(): array public function testResetPeek(): void { $expectedTokens = [ - [ - 'value' => 'price', - 'type' => 'string', - 'position' => 0, - ], - [ - 'value' => '=', - 'type' => 'operator', - 'position' => 5, - ], - [ - 'value' => 10, - 'type' => 'int', - 'position' => 6, - ], + new Token('price', 'string', 0), + new Token('=', 'operator', 5), + new Token(10, 'int', 6), ]; $this->concreteLexer->setInput('price=10'); @@ -90,21 +64,9 @@ public function testResetPeek(): void public function testResetPosition(): void { $expectedTokens = [ - [ - 'value' => 'price', - 'type' => 'string', - 'position' => 0, - ], - [ - 'value' => '=', - 'type' => 'operator', - 'position' => 5, - ], - [ - 'value' => 10, - 'type' => 'int', - 'position' => 6, - ], + new Token('price', 'string', 0), + new Token('=', 'operator', 5), + new Token(10, 'int', 6), ]; $this->concreteLexer->setInput('price=10'); @@ -123,7 +85,7 @@ public function testResetPosition(): void } /** - * @psalm-param list $expectedTokens + * @psalm-param list> $expectedTokens * * @dataProvider dataProvider */ @@ -149,11 +111,7 @@ public function testSkipUntil(): void $this->concreteLexer->skipUntil('operator'); $this->assertEquals( - [ - 'value' => '=', - 'type' => 'operator', - 'position' => 5, - ], + new Token('=', 'operator', 5), $this->concreteLexer->lookahead ); } @@ -165,17 +123,13 @@ public function testUtf8Mismatch(): void $this->assertTrue($this->concreteLexer->moveNext()); $this->assertEquals( - [ - 'value' => "\xE9=10", - 'type' => 'string', - 'position' => 0, - ], + new Token("\xE9=10", 'string', 0), $this->concreteLexer->lookahead ); } /** - * @psalm-param list $expectedTokens + * @psalm-param list> $expectedTokens * * @dataProvider dataProvider */ @@ -188,14 +142,13 @@ public function testPeek(string $input, array $expectedTokens): void $this->assertSame($expectedToken['value'], $actualToken['value']); $this->assertSame($expectedToken['type'], $actualToken['type']); $this->assertSame($expectedToken['position'], $actualToken['position']); - $this->assertCount(3, $actualToken); } $this->assertNull($this->concreteLexer->peek()); } /** - * @psalm-param list $expectedTokens + * @psalm-param list> $expectedTokens * * @dataProvider dataProvider */ @@ -209,7 +162,6 @@ public function testGlimpse(string $input, array $expectedTokens): void $this->assertSame($expectedToken['value'], $actualToken['value']); $this->assertSame($expectedToken['type'], $actualToken['type']); $this->assertSame($expectedToken['position'], $actualToken['position']); - $this->assertCount(3, $actualToken); $this->concreteLexer->moveNext(); } @@ -240,7 +192,7 @@ public function testGetInputUntilPosition( } /** - * @psalm-param list $expectedTokens + * @psalm-param list> $expectedTokens * * @dataProvider dataProvider */ @@ -250,27 +202,27 @@ public function testIsNextToken(string $input, array $expectedTokens): void $this->concreteLexer->moveNext(); for ($i = 0; $i < count($expectedTokens); $i++) { - $this->assertTrue($this->concreteLexer->isNextToken($expectedTokens[$i]['type'])); + $this->assertTrue($this->concreteLexer->isNextToken($expectedTokens[$i]->type)); $this->concreteLexer->moveNext(); } } /** - * @psalm-param list $expectedTokens + * @psalm-param list> $expectedTokens * * @dataProvider dataProvider */ public function testIsNextTokenAny(string $input, array $expectedTokens): void { $allTokenTypes = array_map(static function ($token) { - return $token['type']; + return $token->type; }, $expectedTokens); $this->concreteLexer->setInput($input); $this->concreteLexer->moveNext(); for ($i = 0; $i < count($expectedTokens); $i++) { - $this->assertTrue($this->concreteLexer->isNextTokenAny([$expectedTokens[$i]['type']])); + $this->assertTrue($this->concreteLexer->isNextTokenAny([$expectedTokens[$i]->type])); $this->assertTrue($this->concreteLexer->isNextTokenAny($allTokenTypes)); $this->concreteLexer->moveNext(); } @@ -299,14 +251,14 @@ public function testAddCatchablePatternsToMutableLexer(): void $mutableLexer->setInput('one'); $token = $mutableLexer->glimpse(); - $this->assertEquals('o', $token['value']); + $this->assertEquals('o', $token->value); $mutableLexer = new MutableLexer(); $mutableLexer->addCatchablePattern('[a-z]+'); $mutableLexer->setInput('one'); $token = $mutableLexer->glimpse(); - $this->assertEquals('one', $token['value']); + $this->assertEquals('one', $token->value); } public function testMarkerAnnotationLocaleTr(): void @@ -323,10 +275,10 @@ public function testMarkerAnnotationLocaleTr(): void self::assertTrue($mutableLexer->moveNext()); self::assertNull($mutableLexer->token); self::assertNotNull($mutableLexer->lookahead); - self::assertEquals('@', $mutableLexer->lookahead['value']); + self::assertEquals('@', $mutableLexer->lookahead->value); self::assertTrue($mutableLexer->moveNext()); self::assertNotNull($mutableLexer->token); - self::assertEquals('@', $mutableLexer->token['value']); - self::assertEquals('ODM\Id', $mutableLexer->lookahead['value']); + self::assertEquals('@', $mutableLexer->token->value); + self::assertEquals('ODM\Id', $mutableLexer->lookahead->value); } } diff --git a/tests/Doctrine/Common/Lexer/ConcreteLexer.php b/tests/Doctrine/Common/Lexer/ConcreteLexer.php index 88a0182..b3464e1 100644 --- a/tests/Doctrine/Common/Lexer/ConcreteLexer.php +++ b/tests/Doctrine/Common/Lexer/ConcreteLexer.php @@ -10,6 +10,7 @@ use function is_numeric; use function is_string; +/** @extends AbstractLexer */ class ConcreteLexer extends AbstractLexer { public const INT = 'int'; diff --git a/tests/Doctrine/Common/Lexer/MutableLexer.php b/tests/Doctrine/Common/Lexer/MutableLexer.php index 2132034..66038b5 100644 --- a/tests/Doctrine/Common/Lexer/MutableLexer.php +++ b/tests/Doctrine/Common/Lexer/MutableLexer.php @@ -6,6 +6,7 @@ use Doctrine\Common\Lexer\AbstractLexer; +/** @extends AbstractLexer */ class MutableLexer extends AbstractLexer { /** @var string[] */ diff --git a/tests/Doctrine/Common/Lexer/TokenTest.php b/tests/Doctrine/Common/Lexer/TokenTest.php new file mode 100644 index 0000000..6bc285d --- /dev/null +++ b/tests/Doctrine/Common/Lexer/TokenTest.php @@ -0,0 +1,53 @@ +isA('string')); + self::assertTrue($token->isA('int', 'string')); + self::assertFalse($token->isA('int')); + } + + public function testOffsetGetIsDeprecated(): void + { + $token = new Token('foo', 'string', 1); + self::expectDeprecationWithIdentifier('https://github.com/doctrine/lexer/pull/79'); + self::assertSame('foo', $token['value']); + } + + public function testOffsetExistsIsDeprecated(): void + { + $token = new Token('foo', 'string', 1); + self::expectDeprecationWithIdentifier('https://github.com/doctrine/lexer/pull/79'); + self::assertTrue(isset($token['value'])); + } + + public function testOffsetSetIsDeprecated(): void + { + $token = new Token('foo', 'string', 1); + self::expectDeprecationWithIdentifier('https://github.com/doctrine/lexer/pull/79'); + $token['value'] = 'bar'; + self::assertSame('bar', $token->value); + } + + public function testOffsetUnsetIsDeprecated(): void + { + $token = new Token('foo', 'string', 1); + self::expectDeprecationWithIdentifier('https://github.com/doctrine/lexer/pull/79'); + unset($token['value']); + self::assertNull($token->value); + } +}