fix: do not add false to NodeList (#1314)

Author: simPod

phpstan.neon.dist

@@ -39,6 +39,9 @@ parameters:
         - message: "#^Property GraphQL\\\\Tests\\\\Type\\\\SchemaTest\\:\\:\\$implementingType is never read, only written\\.$#"
           path: tests
 
+        # Cannot satisfy input covariance
+        - '~(expects|should return) array<string, array<string, callable\(GraphQL\\Language\\AST\\Node\): GraphQL\\Language\\VisitorOperation\|void\|false\|null>\|\(callable\(GraphQL\\Language\\AST\\Node\): GraphQL\\Language\\VisitorOperation\|void\|false\|null\)>(,| but returns)?~'
+
 includes:
     - phpstan-baseline.neon
     - phpstan/include-by-php-version.php

src/Language/Visitor.php

@@ -6,6 +6,7 @@
 use GraphQL\Language\AST\NodeKind;
 use GraphQL\Language\AST\NodeList;
 use GraphQL\Utils\TypeInfo;
+use GraphQL\Utils\Utils;
 
 /**
  * Utility for efficient AST traversal and modification.
@@ -88,7 +89,8 @@
  *       ]
  *     ]);
  *
- * @phpstan-type VisitorArray array<string, callable(Node): VisitorOperation|mixed|null>|array<string, array<string, callable(Node): VisitorOperation|mixed|null>>
+ * @phpstan-type NodeVisitor callable(Node): (VisitorOperation|null|false|void)
+ * @phpstan-type VisitorArray array<string, NodeVisitor>|array<string, array<string, NodeVisitor>>
  */
 class Visitor
 {
@@ -218,6 +220,11 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null
                             ++$editOffset;
                         } else {
                             if ($node instanceof NodeList || \is_array($node)) {
+                                if (! $editValue instanceof Node) {
+                                    $notNode = Utils::printSafe($editValue);
+                                    throw new \Exception("Can only add Node to NodeList, got: {$notNode}.");
+                                }
+
                                 $node[$editKey] = $editValue;
                             } else {
                                 $node->{$editKey} = $editValue;
@@ -267,26 +274,23 @@ public static function visit(object $root, array $visitor, ?array $keyMap = null
 
                 if ($visitFn !== null) {
                     $result = $visitFn($node, $key, $parent, $path, $ancestors);
-                    $editValue = null;
 
                     if ($result !== null) {
-                        if ($result instanceof VisitorOperation) {
-                            if ($result instanceof VisitorStop) {
-                                break;
-                            }
+                        if ($result instanceof VisitorStop) {
+                            break;
+                        }
 
-                            if (! $isLeaving && $result instanceof VisitorSkipNode) {
+                        if ($result instanceof VisitorSkipNode) {
+                            if (! $isLeaving) {
                                 \array_pop($path);
-                                continue;
                             }
-
-                            if ($result instanceof VisitorRemoveNode) {
-                                $editValue = null;
-                            }
-                        } else {
-                            $editValue = $result;
+                            continue;
                         }
 
+                        $editValue = $result instanceof VisitorRemoveNode
+                            ? null
+                            : $result;
+
                         $edits[] = [$key, $editValue];
                         if (! $isLeaving) {
                             if (! ($editValue instanceof Node)) {

src/Validator/QueryValidationContext.php

@@ -129,11 +129,8 @@ private function getVariableUsages(HasSelectionSet $node): array
                 Visitor::visitWithTypeInfo(
                     $typeInfo,
                     [
-                        NodeKind::VARIABLE_DEFINITION => static fn (): bool => false,
-                        NodeKind::VARIABLE => static function (VariableNode $variable) use (
-                            &$usages,
-                            $typeInfo
-                        ): void {
+                        NodeKind::VARIABLE_DEFINITION => static fn () => Visitor::skipNode(),
+                        NodeKind::VARIABLE => static function (VariableNode $variable) use (&$usages, $typeInfo): void {
                             $usages[] = [
                                 'node' => $variable,
                                 'type' => $typeInfo->getInputType(),

src/Validator/Rules/SingleFieldSubscription.php

@@ -11,9 +11,6 @@
 
 class SingleFieldSubscription extends ValidationRule
 {
-    /**
-     * @return array<string, callable>
-     */
     public function getVisitor(QueryValidationContext $context): array
     {
         return [