Don't add too many spaces when replacing clauses

When a clause is removed (replaced with empty string) or the replacement string already has spaces or the replacement was at the start or end of the statement there were unnecessary spaced included in the statement. Signed-off-by: Maximilian Krög <maxi_kroeg@web.de>
phpmyadmin · Jun 2, 2024 · b1764f1 · b1764f1
1 parent a472187
commit b1764f1
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
@@ -33,8 +33,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;
 
 /**
@@ -476,6 +478,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.
@@ -500,18 +525,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);
     }
 
     /**
@@ -533,33 +557,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,
@@ -572,7 +572,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,
@@ -588,7 +588,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,
@@ -629,9 +629,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,