Skip to content

Commit 6d94b47

Browse files
simPodsimPod
committed
refactor: make NodeList an actual list
1 parent 76e1656 commit 6d94b47

File tree

13 files changed

+113
-112
lines changed

13 files changed

+113
-112
lines changed

‎src/Language/AST/NodeList.php

Lines changed: 52 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -7,93 +7,100 @@
77
/**
88
* @template T of Node
99
*
10-
* @phpstan-implements \ArrayAccess<array-key, T>
11-
* @phpstan-implements \IteratorAggregate<array-key, T>
10+
* @phpstan-implements \IteratorAggregate<int, T>
1211
*/
13-
class NodeList implements \ArrayAccess, \IteratorAggregate, \Countable
12+
class NodeList implements \IteratorAggregate, \Countable
1413
{
1514
/**
16-
* @var array<Node|array>
15+
* @var array<Node>
1716
*
18-
* @phpstan-var array<T|array<string, mixed>>
17+
* @phpstan-var list<T>
1918
*/
20-
private array $nodes;
19+
private array $nodes = [];
2120

2221
/**
23-
* @param array<Node|array> $nodes
22+
* @param iterable<Node|array> $nodes
2423
*
25-
* @phpstan-param array<T|array<string, mixed>> $nodes
24+
* @phpstan-param iterable<T|array<string, mixed>> $nodes
2625
*/
27-
public function __construct(array $nodes)
26+
public function __construct(iterable $nodes)
2827
{
29-
$this->nodes = $nodes;
28+
foreach ($nodes as $node) {
29+
$this->add($node);
30+
}
3031
}
3132

3233
/**
3334
* @param int|string $offset
3435
*/
35-
#[\ReturnTypeWillChange]
36-
public function offsetExists($offset): bool
36+
public function has($offset): bool
3737
{
3838
return isset($this->nodes[$offset]);
3939
}
4040

4141
/**
42-
* @param int|string $offset
43-
*
4442
* @phpstan-return T
4543
*/
46-
#[\ReturnTypeWillChange]
47-
public function offsetGet($offset): Node
44+
public function get(int $offset): Node
4845
{
49-
$item = $this->nodes[$offset];
46+
return $this->nodes[$offset];
47+
}
5048

51-
if (\is_array($item)) {
52-
// @phpstan-ignore-next-line not really possible to express the correctness of this in PHP
53-
return $this->nodes[$offset] = AST::fromArray($item);
54-
}
49+
/**
50+
* @param Node|array<string, mixed> $value
51+
*
52+
* @phpstan-param T|array<string, mixed> $value
53+
*/
54+
public function set(int $offset, $value): void
55+
{
56+
$this->nodes[$offset] = $this->process($value);
57+
58+
assert($this->nodes === \array_values($this->nodes));
59+
}
60+
61+
/**
62+
* @param Node|array<string, mixed> $value
63+
*
64+
* @phpstan-param T|array<string, mixed> $value
65+
*/
66+
public function add($value): void
67+
{
68+
$value = $this->process($value);
5569

56-
return $item;
70+
$this->nodes[] = $value;
5771
}
5872

5973
/**
60-
* @param int|string|null $offset
6174
* @param Node|array<string, mixed> $value
6275
*
6376
* @phpstan-param T|array<string, mixed> $value
77+
*
78+
* @phpstan-return T
6479
*/
65-
#[\ReturnTypeWillChange]
66-
public function offsetSet($offset, $value): void
80+
private function process($value): Node
6781
{
6882
if (\is_array($value)) {
6983
/** @phpstan-var T $value */
7084
$value = AST::fromArray($value);
7185
}
7286

73-
// Happens when a Node is pushed via []=
74-
if ($offset === null) {
75-
$this->nodes[] = $value;
76-
77-
return;
78-
}
79-
80-
$this->nodes[$offset] = $value;
87+
return $value;
8188
}
8289

83-
/**
84-
* @param int|string $offset
85-
*/
86-
#[\ReturnTypeWillChange]
87-
public function offsetUnset($offset): void
90+
public function remove(Node $node): void
8891
{
89-
unset($this->nodes[$offset]);
92+
$foundKey = \array_search($node, $this->nodes, true);
93+
if ($foundKey === false) {
94+
throw new \InvalidArgumentException('Node not found in NodeList');
95+
}
96+
97+
unset($this->nodes[$foundKey]);
98+
$this->nodes = \array_values($this->nodes);
9099
}
91100

92101
public function getIterator(): \Traversable
93102
{
94-
foreach ($this->nodes as $key => $_) {
95-
yield $key => $this->offsetGet($key);
96-
}
103+
yield from $this->nodes;
97104
}
98105

99106
public function count(): int
@@ -116,7 +123,7 @@ public function splice(int $offset, int $length, $replacement = null): NodeList
116123
}
117124

118125
/**
119-
* @phpstan-param iterable<array-key, T> $list
126+
* @phpstan-param iterable<int, T> $list
120127
*
121128
* @phpstan-return NodeList<T>
122129
*/
@@ -129,14 +136,6 @@ public function merge(iterable $list): NodeList
129136
return new NodeList(\array_merge($this->nodes, $list));
130137
}
131138

132-
/**
133-
* Resets the keys of the stored nodes to contiguous numeric indexes.
134-
*/
135-
public function reindex(): void
136-
{
137-
$this->nodes = array_values($this->nodes);
138-
}
139-
140139
/**
141140
* Returns a clone of this instance and all its children, except Location $loc.
142141
*
@@ -146,8 +145,8 @@ public function cloneDeep(): self
146145
{
147146
/** @var static<T> $cloned */
148147
$cloned = new static([]);
149-
foreach ($this->getIterator() as $key => $node) {
150-
$cloned[$key] = $node->cloneDeep();
148+
foreach ($this->getIterator() as $node) {
149+
$cloned->add($node->cloneDeep());
151150
}
152151

153152
return $cloned;

‎src/Language/Visitor.php

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,11 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null
223223
$node->splice($editKey, 1);
224224
++$editOffset;
225225
} else {
226-
if ($node instanceof NodeList || \is_array($node)) {
226+
if ($node instanceof NodeList) {
227+
if ($editValue !== false) {
228+
$node->set($editKey, $editValue);
229+
}
230+
} elseif (\is_array($node)) {
227231
$node[$editKey] = $editValue;
228232
} else {
229233
$node->{$editKey} = $editValue;
@@ -249,9 +253,13 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null
249253
: null;
250254
$node = $parent !== null
251255
? (
252-
$parent instanceof NodeList || \is_array($parent)
253-
? $parent[$key]
254-
: $parent->{$key}
256+
$parent instanceof NodeList
257+
? $parent->get($key)
258+
: (
259+
\is_array($parent)
260+
? $parent[$key]
261+
: $parent->{$key}
262+
)
255263
)
256264
: $newRoot;
257265
if ($node === null) {

‎src/Validator/Rules/QueryComplexity.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public function getVisitor(QueryValidationContext $context): array
6161
);
6262
},
6363
NodeKind::VARIABLE_DEFINITION => function ($def): VisitorOperation {
64-
$this->variableDefs[] = $def;
64+
$this->variableDefs->add($def);
6565

6666
return Visitor::skipNode();
6767
},

‎tests/Error/ErrorTest.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ public function testConvertsNodesToPositionsAndLocations(): void
3434
}');
3535
$ast = Parser::parse($source);
3636
/** @var OperationDefinitionNode $operationDefinition */
37-
$operationDefinition = $ast->definitions[0];
38-
$fieldNode = $operationDefinition->selectionSet->selections[0];
37+
$operationDefinition = $ast->definitions->get(0);
38+
$fieldNode = $operationDefinition->selectionSet->selections->get(0);
3939
$e = new Error('msg', [$fieldNode]);
4040

4141
self::assertEquals([$fieldNode], $e->getNodes());
@@ -54,8 +54,8 @@ public function testConvertSingleNodeToPositionsAndLocations(): void
5454
}');
5555
$ast = Parser::parse($source);
5656
/** @var OperationDefinitionNode $operationDefinition */
57-
$operationDefinition = $ast->definitions[0];
58-
$fieldNode = $operationDefinition->selectionSet->selections[0];
57+
$operationDefinition = $ast->definitions->get(0);
58+
$fieldNode = $operationDefinition->selectionSet->selections->get(0);
5959
$e = new Error('msg', $fieldNode); // Non-array value.
6060

6161
self::assertEquals([$fieldNode], $e->getNodes());
@@ -73,7 +73,7 @@ public function testConvertsNodeWithStart0ToPositionsAndLocations(): void
7373
field
7474
}');
7575
$ast = Parser::parse($source);
76-
$operationNode = $ast->definitions[0];
76+
$operationNode = $ast->definitions->get(0);
7777
$e = new Error('msg', [$operationNode]);
7878

7979
self::assertEquals([$operationNode], $e->getNodes());
@@ -114,8 +114,8 @@ public function testSerializesToIncludeMessageAndLocations(): void
114114
{
115115
$ast = Parser::parse('{ field }');
116116
/** @var OperationDefinitionNode $operationDefinition */
117-
$operationDefinition = $ast->definitions[0];
118-
$node = $operationDefinition->selectionSet->selections[0];
117+
$operationDefinition = $ast->definitions->get(0);
118+
$node = $operationDefinition->selectionSet->selections->get(0);
119119
$e = new Error('msg', [$node]);
120120

121121
self::assertEquals(

‎tests/Error/PrintErrorTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ public function testPrintsAnErrorWithNodesFromDifferentSources(): void
6363
));
6464

6565
/** @var ObjectTypeDefinitionNode $objectDefinitionA */
66-
$objectDefinitionA = $sourceA->definitions[0];
67-
$fieldTypeA = $objectDefinitionA->fields[0]->type;
66+
$objectDefinitionA = $sourceA->definitions->get(0);
67+
$fieldTypeA = $objectDefinitionA->fields->get(0)->type;
6868

6969
$sourceB = Parser::parse(new Source(
7070
'type Foo {
@@ -74,8 +74,8 @@ public function testPrintsAnErrorWithNodesFromDifferentSources(): void
7474
));
7575

7676
/** @var ObjectTypeDefinitionNode $objectDefinitionB */
77-
$objectDefinitionB = $sourceB->definitions[0];
78-
$fieldTypeB = $objectDefinitionB->fields[0]->type;
77+
$objectDefinitionB = $sourceB->definitions->get(0);
78+
$fieldTypeB = $objectDefinitionB->fields->get(0)->type;
7979

8080
$error = new Error(
8181
'Example error with two nodes',

‎tests/Executor/ExecutorTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,11 +257,11 @@ public function testProvidesInfoAboutCurrentExecutionState(): void
257257
Executor::execute($schema, $ast, $rootValue, null, ['var' => '123']);
258258

259259
/** @var OperationDefinitionNode $operationDefinition */
260-
$operationDefinition = $ast->definitions[0];
260+
$operationDefinition = $ast->definitions->get(0);
261261

262262
self::assertEquals('test', $info->fieldName);
263263
self::assertCount(1, $info->fieldNodes);
264-
self::assertSame($operationDefinition->selectionSet->selections[0], $info->fieldNodes[0]);
264+
self::assertSame($operationDefinition->selectionSet->selections->get(0), $info->fieldNodes[0]);
265265
self::assertSame(Type::string(), $info->returnType);
266266
self::assertSame($schema->getQueryType(), $info->parentType);
267267
self::assertEquals(['result'], $info->path);

‎tests/Language/AST/NodeTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,16 @@ public function testCloneDeep(): void
3030
$cloneFields = $clone->fields;
3131
self::assertNotSameButEquals($nodeFields, $cloneFields);
3232

33-
$nodeId = $nodeFields[0];
34-
$cloneId = $cloneFields[0];
33+
$nodeId = $nodeFields->get(0);
34+
$cloneId = $cloneFields->get(0);
3535
self::assertNotSameButEquals($nodeId, $cloneId);
3636

3737
$nodeIdArgs = $nodeId->arguments;
3838
$cloneIdArgs = $cloneId->arguments;
3939
self::assertNotSameButEquals($nodeIdArgs, $cloneIdArgs);
4040

41-
$nodeArg = $nodeIdArgs[0];
42-
$cloneArg = $cloneIdArgs[0];
41+
$nodeArg = $nodeIdArgs->get(0);
42+
$cloneArg = $cloneIdArgs->get(0);
4343
self::assertNotSameButEquals($nodeArg, $cloneArg);
4444

4545
self::assertSame($node->loc, $clone->loc);

‎tests/Language/NodeListTest.php

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,25 +13,24 @@ public function testConvertArrayToASTNode(): void
1313
{
1414
$nameNode = new NameNode(['value' => 'bar']);
1515

16-
$key = 'foo';
17-
$nodeList = new NodeList([$key => $nameNode->toArray()]);
16+
$nodeList = new NodeList([$nameNode->toArray()]);
1817

19-
self::assertEquals($nameNode, $nodeList[$key]);
18+
self::assertEquals($nameNode, $nodeList->get(0));
2019
}
2120

2221
public function testCloneDeep(): void
2322
{
2423
$nameNode = new NameNode(['value' => 'bar']);
2524

2625
$nodeList = new NodeList([
27-
'array' => $nameNode->toArray(),
28-
'instance' => $nameNode,
26+
$nameNode->toArray(),
27+
$nameNode,
2928
]);
3029

3130
$cloned = $nodeList->cloneDeep();
3231

33-
self::assertNotSameButEquals($nodeList['array'], $cloned['array']);
34-
self::assertNotSameButEquals($nodeList['instance'], $cloned['instance']);
32+
self::assertNotSameButEquals($nodeList->get(0), $cloned->get(0));
33+
self::assertNotSameButEquals($nodeList->get(1), $cloned->get(1));
3534
}
3635

3736
private static function assertNotSameButEquals(object $node, object $clone): void
@@ -44,40 +43,33 @@ public function testThrowsOnInvalidArrays(): void
4443
{
4544
$nodeList = new NodeList([]);
4645

47-
self::expectException(InvariantViolation::class);
46+
$this->expectException(InvariantViolation::class);
4847
// @phpstan-ignore-next-line Wrong on purpose
49-
$nodeList[] = ['not a valid array representation of an AST node'];
48+
$nodeList->add(['not a valid array representation of an AST node']);
5049
}
5150

52-
public function testPushNodes(): void
51+
public function testAddNodes(): void
5352
{
5453
/** @var NodeList<NameNode> $nodeList */
5554
$nodeList = new NodeList([]);
5655
self::assertCount(0, $nodeList);
5756

58-
$nodeList[] = new NameNode(['value' => 'foo']);
57+
$nodeList->add(new NameNode(['value' => 'foo']));
5958
self::assertCount(1, $nodeList);
6059

61-
$nodeList[] = new NameNode(['value' => 'bar']);
60+
$nodeList->add(new NameNode(['value' => 'bar']));
6261
self::assertCount(2, $nodeList);
6362
}
6463

65-
public function testResetContiguousNumericIndexAfterUnset(): void
64+
public function testRemoveDoesNotBreakList(): void
6665
{
6766
$foo = new NameNode(['value' => 'foo']);
6867
$bar = new NameNode(['value' => 'bar']);
6968

7069
$nodeList = new NodeList([$foo, $bar]);
71-
unset($nodeList[0]);
70+
$nodeList->remove($foo);
7271

73-
self::assertArrayNotHasKey(0, $nodeList);
74-
// @phpstan-ignore-next-line just wrong
75-
self::assertArrayHasKey(1, $nodeList);
76-
77-
$nodeList->reindex();
78-
79-
// @phpstan-ignore-next-line just wrong
80-
self::assertArrayHasKey(0, $nodeList);
81-
self::assertSame($bar, $nodeList[0]);
72+
self::assertTrue($nodeList->has(0));
73+
self::assertSame($bar, $nodeList->get(0));
8274
}
8375
}

0 commit comments

Comments
 (0)