Skip to content

Commit 48bd285

Browse files
MauricioFauthMauricioFauth
authored
Merge pull request #551 from MauricioFauth/statement-issues
Fix some issues in Statement class
2 parents 1f2e31e + 0eaf541 commit 48bd285

File tree

2 files changed

+46
-63
lines changed

2 files changed

+46
-63
lines changed

‎psalm-baseline.xml‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -732,20 +732,6 @@
732732
</RedundantCondition>
733733
</file>
734734
<file src="src/Statement.php">
735-
<DocblockTypeContradiction>
736-
<code><![CDATA[empty(static::$clauses[$token->value])]]></code>
737-
</DocblockTypeContradiction>
738-
<InvalidArgument>
739-
<code><![CDATA[$parsedClauses[$token->value]]]></code>
740-
</InvalidArgument>
741-
<InvalidArrayOffset>
742-
<code><![CDATA[$parsedClauses[$token->value]]]></code>
743-
<code><![CDATA[$parsedClauses[$token->value]]]></code>
744-
<code><![CDATA[Parser::KEYWORD_PARSERS[$token->value]]]></code>
745-
<code>Parser::KEYWORD_PARSERS[$tokenValue]</code>
746-
<code><![CDATA[Parser::STATEMENT_PARSERS[$token->value]]]></code>
747-
<code><![CDATA[static::$clauses[$token->value]]]></code>
748-
</InvalidArrayOffset>
749735
<MixedArgument>
750736
<code><![CDATA[$this->$field]]></code>
751737
<code><![CDATA[$this->$field]]></code>
@@ -768,25 +754,13 @@
768754
<PossiblyUnusedReturnValue>
769755
<code>bool</code>
770756
</PossiblyUnusedReturnValue>
771-
<RedundantCondition>
772-
<code><![CDATA[! empty($parsedClauses[$token->value])]]></code>
773-
</RedundantCondition>
774-
<RiskyTruthyFalsyComparison>
775-
<code><![CDATA[! stripos($clauseType, 'JOIN')]]></code>
776-
<code><![CDATA[empty(Parser::KEYWORD_PARSERS[$tokenValue]['options'])]]></code>
777-
<code><![CDATA[stripos($clauseType, 'JOIN')]]></code>
778-
<code><![CDATA[stripos($clauseType, 'JOIN')]]></code>
779-
</RiskyTruthyFalsyComparison>
780757
<UndefinedMethod>
781758
<code><![CDATA[$class::buildAll($this->$field)]]></code>
782759
<code><![CDATA[$class::buildAll($this->$field)]]></code>
783760
<code><![CDATA[$class::buildAll($this->$field)]]></code>
784761
<code><![CDATA[$class::buildAll($this->$field)]]></code>
785762
<code><![CDATA[$class::buildAll($this->$field)]]></code>
786763
</UndefinedMethod>
787-
<UnusedForeachValue>
788-
<code>$index</code>
789-
</UnusedForeachValue>
790764
</file>
791765
<file src="src/Statements/AlterStatement.php">
792766
<MixedArgumentTypeCoercion>

‎src/Statement.php‎

Lines changed: 46 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,20 @@
66

77
use AllowDynamicProperties;
88
use PhpMyAdmin\SqlParser\Components\OptionsArray;
9+
use PhpMyAdmin\SqlParser\Exceptions\ParserException;
910
use PhpMyAdmin\SqlParser\Parsers\OptionsArrays;
11+
use PhpMyAdmin\SqlParser\Statements\SelectStatement;
12+
use PhpMyAdmin\SqlParser\Statements\SetStatement;
13+
use PhpMyAdmin\SqlParser\Utils\Query;
1014
use Stringable;
1115

1216
use function array_flip;
1317
use function array_key_exists;
1418
use function array_keys;
15-
use function in_array;
1619
use function is_array;
17-
use function stripos;
20+
use function is_string;
21+
use function str_contains;
22+
use function strtoupper;
1823
use function trim;
1924

2025
/**
@@ -79,6 +84,8 @@ abstract class Statement implements Stringable
7984
/**
8085
* @param Parser|null $parser the instance that requests parsing
8186
* @param TokensList|null $list the list of tokens to be parsed
87+
*
88+
* @throws ParserException
8289
*/
8390
public function __construct(Parser|null $parser = null, TokensList|null $list = null)
8491
{
@@ -159,7 +166,7 @@ public function build(): string
159166
* @param Parser $parser the instance that requests parsing
160167
* @param TokensList $list the list of tokens to be parsed
161168
*
162-
* @throws Exceptions\ParserException
169+
* @throws ParserException
163170
*/
164171
public function parse(Parser $parser, TokensList $list): void
165172
{
@@ -224,7 +231,7 @@ public function parse(Parser $parser, TokensList $list): void
224231
// ON DUPLICATE KEY UPDATE ...
225232
// has to be parsed in parent statement (INSERT or REPLACE)
226233
// so look for it and break
227-
if ($this instanceof Statements\SelectStatement && $token->value === 'ON') {
234+
if ($this instanceof SelectStatement && $token->value === 'ON') {
228235
++$list->idx; // Skip ON
229236

230237
// look for ON DUPLICATE KEY UPDATE
@@ -261,8 +268,17 @@ public function parse(Parser $parser, TokensList $list): void
261268
$options = [];
262269

263270
// Looking for duplicated clauses.
264-
if (isset(Parser::KEYWORD_PARSERS[$token->value]) || ! empty(Parser::STATEMENT_PARSERS[$token->value])) {
265-
if (! empty($parsedClauses[$token->value])) {
271+
if (
272+
is_string($token->value)
273+
&& (
274+
isset(Parser::KEYWORD_PARSERS[$token->value])
275+
|| (
276+
isset(Parser::STATEMENT_PARSERS[$token->value])
277+
&& Parser::STATEMENT_PARSERS[$token->value] !== ''
278+
)
279+
)
280+
) {
281+
if (array_key_exists($token->value, $parsedClauses)) {
266282
$parser->error('This type of clause was previously parsed.', $token);
267283
break;
268284
}
@@ -271,27 +287,27 @@ public function parse(Parser $parser, TokensList $list): void
271287
}
272288

273289
// Checking if this is the beginning of a clause.
274-
// Fix Issue #221: As `truncate` is not a keyword
290+
// Fix Issue #221: As `truncate` is not a keyword,
275291
// but it might be the beginning of a statement of truncate,
276292
// so let the value use the keyword field for truncate type.
277-
$tokenValue = in_array($token->keyword, ['TRUNCATE']) ? $token->keyword : $token->value;
278-
if (isset(Parser::KEYWORD_PARSERS[$tokenValue]) && $list->idx < $list->count) {
293+
$tokenValue = $token->keyword === 'TRUNCATE' ? $token->keyword : $token->value;
294+
if (is_string($tokenValue) && isset(Parser::KEYWORD_PARSERS[$tokenValue]) && $list->idx < $list->count) {
279295
$class = Parser::KEYWORD_PARSERS[$tokenValue]['class'];
280296
$field = Parser::KEYWORD_PARSERS[$tokenValue]['field'];
281-
if (! empty(Parser::KEYWORD_PARSERS[$tokenValue]['options'])) {
297+
if (isset(Parser::KEYWORD_PARSERS[$tokenValue]['options'])) {
282298
$options = Parser::KEYWORD_PARSERS[$tokenValue]['options'];
283299
}
284300
}
285301

286302
// Checking if this is the beginning of the statement.
287-
if (! empty(Parser::STATEMENT_PARSERS[$token->keyword])) {
288-
if (
289-
! empty(static::$clauses) // Undefined for some statements.
290-
&& empty(static::$clauses[$token->value])
291-
) {
303+
if (
304+
isset(Parser::STATEMENT_PARSERS[$token->keyword])
305+
&& Parser::STATEMENT_PARSERS[$token->keyword] !== ''
306+
) {
307+
if (static::$clauses !== [] && is_string($token->value) && ! isset(static::$clauses[$token->value])) {
292308
// Some keywords (e.g. `SET`) may be the beginning of a
293309
// statement and a clause.
294-
// If such keyword was found and it cannot be a clause of
310+
// If such keyword was found, and it cannot be a clause of
295311
// this statement it means it is a new statement, but no
296312
// delimiter was found between them.
297313
$parser->error(
@@ -311,35 +327,27 @@ public function parse(Parser $parser, TokensList $list): void
311327
$parsedOptions = true;
312328
}
313329
} elseif ($class === null) {
314-
if ($this instanceof Statements\SelectStatement && $token->value === 'WITH ROLLUP') {
330+
if ($this instanceof SelectStatement && $token->value === 'WITH ROLLUP') {
315331
// Handle group options in Select statement
316332
$this->groupOptions = OptionsArrays::parse(
317333
$parser,
318334
$list,
319-
Statements\SelectStatement::STATEMENT_GROUP_OPTIONS,
335+
SelectStatement::STATEMENT_GROUP_OPTIONS,
320336
);
321337
} elseif (
322-
$this instanceof Statements\SelectStatement
338+
$this instanceof SelectStatement
323339
&& ($token->value === 'FOR UPDATE'
324340
|| $token->value === 'LOCK IN SHARE MODE')
325341
) {
326342
// Handle special end options in Select statement
327-
$this->endOptions = OptionsArrays::parse(
328-
$parser,
329-
$list,
330-
Statements\SelectStatement::STATEMENT_END_OPTIONS,
331-
);
343+
$this->endOptions = OptionsArrays::parse($parser, $list, SelectStatement::STATEMENT_END_OPTIONS);
332344
} elseif (
333-
$this instanceof Statements\SetStatement
345+
$this instanceof SetStatement
334346
&& ($token->value === 'COLLATE'
335347
|| $token->value === 'DEFAULT')
336348
) {
337349
// Handle special end options in SET statement
338-
$this->endOptions = OptionsArrays::parse(
339-
$parser,
340-
$list,
341-
Statements\SetStatement::STATEMENT_END_OPTIONS,
342-
);
350+
$this->endOptions = OptionsArrays::parse($parser, $list, SetStatement::STATEMENT_END_OPTIONS);
343351
} else {
344352
// There is no parser for this keyword and isn't the beginning
345353
// of a statement (so no options) either.
@@ -419,7 +427,7 @@ public function __toString(): string
419427
* @param Parser $parser the instance that requests parsing
420428
* @param TokensList $list the list of tokens to be parsed
421429
*
422-
* @throws Exceptions\ParserException
430+
* @throws ParserException
423431
*/
424432
public function validateClauseOrder(Parser $parser, TokensList $list): bool
425433
{
@@ -449,12 +457,12 @@ public function validateClauseOrder(Parser $parser, TokensList $list): bool
449457

450458
$error = 0;
451459
$lastIdx = 0;
452-
foreach ($clauses as $clauseType => $index) {
453-
$clauseStartIdx = Utils\Query::getClauseStartOffset($this, $list, $clauseType);
460+
foreach (array_keys($clauses) as $clauseType) {
461+
$clauseStartIdx = Query::getClauseStartOffset($this, $list, $clauseType);
454462

455463
if (
456464
$clauseStartIdx !== -1
457-
&& $this instanceof Statements\SelectStatement
465+
&& $this instanceof SelectStatement
458466
&& ($clauseType === 'FORCE'
459467
|| $clauseType === 'IGNORE'
460468
|| $clauseType === 'USE')
@@ -466,13 +474,14 @@ public function validateClauseOrder(Parser $parser, TokensList $list): bool
466474

467475
// Handle ordering of Multiple Joins in a query
468476
if ($clauseStartIdx !== -1) {
469-
if ($minJoin === 0 && stripos($clauseType, 'JOIN')) {
477+
$containsJoinClause = str_contains(strtoupper($clauseType), 'JOIN');
478+
if ($minJoin === 0 && $containsJoinClause) {
470479
// First JOIN clause is detected
471480
$minJoin = $maxJoin = $clauseStartIdx;
472-
} elseif ($minJoin !== 0 && ! stripos($clauseType, 'JOIN')) {
481+
} elseif ($minJoin !== 0 && ! $containsJoinClause) {
473482
// After a previous JOIN clause, a non-JOIN clause has been detected
474483
$maxJoin = $lastIdx;
475-
} elseif ($maxJoin < $clauseStartIdx && stripos($clauseType, 'JOIN')) {
484+
} elseif ($maxJoin < $clauseStartIdx && $containsJoinClause) {
476485
$error = 1;
477486
}
478487
}

0 commit comments

Comments
 (0)