Skip to content

Commit 3a3656e

Browse files
committed
✨ Extended string normalization
Correctly normalize mixed string and interpolation. Use `{$c}` over `${c}` after PHP 8.2 deprecation.
1 parent 8bc567f commit 3a3656e

File tree

2 files changed

+92
-23
lines changed

2 files changed

+92
-23
lines changed

phpunit-tests/StringTest.php

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
namespace App\Tests;
66

7+
use Generator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
710
use function var_export;
811

912
/**
@@ -46,15 +49,16 @@ public function testDoubleQuotesWithEscapes(): void
4649

4750
public function testDoubleQuotesEncapsulation(): void
4851
{
52+
// `${a}` is deprecated since PHP8.2
4953
$code = <<<'CODE'
5054
<?php
51-
"encapsed $a or ${a}.";
55+
"encapsed $a or {$a} or ${a}.";
5256
CODE;
5357

5458
$this->assertRepresentation(
5559
$code,
5660
<<<'EOF'
57-
'encapsed ' . $v0 . ' or ' . $v0 . '.';
61+
'encapsed ' . $v0 . ' or ' . $v0 . ' or ' . $v0 . '.';
5862
EOF,
5963
'{"v0":"a"}',
6064
);
@@ -130,35 +134,56 @@ public function testNowdoc(): void
130134
);
131135
}
132136

133-
public function testUselessConcatenation(): void
137+
/** @return Generator<string, array{string, string}, void, void> */
138+
public static function uselessConcatenationProvider(): iterable
134139
{
135-
$code = <<<'CODE'
140+
yield 'basic' => ["'testA' . 'testB' . 'testC';", "'testAtestBtestC';"];
141+
yield 'right' => ["'testA' . ('testB' . 'testC');", "'testAtestBtestC';"];
142+
yield 'left' => ["('testA' . 'testB') . 'testC';", "'testAtestBtestC';"];
143+
yield 'both' => ["('testA' . 'testB') . ('testC' . 'testD');", "'testAtestBtestCtestD';"];
144+
}
145+
146+
#[DataProvider('uselessConcatenationProvider')]
147+
public function testUselessConcatenation(string $input, string $output): void
148+
{
149+
$code = <<<CODE
136150
<?php
137-
'testA' . 'testB' . 'testC';
151+
$input
138152
CODE;
139153

140154
$this->assertRepresentation(
141155
$code,
142-
<<<'EOF'
143-
'testAtestBtestC';
156+
<<<EOF
157+
$output
144158
EOF,
145159
'{}',
146160
);
147161
}
148162

149-
public function testUselessRightConcatenation(): void
163+
/** @return Generator<string, array{string, string}, void, void> */
164+
public static function uselessInterpolatedConcatenationProvider(): iterable
150165
{
151-
$code = <<<'CODE'
166+
yield 'left left' => ['"{$c}testA" . \'testB\';', '$v0 . \'testAtestB\';'];
167+
yield 'left right' => ['"testA{$c}" . \'testB\';', '\'testA\' . $v0 . \'testB\';'];
168+
yield 'right left' => ['\'testA\' . "{$c}testB";', '\'testA\' . $v0 . \'testB\';'];
169+
yield 'right right' => ['\'testA\' . "testB{$c}";', '\'testAtestB\' . $v0;'];
170+
yield 'left left and right right' => ['"{$c}testA" . "testB{$c}";', '$v0 . \'testAtestB\' . $v0;'];
171+
}
172+
173+
#[DataProvider('uselessInterpolatedConcatenationProvider')]
174+
public function testUselessInterpolatedConcatenation(string $input, string $output): void
175+
{
176+
$code = <<<CODE
152177
<?php
153-
'testA' . ('testB' . 'testC');
178+
$input
154179
CODE;
155180

156181
$this->assertRepresentation(
157182
$code,
158-
<<<'EOF'
159-
'testAtestBtestC';
183+
<<<EOF
184+
$output
160185
EOF,
161-
'{}',
186+
'{"v0":"c"}',
162187
);
163188
}
164189
}

src/NormalizeNodeVisitor.php

Lines changed: 54 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,11 @@
2828
use PhpParser\NodeVisitorAbstract;
2929

3030
use function array_map;
31+
use function array_push;
3132
use function array_shift;
33+
use function array_splice;
3234
use function assert;
35+
use function count;
3336
use function is_string;
3437

3538
/**
@@ -151,9 +154,7 @@ private function normalizeString(String_ $string): void
151154
private function normalizeInterpolatedString(InterpolatedString $string): Node
152155
{
153156
$parts = array_map(
154-
static fn (Node $part) => $part instanceof InterpolatedStringPart
155-
? new String_($part->value, ['kind' => String_::KIND_SINGLE_QUOTED])
156-
: $part,
157+
static fn (Node $part) => $part instanceof InterpolatedStringPart ? new String_($part->value) : $part,
157158
$string->parts,
158159
);
159160

@@ -169,16 +170,59 @@ private function normalizeInterpolatedString(InterpolatedString $string): Node
169170
/**
170171
* TRANSFORM: Simplify useless concat such as `'a' . 'b'` => `'ab'`
171172
*/
172-
private function simplifyUselessConcat(Concat $concat): String_|null
173+
private function simplifyUselessConcat(Concat $concat): String_|Concat
173174
{
174-
if ($concat->left instanceof String_ && $concat->right instanceof String_) {
175-
return new String_(
176-
$concat->left->value . $concat->right->value,
177-
['kind' => String_::KIND_SINGLE_QUOTED],
178-
);
175+
// 1. Flatten Concat-tree to an array of nodes: Concat('0', Concat('1', $a)) => ['0','1',$a]
176+
$nodes = $this->unwrapConcat($concat);
177+
// 2. Merge consecutive String_: ['0','1',$a,'2','3',$b,'4','5'] => ['01',$a,'23',$b,'45']
178+
$index = count($nodes) - 1;
179+
while ($index > 0) {
180+
$left = $nodes[$index - 1];
181+
$right = $nodes[$index];
182+
if ($left instanceof String_ && $right instanceof String_) {
183+
array_splice($nodes, $index - 1, 2, [new String_($left->value . $right->value)]);
184+
}
185+
186+
$index--;
179187
}
180188

181-
return null;
189+
// 3. Re-build a Concat-tree, left-based associativity to avoid extra-parentheses:
190+
// ['01',$a,'23'] => Concat(Concat('01', $a), '23')
191+
$node = array_shift($nodes);
192+
assert($node !== null, 'Concat has at least 1 node');
193+
while ($right = array_shift($nodes)) {
194+
$node = new Concat($node, $right);
195+
}
196+
197+
assert(
198+
$node instanceof String_ || $node instanceof Concat,
199+
'Either everything was collapsed to a singled String_ or we have a top-level Concat remaining',
200+
);
201+
202+
return $node;
203+
}
204+
205+
/**
206+
* Unwrap a tree of Concat to a flat array of nodes
207+
*
208+
* @return list<Node\Expr>
209+
*/
210+
private function unwrapConcat(Concat $concat): array
211+
{
212+
$nodes = [];
213+
if ($concat->left instanceof Concat) {
214+
array_push($nodes, ...$this->unwrapConcat($concat->left));
215+
} else {
216+
$nodes[] = $concat->left;
217+
}
218+
219+
if ($concat->right instanceof Concat) {
220+
array_push($nodes, ...$this->unwrapConcat($concat->right));
221+
} else {
222+
$nodes[] = $concat->right;
223+
}
224+
225+
return $nodes;
182226
}
183227

184228
/**

0 commit comments

Comments
 (0)