Merge pull request #558 from MoonE/replace-clause-spaces

Don't add too many spaces when replacing clauses
phpmyadmin · Aug 25, 2024 · f2bafbb · f2bafbb
2 parents 3e6d4f2 + b1764f1
commit f2bafbb
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 23 deletions.
diff --git a/src/Utils/Query.php b/src/Utils/Query.php
@@ -34,8 +34,10 @@
 use function array_flip;
 use function array_keys;
 use function count;
+use function ctype_space;
 use function in_array;
 use function is_string;
+use function mb_substr;
 use function trim;
 
 /**
@@ -479,6 +481,29 @@ public static function getClause(
         return trim($ret);
     }
 
+    /** @param list<string> $parts */
+    private static function glueQueryPartsWithSpaces(array $parts): string
+    {
+        $statement = '';
+        foreach ($parts as $part) {
+            if ($part === '') {
+                continue;
+            }
+
+            if (
+                $statement !== '' &&
+                ! ctype_space(mb_substr($statement, -1)) &&
+                ! ctype_space(mb_substr($part, 0, 1))
+            ) {
+                $statement .= ' ';
+            }
+
+            $statement .= $part;
+        }
+
+        return $statement;
+    }
+
     /**
      * Builds a query by rebuilding the statement from the tokens list supplied
      * and replaces a clause.
@@ -503,18 +528,17 @@ public static function replaceClause(
     ): string {
         // TODO: Update the tokens list and the statement.
 
-        if ($new === null) {
-            $new = $old;
-        }
-
+        $parts = [
+            static::getClause($statement, $list, $old, -1, false),
+            $new ?? $old,
+        ];
         if ($onlyType) {
-            return static::getClause($statement, $list, $old, -1, false) . ' ' .
-                $new . ' ' . static::getClause($statement, $list, $old, 0) . ' ' .
-                static::getClause($statement, $list, $old, 1, false);
+            $parts[] = static::getClause($statement, $list, $old, 0);
         }
 
-        return static::getClause($statement, $list, $old, -1, false) . ' ' .
-            $new . ' ' . static::getClause($statement, $list, $old, 1, false);
+        $parts[] = static::getClause($statement, $list, $old, 1, false);
+
+        return self::glueQueryPartsWithSpaces($parts);
     }
 
     /**
@@ -536,33 +560,30 @@ public static function replaceClauses(Statement $statement, TokensList $list, ar
             return '';
         }
 
-        /**
-         * Value to be returned.
-         */
-        $ret = '';
-
         // If there is only one clause, `replaceClause()` should be used.
         if ($count === 1) {
             return static::replaceClause($statement, $list, $ops[0][0], $ops[0][1]);
         }
 
         // Adding everything before first replacement.
-        $ret .= static::getClause($statement, $list, $ops[0][0], -1) . ' ';
+        $parts = [static::getClause($statement, $list, $ops[0][0], -1)];
 
         // Doing replacements.
         foreach ($ops as $i => $clause) {
-            $ret .= $clause[1] . ' ';
+            $parts[] = $clause[1];
 
             // Adding everything between this and next replacement.
             if ($i + 1 === $count) {
                 continue;
             }
 
-            $ret .= static::getClause($statement, $list, $clause[0], $ops[$i + 1][0]) . ' ';
+            $parts[] = static::getClause($statement, $list, $clause[0], $ops[$i + 1][0]);
         }
 
         // Adding everything after the last replacement.
-        return $ret . static::getClause($statement, $list, $ops[$count - 1][0], 1);
+        $parts[] = static::getClause($statement, $list, $ops[$count - 1][0], 1);
+
+        return self::glueQueryPartsWithSpaces($parts);
     }
 
     /**

diff --git a/tests/Utils/QueryTest.php b/tests/Utils/QueryTest.php
@@ -558,7 +558,7 @@ public function testReplaceClause(): void
         $this->assertEquals(
             'select supplier.city, supplier.id from supplier '
             . 'union select customer.city, customer.id from customer'
-            . ' ORDER BY city ',
+            . ' ORDER BY city',
             Query::replaceClause(
                 $parser->statements[0],
                 $parser->list,
@@ -582,7 +582,7 @@ public function testReplaceClauseOnlyKeyword(): void
         $parser = new Parser('SELECT *, (SELECT 1) FROM film LIMIT 0, 10');
         $this->assertNotNull($parser->list);
         $this->assertEquals(
-            ' SELECT SQL_CALC_FOUND_ROWS *, (SELECT 1) FROM film LIMIT 0, 10',
+            'SELECT SQL_CALC_FOUND_ROWS *, (SELECT 1) FROM film LIMIT 0, 10',
             Query::replaceClause(
                 $parser->statements[0],
                 $parser->list,
@@ -598,7 +598,7 @@ public function testReplaceNonExistingPart(): void
         $parser = new Parser('ALTER TABLE `sale_mast` OPTIMIZE PARTITION p3');
         $this->assertNotNull($parser->list);
         $this->assertEquals(
-            '  ALTER TABLE `sale_mast` OPTIMIZE PARTITION p3',
+            'ALTER TABLE `sale_mast` OPTIMIZE PARTITION p3',
             Query::replaceClause(
                 $parser->statements[0],
                 $parser->list,
@@ -639,9 +639,9 @@ public function testReplaceClauses(): void
         $this->assertEquals(
             'SELECT c.city_id, c.country_id ' .
             'INTO OUTFILE "/dev/null" ' .
-            'FROM city AS c   ' .
+            'FROM city AS c ' .
             'ORDER BY city_id ASC ' .
-            'LIMIT 0, 10 ',
+            'LIMIT 0, 10',
             Query::replaceClauses(
                 $parser->statements[0],
                 $parser->list,