Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Micro optimizations and refactorings #591

Merged
merged 8 commits into from
Oct 25, 2024
Merged

Micro optimizations and refactorings #591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
949d1df
Optimize isString
kamil-tekiela Oct 15, 2024
0e979c8
Optimize isSymbol
kamil-tekiela Oct 15, 2024
0b21c5c
Optimize isWhitespace
kamil-tekiela Oct 15, 2024
bb60ad1
Refactor escapeAll()
kamil-tekiela Oct 15, 2024
b541c5a
Refactor isComment
kamil-tekiela Oct 15, 2024
8d711e8
String offset access should always return a string
kamil-tekiela Oct 15, 2024
3dcaadb
Turn $operators into a const
kamil-tekiela Oct 15, 2024
dbee88a
Context should not be directly loadable
kamil-tekiela Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 0 additions & 51 deletions psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,6 @@
<InvalidPropertyAssignmentValue>
<code><![CDATA[[]]]></code>
</InvalidPropertyAssignmentValue>
<InvalidPropertyFetch>
<code><![CDATA[$context::$keywords]]></code>
</InvalidPropertyFetch>
<MixedAssignment>
<code><![CDATA[self::$keywords]]></code>
</MixedAssignment>
Expand All @@ -176,49 +173,6 @@
<MixedOperand>
<code><![CDATA[$this->str[$this->last]]]></code>
</MixedOperand>
<PossiblyNullArgument>
<code><![CDATA[$this->str[$this->last + 1]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$token]]></code>
<code><![CDATA[$token]]></code>
<code><![CDATA[$token]]></code>
<code><![CDATA[$token]]></code>
<code><![CDATA[$token]]></code>
</PossiblyNullArgument>
<PossiblyNullOperand>
<code><![CDATA[$this->str[$this->last++]]]></code>
<code><![CDATA[$this->str[$this->last++]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[$this->last]]]></code>
<code><![CDATA[$this->str[++$this->last]]]></code>
<code><![CDATA[$this->str[++$this->last]]]></code>
<code><![CDATA[$this->str[++$this->last]]]></code>
<code><![CDATA[$this->str[++$this->last]]]></code>
<code><![CDATA[$this->str[++$this->last]]]></code>
</PossiblyNullOperand>
<PossiblyNullPropertyFetch>
<code><![CDATA[$next->type]]></code>
<code><![CDATA[$next->value]]></code>
Expand All @@ -231,11 +185,6 @@
<RiskyTruthyFalsyComparison>
<code><![CDATA[! $flags]]></code>
<code><![CDATA[! $flags]]></code>
<code><![CDATA[! $flags]]></code>
<code><![CDATA[! $flags]]></code>
<code><![CDATA[Context::isComment($token)]]></code>
<code><![CDATA[Context::isComment($token)]]></code>
<code><![CDATA[Context::isComment($token, $end)]]></code>
<code><![CDATA[empty($delimiter)]]></code>
</RiskyTruthyFalsyComparison>
</file>
Expand Down
139 changes: 48 additions & 91 deletions src/Context.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use PhpMyAdmin\SqlParser\Contexts\ContextMySql50700;

use function array_map;
use function class_exists;
use function explode;
use function in_array;
Expand All @@ -26,7 +27,7 @@
*
* Holds the configuration of the context that is currently used.
*/
abstract class Context
final class Context
{
/**
* The maximum length of a keyword.
Expand Down Expand Up @@ -54,7 +55,7 @@
* The prefix concatenated to the context name when an incomplete class name
* is specified.
*/
public static string $contextPrefix = 'PhpMyAdmin\\SqlParser\\Contexts\\Context';
private const CONTEXT_PREFIX = 'PhpMyAdmin\\SqlParser\\Contexts\\Context';

/**
* List of keywords.
Expand All @@ -78,10 +79,8 @@

/**
* List of operators and their flags.
*
* @var array<string, int>
*/
public static array $operators = [
private const OPERATORS = [
// Some operators (*, =) may have ambiguous flags, because they depend on
// the context they are being used in.
// For example: 1. SELECT * FROM table; # SQL specific (wildcard)
Expand Down Expand Up @@ -366,15 +365,18 @@
*/
public static function isOperator(string $string): int|null
{
return static::$operators[$string] ?? null;
return self::OPERATORS[$string] ?? null;
}

/**
* Checks if the given character is a whitespace.
*/
public static function isWhitespace(string $string): bool
public static function isWhitespace(string $character): bool
{
return $string === ' ' || $string === "\r" || $string === "\n" || $string === "\t";
return match ($character) {
' ', "\r", "\n", "\t" => true,
default => false,
};
}

/**
Expand All @@ -384,39 +386,20 @@
*/
public static function isComment(string $string, bool $end = false): int|null
{
if ($string === '') {
return null;
}

// If comment is Bash style (#):
if (str_starts_with($string, '#')) {
return Token::FLAG_COMMENT_BASH;
}

// If comment is a MySQL command
if (str_starts_with($string, '/*!')) {
return Token::FLAG_COMMENT_MYSQL_CMD;
}

// If comment is opening C style (/*) or is closing C style (*/), warning, it could conflict
// with wildcard and a real opening C style.
// It would look like the following valid SQL statement: "SELECT */* comment */ FROM...".
if (str_starts_with($string, '/*') || str_starts_with($string, '*/')) {
return Token::FLAG_COMMENT_C;
}

// If comment is SQL style (--\s?):
if (
return match (true) {

Check warning on line 389 in src/Context.php

View workflow job for this annotation

GitHub Actions / Mutation tests with PHP 8.2 

Escaped Mutant for Mutator "MatchArmRemoval":

--- Original
+++ New
@@ @@
     {
         return match (true) {
             str_starts_with($string, '#') => Token::FLAG_COMMENT_BASH,
-            str_starts_with($string, '/*!') => Token::FLAG_COMMENT_MYSQL_CMD,
             // If comment is opening C style (/*) or is closing C style (*/), warning, it could conflict
             // with wildcard and a real opening C style.
             // It would look like the following valid SQL statement: "SELECT */* comment */ FROM...".
str_starts_with($string, '#') => Token::FLAG_COMMENT_BASH,
str_starts_with($string, '/*!') => Token::FLAG_COMMENT_MYSQL_CMD,
// If comment is opening C style (/*) or is closing C style (*/), warning, it could conflict
// with wildcard and a real opening C style.
// It would look like the following valid SQL statement: "SELECT */* comment */ FROM...".
str_starts_with($string, '/*') || str_starts_with($string, '*/') => Token::FLAG_COMMENT_C,
str_starts_with($string, '-- ')
|| str_starts_with($string, "--\r")
|| str_starts_with($string, "--\n")
|| str_starts_with($string, "--\t")
|| ($string === '--' && $end)
) {
return Token::FLAG_COMMENT_SQL;
}

return null;
|| str_starts_with($string, "--\r")
|| str_starts_with($string, "--\n")
|| str_starts_with($string, "--\t")
|| ($string === '--' && $end) => Token::FLAG_COMMENT_SQL,
default => null,
};
}

/**
Expand Down Expand Up @@ -445,49 +428,30 @@
*
* @return int|null the appropriate flag for the symbol type
*/
public static function isSymbol(string $string): int|null
public static function isSymbol(string $character): int|null
{
if ($string === '') {
return null;
}

if (str_starts_with($string, '@')) {
return Token::FLAG_SYMBOL_VARIABLE;
}

if (str_starts_with($string, '`')) {
return Token::FLAG_SYMBOL_BACKTICK;
}

if (str_starts_with($string, ':') || str_starts_with($string, '?')) {
return Token::FLAG_SYMBOL_PARAMETER;
}

return null;
return match ($character) {
'@' => Token::FLAG_SYMBOL_VARIABLE,
'`' => Token::FLAG_SYMBOL_BACKTICK,
':', '?' => Token::FLAG_SYMBOL_PARAMETER,
default => null,
};
}

/**
* Checks if the given character is the beginning of a string.
*
* @param string $string string to be checked
* @param string $character a character to be checked
*
* @return int|null the appropriate flag for the string type
*/
public static function isString(string $string): int|null
public static function isString(string $character): int|null
{
if ($string === '') {
return null;
}

if (str_starts_with($string, '\'')) {
return Token::FLAG_STRING_SINGLE_QUOTES;
}

if (str_starts_with($string, '"')) {
return Token::FLAG_STRING_DOUBLE_QUOTES;
}

return null;
return match ($character) {
'\'' => Token::FLAG_STRING_SINGLE_QUOTES,
'"' => Token::FLAG_STRING_DOUBLE_QUOTES,
default => null,
};
}

/**
Expand All @@ -499,7 +463,7 @@
{
// NOTES: Only non-alphanumeric ASCII characters may be separators.
// `~` is the last printable ASCII character.
return $string <= '~'

Check warning on line 466 in src/Context.php

View workflow job for this annotation

GitHub Actions / Mutation tests with PHP 8.2 

Escaped Mutant for Mutator "LessThanOrEqualTo":

--- Original
+++ New
@@ @@
     {
         // NOTES:   Only non-alphanumeric ASCII characters may be separators.
         //          `~` is the last printable ASCII character.
-        return $string <= '~' && $string !== '_' && $string !== '$' && ($string < '0' || $string > '9') && ($string < 'a' || $string > 'z') && ($string < 'A' || $string > 'Z');
+        return $string < '~' && $string !== '_' && $string !== '$' && ($string < '0' || $string > '9') && ($string < 'a' || $string > 'z') && ($string < 'A' || $string > 'Z');
     }
     /**
      * Loads the specified context.
&& $string !== '_'
&& $string !== '$'
&& ($string < '0' || $string > '9')
Expand All @@ -522,22 +486,19 @@
$context = ContextMySql50700::class;
}

if (! class_exists($context)) {
if (! class_exists(self::$contextPrefix . $context)) {
return false;
}

// Could be the fully qualified class name was given, like `ContextDBMS::class`.
if (class_exists('\\' . $context)) {
$context = '\\' . $context;
} else {
// Short context name (must be formatted into class name).
$context = self::$contextPrefix . $context;
$contextClass = $context;
if (! class_exists($contextClass)) {
$contextClass = self::CONTEXT_PREFIX . $context;
if (! class_exists($contextClass)) {
$contextClass = '\\' . $context;

Check warning on line 493 in src/Context.php

View workflow job for this annotation

GitHub Actions / Mutation tests with PHP 8.2 

Escaped Mutant for Mutator "Concat":

--- Original
+++ New
@@ @@
         if (!class_exists($contextClass)) {
             $contextClass = self::CONTEXT_PREFIX . $context;
             if (!class_exists($contextClass)) {
-                $contextClass = '\\' . $context;
+                $contextClass = $context . '\\';
                 if (!class_exists($contextClass)) {
                     return false;
                 }

Check warning on line 493 in src/Context.php

View workflow job for this annotation

GitHub Actions / Mutation tests with PHP 8.2 

Escaped Mutant for Mutator "ConcatOperandRemoval":

--- Original
+++ New
@@ @@
         if (!class_exists($contextClass)) {
             $contextClass = self::CONTEXT_PREFIX . $context;
             if (!class_exists($contextClass)) {
-                $contextClass = '\\' . $context;
+                $contextClass = $context;
                 if (!class_exists($contextClass)) {
                     return false;
                 }
if (! class_exists($contextClass)) {
return false;
}
}
}

self::$loadedContext = $context;
self::$keywords = $context::$keywords;
self::$loadedContext = $contextClass;
self::$keywords = $contextClass::KEYWORDS;

return true;
}
Expand All @@ -557,19 +518,19 @@
public static function loadClosest(string $context = ''): string|null
{
$length = strlen($context);
for ($i = $length; $i > 0;) {

Check warning on line 521 in src/Context.php

View workflow job for this annotation

GitHub Actions / Mutation tests with PHP 8.2 

Escaped Mutant for Mutator "GreaterThan":

--- Original
+++ New
@@ @@
     public static function loadClosest(string $context = '') : string|null
     {
         $length = strlen($context);
-        for ($i = $length; $i > 0;) {
+        for ($i = $length; $i >= 0;) {
             /* Trying to load the new context */
             if (static::load($context)) {
                 return $context;
/* Trying to load the new context */
if (static::load($context)) {
return $context;
}

/* Replace last two non zero digits by zeroes */
do {

Check warning on line 528 in src/Context.php

View workflow job for this annotation

GitHub Actions / Mutation tests with PHP 8.2 

Escaped Mutant for Mutator "DoWhile":

--- Original
+++ New
@@ @@
                 if (!is_numeric($part)) {
                     break 2;
                 }
-            } while (intval($part) === 0 && $i > 0);
+            } while (false);
             $context = substr($context, 0, $i) . '00' . substr($context, $i + 2);
         }
         /* Fallback to loading at least matching engine */
$i -= 2;

Check warning on line 529 in src/Context.php

View workflow job for this annotation

GitHub Actions / Mutation tests with PHP 8.2 

Escaped Mutant for Mutator "DecrementInteger":

--- Original
+++ New
@@ @@
             }
             /* Replace last two non zero digits by zeroes */
             do {
-                $i -= 2;
+                $i -= 1;
                 $part = substr($context, $i, 2);
                 /* No more numeric parts to strip */
                 if (!is_numeric($part)) {
$part = substr($context, $i, 2);

Check warning on line 530 in src/Context.php

View workflow job for this annotation

GitHub Actions / Mutation tests with PHP 8.2 

Escaped Mutant for Mutator "DecrementInteger":

--- Original
+++ New
@@ @@
             /* Replace last two non zero digits by zeroes */
             do {
                 $i -= 2;
-                $part = substr($context, $i, 2);
+                $part = substr($context, $i, 1);
                 /* No more numeric parts to strip */
                 if (!is_numeric($part)) {
                     break 2;

Check warning on line 530 in src/Context.php

View workflow job for this annotation

GitHub Actions / Mutation tests with PHP 8.2 

Escaped Mutant for Mutator "IncrementInteger":

--- Original
+++ New
@@ @@
             /* Replace last two non zero digits by zeroes */
             do {
                 $i -= 2;
-                $part = substr($context, $i, 2);
+                $part = substr($context, $i, 3);
                 /* No more numeric parts to strip */
                 if (!is_numeric($part)) {
                     break 2;
/* No more numeric parts to strip */
if (! is_numeric($part)) {
break 2;

Check warning on line 533 in src/Context.php

View workflow job for this annotation

GitHub Actions / Mutation tests with PHP 8.2 

Escaped Mutant for Mutator "Break_":

--- Original
+++ New
@@ @@
                 $part = substr($context, $i, 2);
                 /* No more numeric parts to strip */
                 if (!is_numeric($part)) {
-                    break 2;
+                    continue;
                 }
             } while (intval($part) === 0 && $i > 0);
             $context = substr($context, 0, $i) . '00' . substr($context, $i + 2);
}
} while (intval($part) === 0 && $i > 0);

Expand Down Expand Up @@ -688,11 +649,7 @@
*/
public static function escapeAll(array $strings): array
{
foreach ($strings as $key => $value) {
$strings[$key] = static::escape($value);
}

return $strings;
return array_map(static::escape(...), $strings);
}

/**
Expand Down
6 changes: 2 additions & 4 deletions src/Contexts/ContextMariaDb100000.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace PhpMyAdmin\SqlParser\Contexts;

use PhpMyAdmin\SqlParser\Context;
use PhpMyAdmin\SqlParser\Token;

/**
Expand All @@ -15,7 +14,7 @@
*
* @see https://mariadb.com/kb/en/reserved-words/
*/
class ContextMariaDb100000 extends Context
final class ContextMariaDb100000
{
/**
* List of keywords.
Expand All @@ -24,11 +23,10 @@ class ContextMariaDb100000 extends Context
*
* @see Token
*
* @var array<string,int>
* @psalm-var non-empty-array<string,Token::FLAG_KEYWORD_*|int>
* @phpstan-var non-empty-array<non-empty-string,Token::FLAG_KEYWORD_*|int>
*/
public static array $keywords = [
public const KEYWORDS = [
'ACTION' => Token::FLAG_KEYWORD,
'AFTER' => Token::FLAG_KEYWORD,
'AGGREGATE' => Token::FLAG_KEYWORD,
Expand Down
6 changes: 2 additions & 4 deletions src/Contexts/ContextMariaDb100100.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace PhpMyAdmin\SqlParser\Contexts;

use PhpMyAdmin\SqlParser\Context;
use PhpMyAdmin\SqlParser\Token;

/**
Expand All @@ -15,7 +14,7 @@
*
* @see https://mariadb.com/kb/en/reserved-words/
*/
class ContextMariaDb100100 extends Context
final class ContextMariaDb100100
{
/**
* List of keywords.
Expand All @@ -24,11 +23,10 @@ class ContextMariaDb100100 extends Context
*
* @see Token
*
* @var array<string,int>
* @psalm-var non-empty-array<string,Token::FLAG_KEYWORD_*|int>
* @phpstan-var non-empty-array<non-empty-string,Token::FLAG_KEYWORD_*|int>
*/
public static array $keywords = [
public const KEYWORDS = [
'ACCOUNT' => Token::FLAG_KEYWORD,
'ACTION' => Token::FLAG_KEYWORD,
'AFTER' => Token::FLAG_KEYWORD,
Expand Down
6 changes: 2 additions & 4 deletions src/Contexts/ContextMariaDb100200.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace PhpMyAdmin\SqlParser\Contexts;

use PhpMyAdmin\SqlParser\Context;
use PhpMyAdmin\SqlParser\Token;

/**
Expand All @@ -15,7 +14,7 @@
*
* @see https://mariadb.com/kb/en/reserved-words/
*/
class ContextMariaDb100200 extends Context
final class ContextMariaDb100200
{
/**
* List of keywords.
Expand All @@ -24,11 +23,10 @@ class ContextMariaDb100200 extends Context
*
* @see Token
*
* @var array<string,int>
* @psalm-var non-empty-array<string,Token::FLAG_KEYWORD_*|int>
* @phpstan-var non-empty-array<non-empty-string,Token::FLAG_KEYWORD_*|int>
*/
public static array $keywords = [
public const KEYWORDS = [
'ACCOUNT' => Token::FLAG_KEYWORD,
'ACTION' => Token::FLAG_KEYWORD,
'AFTER' => Token::FLAG_KEYWORD,
Expand Down
Loading
Loading