Skip to content

Commit 4f0ef97

Browse files
ruudkruudk
authored
Allow replacing a Node with a NodeList in Visitor (#1754)
With this change, it's now possible to do something like this: ```php $editedAst = Visitor::visit( $ast, [ NodeKind::INLINE_FRAGMENT => function ($node) { return $node->selectionSet->selections; }, ] ); ``` It replaces an inline fragment (Node) with the selections (NodeList). This makes it possible to create specific optimizer Visitors that cleanup operations. For example, remove useless inline fragments on the same type. Or inline fragments that don't have a type selection and also don't use directives.
1 parent 9e84668 commit 4f0ef97

File tree

7 files changed

+157
-17
lines changed

7 files changed

+157
-17
lines changed

‎docs/class-reference.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1350,7 +1350,7 @@ visitor API:
13501350
]
13511351
]);
13521352

1353-
@phpstan-type NodeVisitor callable(Node): (VisitorOperation|Node|null|false|void)
1353+
@phpstan-type NodeVisitor callable(Node): (VisitorOperation|Node|NodeList<Node>|null|false|void)
13541354
@phpstan-type VisitorArray array<string, NodeVisitor>|array<string, array<string, NodeVisitor>>
13551355

13561356
@see \GraphQL\Tests\Language\VisitorTest

‎phpstan-baseline.neon

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,3 @@ parameters:
8383
identifier: property.dynamicName
8484
count: 1
8585
path: src/Utils/AST.php
86-
87-
-
88-
message: '#^Method GraphQL\\Validator\\Rules\\KnownDirectives\:\:getDirectiveLocationForASTPath\(\) has parameter \$ancestors with generic class GraphQL\\Language\\AST\\NodeList but does not specify its types\: T$#'
89-
identifier: missingType.generics
90-
count: 1
91-
path: src/Validator/Rules/KnownDirectives.php

‎phpstan.neon.dist

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ parameters:
3535
path: tests
3636

3737
# Cannot satisfy input covariance
38-
- '~(expects|should return) array\<string, array\<string, callable\(GraphQL\\Language\\AST\\Node\)\: \(GraphQL\\Language\\AST\\Node\|GraphQL\\Language\\VisitorOperation\|void\|false\|null\)\>\|\(callable\(GraphQL\\Language\\AST\\Node\)\: \(GraphQL\\Language\\AST\\Node\|GraphQL\\Language\\VisitorOperation\|void\|false\|null\)\)\>(,| but returns)?~'
38+
- '~(expects|should return) array\<string, array\<string, callable\(GraphQL\\Language\\AST\\Node\)\: \(GraphQL\\Language\\AST\\Node\|GraphQL\\Language\\AST\\NodeList<GraphQL\\Language\\AST\\Node>\|GraphQL\\Language\\VisitorOperation\|void\|false\|null\)\>\|\(callable\(GraphQL\\Language\\AST\\Node\)\: \(GraphQL\\Language\\AST\\Node\|GraphQL\\Language\\AST\\NodeList<GraphQL\\Language\\AST\\Node>\|GraphQL\\Language\\VisitorOperation\|void\|false\|null\)\)\>(,| but returns)?~'
3939

4040
# No need to have @throws in methods that are never called
4141
## PHPUnit

‎src/Language/AST/NodeList.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,16 @@ public function count(): int
104104
/**
105105
* Remove a portion of the NodeList and replace it with something else.
106106
*
107-
* @param T|array<T>|null $replacement
107+
* @param T|iterable<T>|null $replacement
108108
*
109109
* @phpstan-return NodeList<T> the NodeList with the extracted elements
110110
*/
111111
public function splice(int $offset, int $length, $replacement = null): NodeList
112112
{
113+
if (is_iterable($replacement) && ! is_array($replacement)) {
114+
$replacement = iterator_to_array($replacement);
115+
}
116+
113117
return new NodeList(
114118
array_splice($this->nodes, $offset, $length, $replacement)
115119
);

‎src/Language/Visitor.php

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@
8989
* ]
9090
* ]);
9191
*
92-
* @phpstan-type NodeVisitor callable(Node): (VisitorOperation|Node|null|false|void)
92+
* @phpstan-type NodeVisitor callable(Node): (VisitorOperation|Node|NodeList<Node>|null|false|void)
9393
* @phpstan-type VisitorArray array<string, NodeVisitor>|array<string, array<string, NodeVisitor>>
9494
*
9595
* @see \GraphQL\Tests\Language\VisitorTest
@@ -217,12 +217,15 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null
217217
$node->splice($editKey, 1);
218218
++$editOffset;
219219
} elseif ($node instanceof NodeList) {
220-
if (! $editValue instanceof Node) {
221-
$notNode = Utils::printSafe($editValue);
222-
throw new \Exception("Can only add Node to NodeList, got: {$notNode}.");
220+
if ($editValue instanceof NodeList) {
221+
$node->splice($editKey, 1, $editValue);
222+
$editOffset -= count($editValue) - 1;
223+
} elseif ($editValue instanceof Node) {
224+
$node[$editKey] = $editValue;
225+
} else {
226+
$notNodeOrNodeList = Utils::printSafe($editValue);
227+
throw new \Exception("Can only add Node or NodeList to NodeList, got: {$notNodeOrNodeList}.");
223228
}
224-
225-
$node[$editKey] = $editValue;
226229
} else {
227230
$node->{$editKey} = $editValue;
228231
}
@@ -478,7 +481,7 @@ public static function visitWithTypeInfo(TypeInfo $typeInfo, array $visitor): ar
478481
/**
479482
* @phpstan-param VisitorArray $visitor
480483
*
481-
* @return (callable(Node $node, string|int|null $key, Node|NodeList<Node>|null $parent, array<int, int|string> $path, array<int, Node|NodeList<Node>> $ancestors): (VisitorOperation|Node|null))|(callable(Node): (VisitorOperation|Node|void|false|null))|null
484+
* @return (callable(Node $node, string|int|null $key, Node|NodeList<Node>|null $parent, array<int, int|string> $path, array<int, Node|NodeList<Node>> $ancestors): (VisitorOperation|Node|null))|(callable(Node): (VisitorOperation|Node|NodeList<Node>|void|false|null))|null
482485
*/
483486
protected static function extractVisitFn(array $visitor, string $kind, bool $isLeaving): ?callable
484487
{

‎src/Validator/Rules/KnownDirectives.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public static function unknownDirectiveMessage(string $directiveName): string
131131
}
132132

133133
/**
134-
* @param array<Node|NodeList> $ancestors
134+
* @param array<Node|NodeList<Node>> $ancestors
135135
*
136136
* @throws \Exception
137137
*/

‎tests/Language/VisitorTest.php

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use GraphQL\Language\AST\DocumentNode;
66
use GraphQL\Language\AST\FieldDefinitionNode;
77
use GraphQL\Language\AST\FieldNode;
8+
use GraphQL\Language\AST\InlineFragmentNode;
89
use GraphQL\Language\AST\InputObjectTypeDefinitionNode;
910
use GraphQL\Language\AST\InputValueDefinitionNode;
1011
use GraphQL\Language\AST\NameNode;
@@ -1732,4 +1733,142 @@ public function testAncestorsForObjectTypeDefinition(): void
17321733
],
17331734
]);
17341735
}
1736+
1737+
public function testAllowsReplacingNodeWithNodeList(): void
1738+
{
1739+
$ast = Parser::parse('
1740+
{
1741+
field1
1742+
... on Type {
1743+
field2
1744+
field3
1745+
}
1746+
field4
1747+
}
1748+
', ['noLocation' => true]);
1749+
1750+
$editedAst = Visitor::visit(
1751+
$ast,
1752+
[
1753+
NodeKind::INLINE_FRAGMENT => [
1754+
'leave' => fn (InlineFragmentNode $node): NodeList => $node->selectionSet->selections,
1755+
],
1756+
]
1757+
);
1758+
1759+
$expected = Parser::parse('
1760+
{
1761+
field1
1762+
field2
1763+
field3
1764+
field4
1765+
}
1766+
', ['noLocation' => true]);
1767+
1768+
self::assertEquals($expected, $editedAst);
1769+
}
1770+
1771+
public function testAllowsReplacingNodeWithNodeListOnEnter(): void
1772+
{
1773+
$ast = Parser::parse('
1774+
{
1775+
field1
1776+
... on Type {
1777+
field2
1778+
field3
1779+
}
1780+
field4
1781+
}
1782+
', ['noLocation' => true]);
1783+
1784+
$editedAst = Visitor::visit(
1785+
$ast,
1786+
[
1787+
NodeKind::INLINE_FRAGMENT => fn (InlineFragmentNode $node): NodeList => $node->selectionSet->selections,
1788+
]
1789+
);
1790+
1791+
$expected = Parser::parse('
1792+
{
1793+
field1
1794+
field2
1795+
field3
1796+
field4
1797+
}
1798+
', ['noLocation' => true]);
1799+
1800+
self::assertEquals($expected, $editedAst);
1801+
}
1802+
1803+
public function testThrowsExceptionWhenAddingNonNodeToNodeList(): void
1804+
{
1805+
$ast = Parser::parse('
1806+
{
1807+
field1
1808+
... on Type {
1809+
field2
1810+
}
1811+
}
1812+
', ['noLocation' => true]);
1813+
1814+
self::expectException(\Exception::class);
1815+
self::expectExceptionMessage('Can only add Node or NodeList to NodeList, got: "invalid string value"');
1816+
1817+
Visitor::visit(
1818+
$ast,
1819+
[
1820+
NodeKind::INLINE_FRAGMENT => [
1821+
'leave' => fn (InlineFragmentNode $node) => 'invalid string value',
1822+
],
1823+
]
1824+
);
1825+
}
1826+
1827+
public function testThrowsExceptionWhenAddingArrayToNodeList(): void
1828+
{
1829+
$ast = Parser::parse('
1830+
{
1831+
field1
1832+
... on Type {
1833+
field2
1834+
}
1835+
}
1836+
', ['noLocation' => true]);
1837+
1838+
self::expectException(\Exception::class);
1839+
self::expectExceptionMessage('Can only add Node or NodeList to NodeList, got:');
1840+
1841+
Visitor::visit(
1842+
$ast,
1843+
[
1844+
NodeKind::INLINE_FRAGMENT => [
1845+
'leave' => fn (InlineFragmentNode $node) => ['invalid', 'array'],
1846+
],
1847+
]
1848+
);
1849+
}
1850+
1851+
public function testThrowsExceptionWhenAddingIntegerToNodeList(): void
1852+
{
1853+
$ast = Parser::parse('
1854+
{
1855+
field1
1856+
... on Type {
1857+
field2
1858+
}
1859+
}
1860+
', ['noLocation' => true]);
1861+
1862+
self::expectException(\Exception::class);
1863+
self::expectExceptionMessage('Can only add Node or NodeList to NodeList, got: 42');
1864+
1865+
Visitor::visit(
1866+
$ast,
1867+
[
1868+
NodeKind::INLINE_FRAGMENT => [
1869+
'leave' => fn (InlineFragmentNode $node) => 42,
1870+
],
1871+
]
1872+
);
1873+
}
17351874
}

0 commit comments

Comments
 (0)