From 8b37c13b36a1eaa9fe0f9246065ebbcfe0d065a1 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 31 Aug 2023 00:07:43 +0100 Subject: [PATCH 1/4] Remove $length from TokenList constructor Signed-off-by: Kamil Tekiela --- src/Statements/WithStatement.php | 2 +- src/TokensList.php | 19 +++---------------- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/src/Statements/WithStatement.php b/src/Statements/WithStatement.php index 4b0469d73..e94b94ebb 100644 --- a/src/Statements/WithStatement.php +++ b/src/Statements/WithStatement.php @@ -330,6 +330,6 @@ private function getSubTokenList(TokensList $list): ParserException|TokensList $length = $list->idx - $idx; - return new TokensList(array_slice($list->tokens, $idx, $length), $length); + return new TokensList(array_slice($list->tokens, $idx, $length)); } } diff --git a/src/TokensList.php b/src/TokensList.php index d669bda4c..d6b5a6af3 100644 --- a/src/TokensList.php +++ b/src/TokensList.php @@ -20,13 +20,6 @@ */ class TokensList implements ArrayAccess { - /** - * The array of tokens. - * - * @var Token[] - */ - public $tokens = []; - /** * The count of tokens. * @@ -42,17 +35,11 @@ class TokensList implements ArrayAccess public $idx = 0; /** - * @param Token[] $tokens the initial array of tokens - * @param int $count the count of tokens in the initial array + * @param Token[] $tokens The array of tokens. */ - public function __construct(array $tokens = [], $count = -1) + public function __construct(public array $tokens = []) { - if ($tokens === []) { - return; - } - - $this->tokens = $tokens; - $this->count = $count === -1 ? count($tokens) : $count; + $this->count = count($tokens); } /** From 49e122ff51c427fdd8b386d8b4a7cbac12f6a26c Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Wed, 30 Aug 2023 23:48:18 +0100 Subject: [PATCH 2/4] Split up build() into a static and instance method Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 20 ---------------- psalm-baseline.xml | 34 --------------------------- src/Components/AlterOperation.php | 10 ++++---- src/Statements/CreateStatement.php | 12 +++++----- src/TokensList.php | 27 ++++++++++----------- src/Utils/Tokens.php | 8 +++---- tests/Builder/CreateStatementTest.php | 21 ++++++++++++++--- tests/Lexer/TokensListTest.php | 3 ++- 8 files changed, 45 insertions(+), 90 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index b16d399b2..c6dd99853 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,10 +1,5 @@ parameters: ignoreErrors: - - - message: "#^Cannot assign new offset to array\\\\|string\\.$#" - count: 3 - path: src/Components/AlterOperation.php - - message: "#^Property PhpMyAdmin\\\\SqlParser\\\\Components\\\\AlterOperation\\:\\:\\$options \\(PhpMyAdmin\\\\SqlParser\\\\Components\\\\OptionsArray\\) does not accept PhpMyAdmin\\\\SqlParser\\\\Components\\\\OptionsArray\\|null\\.$#" count: 1 @@ -15,11 +10,6 @@ parameters: count: 1 path: src/Components/AlterOperation.php - - - message: "#^array\\\\|string does not accept PhpMyAdmin\\\\SqlParser\\\\Token\\.$#" - count: 3 - path: src/Components/AlterOperation.php - - message: "#^Cannot access an offset on array\\, mixed\\>\\|static\\(PhpMyAdmin\\\\SqlParser\\\\Components\\\\ArrayObj\\)\\.$#" count: 1 @@ -500,11 +490,6 @@ parameters: count: 2 path: src/Statements/CreateStatement.php - - - message: "#^Cannot assign new offset to array\\\\|string\\.$#" - count: 4 - path: src/Statements/CreateStatement.php - - message: "#^Cannot call method build\\(\\) on PhpMyAdmin\\\\SqlParser\\\\Components\\\\DataType\\|null\\.$#" count: 1 @@ -550,11 +535,6 @@ parameters: count: 1 path: src/Statements/CreateStatement.php - - - message: "#^array\\\\|string does not accept PhpMyAdmin\\\\SqlParser\\\\Token\\.$#" - count: 4 - path: src/Statements/CreateStatement.php - - message: "#^Argument of an invalid type array\\\\|null supplied for foreach, only iterables are supported\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index f56d26a55..3a4651799 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1060,11 +1060,6 @@ withers]]> - - - is_array($list) - - @@ -1329,41 +1324,12 @@ $find[$k] - - tokens[$j]]]> - - - $newList - $newList - - - tokens[$i]]]> - tokens[$j]]]> - tokens[$j]]]> - - - $newList[] - $newList[] - - - tokens[$i]->type]]> - tokens[$j]->type]]> - - $list - - count]]> - tokens]]> - - - count]]> - tokens]]> - diff --git a/src/Components/AlterOperation.php b/src/Components/AlterOperation.php index e0d92e406..4b74f74f0 100644 --- a/src/Components/AlterOperation.php +++ b/src/Components/AlterOperation.php @@ -258,10 +258,8 @@ final class AlterOperation implements Component /** * Unparsed tokens. - * - * @var Token[]|string */ - public $unknown = []; + public TokensList $unknown; /** * @param OptionsArray $options options of alter operation @@ -273,12 +271,12 @@ public function __construct( $options = null, $field = null, $partitions = null, - $unknown = [] + array $unknown = [] ) { $this->partitions = $partitions; $this->options = $options; $this->field = $field; - $this->unknown = $unknown; + $this->unknown = new TokensList($unknown); } /** @@ -525,7 +523,7 @@ public function build(): string $ret .= $this->field . ' '; } - $ret .= $afterFieldsOptions . TokensList::build($this->unknown); + $ret .= $afterFieldsOptions . $this->unknown->build(); if (isset($this->partitions)) { $ret .= PartitionDefinition::buildAll($this->partitions); diff --git a/src/Statements/CreateStatement.php b/src/Statements/CreateStatement.php index 24896bd7c..627d4b3e2 100644 --- a/src/Statements/CreateStatement.php +++ b/src/Statements/CreateStatement.php @@ -390,9 +390,9 @@ class CreateStatement extends Statement * For views, it is the select statement that creates the view. * Used by `CREATE FUNCTION`, `CREATE PROCEDURE` and `CREATE VIEW`. * - * @var Token[]|string + * @var Token[] */ - public $body = []; + public array $body = []; public function build(): string { @@ -476,7 +476,7 @@ public function build(): string . $this->options->build() . ' ' . $this->name->build() . ' ' . $fields . ' AS ' . $builtStatement - . (! empty($this->body) ? TokensList::build($this->body) : '') . ' ' + . TokensList::buildFromArray($this->body) . ' ' . ($this->entityOptions?->build() ?? ''); } @@ -486,7 +486,7 @@ public function build(): string . $this->name->build() . ' ' . $this->entityOptions->build() . ' ' . 'ON ' . $this->table->build() . ' ' - . 'FOR EACH ROW ' . TokensList::build($this->body); + . 'FOR EACH ROW ' . TokensList::buildFromArray($this->body); } if ($this->options->has('PROCEDURE') || $this->options->has('FUNCTION')) { @@ -500,13 +500,13 @@ public function build(): string . $this->name->build() . ' ' . ParameterDefinition::buildAll($this->parameters) . ' ' . $tmp . ' ' . $this->entityOptions->build() . ' ' - . TokensList::build($this->body); + . TokensList::buildFromArray($this->body); } return 'CREATE ' . $this->options->build() . ' ' . $this->name->build() . ' ' - . TokensList::build($this->body); + . TokensList::buildFromArray($this->body); } /** diff --git a/src/TokensList.php b/src/TokensList.php index d6b5a6af3..8086c9ca4 100644 --- a/src/TokensList.php +++ b/src/TokensList.php @@ -9,7 +9,6 @@ use function count; use function in_array; use function is_array; -use function is_string; /** * Defines an array of tokens and utility functions to iterate through it. @@ -44,24 +43,22 @@ public function __construct(public array $tokens = []) /** * Builds an array of tokens by merging their raw value. - * - * @param string|Token[]|TokensList $list the tokens to be built */ - public static function build($list): string + public function build(): string { - if (is_string($list)) { - return $list; - } - - if ($list instanceof self) { - $list = $list->tokens; - } + return static::buildFromArray($this->tokens); + } + /** + * Builds an array of tokens by merging their raw value. + * + * @param Token[] $list the tokens to be built + */ + public static function buildFromArray(array $list): string + { $ret = ''; - if (is_array($list)) { - foreach ($list as $tok) { - $ret .= $tok->token; - } + foreach ($list as $token) { + $ret .= $token->token; } return $ret; diff --git a/src/Utils/Tokens.php b/src/Utils/Tokens.php index c7f9c5cae..9c6d4c37d 100644 --- a/src/Utils/Tokens.php +++ b/src/Utils/Tokens.php @@ -64,16 +64,14 @@ public static function replaceTokens( $isList = $list instanceof TokensList; // Parsing the tokens. - if (! $isList) { + if (! $list instanceof TokensList) { $list = Lexer::getTokens($list); } /** * The list to be returned. - * - * @var Token[] */ - $newList = []; + $newList = new TokensList(); /** * The length of the find pattern is calculated only once. @@ -148,6 +146,6 @@ public static function replaceTokens( } } - return $isList ? new TokensList($newList) : TokensList::build($newList); + return $isList ? $newList : $newList->build(); } } diff --git a/tests/Builder/CreateStatementTest.php b/tests/Builder/CreateStatementTest.php index 59c63929f..ccb33e1b9 100644 --- a/tests/Builder/CreateStatementTest.php +++ b/tests/Builder/CreateStatementTest.php @@ -13,6 +13,7 @@ use PhpMyAdmin\SqlParser\Parser; use PhpMyAdmin\SqlParser\Statements\CreateStatement; use PhpMyAdmin\SqlParser\Tests\TestCase; +use PhpMyAdmin\SqlParser\Token; use PhpMyAdmin\SqlParser\TokensList; use PHPUnit\Framework\Attributes\DataProvider; @@ -524,7 +525,7 @@ public function testBuilderCreateProcedure(): void $this->assertSame( 'SELECT _var', - TokensList::build($stmt->body) + TokensList::buildFromArray($stmt->body) ); } @@ -652,7 +653,7 @@ public function testBuilderCreateFunction(): void . ' RETURN TRUE;' . "\n" . ' END IF;' . "\n" . 'END', - TokensList::build($stmt->body) + TokensList::buildFromArray($stmt->body) ); } @@ -664,7 +665,21 @@ public function testBuilderTrigger(): void $stmt->name = new Expression('ins_sum'); $stmt->entityOptions = new OptionsArray(['BEFORE', 'INSERT']); $stmt->table = new Expression('account'); - $stmt->body = 'SET @sum = @sum + NEW.amount'; + $stmt->body = [ + new Token('SET', Token::TYPE_KEYWORD), + new Token(' ', Token::TYPE_WHITESPACE), + new Token('@sum', Token::TYPE_NONE), + new Token(' ', Token::TYPE_WHITESPACE), + new Token('=', Token::TYPE_OPERATOR), + new Token(' ', Token::TYPE_WHITESPACE), + new Token('@sum', Token::TYPE_NONE), + new Token(' ', Token::TYPE_WHITESPACE), + new Token('+', Token::TYPE_OPERATOR), + new Token(' ', Token::TYPE_WHITESPACE), + new Token('NEW', Token::TYPE_KEYWORD), + new Token('.', Token::TYPE_OPERATOR), + new Token('amount', Token::TYPE_NONE), + ]; $this->assertEquals( 'CREATE TRIGGER ins_sum BEFORE INSERT ON account ' . diff --git a/tests/Lexer/TokensListTest.php b/tests/Lexer/TokensListTest.php index 4b3e44960..26c0276ee 100644 --- a/tests/Lexer/TokensListTest.php +++ b/tests/Lexer/TokensListTest.php @@ -45,7 +45,8 @@ public function setUp(): void public function testBuild(): void { $list = new TokensList($this->tokens); - $this->assertEquals('SELECT * FROM `test` WHERE name=fa', TokensList::build($list)); + $this->assertEquals('SELECT * FROM `test` WHERE name=fa', $list->build()); + $this->assertEquals('SELECT * FROM `test` WHERE name=fa', TokensList::buildFromArray($this->tokens)); } public function testAdd(): void From da494f7812e89fa82a8db40e87a48fb3cb5baa09 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 31 Aug 2023 00:37:52 +0100 Subject: [PATCH 3/4] Remove class Tokens It was not used anywhere in PMA project. It's very likely broken. It has serious feature envy on TokensList. The two methods had very unclear purpose and behaviour. Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 25 ------ psalm-baseline.xml | 20 ----- src/Lexer.php | 15 ---- src/Utils/Tokens.php | 151 ------------------------------------- tests/Utils/TokensTest.php | 121 ----------------------------- 5 files changed, 332 deletions(-) delete mode 100644 src/Utils/Tokens.php delete mode 100644 tests/Utils/TokensTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index c6dd99853..247df1f9b 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -810,26 +810,6 @@ parameters: count: 2 path: src/Utils/Table.php - - - message: "#^Binary operation \"&\" between int\\|string and int results in an error\\.$#" - count: 1 - path: src/Utils/Tokens.php - - - - message: "#^Cannot cast mixed to string\\.$#" - count: 1 - path: src/Utils/Tokens.php - - - - message: "#^Parameter \\#1 \\$string1 of function strcasecmp expects string, int\\|string given\\.$#" - count: 1 - path: src/Utils/Tokens.php - - - - message: "#^Parameter \\#2 \\$pattern of static method PhpMyAdmin\\\\SqlParser\\\\Utils\\\\Tokens\\:\\:match\\(\\) expects array\\, PhpMyAdmin\\\\SqlParser\\\\Token given\\.$#" - count: 1 - path: src/Utils/Tokens.php - - message: "#^Cannot call method __toString\\(\\) on PhpMyAdmin\\\\SqlParser\\\\Components\\\\Expression\\|null\\.$#" count: 2 @@ -940,11 +920,6 @@ parameters: count: 1 path: tests/Utils/TableTest.php - - - message: "#^Parameter \\#2 \\$find of static method PhpMyAdmin\\\\SqlParser\\\\Utils\\\\Tokens\\:\\:replaceTokens\\(\\) expects array\\, array\\\\> given\\.$#" - count: 1 - path: tests/Utils/TokensTest.php - - message: "#^Expression \"\\$str1\\[\\$i\\]\" on a separate line does not do anything\\.$#" count: 1 diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 3a4651799..274a9a277 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1320,17 +1320,6 @@ has - - - $find[$k] - - - - - - - - provideBuilderForRenameColumn @@ -1769,15 +1758,6 @@ getForeignKeysProvider - - - $find - - - matchProvider - replaceTokensProvider - - $testContents diff --git a/src/Lexer.php b/src/Lexer.php index f36f16e50..5048f15bb 100644 --- a/src/Lexer.php +++ b/src/Lexer.php @@ -144,21 +144,6 @@ class Lexer extends Core */ public $delimiterLen; - /** - * Gets the tokens list parsed by a new instance of a lexer. - * - * @param string|UtfString $str the query to be lexed - * @param bool $strict whether strict mode should be - * enabled or not - * @param string $delimiter the delimiter to be used - */ - public static function getTokens($str, $strict = false, $delimiter = null): TokensList - { - $lexer = new self($str, $strict, $delimiter); - - return $lexer->list; - } - /** * @param string|UtfString $str the query to be lexed * @param bool $strict whether strict mode should be diff --git a/src/Utils/Tokens.php b/src/Utils/Tokens.php deleted file mode 100644 index 9c6d4c37d..000000000 --- a/src/Utils/Tokens.php +++ /dev/null @@ -1,151 +0,0 @@ - $pattern the pattern to be matches - */ - public static function match(Token $token, array $pattern): bool - { - // Token. - if (isset($pattern['token']) && ($pattern['token'] !== $token->token)) { - return false; - } - - // Value. - if (isset($pattern['value']) && ($pattern['value'] !== $token->value)) { - return false; - } - - if (isset($pattern['value_str']) && strcasecmp($pattern['value_str'], (string) $token->value)) { - return false; - } - - // Type. - if (isset($pattern['type']) && ($pattern['type'] !== $token->type)) { - return false; - } - - // Flags. - return ! isset($pattern['flags']) - || (! (($pattern['flags'] & $token->flags) === 0)); - } - - /** - * @param Token[] $find - * @param Token[] $replace - */ - public static function replaceTokens( - TokensList|string|UtfString $list, - array $find, - array $replace - ): TokensList|string { - /** - * Whether the first parameter is a list. - */ - $isList = $list instanceof TokensList; - - // Parsing the tokens. - if (! $list instanceof TokensList) { - $list = Lexer::getTokens($list); - } - - /** - * The list to be returned. - */ - $newList = new TokensList(); - - /** - * The length of the find pattern is calculated only once. - * - * @var int - */ - $findCount = count($find); - - /** - * The starting index of the pattern. - * - * @var int - */ - $i = 0; - - while ($i < $list->count) { - // A sequence may not start with a comment. - if ($list->tokens[$i]->type === Token::TYPE_COMMENT) { - $newList[] = $list->tokens[$i]; - ++$i; - continue; - } - - /** - * The index used to parse `$list->tokens`. - * - * This index might be running faster than `$k` because some tokens - * are skipped. - */ - $j = $i; - - /** - * The index used to parse `$find`. - * - * This index might be running slower than `$j` because some tokens - * are skipped. - * - * @var int - */ - $k = 0; - - // Checking if the next tokens match the pattern described. - while (($j < $list->count) && ($k < $findCount)) { - // Comments are being skipped. - if ($list->tokens[$j]->type === Token::TYPE_COMMENT) { - ++$j; - } - - if (! static::match($list->tokens[$j], $find[$k])) { - // This token does not match the pattern. - break; - } - - // Going to next token and segment of find pattern. - ++$j; - ++$k; - } - - // Checking if the sequence was found. - if ($k === $findCount) { - // Inserting new tokens. - foreach ($replace as $token) { - $newList[] = $token; - } - - // Skipping next `$findCount` tokens. - $i = $j; - } else { - // Adding the same token. - $newList[] = $list->tokens[$i]; - ++$i; - } - } - - return $isList ? $newList : $newList->build(); - } -} diff --git a/tests/Utils/TokensTest.php b/tests/Utils/TokensTest.php deleted file mode 100644 index 6246ddc70..000000000 --- a/tests/Utils/TokensTest.php +++ /dev/null @@ -1,121 +0,0 @@ -[] $find - * @param Token[] $replace - */ - #[DataProvider('replaceTokensProvider')] - public function testReplaceTokens(string $list, array $find, array $replace, string $expected): void - { - $this->assertEquals($expected, Tokens::replaceTokens($list, $find, $replace)); - } - - /** - * @return array[]|Token[]>> - * @psalm-return list>, Token[], string}> - */ - public static function replaceTokensProvider(): array - { - return [ - [ - 'SELECT * FROM /*x*/a/*c*/.b', - [ - ['value_str' => 'a'], - ['token' => '.'], - ], - [ - new Token('c'), - new Token('.'), - ], - 'SELECT * FROM /*x*/c.b', - ], - ]; - } - - /** - * @param array $pattern - */ - #[DataProvider('matchProvider')] - public function testMatch(Token $token, array $pattern, bool $expected): void - { - $this->assertSame($expected, Tokens::match($token, $pattern)); - } - - /** - * @return array>> - * @psalm-return list, bool}> - */ - public static function matchProvider(): array - { - return [ - [ - new Token(''), - [], - true, - ], - - [ - new Token('"abc"', Token::TYPE_STRING, Token::FLAG_STRING_DOUBLE_QUOTES), - ['token' => '"abc"'], - true, - ], - [ - new Token('"abc"', Token::TYPE_STRING, Token::FLAG_STRING_DOUBLE_QUOTES), - ['value' => 'abc'], - true, - ], - [ - new Token('"abc"', Token::TYPE_STRING, Token::FLAG_STRING_DOUBLE_QUOTES), - ['value_str' => 'ABC'], - true, - ], - [ - new Token('"abc"', Token::TYPE_STRING, Token::FLAG_STRING_DOUBLE_QUOTES), - ['type' => Token::TYPE_STRING], - true, - ], - [ - new Token('"abc"', Token::TYPE_STRING, Token::FLAG_STRING_DOUBLE_QUOTES), - ['flags' => Token::FLAG_STRING_DOUBLE_QUOTES], - true, - ], - - [ - new Token('"abc"', Token::TYPE_STRING, Token::FLAG_STRING_DOUBLE_QUOTES), - ['token' => '"abcd"'], - false, - ], - [ - new Token('"abc"', Token::TYPE_STRING, Token::FLAG_STRING_DOUBLE_QUOTES), - ['value' => 'abcd'], - false, - ], - [ - new Token('"abc"', Token::TYPE_STRING, Token::FLAG_STRING_DOUBLE_QUOTES), - ['value_str' => 'ABCd'], - false, - ], - [ - new Token('"abc"', Token::TYPE_STRING, Token::FLAG_STRING_DOUBLE_QUOTES), - ['type' => Token::TYPE_NUMBER], - false, - ], - [ - new Token('"abc"', Token::TYPE_STRING, Token::FLAG_STRING_DOUBLE_QUOTES), - ['flags' => Token::FLAG_STRING_SINGLE_QUOTES], - false, - ], - ]; - } -} From c154f5c7ff2f872ee3e4031457e1961a0a823f4c Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Thu, 31 Aug 2023 00:51:08 +0100 Subject: [PATCH 4/4] Revert change to AlterOperation for now Signed-off-by: Kamil Tekiela --- src/Components/AlterOperation.php | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/Components/AlterOperation.php b/src/Components/AlterOperation.php index 4b74f74f0..a60656dea 100644 --- a/src/Components/AlterOperation.php +++ b/src/Components/AlterOperation.php @@ -256,11 +256,6 @@ final class AlterOperation implements Component */ public $partitions; - /** - * Unparsed tokens. - */ - public TokensList $unknown; - /** * @param OptionsArray $options options of alter operation * @param Expression|string|null $field altered field @@ -271,12 +266,12 @@ public function __construct( $options = null, $field = null, $partitions = null, - array $unknown = [] + public array $unknown = [] ) { $this->partitions = $partitions; $this->options = $options; $this->field = $field; - $this->unknown = new TokensList($unknown); + $this->unknown = $unknown; } /** @@ -523,7 +518,7 @@ public function build(): string $ret .= $this->field . ' '; } - $ret .= $afterFieldsOptions . $this->unknown->build(); + $ret .= $afterFieldsOptions . TokensList::buildFromArray($this->unknown); if (isset($this->partitions)) { $ret .= PartitionDefinition::buildAll($this->partitions);