From 4999c9b67342655d2527c65c7d5f5378561122bb Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 21 Jan 2025 10:48:34 +0100 Subject: [PATCH 1/2] Calculate query complexity with fragments correctly independent of order https://github.com/webonyx/graphql-php/issues/785 --- tests/Validator/QueryComplexityTest.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/Validator/QueryComplexityTest.php b/tests/Validator/QueryComplexityTest.php index 8ea6da018..f7461f230 100644 --- a/tests/Validator/QueryComplexityTest.php +++ b/tests/Validator/QueryComplexityTest.php @@ -59,6 +59,19 @@ public function testFragmentQueries(): void $this->assertDocumentValidators($query, 2, 3); } + /** @dataProvider fragmentQueriesOnRootProvider */ + public function testFragmentQueriesOnRoot(string $query): void + { + $this->assertDocumentValidators($query, 12, 13); + } + + /** @return iterable> */ + public function fragmentQueriesOnRootProvider(): iterable + { + yield ['fragment humanFragment on QueryRoot { human { dogs { name } } } query { ...humanFragment }']; // success example + yield ['query { ...humanFragment } fragment humanFragment on QueryRoot { human { dogs { name } } }']; // failing example, changed order see https://github.com/webonyx/graphql-php/issues/785 + } + public function testAliasesQueries(): void { $query = 'query MyQuery { thomas: human(name: "Thomas") { firstName } jeremy: human(name: "Jeremy") { firstName } }'; From 599f7113eb7411e25cf51cd500fbf8ac61e7c9bb Mon Sep 17 00:00:00 2001 From: spawnia <12158000+spawnia@users.noreply.github.com> Date: Tue, 21 Jan 2025 09:49:33 +0000 Subject: [PATCH 2/2] Apply php-cs-fixer changes --- benchmarks/HugeSchemaBench.php | 4 +-- benchmarks/Utils/SchemaGenerator.php | 2 +- examples/00-hello-world/graphql.php | 4 +-- examples/03-standard-server/graphql.php | 4 +-- src/Executor/ReferenceExecutor.php | 26 +++----------- src/Executor/Values.php | 20 +++-------- src/Language/Lexer.php | 48 +++++-------------------- src/Language/Parser.php | 12 ++----- src/Type/Definition/EnumType.php | 5 +-- src/Utils/ASTDefinitionBuilder.php | 9 +---- src/Utils/BuildClientSchema.php | 12 +++---- src/Utils/BuildSchema.php | 32 ++++++++--------- src/Validator/Rules/QueryComplexity.php | 16 ++------- tests/Executor/ExecutorTest.php | 10 +----- tests/GraphQLTest.php | 22 ++++++------ 15 files changed, 64 insertions(+), 162 deletions(-) diff --git a/benchmarks/HugeSchemaBench.php b/benchmarks/HugeSchemaBench.php index 858d8cd81..fe6bb1821 100644 --- a/benchmarks/HugeSchemaBench.php +++ b/benchmarks/HugeSchemaBench.php @@ -70,8 +70,8 @@ private function createLazySchema(): Schema { return new Schema( (new SchemaConfig()) - ->setQuery($this->schemaGenerator->buildQueryType()) - ->setTypeLoader(fn (string $name): Type => $this->schemaGenerator->loadType($name)) + ->setQuery($this->schemaGenerator->buildQueryType()) + ->setTypeLoader(fn (string $name): Type => $this->schemaGenerator->loadType($name)) ); } } diff --git a/benchmarks/Utils/SchemaGenerator.php b/benchmarks/Utils/SchemaGenerator.php index 71809a58d..298dce943 100644 --- a/benchmarks/Utils/SchemaGenerator.php +++ b/benchmarks/Utils/SchemaGenerator.php @@ -35,7 +35,7 @@ public function buildSchema(): Schema { return new Schema( (new SchemaConfig()) - ->setQuery($this->buildQueryType()) + ->setQuery($this->buildQueryType()) ); } diff --git a/examples/00-hello-world/graphql.php b/examples/00-hello-world/graphql.php index 953571f22..d678a35dd 100644 --- a/examples/00-hello-world/graphql.php +++ b/examples/00-hello-world/graphql.php @@ -49,8 +49,8 @@ // https://webonyx.github.io/graphql-php/schema-definition/#configuration-options $schema = new Schema( (new SchemaConfig()) - ->setQuery($queryType) - ->setMutation($mutationType) + ->setQuery($queryType) + ->setMutation($mutationType) ); $rawInput = file_get_contents('php://input'); diff --git a/examples/03-standard-server/graphql.php b/examples/03-standard-server/graphql.php index 12aaff40d..7937275d6 100644 --- a/examples/03-standard-server/graphql.php +++ b/examples/03-standard-server/graphql.php @@ -48,8 +48,8 @@ // https://webonyx.github.io/graphql-php/schema-definition/#configuration-options $schema = new Schema( (new SchemaConfig()) - ->setQuery($queryType) - ->setMutation($mutationType) + ->setQuery($queryType) + ->setMutation($mutationType) ); $rootValue = ['prefix' => 'You said: ']; diff --git a/src/Executor/ReferenceExecutor.php b/src/Executor/ReferenceExecutor.php index e0b9bc682..c9c8ad202 100644 --- a/src/Executor/ReferenceExecutor.php +++ b/src/Executor/ReferenceExecutor.php @@ -355,10 +355,7 @@ protected function getOperationRootType(Schema $schema, OperationDefinitionNode case 'query': $queryType = $schema->getQueryType(); if ($queryType === null) { - throw new Error( - 'Schema does not define the required query root type.', - [$operation] - ); + throw new Error('Schema does not define the required query root type.', [$operation]); } return $queryType; @@ -366,10 +363,7 @@ protected function getOperationRootType(Schema $schema, OperationDefinitionNode case 'mutation': $mutationType = $schema->getMutationType(); if ($mutationType === null) { - throw new Error( - 'Schema is not configured for mutations.', - [$operation] - ); + throw new Error('Schema is not configured for mutations.', [$operation]); } return $mutationType; @@ -377,19 +371,13 @@ protected function getOperationRootType(Schema $schema, OperationDefinitionNode case 'subscription': $subscriptionType = $schema->getSubscriptionType(); if ($subscriptionType === null) { - throw new Error( - 'Schema is not configured for subscriptions.', - [$operation] - ); + throw new Error('Schema is not configured for subscriptions.', [$operation]); } return $subscriptionType; default: - throw new Error( - 'Can only execute queries, mutations and subscriptions.', - [$operation] - ); + throw new Error('Can only execute queries, mutations and subscriptions.', [$operation]); } } @@ -1075,11 +1063,7 @@ protected function completeLeafValue(LeafType $returnType, &$result) } catch (\Throwable $error) { $safeReturnType = Utils::printSafe($returnType); $safeResult = Utils::printSafe($result); - throw new InvariantViolation( - "Expected a value of type {$safeReturnType} but received: {$safeResult}. {$error->getMessage()}", - 0, - $error - ); + throw new InvariantViolation("Expected a value of type {$safeReturnType} but received: {$safeResult}. {$error->getMessage()}", 0, $error); } } diff --git a/src/Executor/Values.php b/src/Executor/Values.php index bca9a43f0..0e7b0bf98 100644 --- a/src/Executor/Values.php +++ b/src/Executor/Values.php @@ -238,23 +238,14 @@ public static function getArgumentValuesForMap($def, array $argumentValueMap, ?a $safeArgType = Utils::printSafe($argType); if ($isNull) { - throw new Error( - "Argument \"{$name}\" of non-null type \"{$safeArgType}\" must not be null.", - $referenceNode - ); + throw new Error("Argument \"{$name}\" of non-null type \"{$safeArgType}\" must not be null.", $referenceNode); } if ($argumentValueNode instanceof VariableNode) { - throw new Error( - "Argument \"{$name}\" of required type \"{$safeArgType}\" was provided the variable \"\${$argumentValueNode->name->value}\" which was not provided a runtime value.", - [$argumentValueNode] - ); + throw new Error("Argument \"{$name}\" of required type \"{$safeArgType}\" was provided the variable \"\${$argumentValueNode->name->value}\" which was not provided a runtime value.", [$argumentValueNode]); } - throw new Error( - "Argument \"{$name}\" of required type \"{$safeArgType}\" was not provided.", - $referenceNode - ); + throw new Error("Argument \"{$name}\" of required type \"{$safeArgType}\" was not provided.", $referenceNode); } elseif ($hasValue) { assert($argumentValueNode instanceof Node); @@ -275,10 +266,7 @@ public static function getArgumentValuesForMap($def, array $argumentValueMap, ?a // execution. This is a runtime check to ensure execution does not // continue with an invalid argument value. $invalidValue = Printer::doPrint($argumentValueNode); - throw new Error( - "Argument \"{$name}\" has invalid value {$invalidValue}.", - [$argumentValueNode] - ); + throw new Error("Argument \"{$name}\" has invalid value {$invalidValue}.", [$argumentValueNode]); } $coercedValues[$name] = $coercedValue; diff --git a/src/Language/Lexer.php b/src/Language/Lexer.php index cec753d85..93991e22a 100644 --- a/src/Language/Lexer.php +++ b/src/Language/Lexer.php @@ -244,11 +244,7 @@ private function readToken(Token $prev): Token ->readString($line, $col, $prev); } - throw new SyntaxError( - $this->source, - $position, - $this->unexpectedCharacterMessage($code) - ); + throw new SyntaxError($this->source, $position, $this->unexpectedCharacterMessage($code)); } /** @throws \JsonException */ @@ -329,11 +325,7 @@ private function readNumber(int $line, int $col, Token $prev): Token [$char, $code] = $this->moveStringCursor(1, 1)->readChar(); if ($code >= 48 && $code <= 57) { - throw new SyntaxError( - $this->source, - $this->position, - 'Invalid number, unexpected digit after 0: ' . Utils::printCharCode($code) - ); + throw new SyntaxError($this->source, $this->position, 'Invalid number, unexpected digit after 0: ' . Utils::printCharCode($code)); } } else { $value .= $this->readDigits(); @@ -398,11 +390,7 @@ private function readDigits(): string $code = null; } - throw new SyntaxError( - $this->source, - $this->position, - 'Invalid number, expected digit but got: ' . Utils::printCharCode($code) - ); + throw new SyntaxError($this->source, $this->position, 'Invalid number, expected digit but got: ' . Utils::printCharCode($code)); } /** @@ -477,11 +465,7 @@ private function readString(int $line, int $col, Token $prev): Token $position = $this->position; [$hex] = $this->readChars(4); if (\preg_match('/[0-9a-fA-F]{4}/', $hex) !== 1) { - throw new SyntaxError( - $this->source, - $position - 1, - "Invalid character escape sequence: \\u{$hex}" - ); + throw new SyntaxError($this->source, $position - 1, "Invalid character escape sequence: \\u{$hex}"); } $code = \hexdec($hex); @@ -492,11 +476,7 @@ private function readString(int $line, int $col, Token $prev): Token if ($highOrderByte >= 0xD8 && $highOrderByte <= 0xDF) { [$utf16Continuation] = $this->readChars(6); if (\preg_match('/^\\\u[0-9a-fA-F]{4}$/', $utf16Continuation) !== 1) { - throw new SyntaxError( - $this->source, - $this->position - 5, - 'Invalid UTF-16 trailing surrogate: ' . $utf16Continuation - ); + throw new SyntaxError($this->source, $this->position - 5, 'Invalid UTF-16 trailing surrogate: ' . $utf16Continuation); } $surrogatePairHex = $hex . \substr($utf16Continuation, 2, 4); @@ -513,11 +493,7 @@ private function readString(int $line, int $col, Token $prev): Token continue 2; default: $chr = Utils::chr($code); - throw new SyntaxError( - $this->source, - $this->position - 1, - "Invalid character escape sequence: \\{$chr}" - ); + throw new SyntaxError($this->source, $this->position - 1, "Invalid character escape sequence: \\{$chr}"); } $chunk = ''; @@ -528,11 +504,7 @@ private function readString(int $line, int $col, Token $prev): Token [$char, $code, $bytes] = $this->readChar(); } - throw new SyntaxError( - $this->source, - $this->position, - 'Unterminated string.' - ); + throw new SyntaxError($this->source, $this->position, 'Unterminated string.'); } /** @@ -612,11 +584,7 @@ private function readBlockString(int $line, int $col, Token $prev): Token [$char, $code, $bytes] = $this->readChar(); } - throw new SyntaxError( - $this->source, - $this->position, - 'Unterminated string.' - ); + throw new SyntaxError($this->source, $this->position, 'Unterminated string.'); } /** diff --git a/src/Language/Parser.php b/src/Language/Parser.php index 5186fa69e..e387ca06d 100644 --- a/src/Language/Parser.php +++ b/src/Language/Parser.php @@ -379,11 +379,7 @@ private function expect(string $kind): Token return $token; } - throw new SyntaxError( - $this->lexer->source, - $token->start, - "Expected {$kind}, found {$token->getDescription()}" - ); + throw new SyntaxError($this->lexer->source, $token->start, "Expected {$kind}, found {$token->getDescription()}"); } /** @@ -397,11 +393,7 @@ private function expectKeyword(string $value): void { $token = $this->lexer->token; if ($token->kind !== Token::NAME || $token->value !== $value) { - throw new SyntaxError( - $this->lexer->source, - $token->start, - "Expected \"{$value}\", found {$token->getDescription()}" - ); + throw new SyntaxError($this->lexer->source, $token->start, "Expected \"{$value}\", found {$token->getDescription()}"); } $this->lexer->advance(); diff --git a/src/Type/Definition/EnumType.php b/src/Type/Definition/EnumType.php index 567abed8f..4d780862e 100644 --- a/src/Type/Definition/EnumType.php +++ b/src/Type/Definition/EnumType.php @@ -197,10 +197,7 @@ public function parseLiteral(Node $valueNode, ?array $variables = null) { if (! $valueNode instanceof EnumValueNode) { $valueStr = Printer::doPrint($valueNode); - throw new Error( - "Enum \"{$this->name}\" cannot represent non-enum value: {$valueStr}.{$this->didYouMean($valueStr)}", - $valueNode - ); + throw new Error("Enum \"{$this->name}\" cannot represent non-enum value: {$valueStr}.{$this->didYouMean($valueStr)}", $valueNode); } $name = $valueNode->value; diff --git a/src/Utils/ASTDefinitionBuilder.php b/src/Utils/ASTDefinitionBuilder.php index 7dfd7a571..886c4c8b3 100644 --- a/src/Utils/ASTDefinitionBuilder.php +++ b/src/Utils/ASTDefinitionBuilder.php @@ -275,14 +275,7 @@ private function internalBuildType(string $typeName, ?Node $typeNode = null): Ty ); } catch (\Throwable $e) { $class = static::class; - throw new Error( - "Type config decorator passed to {$class} threw an error when building {$typeName} type: {$e->getMessage()}", - null, - null, - [], - null, - $e - ); + throw new Error("Type config decorator passed to {$class} threw an error when building {$typeName} type: {$e->getMessage()}", null, null, [], null, $e); } // @phpstan-ignore-next-line should not happen, but function types are not enforced by PHP diff --git a/src/Utils/BuildClientSchema.php b/src/Utils/BuildClientSchema.php index 231702e40..54b78b7a5 100644 --- a/src/Utils/BuildClientSchema.php +++ b/src/Utils/BuildClientSchema.php @@ -151,12 +151,12 @@ public function buildSchema(): Schema return new Schema( (new SchemaConfig()) - ->setQuery($queryType) - ->setMutation($mutationType) - ->setSubscription($subscriptionType) - ->setTypes($this->typeMap) - ->setDirectives($directives) - ->setAssumeValid($this->options['assumeValid'] ?? false) + ->setQuery($queryType) + ->setMutation($mutationType) + ->setSubscription($subscriptionType) + ->setTypes($this->typeMap) + ->setDirectives($directives) + ->setAssumeValid($this->options['assumeValid'] ?? false) ); } diff --git a/src/Utils/BuildSchema.php b/src/Utils/BuildSchema.php index 87f7d73b9..2d04e4d03 100644 --- a/src/Utils/BuildSchema.php +++ b/src/Utils/BuildSchema.php @@ -245,24 +245,24 @@ static function (string $typeName): Type { return new Schema( (new SchemaConfig()) // @phpstan-ignore-next-line - ->setQuery(isset($operationTypes['query']) - ? $definitionBuilder->maybeBuildType($operationTypes['query']) - : null) + ->setQuery(isset($operationTypes['query']) + ? $definitionBuilder->maybeBuildType($operationTypes['query']) + : null) // @phpstan-ignore-next-line - ->setMutation(isset($operationTypes['mutation']) - ? $definitionBuilder->maybeBuildType($operationTypes['mutation']) - : null) + ->setMutation(isset($operationTypes['mutation']) + ? $definitionBuilder->maybeBuildType($operationTypes['mutation']) + : null) // @phpstan-ignore-next-line - ->setSubscription(isset($operationTypes['subscription']) - ? $definitionBuilder->maybeBuildType($operationTypes['subscription']) - : null) - ->setTypeLoader(static fn (string $name): ?Type => $definitionBuilder->maybeBuildType($name)) - ->setDirectives($directives) - ->setAstNode($schemaDef) - ->setTypes(fn (): array => \array_map( - static fn (TypeDefinitionNode $def): Type => $definitionBuilder->buildType($def->getName()->value), - $typeDefinitionsMap, - )) + ->setSubscription(isset($operationTypes['subscription']) + ? $definitionBuilder->maybeBuildType($operationTypes['subscription']) + : null) + ->setTypeLoader(static fn (string $name): ?Type => $definitionBuilder->maybeBuildType($name)) + ->setDirectives($directives) + ->setAstNode($schemaDef) + ->setTypes(fn (): array => \array_map( + static fn (TypeDefinitionNode $def): Type => $definitionBuilder->buildType($def->getName()->value), + $typeDefinitionsMap, + )) ); } diff --git a/src/Validator/Rules/QueryComplexity.php b/src/Validator/Rules/QueryComplexity.php index 02736ea21..6b00e36c7 100644 --- a/src/Validator/Rules/QueryComplexity.php +++ b/src/Validator/Rules/QueryComplexity.php @@ -179,13 +179,7 @@ protected function directiveExcludesField(FieldNode $node): bool $this->getRawVariableValues() ); if ($errors !== null && $errors !== []) { - throw new Error(\implode( - "\n\n", - \array_map( - static fn (Error $error): string => $error->getMessage(), - $errors - ) - )); + throw new Error(\implode("\n\n", \array_map(static fn (Error $error): string => $error->getMessage(), $errors))); } if ($directiveNode->name->value === Directive::INCLUDE_NAME) { @@ -248,13 +242,7 @@ protected function buildFieldArguments(FieldNode $node): array ); if (is_array($errors) && $errors !== []) { - throw new Error(\implode( - "\n\n", - \array_map( - static fn ($error) => $error->getMessage(), - $errors - ) - )); + throw new Error(\implode("\n\n", \array_map(static fn ($error) => $error->getMessage(), $errors))); } $args = Values::getArgumentValues($fieldDef, $node, $variableValues); diff --git a/tests/Executor/ExecutorTest.php b/tests/Executor/ExecutorTest.php index e8f5be331..4538f41f7 100644 --- a/tests/Executor/ExecutorTest.php +++ b/tests/Executor/ExecutorTest.php @@ -463,15 +463,7 @@ public function testNullsOutErrorSubtrees(): void throw new UserError('Error getting asyncReturnError'); }), 'asyncReturnErrorWithExtensions' => static fn (): Deferred => new Deferred(static function (): void { - throw new Error( - 'Error getting asyncReturnErrorWithExtensions', - null, - null, - [], - null, - null, - ['foo' => 'bar'] - ); + throw new Error('Error getting asyncReturnErrorWithExtensions', null, null, [], null, null, ['foo' => 'bar']); }), ]; diff --git a/tests/GraphQLTest.php b/tests/GraphQLTest.php index bb5ec9e72..04c869f99 100644 --- a/tests/GraphQLTest.php +++ b/tests/GraphQLTest.php @@ -18,20 +18,20 @@ public function testPromiseToExecute(): void $promiseAdapter = new SyncPromiseAdapter(); $schema = new Schema( (new SchemaConfig()) - ->setQuery(new ObjectType([ - 'name' => 'Query', - 'fields' => [ - 'sayHi' => [ - 'type' => Type::nonNull(Type::string()), - 'args' => [ - 'name' => [ - 'type' => Type::nonNull(Type::string()), + ->setQuery(new ObjectType([ + 'name' => 'Query', + 'fields' => [ + 'sayHi' => [ + 'type' => Type::nonNull(Type::string()), + 'args' => [ + 'name' => [ + 'type' => Type::nonNull(Type::string()), + ], ], + 'resolve' => static fn ($rootValue, array $args): Promise => $promiseAdapter->createFulfilled("Hi {$args['name']}!"), ], - 'resolve' => static fn ($rootValue, array $args): Promise => $promiseAdapter->createFulfilled("Hi {$args['name']}!"), ], - ], - ])) + ])) ); $promise = GraphQL::promiseToExecute($promiseAdapter, $schema, '{ sayHi(name: "John") }');