From c60b263c41eb145c20f7f6cdc2ddd439d23f620f Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 21 Feb 2023 15:23:16 +0100 Subject: [PATCH 1/2] Preserve NodeList lazyness --- CHANGELOG.md | 4 ++ src/Language/AST/NodeList.php | 68 ++++++++++++++------------------- src/Language/Visitor.php | 42 +++++++++----------- tests/Language/NodeListTest.php | 11 +++--- 4 files changed, 57 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f91726107..36bf60c8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +### Changed + +- Make `NodeList` an actual list + ## v15.1.0 ### Added diff --git a/src/Language/AST/NodeList.php b/src/Language/AST/NodeList.php index 85a848331..3e78f3b01 100644 --- a/src/Language/AST/NodeList.php +++ b/src/Language/AST/NodeList.php @@ -12,22 +12,20 @@ class NodeList implements \IteratorAggregate, \Countable { /** - * @var array + * @var list> * - * @phpstan-var list + * @phpstan-var list> $nodes */ - private array $nodes = []; + protected array $nodes; /** - * @param iterable $nodes + * @param list> $nodes * - * @phpstan-param iterable> $nodes + * @phpstan-param list> $nodes */ - public function __construct(iterable $nodes) + public function __construct(array $nodes) { - foreach ($nodes as $node) { - $this->nodes[] = $this->process($node); - } + $this->nodes = $nodes; } public function has(int $offset): bool @@ -40,7 +38,12 @@ public function has(int $offset): bool */ public function get(int $offset): Node { - return $this->nodes[$offset]; + $node = $this->nodes[$offset]; + + // @phpstan-ignore-next-line not really possible to express the correctness of this in PHP + return \is_array($node) + ? ($this->nodes[$offset] = AST::fromArray($node)) + : $node; } /** @@ -61,37 +64,17 @@ public function add(Node $value): void $this->nodes[] = $value; } - /** - * @param Node|array $value - * - * @phpstan-param T|array $value - * - * @phpstan-return T - */ - private function process($value): Node + public function unset(int $offset): void { - if (\is_array($value)) { - /** @phpstan-var T $value */ - $value = AST::fromArray($value); - } - - return $value; - } - - public function remove(Node $node): void - { - $foundKey = \array_search($node, $this->nodes, true); - if ($foundKey === false) { - throw new \InvalidArgumentException('Node not found in NodeList'); - } - - unset($this->nodes[$foundKey]); + unset($this->nodes[$offset]); $this->nodes = \array_values($this->nodes); } public function getIterator(): \Traversable { - yield from $this->nodes; + foreach ($this->nodes as $key => $_) { + yield $key => $this->get($key); + } } public function count(): int @@ -102,15 +85,17 @@ public function count(): int /** * Remove a portion of the NodeList and replace it with something else. * - * @param T|array|null $replacement + * @param Node|array|null $replacement + * @phpstan-param T|array|null $replacement * * @phpstan-return NodeList the NodeList with the extracted elements */ public function splice(int $offset, int $length, $replacement = null): NodeList { - return new NodeList( - \array_splice($this->nodes, $offset, $length, $replacement) - ); + $spliced = \array_splice($this->nodes, $offset, $length, $replacement); + + // @phpstan-ignore-next-line generic type mismatch + return new NodeList($spliced); } /** @@ -124,7 +109,10 @@ public function merge(iterable $list): NodeList $list = \iterator_to_array($list); } - return new NodeList(\array_merge($this->nodes, $list)); + $merged = \array_merge($this->nodes, $list); + + // @phpstan-ignore-next-line generic type mismatch + return new NodeList($merged); } /** diff --git a/src/Language/Visitor.php b/src/Language/Visitor.php index 223d38953..bc38ef1e8 100644 --- a/src/Language/Visitor.php +++ b/src/Language/Visitor.php @@ -174,13 +174,13 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null /** * @var list, * }> $stack */ $stack = []; - $inArray = $root instanceof NodeList; + $inList = $root instanceof NodeList; $keys = [$root]; $index = -1; $edits = []; @@ -210,12 +210,12 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null $editOffset = 0; foreach ($edits as [$editKey, $editValue]) { - if ($inArray) { + if ($inList) { $editKey -= $editOffset; } - if ($inArray && $editValue === null) { - assert($node instanceof NodeList, 'Follows from $inArray'); + if ($inList && $editValue === null) { + assert($node instanceof NodeList, 'Follows from $inList'); $node->splice($editKey, 1); ++$editOffset; } elseif ($node instanceof NodeList) { @@ -236,23 +236,19 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null 'index' => $index, 'keys' => $keys, 'edits' => $edits, - 'inArray' => $inArray, + 'inList' => $inList, ] = \array_pop($stack); } else { - $key = $parent !== null - ? ( - $inArray - ? $index - : $keys[$index] - ) - : null; - $node = $parent !== null - ? ( - $parent instanceof NodeList - ? $parent->get($key) - : $parent->{$key} - ) - : $newRoot; + if ($parent === null) { + $node = $newRoot; + } else { + $key = $inList + ? $index + : $keys[$index]; + $node = $parent instanceof NodeList + ? $parent->get($key) + : $parent->{$key}; + } if ($node === null) { continue; } @@ -310,14 +306,14 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null \array_pop($path); } else { $stack[] = [ - 'inArray' => $inArray, + 'inList' => $inList, 'index' => $index, 'keys' => $keys, 'edits' => $edits, ]; - $inArray = $node instanceof NodeList; + $inList = $node instanceof NodeList; - $keys = ($inArray ? $node : $visitorKeys[$node->kind]) ?? []; + $keys = ($inList ? $node : $visitorKeys[$node->kind]) ?? []; $index = -1; $edits = []; if ($parent !== null) { diff --git a/tests/Language/NodeListTest.php b/tests/Language/NodeListTest.php index 124e6d3cf..598b7be02 100644 --- a/tests/Language/NodeListTest.php +++ b/tests/Language/NodeListTest.php @@ -41,10 +41,11 @@ private static function assertNotSameButEquals(object $node, object $clone): voi public function testThrowsOnInvalidArrays(): void { - $this->expectException(InvariantViolation::class); - // @phpstan-ignore-next-line Wrong on purpose - new NodeList([['not a valid array representation of an AST node']]); + $nodeList = new NodeList([['not a valid array representation of an AST node']]); + + $this->expectException(InvariantViolation::class); + iterator_to_array($nodeList); } public function testAddNodes(): void @@ -60,13 +61,13 @@ public function testAddNodes(): void self::assertCount(2, $nodeList); } - public function testRemoveDoesNotBreakList(): void + public function testUnsetDoesNotBreakList(): void { $foo = new NameNode(['value' => 'foo']); $bar = new NameNode(['value' => 'bar']); $nodeList = new NodeList([$foo, $bar]); - $nodeList->remove($foo); + $nodeList->unset(0); self::assertTrue($nodeList->has(0)); self::assertSame($bar, $nodeList->get(0)); From 5cbdc86762030885b838f847ca138833ef7ab8cf Mon Sep 17 00:00:00 2001 From: spawnia Date: Tue, 21 Feb 2023 14:24:16 +0000 Subject: [PATCH 2/2] Apply php-cs-fixer changes --- src/Language/AST/NodeList.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Language/AST/NodeList.php b/src/Language/AST/NodeList.php index 3e78f3b01..75e4767b1 100644 --- a/src/Language/AST/NodeList.php +++ b/src/Language/AST/NodeList.php @@ -86,6 +86,7 @@ public function count(): int * Remove a portion of the NodeList and replace it with something else. * * @param Node|array|null $replacement + * * @phpstan-param T|array|null $replacement * * @phpstan-return NodeList the NodeList with the extracted elements