Refactor AbstractCommand::getRawSql() (#785)

* Refactor `AbstractCommand::getRawSql()` * Apply fixes from StyleCI * Update test * Remove unnecessary psalm annotation * Update type annotation * Add line to CHANGELOG.md * Update test * Improve * minor improvement --------- Co-authored-by: StyleCI Bot <bot@styleci.io> Co-authored-by: Sergei Predvoditelev <sergei@predvoditelev.ru>
yiisoft · Nov 26, 2023 · c4c4a9d · c4c4a9d
1 parent f06cf26
commit c4c4a9d
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,8 @@
   `DbArrayHelper::populate()` methods to `array[]` (@vjik)
 - Enh #779: Specify populate closure type in `BatchQueryResultInterface` (@vjik)
 - Enh #778: Deprecate unnecessary argument `$rawSql` of `AbstractCommand::internalExecute()` (@Tigrov)
+- Enh #785: Refactor `AbstractCommand::getRawSql()` (@Tigrov)
+- Bug #785: Fix bug of `AbstractCommand::getRawSql()` when a param value is `Stringable` object (@Tigrov)
 - Enh #786: Refactor `AbstractSchema::getDataType()` (@Tigrov)
 - Enh #784: Specify result type of `ConstraintSchemaInterface::getTableIndexes()` method to `IndexConstraint[]` (@vjik)
 - Enh #784: Remove unused code in `AbstractSchema::getTableIndexes()` (@vjik)

diff --git a/src/Command/AbstractCommand.php b/src/Command/AbstractCommand.php
@@ -16,15 +16,13 @@
 use function explode;
 use function get_resource_type;
 use function is_array;
-use function is_bool;
 use function is_int;
-use function is_object;
 use function is_resource;
 use function is_scalar;
 use function is_string;
 use function preg_replace_callback;
+use function str_starts_with;
 use function stream_get_contents;
-use function strncmp;
 
 /**
  * Represents an SQL statement to execute in a database.
@@ -110,12 +108,12 @@ abstract class AbstractCommand implements CommandInterface
      * @var string|null Transaction isolation level.
      */
     protected string|null $isolationLevel = null;
+
     /**
-     * @var array Parameters to use.
-     *
-     * @psalm-var ParamInterface[]
+     * @var ParamInterface[] Parameters to use.
      */
     protected array $params = [];
+
     /**
      * @var string|null Name of the table to refresh schema for. Null means not to refresh the schema.
      */
@@ -343,44 +341,42 @@ public function getRawSql(): string
         }
 
         $params = [];
+        $quoter = $this->getQueryBuilder()->quoter();
 
-        /** @psalm-var mixed $value */
-        foreach ($this->params as $name => $value) {
-            if (is_string($name) && strncmp(':', $name, 1)) {
+        foreach ($this->params as $name => $param) {
+            if (is_string($name) && !str_starts_with($name, ':')) {
                 $name = ':' . $name;
             }
 
-            if ($value instanceof ParamInterface) {
-                /** @psalm-var mixed $value */
-                $value = $value->getValue();
-            }
-
-            if (is_string($value)) {
-                /** @psalm-var mixed */
-                $params[$name] = $this->getQueryBuilder()->quoter()->quoteValue($value);
-            } elseif (is_bool($value)) {
-                /** @psalm-var string */
-                $params[$name] = $value ? 'TRUE' : 'FALSE';
-            } elseif ($value === null) {
-                $params[$name] = 'NULL';
-            } elseif ((!is_object($value) && !is_resource($value)) || $value instanceof Expression) {
-                /** @psalm-var mixed */
-                $params[$name] = $value;
-            }
+            $value = $param->getValue();
+
+            $params[$name] = match ($param->getType()) {
+                DataType::INTEGER => (string)$value,
+                DataType::STRING, DataType::LOB => match (true) {
+                    $value instanceof Expression => (string)$value,
+                    is_resource($value) => $name,
+                    default => $quoter->quoteValue((string)$value),
+                },
+                DataType::BOOLEAN => $value ? 'TRUE' : 'FALSE',
+                DataType::NULL => 'NULL',
+                default => $name,
+            };
         }
 
+        /** @var string[] $params */
         if (!isset($params[0])) {
-            return preg_replace_callback('#(:\w+)#', static function (array $matches) use ($params): string {
-                $m = $matches[1];
-                return (string)($params[$m] ?? $m);
-            }, $this->sql);
+            return preg_replace_callback(
+                '#(:\w+)#',
+                static fn (array $matches): string => $params[$matches[1]] ?? $matches[1],
+                $this->sql
+            );
         }
 
         // Support unnamed placeholders should be dropped
         $sql = '';
 
         foreach (explode('?', $this->sql) as $i => $part) {
-            $sql .= $part . (string)($params[$i] ?? '');
+            $sql .= $part . ($params[$i] ?? '');
         }
 
         return $sql;

diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php
@@ -8,6 +8,7 @@
 use Yiisoft\Db\Query\Query;
 use Yiisoft\Db\Schema\SchemaInterface;
 use Yiisoft\Db\Tests\Support\DbHelper;
+use Yiisoft\Db\Tests\Support\Stringable;
 use Yiisoft\Db\Tests\Support\TestTrait;
 
 class CommandProvider
@@ -637,6 +638,18 @@ public static function rawSql(): array
                     static::$driverName,
                 ),
             ],
+            [
+                <<<SQL
+                SELECT * FROM [[customer]] WHERE [[name]] = :name
+                SQL,
+                ['name' => new Stringable('Alfa')],
+                DbHelper::replaceQuotes(
+                    <<<SQL
+                    SELECT * FROM [[customer]] WHERE [[name]] = 'Alfa'
+                    SQL,
+                    static::$driverName,
+                ),
+            ],
         ];
     }
 

diff --git a/tests/Support/Stringable.php b/tests/Support/Stringable.php
@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Db\Tests\Support;
+
+class Stringable implements \Stringable
+{
+    public function __construct(private string $value)
+    {
+    }
+
+    public function __toString(): string
+    {
+        return $this->value;
+    }
+}