Skip to content

Commit 75679ca

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 75679ca

File tree

2 files changed

+41
-25
lines changed

2 files changed

+41
-25
lines changed

src/Utils/Query.php

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
use function count;
3737
use function in_array;
3838
use function is_string;
39+
use function preg_match;
3940
use function trim;
4041

4142
/**
@@ -726,6 +727,27 @@ public static function getClause($statement, $list, $clause, $type = 0, $skipFir
726727
return trim($ret);
727728
}
728729

730+
/**
731+
* @param list<string> $parts
732+
*/
733+
private static function glueQueryPartsWithSpaces(array $parts): string
734+
{
735+
$statement = '';
736+
foreach ($parts as $part) {
737+
if ($part === '') {
738+
continue;
739+
}
740+
741+
if ($statement !== '' && ! preg_match('/\\s$/', $statement) && ! preg_match('/^\\s/', $part)) {
742+
$statement .= ' ';
743+
}
744+
745+
$statement .= $part;
746+
}
747+
748+
return $statement;
749+
}
750+
729751
/**
730752
* Builds a query by rebuilding the statement from the tokens list supplied
731753
* and replaces a clause.
@@ -747,18 +769,17 @@ public static function replaceClause($statement, $list, $old, $new = null, $only
747769
{
748770
// TODO: Update the tokens list and the statement.
749771

750-
if ($new === null) {
751-
$new = $old;
752-
}
753-
772+
$parts = [
773+
static::getClause($statement, $list, $old, -1, false),
774+
$new ?? $old,
775+
];
754776
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);
777+
$parts[] = static::getClause($statement, $list, $old, 0);
758778
}
759779

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

764785
/**
@@ -782,35 +803,30 @@ public static function replaceClauses($statement, $list, array $ops)
782803
return '';
783804
}
784805

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

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

800814
// Doing replacements.
801815
foreach ($ops as $i => $clause) {
802-
$ret .= $clause[1] . ' ';
816+
$parts[] = $clause[1];
803817

804818
// Adding everything between this and next replacement.
805819
if ($i + 1 === $count) {
806820
continue;
807821
}
808822

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

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

816832
/**

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)