Skip to content

Commit f042584

Browse files
spawniaspawnia
authored
Preserve NodeList lazyness (#1321)
1 parent 55180f9 commit f042584

File tree

4 files changed

+58
-68
lines changed

4 files changed

+58
-68
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ You can find and compare releases at the [GitHub release page](https://github.co
99

1010
## Unreleased
1111

12+
### Changed
13+
14+
- Make `NodeList` an actual list
15+
1216
## v15.1.0
1317

1418
### Added

‎src/Language/AST/NodeList.php‎

Lines changed: 29 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,20 @@
1212
class NodeList implements \IteratorAggregate, \Countable
1313
{
1414
/**
15-
* @var array<Node>
15+
* @var list<Node|array<string, mixed>>
1616
*
17-
* @phpstan-var list<T>
17+
* @phpstan-var list<T|array<string, mixed>> $nodes
1818
*/
19-
private array $nodes = [];
19+
protected array $nodes;
2020

2121
/**
22-
* @param iterable<Node|array> $nodes
22+
* @param list<Node|array<string, mixed>> $nodes
2323
*
24-
* @phpstan-param iterable<T|array<string, mixed>> $nodes
24+
* @phpstan-param list<T|array<string, mixed>> $nodes
2525
*/
26-
public function __construct(iterable $nodes)
26+
public function __construct(array $nodes)
2727
{
28-
foreach ($nodes as $node) {
29-
$this->nodes[] = $this->process($node);
30-
}
28+
$this->nodes = $nodes;
3129
}
3230

3331
public function has(int $offset): bool
@@ -40,7 +38,12 @@ public function has(int $offset): bool
4038
*/
4139
public function get(int $offset): Node
4240
{
43-
return $this->nodes[$offset];
41+
$node = $this->nodes[$offset];
42+
43+
// @phpstan-ignore-next-line not really possible to express the correctness of this in PHP
44+
return \is_array($node)
45+
? ($this->nodes[$offset] = AST::fromArray($node))
46+
: $node;
4447
}
4548

4649
/**
@@ -61,37 +64,17 @@ public function add(Node $value): void
6164
$this->nodes[] = $value;
6265
}
6366

64-
/**
65-
* @param Node|array<string, mixed> $value
66-
*
67-
* @phpstan-param T|array<string, mixed> $value
68-
*
69-
* @phpstan-return T
70-
*/
71-
private function process($value): Node
72-
{
73-
if (\is_array($value)) {
74-
/** @phpstan-var T $value */
75-
$value = AST::fromArray($value);
76-
}
77-
78-
return $value;
79-
}
80-
81-
public function remove(Node $node): void
67+
public function unset(int $offset): void
8268
{
83-
$foundKey = \array_search($node, $this->nodes, true);
84-
if ($foundKey === false) {
85-
throw new \InvalidArgumentException('Node not found in NodeList');
86-
}
87-
88-
unset($this->nodes[$foundKey]);
69+
unset($this->nodes[$offset]);
8970
$this->nodes = \array_values($this->nodes);
9071
}
9172

9273
public function getIterator(): \Traversable
9374
{
94-
yield from $this->nodes;
75+
foreach ($this->nodes as $key => $_) {
76+
yield $key => $this->get($key);
77+
}
9578
}
9679

9780
public function count(): int
@@ -102,15 +85,18 @@ public function count(): int
10285
/**
10386
* Remove a portion of the NodeList and replace it with something else.
10487
*
105-
* @param T|array<T>|null $replacement
88+
* @param Node|array<Node>|null $replacement
89+
*
90+
* @phpstan-param T|array<T>|null $replacement
10691
*
10792
* @phpstan-return NodeList<T> the NodeList with the extracted elements
10893
*/
10994
public function splice(int $offset, int $length, $replacement = null): NodeList
11095
{
111-
return new NodeList(
112-
\array_splice($this->nodes, $offset, $length, $replacement)
113-
);
96+
$spliced = \array_splice($this->nodes, $offset, $length, $replacement);
97+
98+
// @phpstan-ignore-next-line generic type mismatch
99+
return new NodeList($spliced);
114100
}
115101

116102
/**
@@ -124,7 +110,10 @@ public function merge(iterable $list): NodeList
124110
$list = \iterator_to_array($list);
125111
}
126112

127-
return new NodeList(\array_merge($this->nodes, $list));
113+
$merged = \array_merge($this->nodes, $list);
114+
115+
// @phpstan-ignore-next-line generic type mismatch
116+
return new NodeList($merged);
128117
}
129118

130119
/**

‎src/Language/Visitor.php‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -174,13 +174,13 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null
174174

175175
/**
176176
* @var list<array{
177-
* inArray: bool,
177+
* inList: bool,
178178
* index: int,
179179
* keys: Node|NodeList|mixed,
180180
* edits: array<int, array{mixed, mixed}>,
181181
* }> $stack */
182182
$stack = [];
183-
$inArray = $root instanceof NodeList;
183+
$inList = $root instanceof NodeList;
184184
$keys = [$root];
185185
$index = -1;
186186
$edits = [];
@@ -210,12 +210,12 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null
210210

211211
$editOffset = 0;
212212
foreach ($edits as [$editKey, $editValue]) {
213-
if ($inArray) {
213+
if ($inList) {
214214
$editKey -= $editOffset;
215215
}
216216

217-
if ($inArray && $editValue === null) {
218-
assert($node instanceof NodeList, 'Follows from $inArray');
217+
if ($inList && $editValue === null) {
218+
assert($node instanceof NodeList, 'Follows from $inList');
219219
$node->splice($editKey, 1);
220220
++$editOffset;
221221
} elseif ($node instanceof NodeList) {
@@ -236,23 +236,19 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null
236236
'index' => $index,
237237
'keys' => $keys,
238238
'edits' => $edits,
239-
'inArray' => $inArray,
239+
'inList' => $inList,
240240
] = \array_pop($stack);
241241
} else {
242-
$key = $parent !== null
243-
? (
244-
$inArray
245-
? $index
246-
: $keys[$index]
247-
)
248-
: null;
249-
$node = $parent !== null
250-
? (
251-
$parent instanceof NodeList
252-
? $parent->get($key)
253-
: $parent->{$key}
254-
)
255-
: $newRoot;
242+
if ($parent === null) {
243+
$node = $newRoot;
244+
} else {
245+
$key = $inList
246+
? $index
247+
: $keys[$index];
248+
$node = $parent instanceof NodeList
249+
? $parent->get($key)
250+
: $parent->{$key};
251+
}
256252
if ($node === null) {
257253
continue;
258254
}
@@ -310,14 +306,14 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null
310306
\array_pop($path);
311307
} else {
312308
$stack[] = [
313-
'inArray' => $inArray,
309+
'inList' => $inList,
314310
'index' => $index,
315311
'keys' => $keys,
316312
'edits' => $edits,
317313
];
318-
$inArray = $node instanceof NodeList;
314+
$inList = $node instanceof NodeList;
319315

320-
$keys = ($inArray ? $node : $visitorKeys[$node->kind]) ?? [];
316+
$keys = ($inList ? $node : $visitorKeys[$node->kind]) ?? [];
321317
$index = -1;
322318
$edits = [];
323319
if ($parent !== null) {

‎tests/Language/NodeListTest.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,11 @@ private static function assertNotSameButEquals(object $node, object $clone): voi
4141

4242
public function testThrowsOnInvalidArrays(): void
4343
{
44-
$this->expectException(InvariantViolation::class);
45-
4644
// @phpstan-ignore-next-line Wrong on purpose
47-
new NodeList([['not a valid array representation of an AST node']]);
45+
$nodeList = new NodeList([['not a valid array representation of an AST node']]);
46+
47+
$this->expectException(InvariantViolation::class);
48+
iterator_to_array($nodeList);
4849
}
4950

5051
public function testAddNodes(): void
@@ -60,13 +61,13 @@ public function testAddNodes(): void
6061
self::assertCount(2, $nodeList);
6162
}
6263

63-
public function testRemoveDoesNotBreakList(): void
64+
public function testUnsetDoesNotBreakList(): void
6465
{
6566
$foo = new NameNode(['value' => 'foo']);
6667
$bar = new NameNode(['value' => 'bar']);
6768

6869
$nodeList = new NodeList([$foo, $bar]);
69-
$nodeList->remove($foo);
70+
$nodeList->unset(0);
7071

7172
self::assertTrue($nodeList->has(0));
7273
self::assertSame($bar, $nodeList->get(0));

0 commit comments

Comments
 (0)