Skip to content

Commit

Permalink
Merge pull request #551 from MauricioFauth/statement-issues
Browse files Browse the repository at this point in the history 
Fix some issues in Statement class
  • Loading branch information
@MauricioFauth
MauricioFauth committed Feb 8, 2024
2 parents 1f2e31e + 0eaf541 commit 48bd285
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 63 deletions.
26 changes: 0 additions & 26 deletions psalm-baseline.xml
View file
Expand Up @@ -732,20 +732,6 @@
</RedundantCondition>
</file>
<file src="src/Statement.php">
<DocblockTypeContradiction>
<code><![CDATA[empty(static::$clauses[$token->value])]]></code>
</DocblockTypeContradiction>
<InvalidArgument>
<code><![CDATA[$parsedClauses[$token->value]]]></code>
</InvalidArgument>
<InvalidArrayOffset>
<code><![CDATA[$parsedClauses[$token->value]]]></code>
<code><![CDATA[$parsedClauses[$token->value]]]></code>
<code><![CDATA[Parser::KEYWORD_PARSERS[$token->value]]]></code>
<code>Parser::KEYWORD_PARSERS[$tokenValue]</code>
<code><![CDATA[Parser::STATEMENT_PARSERS[$token->value]]]></code>
<code><![CDATA[static::$clauses[$token->value]]]></code>
</InvalidArrayOffset>
<MixedArgument>
<code><![CDATA[$this->$field]]></code>
<code><![CDATA[$this->$field]]></code>
Expand All @@ -768,25 +754,13 @@
<PossiblyUnusedReturnValue>
<code>bool</code>
</PossiblyUnusedReturnValue>
<RedundantCondition>
<code><![CDATA[! empty($parsedClauses[$token->value])]]></code>
</RedundantCondition>
<RiskyTruthyFalsyComparison>
<code><![CDATA[! stripos($clauseType, 'JOIN')]]></code>
<code><![CDATA[empty(Parser::KEYWORD_PARSERS[$tokenValue]['options'])]]></code>
<code><![CDATA[stripos($clauseType, 'JOIN')]]></code>
<code><![CDATA[stripos($clauseType, 'JOIN')]]></code>
</RiskyTruthyFalsyComparison>
<UndefinedMethod>
<code><![CDATA[$class::buildAll($this->$field)]]></code>
<code><![CDATA[$class::buildAll($this->$field)]]></code>
<code><![CDATA[$class::buildAll($this->$field)]]></code>
<code><![CDATA[$class::buildAll($this->$field)]]></code>
<code><![CDATA[$class::buildAll($this->$field)]]></code>
</UndefinedMethod>
<UnusedForeachValue>
<code>$index</code>
</UnusedForeachValue>
</file>
<file src="src/Statements/AlterStatement.php">
<MixedArgumentTypeCoercion>
Expand Down
83 changes: 46 additions & 37 deletions src/Statement.php
View file
Expand Up @@ -6,15 +6,20 @@

use AllowDynamicProperties;
use PhpMyAdmin\SqlParser\Components\OptionsArray;
use PhpMyAdmin\SqlParser\Exceptions\ParserException;
use PhpMyAdmin\SqlParser\Parsers\OptionsArrays;
use PhpMyAdmin\SqlParser\Statements\SelectStatement;
use PhpMyAdmin\SqlParser\Statements\SetStatement;
use PhpMyAdmin\SqlParser\Utils\Query;
use Stringable;

use function array_flip;
use function array_key_exists;
use function array_keys;
use function in_array;
use function is_array;
use function stripos;
use function is_string;
use function str_contains;
use function strtoupper;
use function trim;

/**
Expand Down Expand Up @@ -79,6 +84,8 @@ abstract class Statement implements Stringable
/**
* @param Parser|null $parser the instance that requests parsing
* @param TokensList|null $list the list of tokens to be parsed
*
* @throws ParserException
*/
public function __construct(Parser|null $parser = null, TokensList|null $list = null)
{
Expand Down Expand Up @@ -159,7 +166,7 @@ public function build(): string
* @param Parser $parser the instance that requests parsing
* @param TokensList $list the list of tokens to be parsed
*
* @throws Exceptions\ParserException
* @throws ParserException
*/
public function parse(Parser $parser, TokensList $list): void
{
Expand Down Expand Up @@ -224,7 +231,7 @@ public function parse(Parser $parser, TokensList $list): void
// ON DUPLICATE KEY UPDATE ...
// has to be parsed in parent statement (INSERT or REPLACE)
// so look for it and break
if ($this instanceof Statements\SelectStatement && $token->value === 'ON') {
if ($this instanceof SelectStatement && $token->value === 'ON') {
++$list->idx; // Skip ON

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

// Looking for duplicated clauses.
if (isset(Parser::KEYWORD_PARSERS[$token->value]) || ! empty(Parser::STATEMENT_PARSERS[$token->value])) {
if (! empty($parsedClauses[$token->value])) {
if (
is_string($token->value)
&& (
isset(Parser::KEYWORD_PARSERS[$token->value])
|| (
isset(Parser::STATEMENT_PARSERS[$token->value])
&& Parser::STATEMENT_PARSERS[$token->value] !== ''
)
)
) {
if (array_key_exists($token->value, $parsedClauses)) {
$parser->error('This type of clause was previously parsed.', $token);
break;
}
Expand All @@ -271,27 +287,27 @@ public function parse(Parser $parser, TokensList $list): void
}

// Checking if this is the beginning of a clause.
// Fix Issue #221: As `truncate` is not a keyword
// Fix Issue #221: As `truncate` is not a keyword,
// but it might be the beginning of a statement of truncate,
// so let the value use the keyword field for truncate type.
$tokenValue = in_array($token->keyword, ['TRUNCATE']) ? $token->keyword : $token->value;
if (isset(Parser::KEYWORD_PARSERS[$tokenValue]) && $list->idx < $list->count) {
$tokenValue = $token->keyword === 'TRUNCATE' ? $token->keyword : $token->value;
if (is_string($tokenValue) && isset(Parser::KEYWORD_PARSERS[$tokenValue]) && $list->idx < $list->count) {
$class = Parser::KEYWORD_PARSERS[$tokenValue]['class'];
$field = Parser::KEYWORD_PARSERS[$tokenValue]['field'];
if (! empty(Parser::KEYWORD_PARSERS[$tokenValue]['options'])) {
if (isset(Parser::KEYWORD_PARSERS[$tokenValue]['options'])) {
$options = Parser::KEYWORD_PARSERS[$tokenValue]['options'];
}
}

// Checking if this is the beginning of the statement.
if (! empty(Parser::STATEMENT_PARSERS[$token->keyword])) {
if (
! empty(static::$clauses) // Undefined for some statements.
&& empty(static::$clauses[$token->value])
) {
if (
isset(Parser::STATEMENT_PARSERS[$token->keyword])
&& Parser::STATEMENT_PARSERS[$token->keyword] !== ''
) {
if (static::$clauses !== [] && is_string($token->value) && ! isset(static::$clauses[$token->value])) {
// Some keywords (e.g. `SET`) may be the beginning of a
// statement and a clause.
// If such keyword was found and it cannot be a clause of
// If such keyword was found, and it cannot be a clause of
// this statement it means it is a new statement, but no
// delimiter was found between them.
$parser->error(
Expand All @@ -311,35 +327,27 @@ public function parse(Parser $parser, TokensList $list): void
$parsedOptions = true;
}
} elseif ($class === null) {
if ($this instanceof Statements\SelectStatement && $token->value === 'WITH ROLLUP') {
if ($this instanceof SelectStatement && $token->value === 'WITH ROLLUP') {
// Handle group options in Select statement
$this->groupOptions = OptionsArrays::parse(
$parser,
$list,
Statements\SelectStatement::STATEMENT_GROUP_OPTIONS,
SelectStatement::STATEMENT_GROUP_OPTIONS,
);
} elseif (
$this instanceof Statements\SelectStatement
$this instanceof SelectStatement
&& ($token->value === 'FOR UPDATE'
|| $token->value === 'LOCK IN SHARE MODE')
) {
// Handle special end options in Select statement
$this->endOptions = OptionsArrays::parse(
$parser,
$list,
Statements\SelectStatement::STATEMENT_END_OPTIONS,
);
$this->endOptions = OptionsArrays::parse($parser, $list, SelectStatement::STATEMENT_END_OPTIONS);
} elseif (
$this instanceof Statements\SetStatement
$this instanceof SetStatement
&& ($token->value === 'COLLATE'
|| $token->value === 'DEFAULT')
) {
// Handle special end options in SET statement
$this->endOptions = OptionsArrays::parse(
$parser,
$list,
Statements\SetStatement::STATEMENT_END_OPTIONS,
);
$this->endOptions = OptionsArrays::parse($parser, $list, SetStatement::STATEMENT_END_OPTIONS);
} else {
// There is no parser for this keyword and isn't the beginning
// of a statement (so no options) either.
Expand Down Expand Up @@ -419,7 +427,7 @@ public function __toString(): string
* @param Parser $parser the instance that requests parsing
* @param TokensList $list the list of tokens to be parsed
*
* @throws Exceptions\ParserException
* @throws ParserException
*/
public function validateClauseOrder(Parser $parser, TokensList $list): bool
{
Expand Down Expand Up @@ -449,12 +457,12 @@ public function validateClauseOrder(Parser $parser, TokensList $list): bool

$error = 0;
$lastIdx = 0;
foreach ($clauses as $clauseType => $index) {
$clauseStartIdx = Utils\Query::getClauseStartOffset($this, $list, $clauseType);
foreach (array_keys($clauses) as $clauseType) {
$clauseStartIdx = Query::getClauseStartOffset($this, $list, $clauseType);

if (
$clauseStartIdx !== -1
&& $this instanceof Statements\SelectStatement
&& $this instanceof SelectStatement
&& ($clauseType === 'FORCE'
|| $clauseType === 'IGNORE'
|| $clauseType === 'USE')
Expand All @@ -466,13 +474,14 @@ public function validateClauseOrder(Parser $parser, TokensList $list): bool

// Handle ordering of Multiple Joins in a query
if ($clauseStartIdx !== -1) {
if ($minJoin === 0 && stripos($clauseType, 'JOIN')) {
$containsJoinClause = str_contains(strtoupper($clauseType), 'JOIN');
if ($minJoin === 0 && $containsJoinClause) {
// First JOIN clause is detected
$minJoin = $maxJoin = $clauseStartIdx;
} elseif ($minJoin !== 0 && ! stripos($clauseType, 'JOIN')) {
} elseif ($minJoin !== 0 && ! $containsJoinClause) {
// After a previous JOIN clause, a non-JOIN clause has been detected
$maxJoin = $lastIdx;
} elseif ($maxJoin < $clauseStartIdx && stripos($clauseType, 'JOIN')) {
} elseif ($maxJoin < $clauseStartIdx && $containsJoinClause) {
$error = 1;
}
}
Expand Down

0 comments on commit 48bd285

Please sign in to comment.