From b1764f18db91dcac43473809013818487b2b0206 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Kr=C3=B6g?= Date: Thu, 9 May 2024 23:33:51 +0200 Subject: [PATCH] Don't add too many spaces when replacing clauses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/Utils/Query.php | 57 ++++++++++++++++++++++++++------------- tests/Utils/QueryTest.php | 10 +++---- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/src/Utils/Query.php b/src/Utils/Query.php index e80b17bf..fd5d0410 100644 --- 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 $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 index b1183a4e..446fadb8 100644 --- 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,