From e204155a419f4a548d27274969f2eff9b7796605 Mon Sep 17 00:00:00 2001 From: Kamil Tekiela Date: Tue, 24 Sep 2024 14:53:05 +0100 Subject: [PATCH] Refactor OptionsArray Signed-off-by: Kamil Tekiela --- phpstan-baseline.neon | 55 +++++++++++--------- psalm-baseline.xml | 75 +++++++-------------------- src/Components/OptionsArray.php | 55 +++++++++++++------- src/Parsers/OptionsArrays.php | 11 ++-- src/Statements/CreateStatement.php | 14 ++--- src/Utils/Table.php | 14 +++-- tests/Components/OptionsArrayTest.php | 9 ++-- tests/Parser/LoadStatementTest.php | 2 +- tests/Parser/SelectStatementTest.php | 2 +- 9 files changed, 111 insertions(+), 126 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 2f960300..c8bc2e4b 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -20,11 +20,6 @@ parameters: count: 1 path: src/Components/DataType.php - - - message: "#^Parameter \\#2 \\$string2 of function strcasecmp expects string, mixed given\\.$#" - count: 2 - path: src/Components/OptionsArray.php - - message: "#^Parameter \\#1 \\$str of static method PhpMyAdmin\\\\SqlParser\\\\Context\\:\\:escape\\(\\) expects string, string\\|null given\\.$#" count: 1 @@ -256,48 +251,58 @@ parameters: path: src/Parsers/Limits.php - - message: "#^Cannot access offset 'equals' on mixed\\.$#" - count: 1 + message: "#^Cannot access offset 1 on mixed\\.$#" + count: 2 path: src/Parsers/OptionsArrays.php - - message: "#^Cannot access offset 'expr' on mixed\\.$#" - count: 2 + message: "#^Cannot access offset 2 on mixed\\.$#" + count: 1 path: src/Parsers/OptionsArrays.php - - message: "#^Cannot access offset 'name' on mixed\\.$#" + message: "#^Cannot assign offset 'equals' to array\\\\|string\\.$#" count: 1 path: src/Parsers/OptionsArrays.php - - message: "#^Cannot access offset 'value' on mixed\\.$#" + message: "#^Cannot assign offset 'expr' to array\\\\|string\\.$#" count: 2 path: src/Parsers/OptionsArrays.php - - message: "#^Cannot access offset 1 on mixed\\.$#" - count: 2 + message: "#^Offset 'expr' does not exist on array\\{name\\: string, equals\\: bool, expr\\: PhpMyAdmin\\\\SqlParser\\\\Components\\\\Expression\\|string, value\\: string\\|null\\}\\|string\\.$#" + count: 1 path: src/Parsers/OptionsArrays.php - - message: "#^Cannot access offset 2 on mixed\\.$#" + message: "#^Offset 'name' does not exist on array\\{name\\: string, equals\\: bool, expr\\: PhpMyAdmin\\\\SqlParser\\\\Components\\\\Expression\\|string, value\\: string\\|null\\}\\|string\\.$#" count: 1 path: src/Parsers/OptionsArrays.php - - message: "#^Parameter \\#2 \\.\\.\\.\\$values of function sprintf expects bool\\|float\\|int\\|string\\|null, mixed given\\.$#" + message: "#^Parameter \\#3 \\$options of static method PhpMyAdmin\\\\SqlParser\\\\Parsers\\\\Expressions\\:\\:parse\\(\\) expects array\\, mixed given\\.$#" count: 1 path: src/Parsers/OptionsArrays.php - - message: "#^Parameter \\#3 \\$options of static method PhpMyAdmin\\\\SqlParser\\\\Parsers\\\\Expressions\\:\\:parse\\(\\) expects array\\, mixed given\\.$#" + message: "#^Property PhpMyAdmin\\\\SqlParser\\\\Components\\\\OptionsArray\\:\\:\\$options \\(array\\\\) does not accept non\\-empty\\-array\\\\.$#" + count: 2 + path: src/Parsers/OptionsArrays.php + + - + message: "#^Property PhpMyAdmin\\\\SqlParser\\\\Components\\\\OptionsArray\\:\\:\\$options \\(array\\\\) does not accept non\\-empty\\-array\\\\.$#" count: 1 path: src/Parsers/OptionsArrays.php - - message: "#^Property PhpMyAdmin\\\\SqlParser\\\\Components\\\\OptionsArray\\:\\:\\$options \\(array\\\\) does not accept array\\\\.$#" - count: 8 + message: "#^Property PhpMyAdmin\\\\SqlParser\\\\Components\\\\OptionsArray\\:\\:\\$options \\(array\\\\) does not accept non\\-empty\\-array\\\\.$#" + count: 3 + path: src/Parsers/OptionsArrays.php + + - + message: "#^Property PhpMyAdmin\\\\SqlParser\\\\Components\\\\OptionsArray\\:\\:\\$options \\(array\\\\) does not accept non\\-empty\\-array\\\\.$#" + count: 3 path: src/Parsers/OptionsArrays.php - @@ -506,18 +511,18 @@ parameters: path: src/Statements/CreateStatement.php - - message: "#^Cannot call method has\\(\\) on PhpMyAdmin\\\\SqlParser\\\\Components\\\\OptionsArray\\|null\\.$#" - count: 11 + message: "#^Cannot call method get\\(\\) on PhpMyAdmin\\\\SqlParser\\\\Components\\\\OptionsArray\\|null\\.$#" + count: 2 path: src/Statements/CreateStatement.php - - message: "#^Parameter \\#1 \\$component of static method PhpMyAdmin\\\\SqlParser\\\\Parsers\\\\ParameterDefinitions\\:\\:buildAll\\(\\) expects array\\, array\\\\|null given\\.$#" - count: 1 + message: "#^Cannot call method has\\(\\) on PhpMyAdmin\\\\SqlParser\\\\Components\\\\OptionsArray\\|null\\.$#" + count: 9 path: src/Statements/CreateStatement.php - - message: "#^Parameter \\#3 \\$subject of function str_replace expects array\\|string, mixed given\\.$#" - count: 2 + message: "#^Parameter \\#1 \\$component of static method PhpMyAdmin\\\\SqlParser\\\\Parsers\\\\ParameterDefinitions\\:\\:buildAll\\(\\) expects array\\, array\\\\|null given\\.$#" + count: 1 path: src/Statements/CreateStatement.php - @@ -896,7 +901,7 @@ parameters: path: src/Utils/Table.php - - message: "#^Method PhpMyAdmin\\\\SqlParser\\\\Utils\\\\Table\\:\\:getFields\\(\\) should return array\\ but returns array\\\\.$#" + message: "#^Method PhpMyAdmin\\\\SqlParser\\\\Utils\\\\Table\\:\\:getFields\\(\\) should return array\\ but returns array\\\\.$#" count: 1 path: src/Utils/Table.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index aeef4c62..8666dba1 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -86,27 +86,6 @@ type]]> - - - - - - - - - - - - - - - - - - - - - expr]]> @@ -498,25 +477,12 @@ options[$lastOptionId]['name']]]> - options[$lastOptionId]) - ? $ret->options[$lastOptionId]['name'] - : $ret->options[$lastOptionId]]]> - options[$lastOptionId]['expr']]]> - options[$lastOptionId]['name']]]> - options[$lastOptionId]['value']]]> - - options[$lastOptionId]['equals']]]> - options[$lastOptionId]['expr']]]> - options[$lastOptionId]['expr']]]> - options[$lastOptionId]['value']]]> - options[$lastOptionId]['value']]]> - options[$lastOptionId]]]> options[$lastOptionId]]]> @@ -529,6 +495,7 @@ options[$lastOptionId]]]> options[$lastOptionId]]]> options[$lastOptionId]]]> + options[$lastOptionId]]]> options[$lastOptionId]]]> @@ -549,9 +516,18 @@ options]]> options]]> options]]> - options]]> - options]]> + + options[$lastOptionId]['equals']]]> + options[$lastOptionId]['expr']]]> + options[$lastOptionId]['expr']]]> + options[$lastOptionId]['expr']]]> + options[$lastOptionId]['expr']]]> + options[$lastOptionId]['name']]]> + options[$lastOptionId]['value']]]> + options[$lastOptionId]['value']]]> + options[$lastOptionId]['value']]]> + @@ -559,9 +535,11 @@ - - options[$lastOptionId]['expr']->expr]]> - + + options]]> + options]]> + options]]> + @@ -679,14 +657,8 @@ - - $field]]> - - - - $field]]> @@ -694,10 +666,6 @@ - - - - parameters]]> @@ -723,7 +691,7 @@ - + @@ -1191,13 +1159,6 @@ - - - - - name]['default_value']]]> - name]['expr']]]> - $options The array of options. Options that have a value - * must be an array with at least two keys `name` and - * `expr` or `value`. + * @param array> $options $options The array of options. + * Options that have a value must be an array with at least two keys `name` and `expr` or `value`. + * @psalm-param array $options */ public function __construct(public array $options = []) { @@ -27,7 +29,7 @@ public function __construct(public array $options = []) public function build(): string { - if (empty($this->options)) { + if ($this->options === []) { return ''; } @@ -37,34 +39,49 @@ public function build(): string $options[] = $option; } else { $options[] = $option['name'] - . (! empty($option['equals']) ? '=' : ' ') - . (! empty($option['expr']) ? $option['expr'] : $option['value']); + . ($option['equals'] ? '=' : ' ') + . ($option['expr'] !== '' ? $option['expr'] : ($option['value'] ?? '')); } } return implode(' ', $options); } + public function has(string $key): bool + { + foreach ($this->options as $option) { + if (is_array($option)) { + if (strcasecmp($key, $option['name']) === 0) { + return ($option['value'] ?? '') !== ''; + } + } elseif (strcasecmp($key, $option) === 0) { + return true; + } + } + + return false; + } + /** - * Checks if it has the specified option and returns it value or true. + * Checks if it has the specified option and returns its value. * * @param string $key the key to be checked * @param bool $getExpr Gets the expression instead of the value. * The value is the processed form of the expression. */ - public function has(string $key, bool $getExpr = false): mixed + public function get(string $key, bool $getExpr = false): string|Expression { foreach ($this->options as $option) { if (is_array($option)) { - if (! strcasecmp($key, $option['name'])) { - return $getExpr ? $option['expr'] : $option['value']; + if (strcasecmp($key, $option['name']) === 0) { + return $getExpr ? $option['expr'] : ($option['value'] ?? ''); } - } elseif (! strcasecmp($key, $option)) { - return true; + } elseif (strcasecmp($key, $option) === 0) { + return $option; } } - return false; + return ''; } /** @@ -78,12 +95,12 @@ public function remove(string $key): bool { foreach ($this->options as $idx => $option) { if (is_array($option)) { - if (! strcasecmp($key, $option['name'])) { + if (strcasecmp($key, $option['name']) === 0) { unset($this->options[$idx]); return true; } - } elseif (! strcasecmp($key, $option)) { + } elseif (strcasecmp($key, $option) === 0) { unset($this->options[$idx]); return true; @@ -107,7 +124,7 @@ public function merge(OptionsArray $options): void */ public function isEmpty(): bool { - return empty($this->options); + return $this->options === []; } public function __toString(): string diff --git a/src/Parsers/OptionsArrays.php b/src/Parsers/OptionsArrays.php index 862d4e94..f3dea83c 100644 --- a/src/Parsers/OptionsArrays.php +++ b/src/Parsers/OptionsArrays.php @@ -161,6 +161,8 @@ public static function parse(Parser $parser, TokensList $list, array $options = 'equals' => $lastOption[1] === 'expr=', // @var Expression The parsed expression. 'expr' => '', + // No value when it is an expression. + 'value' => null, ]; $state = 1; } @@ -179,14 +181,15 @@ public static function parse(Parser $parser, TokensList $list, array $options = } if ($lastOption[1] === 'expr' || $lastOption[1] === 'expr=') { - $ret->options[$lastOptionId]['expr'] = Expressions::parse( + $expression = Expressions::parse( $parser, $list, empty($lastOption[2]) ? [] : $lastOption[2], ); - if ($ret->options[$lastOptionId]['expr'] !== null) { - $ret->options[$lastOptionId]['value'] - = $ret->options[$lastOptionId]['expr']->expr; + $ret->options[$lastOptionId]['expr'] = ''; + if ($expression !== null) { + $ret->options[$lastOptionId]['value'] = $expression->expr; + $ret->options[$lastOptionId]['expr'] = $expression; } $lastOption = null; diff --git a/src/Statements/CreateStatement.php b/src/Statements/CreateStatement.php index 8ce6007b..7b2b65a5 100644 --- a/src/Statements/CreateStatement.php +++ b/src/Statements/CreateStatement.php @@ -781,7 +781,7 @@ public function parse(Parser $parser, TokensList $list): void /** @return list */ public function getForeignKeys(): array { - if (empty($this->fields) || (! is_array($this->fields)) || (! $this->options->has('TABLE'))) { + if (empty($this->fields) || ! is_array($this->fields) || ! $this->options->has('TABLE')) { return []; } @@ -810,16 +810,16 @@ public function getForeignKeys(): array $foreignKey->refTableName = $field->references->table->table; $foreignKey->refIndexList = $field->references->columns; - $opt = $field->references->options->has('ON UPDATE'); + $opt = $field->references->options->get('ON UPDATE'); - if ($opt) { - $foreignKey->onUpdate = str_replace(' ', '_', $opt); + if ($opt !== '') { + $foreignKey->onUpdate = str_replace(' ', '_', (string) $opt); } - $opt = $field->references->options->has('ON DELETE'); + $opt = $field->references->options->get('ON DELETE'); - if ($opt) { - $foreignKey->onDelete = str_replace(' ', '_', $opt); + if ($opt !== '') { + $foreignKey->onDelete = str_replace(' ', '_', (string) $opt); } } diff --git a/src/Utils/Table.php b/src/Utils/Table.php index 2db8c4ce..6b92893b 100644 --- a/src/Utils/Table.php +++ b/src/Utils/Table.php @@ -30,7 +30,7 @@ class Table */ public static function getFields(CreateStatement $statement): array { - if (empty($statement->fields) || (! is_array($statement->fields)) || (! $statement->options->has('TABLE'))) { + if (empty($statement->fields) || ! is_array($statement->fields) || ! $statement->options->has('TABLE')) { return []; } @@ -57,24 +57,22 @@ public static function getFields(CreateStatement $statement): array } } - $option = $field->options->has('DEFAULT'); + $option = $field->options->get('DEFAULT'); - if ($option) { + if ($option !== '') { $ret[$field->name]['default_value'] = $option; if ($option === 'CURRENT_TIMESTAMP') { $ret[$field->name]['default_current_timestamp'] = true; } } - $option = $field->options->has('ON UPDATE'); - - if ($option === 'CURRENT_TIMESTAMP') { + if ($field->options->get('ON UPDATE') === 'CURRENT_TIMESTAMP') { $ret[$field->name]['on_update_current_timestamp'] = true; } - $option = $field->options->has('AS'); + $option = $field->options->get('AS'); - if (! $option) { + if ($option === '') { continue; } diff --git a/tests/Components/OptionsArrayTest.php b/tests/Components/OptionsArrayTest.php index 03e83aa5..1785f3ef 100644 --- a/tests/Components/OptionsArrayTest.php +++ b/tests/Components/OptionsArrayTest.php @@ -58,10 +58,10 @@ public function testParseExpr(): void ], ], ); - $sumValue = $component->has('SUM', true); + $sumValue = $component->get('SUM', true); $this->assertInstanceOf(Expression::class, $sumValue); $this->assertEquals('(3 + 5)', (string) $sumValue); - $this->assertEquals('8', $component->has('RESULT')); + $this->assertEquals('8', $component->get('RESULT')); } public function testHas(): void @@ -79,7 +79,7 @@ public function testHas(): void ], ); $this->assertTrue($component->has('A')); - $this->assertEquals('test', $component->has('B')); + $this->assertEquals('test', $component->get('B')); $this->assertTrue($component->has('C')); $this->assertFalse($component->has('D')); } @@ -105,7 +105,7 @@ public function testRemove(): void 'C' => 3, ], ); - $this->assertEquals('test', $component->has('B')); + $this->assertEquals('test', $component->get('B')); $component->remove('B'); $this->assertFalse($component->has('B')); } @@ -127,6 +127,7 @@ public function testBuild(): void 'name' => 'MAX_STATEMENT_TIME', 'value' => '42', 'equals' => true, + 'expr' => '', ], ], ); diff --git a/tests/Parser/LoadStatementTest.php b/tests/Parser/LoadStatementTest.php index ecd873a2..ca99f4bd 100644 --- a/tests/Parser/LoadStatementTest.php +++ b/tests/Parser/LoadStatementTest.php @@ -16,7 +16,7 @@ public function testLoadOptions(): void $parser = new Parser($data['query']); $stmt = $parser->statements[0]; $this->assertNotNull($stmt->options); - $this->assertEquals(10, $stmt->options->has('CONCURRENT')); + $this->assertTrue($stmt->options->has('CONCURRENT')); } #[DataProvider('loadProvider')] diff --git a/tests/Parser/SelectStatementTest.php b/tests/Parser/SelectStatementTest.php index eccb3561..16178d6f 100644 --- a/tests/Parser/SelectStatementTest.php +++ b/tests/Parser/SelectStatementTest.php @@ -16,7 +16,7 @@ public function testSelectOptions(): void $parser = new Parser($data['query']); $stmt = $parser->statements[0]; $this->assertNotNull($stmt->options); - $this->assertEquals(10, $stmt->options->has('MAX_STATEMENT_TIME')); + $this->assertEquals(10, $stmt->options->get('MAX_STATEMENT_TIME')); } #[DataProvider('selectProvider')]