From 226aeda56d9b2ad636fdfd21234b0b63fc64ccea Mon Sep 17 00:00:00 2001 From: Ruud Kamphuis Date: Tue, 17 Mar 2026 10:58:22 +0100 Subject: [PATCH 1/3] Fix parseLiteral not called on per-schema scalar overrides for inline arguments AST::valueFromAST() called parseLiteral() on the built-in scalar singleton from field definitions, not the custom override registered via the type loader. This meant inline literal arguments like `node(id: 123)` silently skipped custom parsing logic. Pass the Schema into valueFromAST() and resolve built-in scalars from it before calling parseLiteral(), mirroring the parseValue() fix in Value::coerceInputValue(). Also extract Type::isBuiltInScalarName() to centralize the name check without requiring an instanceof guard. --- src/Executor/ReferenceExecutor.php | 3 +- src/Executor/Values.php | 8 +++--- src/Type/Definition/Type.php | 10 +++++-- src/Utils/AST.php | 21 ++++++++++---- tests/Type/Definition/TypeTest.php | 16 +++++++++++ tests/Type/ScalarOverridesTest.php | 45 ++++++++++++++++++++++++++++++ 6 files changed, 91 insertions(+), 12 deletions(-) diff --git a/src/Executor/ReferenceExecutor.php b/src/Executor/ReferenceExecutor.php index 55a865e99..531c9c59d 100644 --- a/src/Executor/ReferenceExecutor.php +++ b/src/Executor/ReferenceExecutor.php @@ -738,7 +738,8 @@ protected function resolveFieldValueOrError( $args = $this->fieldArgsCache[$fieldDef][$fieldNode] ??= $argsMapper(Values::getArgumentValues( $fieldDef, $fieldNode, - $this->exeContext->variableValues + $this->exeContext->variableValues, + $this->exeContext->schema, ), $fieldDef, $fieldNode, $contextValue); return $resolveFn($rootValue, $args, $contextValue, $info); diff --git a/src/Executor/Values.php b/src/Executor/Values.php index e63737e79..09ac6bd94 100644 --- a/src/Executor/Values.php +++ b/src/Executor/Values.php @@ -180,7 +180,7 @@ public static function getDirectiveValues(Directive $directiveDef, Node $node, ? * * @return array */ - public static function getArgumentValues($def, Node $node, ?array $variableValues = null): array + public static function getArgumentValues($def, Node $node, ?array $variableValues = null, ?Schema $schema = null): array { if ($def->args === []) { return []; @@ -196,7 +196,7 @@ public static function getArgumentValues($def, Node $node, ?array $variableValue } } - return static::getArgumentValuesForMap($def, $argumentValueMap, $variableValues, $node); + return static::getArgumentValuesForMap($def, $argumentValueMap, $variableValues, $node, $schema); } /** @@ -209,7 +209,7 @@ public static function getArgumentValues($def, Node $node, ?array $variableValue * * @return array */ - public static function getArgumentValuesForMap($def, array $argumentValueMap, ?array $variableValues = null, ?Node $referenceNode = null): array + public static function getArgumentValuesForMap($def, array $argumentValueMap, ?array $variableValues = null, ?Node $referenceNode = null, ?Schema $schema = null): array { /** @var array $coercedValues */ $coercedValues = []; @@ -260,7 +260,7 @@ public static function getArgumentValuesForMap($def, array $argumentValueMap, ?a // usage here is of the correct type. $coercedValues[$name] = $variableValues[$variableName] ?? null; } else { - $coercedValue = AST::valueFromAST($argumentValueNode, $argType, $variableValues); + $coercedValue = AST::valueFromAST($argumentValueNode, $argType, $variableValues, $schema); if (Utils::undefined() === $coercedValue) { // Note: ValuesOfCorrectType validation should catch this before // execution. This is a runtime check to ensure execution does not diff --git a/src/Type/Definition/Type.php b/src/Type/Definition/Type.php index 8a9b05647..805c4dca6 100644 --- a/src/Type/Definition/Type.php +++ b/src/Type/Definition/Type.php @@ -203,7 +203,7 @@ public static function overrideStandardTypes(array $types): void throw new InvariantViolation("Expecting instance of {$typeClass}, got {$notType}"); } - if (! in_array($type->name, self::BUILT_IN_SCALAR_NAMES, true)) { + if (! self::isBuiltInScalarName($type->name)) { $standardTypeNames = implode(', ', self::BUILT_IN_SCALAR_NAMES); $notStandardTypeName = Utils::printSafe($type->name); throw new InvariantViolation("Expecting one of the following names for a standard type: {$standardTypeNames}; got {$notStandardTypeName}"); @@ -228,7 +228,13 @@ public static function overrideStandardTypes(array $types): void public static function isBuiltInScalar($type): bool { return $type instanceof ScalarType - && in_array($type->name, self::BUILT_IN_SCALAR_NAMES, true); + && self::isBuiltInScalarName($type->name); + } + + /** Checks if the given name is one of the built-in scalar type names (ID, String, Int, Float, Boolean). */ + public static function isBuiltInScalarName(string $name): bool + { + return in_array($name, self::BUILT_IN_SCALAR_NAMES, true); } /** diff --git a/src/Utils/AST.php b/src/Utils/AST.php index 891ccfd63..2c6b07cb7 100644 --- a/src/Utils/AST.php +++ b/src/Utils/AST.php @@ -37,6 +37,7 @@ use GraphQL\Type\Definition\NullableType; use GraphQL\Type\Definition\ScalarType; use GraphQL\Type\Definition\Type; +use GraphQL\Type\Schema; /** * Various utilities dealing with AST. @@ -307,7 +308,7 @@ public static function astFromValue($value, InputType $type): ?ValueNode * * @api */ - public static function valueFromAST(?ValueNode $valueNode, Type $type, ?array $variables = null) + public static function valueFromAST(?ValueNode $valueNode, Type $type, ?array $variables = null, ?Schema $schema = null) { $undefined = Utils::undefined(); @@ -323,7 +324,7 @@ public static function valueFromAST(?ValueNode $valueNode, Type $type, ?array $v return $undefined; } - return self::valueFromAST($valueNode, $type->getWrappedType(), $variables); + return self::valueFromAST($valueNode, $type->getWrappedType(), $variables, $schema); } if ($valueNode instanceof NullValueNode) { @@ -362,7 +363,7 @@ public static function valueFromAST(?ValueNode $valueNode, Type $type, ?array $v $coercedValues[] = null; } else { - $itemValue = self::valueFromAST($itemNode, $itemType, $variables); + $itemValue = self::valueFromAST($itemNode, $itemType, $variables, $schema); if ($undefined === $itemValue) { // Invalid: intentionally return no value. return $undefined; @@ -375,7 +376,7 @@ public static function valueFromAST(?ValueNode $valueNode, Type $type, ?array $v return $coercedValues; } - $coercedValue = self::valueFromAST($valueNode, $itemType, $variables); + $coercedValue = self::valueFromAST($valueNode, $itemType, $variables, $schema); if ($undefined === $coercedValue) { // Invalid: intentionally return no value. return $undefined; @@ -416,7 +417,8 @@ public static function valueFromAST(?ValueNode $valueNode, Type $type, ?array $v $fieldValue = self::valueFromAST( $fieldNode->value, $field->getType(), - $variables + $variables, + $schema, ); if ($undefined === $fieldValue) { @@ -440,6 +442,15 @@ public static function valueFromAST(?ValueNode $valueNode, Type $type, ?array $v assert($type instanceof ScalarType, 'only remaining option'); + // Account for type loader returning a different scalar instance than + // the built-in singleton used in field definitions. Resolve the actual + // type from the schema to ensure the correct parseLiteral() is called. + if ($schema !== null && Type::isBuiltInScalarName($type->name)) { + $schemaType = $schema->getType($type->name); + assert($schemaType instanceof ScalarType, "Schema must provide a ScalarType for built-in scalar \"{$type->name}\"."); + $type = $schemaType; + } + // Scalars fulfill parsing a literal value via parseLiteral(). // Invalid values represent a failure to parse correctly, in which case // no value is returned. diff --git a/tests/Type/Definition/TypeTest.php b/tests/Type/Definition/TypeTest.php index 45f1a75f1..3bc2d1789 100644 --- a/tests/Type/Definition/TypeTest.php +++ b/tests/Type/Definition/TypeTest.php @@ -48,4 +48,20 @@ public function testIsBuiltInScalarReturnsFalseForCustomScalarWithNonBuiltInName { self::assertFalse(Type::isBuiltInScalar(new CustomScalarType(['name' => 'MyScalar']))); // @phpstan-ignore staticMethod.alreadyNarrowedType } + + public function testIsBuiltInScalarNameReturnsTrueForBuiltInNames(): void + { + self::assertTrue(Type::isBuiltInScalarName(Type::STRING)); + self::assertTrue(Type::isBuiltInScalarName(Type::INT)); + self::assertTrue(Type::isBuiltInScalarName(Type::FLOAT)); + self::assertTrue(Type::isBuiltInScalarName(Type::BOOLEAN)); + self::assertTrue(Type::isBuiltInScalarName(Type::ID)); + } + + public function testIsBuiltInScalarNameReturnsFalseForNonBuiltInNames(): void + { + self::assertFalse(Type::isBuiltInScalarName('MyScalar')); + self::assertFalse(Type::isBuiltInScalarName('Query')); + self::assertFalse(Type::isBuiltInScalarName('')); + } } diff --git a/tests/Type/ScalarOverridesTest.php b/tests/Type/ScalarOverridesTest.php index ebee4ca88..607d54f6c 100644 --- a/tests/Type/ScalarOverridesTest.php +++ b/tests/Type/ScalarOverridesTest.php @@ -4,6 +4,7 @@ use GraphQL\Error\InvariantViolation; use GraphQL\GraphQL; +use GraphQL\Language\AST\IntValueNode; use GraphQL\Language\AST\StringValueNode; use GraphQL\Type\Definition\CustomScalarType; use GraphQL\Type\Definition\InputObjectType; @@ -316,6 +317,50 @@ public function testTypeLoaderOverrideWithInputObjectFieldOfOverriddenBuiltInSca self::assertSame(['data' => ['node' => 'custom-abc-123:test']], $result->toArray()); } + public function testTypeLoaderOverrideCallsParseLiteralForInlineArgument(): void + { + $parseLiteralCalled = false; + + $customID = new CustomScalarType([ + 'name' => Type::ID, + 'serialize' => static fn ($value): string => (string) $value, + 'parseValue' => static fn ($value): string => 'parsed-' . $value, + 'parseLiteral' => static function ($node) use (&$parseLiteralCalled): string { + $parseLiteralCalled = true; + + assert($node instanceof IntValueNode || $node instanceof StringValueNode); + + return 'literal-' . $node->value; + }, + ]); + + $queryType = new ObjectType([ + 'name' => 'Query', + 'fields' => [ + 'node' => [ + 'type' => Type::string(), + 'args' => [ + 'id' => Type::nonNull(Type::id()), + ], + 'resolve' => static fn ($root, array $args): string => 'node-' . $args['id'], + ], + ], + ]); + + $types = ['Query' => $queryType, 'ID' => $customID]; + + $schema = new Schema([ + 'query' => $queryType, + 'typeLoader' => static fn (string $name): ?Type => $types[$name] ?? null, + ]); + + $result = GraphQL::executeQuery($schema, '{ node(id: 123) }'); + + self::assertEmpty($result->errors, isset($result->errors[0]) ? $result->errors[0]->getMessage() : ''); + self::assertTrue($parseLiteralCalled, 'Expected custom parseLiteral to be called for inline literal argument'); + self::assertSame(['data' => ['node' => 'node-literal-123']], $result->toArray()); + } + /** @throws InvariantViolation */ private static function createCustomID(\Closure $parseValue): CustomScalarType { From aa742b3aec56c0a740e9b9b079efc246932f67b3 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 17 Mar 2026 10:00:16 +0000 Subject: [PATCH 2/3] Autofix --- docs/class-reference.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/class-reference.md b/docs/class-reference.md index 21b10dc2c..326c88d8d 100644 --- a/docs/class-reference.md +++ b/docs/class-reference.md @@ -2955,7 +2955,8 @@ static function astFromValue($value, GraphQL\Type\Definition\InputType $type): ? static function valueFromAST( ?GraphQL\Language\AST\ValueNode $valueNode, GraphQL\Type\Definition\Type $type, - ?array $variables = null + ?array $variables = null, + ?GraphQL\Type\Schema $schema = null ) ``` From 1380b379d4d5aeaed9b131134003755b52a2287f Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 17 Mar 2026 14:47:50 +0100 Subject: [PATCH 3/3] Thread schema through directive argument coercion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Directive arguments (e.g. @include(if: true)) were not using schema-resolved scalar overrides because getDirectiveValues() did not pass the schema to getArgumentValues(). 🤖 Generated with Claude Code --- src/Executor/ReferenceExecutor.php | 8 ++++-- src/Executor/Values.php | 4 +-- src/Utils/AST.php | 7 ++--- tests/Type/ScalarOverridesTest.php | 42 ++++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src/Executor/ReferenceExecutor.php b/src/Executor/ReferenceExecutor.php index 531c9c59d..f87bd06c0 100644 --- a/src/Executor/ReferenceExecutor.php +++ b/src/Executor/ReferenceExecutor.php @@ -466,10 +466,13 @@ protected function shouldIncludeNode(SelectionNode $node): bool { $variableValues = $this->exeContext->variableValues; + $schema = $this->exeContext->schema; + $skip = Values::getDirectiveValues( Directive::skipDirective(), $node, - $variableValues + $variableValues, + $schema, ); if (isset($skip['if']) && $skip['if'] === true) { return false; @@ -478,7 +481,8 @@ protected function shouldIncludeNode(SelectionNode $node): bool $include = Values::getDirectiveValues( Directive::includeDirective(), $node, - $variableValues + $variableValues, + $schema, ); return ! isset($include['if']) || $include['if'] !== false; diff --git a/src/Executor/Values.php b/src/Executor/Values.php index 09ac6bd94..185851fad 100644 --- a/src/Executor/Values.php +++ b/src/Executor/Values.php @@ -154,13 +154,13 @@ public static function getVariableValues(Schema $schema, NodeList $varDefNodes, * * @return array|null */ - public static function getDirectiveValues(Directive $directiveDef, Node $node, ?array $variableValues = null): ?array + public static function getDirectiveValues(Directive $directiveDef, Node $node, ?array $variableValues = null, ?Schema $schema = null): ?array { $directiveDefName = $directiveDef->name; foreach ($node->directives as $directive) { if ($directive->name->value === $directiveDefName) { - return self::getArgumentValues($directiveDef, $directive, $variableValues); + return self::getArgumentValues($directiveDef, $directive, $variableValues, $schema); } } diff --git a/src/Utils/AST.php b/src/Utils/AST.php index 2c6b07cb7..c21229fcb 100644 --- a/src/Utils/AST.php +++ b/src/Utils/AST.php @@ -441,13 +441,14 @@ public static function valueFromAST(?ValueNode $valueNode, Type $type, ?array $v } assert($type instanceof ScalarType, 'only remaining option'); + $typeName = $type->name; // Account for type loader returning a different scalar instance than // the built-in singleton used in field definitions. Resolve the actual // type from the schema to ensure the correct parseLiteral() is called. - if ($schema !== null && Type::isBuiltInScalarName($type->name)) { - $schemaType = $schema->getType($type->name); - assert($schemaType instanceof ScalarType, "Schema must provide a ScalarType for built-in scalar \"{$type->name}\"."); + if ($schema !== null && Type::isBuiltInScalarName($typeName)) { + $schemaType = $schema->getType($typeName); + assert($schemaType instanceof ScalarType, "Schema must provide a ScalarType for built-in scalar \"{$typeName}\"."); $type = $schemaType; } diff --git a/tests/Type/ScalarOverridesTest.php b/tests/Type/ScalarOverridesTest.php index 607d54f6c..b54615227 100644 --- a/tests/Type/ScalarOverridesTest.php +++ b/tests/Type/ScalarOverridesTest.php @@ -4,6 +4,7 @@ use GraphQL\Error\InvariantViolation; use GraphQL\GraphQL; +use GraphQL\Language\AST\BooleanValueNode; use GraphQL\Language\AST\IntValueNode; use GraphQL\Language\AST\StringValueNode; use GraphQL\Type\Definition\CustomScalarType; @@ -361,6 +362,47 @@ public function testTypeLoaderOverrideCallsParseLiteralForInlineArgument(): void self::assertSame(['data' => ['node' => 'node-literal-123']], $result->toArray()); } + public function testTypeLoaderOverrideCallsParseLiteralForDirectiveArgument(): void + { + $parseLiteralCalled = false; + + $customBoolean = new CustomScalarType([ + 'name' => Type::BOOLEAN, + 'serialize' => static fn ($value): bool => (bool) $value, + 'parseValue' => static fn ($value): bool => (bool) $value, + 'parseLiteral' => static function ($node) use (&$parseLiteralCalled): bool { + $parseLiteralCalled = true; + + self::assertInstanceOf(BooleanValueNode::class, $node); + + return $node->value; + }, + ]); + + $queryType = new ObjectType([ + 'name' => 'Query', + 'fields' => [ + 'greeting' => [ + 'type' => Type::string(), + 'resolve' => static fn (): string => 'hello', + ], + ], + ]); + + $types = ['Query' => $queryType, 'Boolean' => $customBoolean]; + + $schema = new Schema([ + 'query' => $queryType, + 'typeLoader' => static fn (string $name): ?Type => $types[$name] ?? null, + ]); + + $result = GraphQL::executeQuery($schema, '{ greeting @include(if: true) }'); + + self::assertEmpty($result->errors, isset($result->errors[0]) ? $result->errors[0]->getMessage() : ''); + self::assertTrue($parseLiteralCalled, 'Expected custom parseLiteral to be called for inline directive argument'); + self::assertSame(['data' => ['greeting' => 'hello']], $result->toArray()); + } + /** @throws InvariantViolation */ private static function createCustomID(\Closure $parseValue): CustomScalarType {