Skip to content

Commit 4945c8f

Browse files
committed
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>
1 parent 29f982a commit 4945c8f

File tree

2 files changed

+42
-25
lines changed

2 files changed

+42
-25
lines changed

src/Utils/Query.php

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@
3434
use function array_flip;
3535
use function array_keys;
3636
use function count;
37+
use function ctype_space;
3738
use function in_array;
3839
use function is_string;
40+
use function mb_substr;
3941
use function trim;
4042

4143
/**
@@ -726,6 +728,27 @@ public static function getClause($statement, $list, $clause, $type = 0, $skipFir
726728
return trim($ret);
727729
}
728730

731+
/**
732+
* @param list<string> $parts
733+
*/
734+
private static function glueQueryPartsWithSpaces(array $parts): string
735+
{
736+
$statement = '';
737+
foreach ($parts as $part) {
738+
if ($part === '') {
739+
continue;
740+
}
741+
742+
if ($statement !== '' && ! ctype_space(mb_substr($statement, -1)) && ! ctype_space(mb_substr($part, 0, 1))) {
743+
$statement .= ' ';
744+
}
745+
746+
$statement .= $part;
747+
}
748+
749+
return $statement;
750+
}
751+
729752
/**
730753
* Builds a query by rebuilding the statement from the tokens list supplied
731754
* and replaces a clause.
@@ -747,18 +770,17 @@ public static function replaceClause($statement, $list, $old, $new = null, $only
747770
{
748771
// TODO: Update the tokens list and the statement.
749772

750-
if ($new === null) {
751-
$new = $old;
752-
}
753-
773+
$parts = [
774+
static::getClause($statement, $list, $old, -1, false),
775+
$new ?? $old,
776+
];
754777
if ($onlyType) {
755-
return static::getClause($statement, $list, $old, -1, false) . ' ' .
756-
$new . ' ' . static::getClause($statement, $list, $old, 0) . ' ' .
757-
static::getClause($statement, $list, $old, 1, false);
778+
$parts[] = static::getClause($statement, $list, $old, 0);
758779
}
759780

760-
return static::getClause($statement, $list, $old, -1, false) . ' ' .
761-
$new . ' ' . static::getClause($statement, $list, $old, 1, false);
781+
$parts[] = static::getClause($statement, $list, $old, 1, false);
782+
783+
return self::glueQueryPartsWithSpaces($parts);
762784
}
763785

764786
/**
@@ -782,35 +804,30 @@ public static function replaceClauses($statement, $list, array $ops)
782804
return '';
783805
}
784806

785-
/**
786-
* Value to be returned.
787-
*
788-
* @var string
789-
*/
790-
$ret = '';
791-
792807
// If there is only one clause, `replaceClause()` should be used.
793808
if ($count === 1) {
794809
return static::replaceClause($statement, $list, $ops[0][0], $ops[0][1]);
795810
}
796811

797812
// Adding everything before first replacement.
798-
$ret .= static::getClause($statement, $list, $ops[0][0], -1) . ' ';
813+
$parts = [static::getClause($statement, $list, $ops[0][0], -1)];
799814

800815
// Doing replacements.
801816
foreach ($ops as $i => $clause) {
802-
$ret .= $clause[1] . ' ';
817+
$parts[] = $clause[1];
803818

804819
// Adding everything between this and next replacement.
805820
if ($i + 1 === $count) {
806821
continue;
807822
}
808823

809-
$ret .= static::getClause($statement, $list, $clause[0], $ops[$i + 1][0]) . ' ';
824+
$parts[] = static::getClause($statement, $list, $clause[0], $ops[$i + 1][0]);
810825
}
811826

812827
// Adding everything after the last replacement.
813-
return $ret . static::getClause($statement, $list, $ops[$count - 1][0], 1);
828+
$parts[] = static::getClause($statement, $list, $ops[$count - 1][0], 1);
829+
830+
return self::glueQueryPartsWithSpaces($parts);
814831
}
815832

816833
/**

tests/Utils/QueryTest.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -593,7 +593,7 @@ public function testReplaceClause(): void
593593
$this->assertEquals(
594594
'select supplier.city, supplier.id from supplier '
595595
. 'union select customer.city, customer.id from customer'
596-
. ' ORDER BY city ',
596+
. ' ORDER BY city',
597597
Query::replaceClause(
598598
$parser->statements[0],
599599
$parser->list,
@@ -606,7 +606,7 @@ public function testReplaceClauseOnlyKeyword(): void
606606
{
607607
$parser = new Parser('SELECT *, (SELECT 1) FROM film LIMIT 0, 10');
608608
$this->assertEquals(
609-
' SELECT SQL_CALC_FOUND_ROWS *, (SELECT 1) FROM film LIMIT 0, 10',
609+
'SELECT SQL_CALC_FOUND_ROWS *, (SELECT 1) FROM film LIMIT 0, 10',
610610
Query::replaceClause(
611611
$parser->statements[0],
612612
$parser->list,
@@ -621,7 +621,7 @@ public function testReplaceNonExistingPart(): void
621621
{
622622
$parser = new Parser('ALTER TABLE `sale_mast` OPTIMIZE PARTITION p3');
623623
$this->assertEquals(
624-
' ALTER TABLE `sale_mast` OPTIMIZE PARTITION p3',
624+
'ALTER TABLE `sale_mast` OPTIMIZE PARTITION p3',
625625
Query::replaceClause(
626626
$parser->statements[0],
627627
$parser->list,
@@ -660,9 +660,9 @@ public function testReplaceClauses(): void
660660
$this->assertEquals(
661661
'SELECT c.city_id, c.country_id ' .
662662
'INTO OUTFILE "/dev/null" ' .
663-
'FROM city AS c ' .
663+
'FROM city AS c ' .
664664
'ORDER BY city_id ASC ' .
665-
'LIMIT 0, 10 ',
665+
'LIMIT 0, 10',
666666
Query::replaceClauses(
667667
$parser->statements[0],
668668
$parser->list,

0 commit comments

Comments
 (0)