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 7ab080316..75e4767b1 100644 --- a/src/Language/AST/NodeList.php +++ b/src/Language/AST/NodeList.php @@ -7,92 +7,73 @@ /** * @template T of Node * - * @phpstan-implements \ArrayAccess - * @phpstan-implements \IteratorAggregate + * @phpstan-implements \IteratorAggregate */ -class NodeList implements \ArrayAccess, \IteratorAggregate, \Countable +class NodeList implements \IteratorAggregate, \Countable { /** - * @var array + * @var list> * - * @phpstan-var array> + * @phpstan-var list> $nodes */ - private array $nodes; + protected array $nodes; /** - * @param array $nodes + * @param list> $nodes * - * @phpstan-param array> $nodes + * @phpstan-param list> $nodes */ public function __construct(array $nodes) { $this->nodes = $nodes; } - /** - * @param int|string $offset - */ - #[\ReturnTypeWillChange] - public function offsetExists($offset): bool + public function has(int $offset): bool { return isset($this->nodes[$offset]); } /** - * @param int|string $offset - * * @phpstan-return T */ - #[\ReturnTypeWillChange] - public function offsetGet($offset): Node + public function get(int $offset): Node { - $item = $this->nodes[$offset]; + $node = $this->nodes[$offset]; - if (\is_array($item)) { - // @phpstan-ignore-next-line not really possible to express the correctness of this in PHP - return $this->nodes[$offset] = AST::fromArray($item); - } - - return $item; + // @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; } /** - * @param int|string|null $offset - * @param Node|array $value - * - * @phpstan-param T|array $value + * @phpstan-param T $value */ - #[\ReturnTypeWillChange] - public function offsetSet($offset, $value): void + public function set(int $offset, Node $value): void { - if (\is_array($value)) { - /** @phpstan-var T $value */ - $value = AST::fromArray($value); - } - - // Happens when a Node is pushed via []= - if ($offset === null) { - $this->nodes[] = $value; - - return; - } - $this->nodes[$offset] = $value; + + assert($this->nodes === \array_values($this->nodes)); } /** - * @param int|string $offset + * @phpstan-param T $value */ - #[\ReturnTypeWillChange] - public function offsetUnset($offset): void + public function add(Node $value): void + { + $this->nodes[] = $value; + } + + public function unset(int $offset): void { unset($this->nodes[$offset]); + $this->nodes = \array_values($this->nodes); } public function getIterator(): \Traversable { foreach ($this->nodes as $key => $_) { - yield $key => $this->offsetGet($key); + yield $key => $this->get($key); } } @@ -104,19 +85,22 @@ 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); } /** - * @phpstan-param iterable $list + * @phpstan-param iterable $list * * @phpstan-return NodeList */ @@ -126,15 +110,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); - /** - * Resets the keys of the stored nodes to contiguous numeric indexes. - */ - public function reindex(): void - { - $this->nodes = array_values($this->nodes); + // @phpstan-ignore-next-line generic type mismatch + return new NodeList($merged); } /** @@ -146,8 +125,8 @@ public function cloneDeep(): self { /** @var static $cloned */ $cloned = new static([]); - foreach ($this->getIterator() as $key => $node) { - $cloned[$key] = $node->cloneDeep(); + foreach ($this->getIterator() as $node) { + $cloned->add($node->cloneDeep()); } return $cloned; diff --git a/src/Language/Visitor.php b/src/Language/Visitor.php index 5eaa1fca8..bc38ef1e8 100644 --- a/src/Language/Visitor.php +++ b/src/Language/Visitor.php @@ -224,7 +224,7 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null throw new \Exception("Can only add Node to NodeList, got: {$notNode}."); } - $node[$editKey] = $editValue; + $node->set($editKey, $editValue); } else { $node->{$editKey} = $editValue; } @@ -246,7 +246,7 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null ? $index : $keys[$index]; $node = $parent instanceof NodeList - ? $parent[$key] + ? $parent->get($key) : $parent->{$key}; } if ($node === null) { diff --git a/src/Validator/Rules/QueryComplexity.php b/src/Validator/Rules/QueryComplexity.php index 6fd2f51db..ba4e6efca 100644 --- a/src/Validator/Rules/QueryComplexity.php +++ b/src/Validator/Rules/QueryComplexity.php @@ -61,7 +61,7 @@ public function getVisitor(QueryValidationContext $context): array ); }, NodeKind::VARIABLE_DEFINITION => function ($def): VisitorOperation { - $this->variableDefs[] = $def; + $this->variableDefs->add($def); return Visitor::skipNode(); }, diff --git a/tests/Error/ErrorTest.php b/tests/Error/ErrorTest.php index c1e2d07be..15ea631c3 100644 --- a/tests/Error/ErrorTest.php +++ b/tests/Error/ErrorTest.php @@ -34,8 +34,8 @@ public function testConvertsNodesToPositionsAndLocations(): void }'); $ast = Parser::parse($source); /** @var OperationDefinitionNode $operationDefinition */ - $operationDefinition = $ast->definitions[0]; - $fieldNode = $operationDefinition->selectionSet->selections[0]; + $operationDefinition = $ast->definitions->get(0); + $fieldNode = $operationDefinition->selectionSet->selections->get(0); $e = new Error('msg', [$fieldNode]); self::assertEquals([$fieldNode], $e->getNodes()); @@ -54,8 +54,8 @@ public function testConvertSingleNodeToPositionsAndLocations(): void }'); $ast = Parser::parse($source); /** @var OperationDefinitionNode $operationDefinition */ - $operationDefinition = $ast->definitions[0]; - $fieldNode = $operationDefinition->selectionSet->selections[0]; + $operationDefinition = $ast->definitions->get(0); + $fieldNode = $operationDefinition->selectionSet->selections->get(0); $e = new Error('msg', $fieldNode); // Non-array value. self::assertEquals([$fieldNode], $e->getNodes()); @@ -73,7 +73,7 @@ public function testConvertsNodeWithStart0ToPositionsAndLocations(): void field }'); $ast = Parser::parse($source); - $operationNode = $ast->definitions[0]; + $operationNode = $ast->definitions->get(0); $e = new Error('msg', [$operationNode]); self::assertEquals([$operationNode], $e->getNodes()); @@ -114,8 +114,8 @@ public function testSerializesToIncludeMessageAndLocations(): void { $ast = Parser::parse('{ field }'); /** @var OperationDefinitionNode $operationDefinition */ - $operationDefinition = $ast->definitions[0]; - $node = $operationDefinition->selectionSet->selections[0]; + $operationDefinition = $ast->definitions->get(0); + $node = $operationDefinition->selectionSet->selections->get(0); $e = new Error('msg', [$node]); self::assertEquals( diff --git a/tests/Error/PrintErrorTest.php b/tests/Error/PrintErrorTest.php index 8899998bf..4f2f5bd68 100644 --- a/tests/Error/PrintErrorTest.php +++ b/tests/Error/PrintErrorTest.php @@ -63,8 +63,8 @@ public function testPrintsAnErrorWithNodesFromDifferentSources(): void )); /** @var ObjectTypeDefinitionNode $objectDefinitionA */ - $objectDefinitionA = $sourceA->definitions[0]; - $fieldTypeA = $objectDefinitionA->fields[0]->type; + $objectDefinitionA = $sourceA->definitions->get(0); + $fieldTypeA = $objectDefinitionA->fields->get(0)->type; $sourceB = Parser::parse(new Source( 'type Foo { @@ -74,8 +74,8 @@ public function testPrintsAnErrorWithNodesFromDifferentSources(): void )); /** @var ObjectTypeDefinitionNode $objectDefinitionB */ - $objectDefinitionB = $sourceB->definitions[0]; - $fieldTypeB = $objectDefinitionB->fields[0]->type; + $objectDefinitionB = $sourceB->definitions->get(0); + $fieldTypeB = $objectDefinitionB->fields->get(0)->type; $error = new Error( 'Example error with two nodes', diff --git a/tests/Executor/ExecutorTest.php b/tests/Executor/ExecutorTest.php index 97dd0955f..9afe125d1 100644 --- a/tests/Executor/ExecutorTest.php +++ b/tests/Executor/ExecutorTest.php @@ -257,11 +257,11 @@ public function testProvidesInfoAboutCurrentExecutionState(): void Executor::execute($schema, $ast, $rootValue, null, ['var' => '123']); /** @var OperationDefinitionNode $operationDefinition */ - $operationDefinition = $ast->definitions[0]; + $operationDefinition = $ast->definitions->get(0); self::assertEquals('test', $info->fieldName); self::assertCount(1, $info->fieldNodes); - self::assertSame($operationDefinition->selectionSet->selections[0], $info->fieldNodes[0]); + self::assertSame($operationDefinition->selectionSet->selections->get(0), $info->fieldNodes[0]); self::assertSame(Type::string(), $info->returnType); self::assertSame($schema->getQueryType(), $info->parentType); self::assertEquals(['result'], $info->path); diff --git a/tests/Language/AST/NodeTest.php b/tests/Language/AST/NodeTest.php index 7c6292b67..5004519ef 100644 --- a/tests/Language/AST/NodeTest.php +++ b/tests/Language/AST/NodeTest.php @@ -30,16 +30,16 @@ public function testCloneDeep(): void $cloneFields = $clone->fields; self::assertNotSameButEquals($nodeFields, $cloneFields); - $nodeId = $nodeFields[0]; - $cloneId = $cloneFields[0]; + $nodeId = $nodeFields->get(0); + $cloneId = $cloneFields->get(0); self::assertNotSameButEquals($nodeId, $cloneId); $nodeIdArgs = $nodeId->arguments; $cloneIdArgs = $cloneId->arguments; self::assertNotSameButEquals($nodeIdArgs, $cloneIdArgs); - $nodeArg = $nodeIdArgs[0]; - $cloneArg = $cloneIdArgs[0]; + $nodeArg = $nodeIdArgs->get(0); + $cloneArg = $cloneIdArgs->get(0); self::assertNotSameButEquals($nodeArg, $cloneArg); self::assertSame($node->loc, $clone->loc); diff --git a/tests/Language/NodeListTest.php b/tests/Language/NodeListTest.php index 2033493c0..598b7be02 100644 --- a/tests/Language/NodeListTest.php +++ b/tests/Language/NodeListTest.php @@ -13,10 +13,9 @@ public function testConvertArrayToASTNode(): void { $nameNode = new NameNode(['value' => 'bar']); - $key = 'foo'; - $nodeList = new NodeList([$key => $nameNode->toArray()]); + $nodeList = new NodeList([$nameNode->toArray()]); - self::assertEquals($nameNode, $nodeList[$key]); + self::assertEquals($nameNode, $nodeList->get(0)); } public function testCloneDeep(): void @@ -24,14 +23,14 @@ public function testCloneDeep(): void $nameNode = new NameNode(['value' => 'bar']); $nodeList = new NodeList([ - 'array' => $nameNode->toArray(), - 'instance' => $nameNode, + $nameNode->toArray(), + $nameNode, ]); $cloned = $nodeList->cloneDeep(); - self::assertNotSameButEquals($nodeList['array'], $cloned['array']); - self::assertNotSameButEquals($nodeList['instance'], $cloned['instance']); + self::assertNotSameButEquals($nodeList->get(0), $cloned->get(0)); + self::assertNotSameButEquals($nodeList->get(1), $cloned->get(1)); } private static function assertNotSameButEquals(object $node, object $clone): void @@ -42,42 +41,35 @@ private static function assertNotSameButEquals(object $node, object $clone): voi public function testThrowsOnInvalidArrays(): void { - $nodeList = new NodeList([]); - - self::expectException(InvariantViolation::class); // @phpstan-ignore-next-line Wrong on purpose - $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 testPushNodes(): void + public function testAddNodes(): void { /** @var NodeList $nodeList */ $nodeList = new NodeList([]); self::assertCount(0, $nodeList); - $nodeList[] = new NameNode(['value' => 'foo']); + $nodeList->add(new NameNode(['value' => 'foo'])); self::assertCount(1, $nodeList); - $nodeList[] = new NameNode(['value' => 'bar']); + $nodeList->add(new NameNode(['value' => 'bar'])); self::assertCount(2, $nodeList); } - public function testResetContiguousNumericIndexAfterUnset(): void + public function testUnsetDoesNotBreakList(): void { $foo = new NameNode(['value' => 'foo']); $bar = new NameNode(['value' => 'bar']); $nodeList = new NodeList([$foo, $bar]); - unset($nodeList[0]); - - self::assertArrayNotHasKey(0, $nodeList); - // @phpstan-ignore-next-line just wrong - self::assertArrayHasKey(1, $nodeList); - - $nodeList->reindex(); + $nodeList->unset(0); - // @phpstan-ignore-next-line just wrong - self::assertArrayHasKey(0, $nodeList); - self::assertSame($bar, $nodeList[0]); + self::assertTrue($nodeList->has(0)); + self::assertSame($bar, $nodeList->get(0)); } } diff --git a/tests/Language/ParserTest.php b/tests/Language/ParserTest.php index bb4c9e01b..b13965bfd 100644 --- a/tests/Language/ParserTest.php +++ b/tests/Language/ParserTest.php @@ -220,7 +220,7 @@ public function testParsesMultiByteCharacters(): void ]); /** @var OperationDefinitionNode $operationDefinition */ - $operationDefinition = $result->definitions[0]; + $operationDefinition = $result->definitions->get(0); self::assertEquals($expected, $operationDefinition->selectionSet); } diff --git a/tests/Language/SerializationTest.php b/tests/Language/SerializationTest.php index b8fb51379..d6190ef6a 100644 --- a/tests/Language/SerializationTest.php +++ b/tests/Language/SerializationTest.php @@ -58,8 +58,8 @@ private static function assertNodesAreEqual(Node $expected, Node $actual, array foreach ($expectedValue as $index => $listNode) { $tmpPath2 = $tmpPath; - $tmpPath2[] = $index; - self::assertNodesAreEqual($listNode, $actualValue[$index], $tmpPath2); + $tmpPath2[] = (string) $index; + self::assertNodesAreEqual($listNode, $actualValue->get($index), $tmpPath2); } } elseif ($expectedValue instanceof Location) { self::assertInstanceOf(Location::class, $actualValue, $err); diff --git a/tests/Language/VisitorTest.php b/tests/Language/VisitorTest.php index 6cf0c1524..c1bd962ad 100644 --- a/tests/Language/VisitorTest.php +++ b/tests/Language/VisitorTest.php @@ -49,7 +49,7 @@ private function checkVisitorFnArgs(DocumentNode $ast, array $args, bool $isEdit if ($parent instanceof NodeList) { self::assertIsInt($key); - self::assertTrue(isset($parent[$key])); + self::assertTrue($parent->has($key)); } else { self::assertIsString($key); self::assertTrue(property_exists($parent, $key)); @@ -66,7 +66,8 @@ private function checkVisitorFnArgs(DocumentNode $ast, array $args, bool $isEdit } if ($parent instanceof NodeList) { - self::assertEquals($node, $parent[$key]); + self::assertIsInt($key); + self::assertEquals($node, $parent->get($key)); } else { /** @phpstan-ignore-next-line */ self::assertEquals($node, $parent->{$key}); @@ -91,7 +92,8 @@ private function getNodeByPath(DocumentNode $ast, array $path): object foreach ($path as $key) { if ($result instanceof NodeList) { - $result = $result[$key]; + self::assertIsInt($key); + $result = $result->get($key); } else { /** @phpstan-ignore-next-line */ $result = $result->{$key}; @@ -221,7 +223,7 @@ public function testAllowsEditingANodeBothOnEnterAndOnLeave(): void self::assertNotEquals($ast, $editedAst); $expected = $ast->cloneDeep(); - $operationNode = $expected->definitions[0]; + $operationNode = $expected->definitions->get(0); assert($operationNode instanceof OperationDefinitionNode); $operationNode->directives = new NodeList([$directive1, $directive2]); diff --git a/tests/Utils/AstGetOperationAstTest.php b/tests/Utils/AstGetOperationAstTest.php index 9923b3ab2..3eb1db632 100644 --- a/tests/Utils/AstGetOperationAstTest.php +++ b/tests/Utils/AstGetOperationAstTest.php @@ -18,7 +18,7 @@ public function testGetsAnOperationFromASimpleDocument(): void $doc = Parser::parse('{ field }'); self::assertEquals( AST::getOperationAST($doc), - $doc->definitions->offsetGet(0) + $doc->definitions->get(0) ); } @@ -28,7 +28,7 @@ public function testGetsAnOperationFromASimpleDocument(): void public function testGetsAnOperationFromADcoumentWithNamedOpMutation(): void { $doc = Parser::parse('mutation Test { field }'); - self::assertEquals(AST::getOperationAST($doc), $doc->definitions->offsetGet(0)); + self::assertEquals(AST::getOperationAST($doc), $doc->definitions->get(0)); } /** @@ -37,7 +37,7 @@ public function testGetsAnOperationFromADcoumentWithNamedOpMutation(): void public function testGetsAnOperationFromADcoumentWithNamedOpSubscription(): void { $doc = Parser::parse('subscription Test { field }'); - self::assertEquals(AST::getOperationAST($doc), $doc->definitions->offsetGet(0)); + self::assertEquals(AST::getOperationAST($doc), $doc->definitions->get(0)); } /** @@ -99,8 +99,8 @@ public function testGetsNamedOperation(): void mutation TestM { field } subscription TestS { field } '); - self::assertEquals(AST::getOperationAST($doc, 'TestQ'), $doc->definitions->offsetGet(0)); - self::assertEquals(AST::getOperationAST($doc, 'TestM'), $doc->definitions->offsetGet(1)); - self::assertEquals(AST::getOperationAST($doc, 'TestS'), $doc->definitions->offsetGet(2)); + self::assertEquals(AST::getOperationAST($doc, 'TestQ'), $doc->definitions->get(0)); + self::assertEquals(AST::getOperationAST($doc, 'TestM'), $doc->definitions->get(1)); + self::assertEquals(AST::getOperationAST($doc, 'TestS'), $doc->definitions->get(2)); } } diff --git a/tests/Utils/BuildSchemaTest.php b/tests/Utils/BuildSchemaTest.php index e9fd2b9a0..6d700bbb9 100644 --- a/tests/Utils/BuildSchemaTest.php +++ b/tests/Utils/BuildSchemaTest.php @@ -429,7 +429,7 @@ interface EmptyInterface GRAPHQL; - $definition = Parser::parse($sdl)->definitions[0]; + $definition = Parser::parse($sdl)->definitions->get(0); self::assertInstanceOf(InterfaceTypeDefinitionNode::class, $definition); self::assertCount(0, $definition->interfaces, 'The interfaces property must be an empty list.');