diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index b37efd3a77..711c28ff9f 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -74,7 +74,7 @@ jobs: key: php-${{ matrix.php-version }}-composer-${{ matrix.laravel-version }}-os-${{ matrix.os }}-composer-${{ matrix.composer.name }} - name: "Remove conflicting dependencies that are not needed here" - run: composer remove --dev --no-update phpbench/phpbench rector/rector + run: composer remove --dev --no-update phpbench/phpbench - name: "Remove Pennant for Laravel 9 because it is not compatible" if: matrix.laravel-version == '^9' @@ -168,7 +168,7 @@ jobs: key: php-${{ matrix.php-version }}-composer-${{ matrix.laravel-version }}-os-${{ matrix.os }}-composer-${{ matrix.composer.name }} - name: "Remove conflicting dependencies that are not needed here" - run: composer remove --dev --no-update larastan/larastan phpstan/phpstan-mockery phpbench/phpbench rector/rector + run: composer remove --dev --no-update phpstan/phpstan-mockery phpbench/phpbench - name: "Remove Pennant for Laravel 9 because it is not compatible" if: matrix.laravel-version == '^9' @@ -224,7 +224,7 @@ jobs: path: ~/.composer/cache key: php-${{ matrix.php-version }}-composer-${{ matrix.laravel-version }} - - run: composer remove --dev phpbench/phpbench rector/rector --no-update + - run: composer remove --dev phpbench/phpbench --no-update - run: composer require laravel/framework:${{ matrix.laravel-version }} --no-interaction --prefer-dist --no-progress diff --git a/.gitignore b/.gitignore index 06e14c1c94..78e80e43b7 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ .vscode phpunit.xml .gitpod.yml +docs/superpowers # Generated files .phpunit.result.cache diff --git a/CHANGELOG.md b/CHANGELOG.md index c50455f9df..7e1940c1d1 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 +### Added + +- Add `RootResolverSignatureRector` rule to auto-fix root resolver `__invoke` signatures https://github.com/nuwave/lighthouse/pull/2779 + ## v6.68.0 ### Added diff --git a/docs/master/api-reference/resolvers.md b/docs/master/api-reference/resolvers.md index e91fabbe94..49d42dd18d 100644 --- a/docs/master/api-reference/resolvers.md +++ b/docs/master/api-reference/resolvers.md @@ -22,6 +22,77 @@ function (mixed $root, array $args, GraphQLContext $context, ResolveInfo $resolv The return value of this must fit the return type defined for the corresponding field from the schema. +## Root resolvers + +Root resolvers are classes with an `__invoke` method that sit directly in the configured +`lighthouse.namespaces.queries` or `lighthouse.namespaces.mutations` namespaces. +Lighthouse calls them with positional arguments `($root, $args, $context, $resolveInfo)` where `$root` is always `null` for root types. + +Omitting `$root` causes Lighthouse's positional `null` to bind to `$args`, producing TypeErrors at runtime. +The canonical signature for a root resolver is: + +```php +use Nuwave\Lighthouse\Execution\ResolveInfo; +use Nuwave\Lighthouse\Support\Contracts\GraphQLContext; + +class MyQuery +{ + public function __invoke(mixed $root, array $args, GraphQLContext $context, ResolveInfo $resolveInfo) + { + // ... + } +} +``` + +On PHP 8.2+, you can use the more precise `null` type instead of `mixed`. +The Rector rule below automatically picks the correct type for your PHP version. + +### Rector rule + +Lighthouse ships a Rector rule that enforces correct `__invoke` signatures on root resolvers. +Enable it in your `rector.php`: + +```php +use Nuwave\Lighthouse\Rector\RootResolverSignatureRector; +use Rector\Config\RectorConfig; + +return static function (RectorConfig $rectorConfig): void { + $rectorConfig->rule(RootResolverSignatureRector::class); + + // Required: Larastan bootstrap makes config() available + $rectorConfig->bootstrapFiles([ + __DIR__ . '/vendor/larastan/larastan/bootstrap.php', + ]); +}; +``` + +The rule fixes: + +- Missing `$root` parameter (detected when there is a single param typed `array` or untyped) +- Missing `$args` parameter when only `$root` is present +- Wrong type on the `$root` parameter (must be `null` on PHP 8.2+, `mixed` on earlier versions) +- Wrong type on `$args` (must be `array`) +- Wrong type on `$context` if present (must implement `GraphQLContext`) +- Wrong type on `$resolveInfo` if present (must be or extend `ResolveInfo`) + +#### Name normalization + +Optionally configure preferred parameter names: + +```php +$rectorConfig->ruleWithConfiguration(RootResolverSignatureRector::class, [ + 'paramNames' => ['_', 'args', 'context', 'resolveInfo'], +]); +``` + +Use `null` at any position to skip renaming that parameter: + +```php +$rectorConfig->ruleWithConfiguration(RootResolverSignatureRector::class, [ + 'paramNames' => [null, null, 'context', 'resolveInfo'], +]); +``` + ## Complexity function signature The complexity function is used to calculate a query complexity score for a field. diff --git a/src/Rector/RootResolverSignatureRector.php b/src/Rector/RootResolverSignatureRector.php new file mode 100644 index 0000000000..d5e65d4ab1 --- /dev/null +++ b/src/Rector/RootResolverSignatureRector.php @@ -0,0 +1,297 @@ + */ + protected array $paramNames = []; + + public function __construct( + protected PhpVersionProvider $phpVersionProvider, + protected VariableRenamer $variableRenamer, + ) {} + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition('Fix root resolver __invoke signatures to match Lighthouse calling convention', [ + new CodeSample( + <<<'CODE_SAMPLE' +namespace App\GraphQL\Queries; + +class Users +{ + public function __invoke(array $args) + { + return []; + } +} +CODE_SAMPLE, + <<<'CODE_SAMPLE' +namespace App\GraphQL\Queries; + +class Users +{ + public function __invoke(null $root, array $args) + { + return []; + } +} +CODE_SAMPLE, + ), + ]); + } + + /** @return array> */ + public function getNodeTypes(): array + { + return [Class_::class]; + } + + /** @param array $configuration */ + public function configure(array $configuration): void + { + $paramNames = $configuration['paramNames'] ?? []; + if (! is_array($paramNames)) { + throw new InvalidConfigurationException('paramNames must be an array.'); + } + + if (count($paramNames) > 4) { + throw new InvalidConfigurationException('paramNames must have at most 4 elements.'); + } + + foreach ($paramNames as $name) { + if ($name !== null && ! is_string($name)) { + throw new InvalidConfigurationException('Each paramNames element must be a string or null.'); + } + } + + $this->paramNames = $paramNames; + } + + /** @param Class_ $node */ + public function refactor(Node $node): ?Node + { + if (! $this->isRootResolver($node)) { + return null; + } + + $invokeMethod = $node->getMethod('__invoke'); + if (! $invokeMethod instanceof ClassMethod) { + return null; + } + + if ($invokeMethod->params === []) { + return null; + } + + $changed = false; + + if ($this->isMissingRootParam($invokeMethod)) { + $this->prependRootParam($invokeMethod); + $changed = true; + } elseif ($this->fixParamType($invokeMethod, 0, $this->rootTypeIdentifier())) { + $changed = true; + } + + if ($this->ensureArgsParam($invokeMethod)) { + $changed = true; + } + + if ($this->fixParamType($invokeMethod, 1, new Identifier('array'))) { + $changed = true; + } + + if (isset($invokeMethod->params[2]) && $this->fixObjectParam($invokeMethod, 2, \Nuwave\Lighthouse\Support\Contracts\GraphQLContext::class)) { + $changed = true; + } + + if (isset($invokeMethod->params[3]) && $this->fixObjectParam($invokeMethod, 3, \Nuwave\Lighthouse\Execution\ResolveInfo::class)) { + $changed = true; + } + + if ($this->normalizeNames($invokeMethod)) { + $changed = true; + } + + if (! $changed) { + return null; + } + + return $node; + } + + protected function isRootResolver(Class_ $node): bool + { + $fqcn = $node->namespacedName?->toString(); + if ($fqcn === null) { + return false; + } + + foreach ($this->resolverNamespaces() as $namespace) { + if ($this->isDirectChildOfNamespace($fqcn, $namespace)) { + return true; + } + } + + return false; + } + + /** + * Subscriptions are excluded — their resolver convention differs (they use subscriber classes, not __invoke). + * + * @return non-empty-list + */ + protected function resolverNamespaces(): array + { + $namespaces = [ + ...RootType::namespaces(RootType::QUERY), + ...RootType::namespaces(RootType::MUTATION), + ]; + + if ($namespaces === []) { + throw new \RuntimeException('Lighthouse resolver namespaces are empty. Ensure your Rector config includes bootstrapFiles with the Larastan bootstrap and Lighthouse config loaded.'); + } + + return $namespaces; + } + + protected function isDirectChildOfNamespace(string $fqcn, string $namespace): bool + { + return str_starts_with($fqcn, $namespace . '\\') + && ! str_contains(substr($fqcn, strlen($namespace) + 1), '\\'); + } + + protected function isMissingRootParam(ClassMethod $method): bool + { + if (count($method->params) !== 1) { + return false; + } + + $firstParam = $method->params[0]; + + return $firstParam->type === null + || ($firstParam->type instanceof Identifier && $firstParam->type->name === 'array'); + } + + protected function prependRootParam(ClassMethod $method): void + { + $rootParam = new Param( + new Variable('root'), + null, + $this->rootTypeIdentifier(), + ); + + array_unshift($method->params, $rootParam); + } + + protected function rootTypeIdentifier(): Identifier + { + if ($this->phpVersionProvider->isAtLeastPhpVersion(PhpVersion::PHP_82)) { + return new Identifier('null'); + } + + return new Identifier('mixed'); + } + + protected function fixParamType(ClassMethod $method, int $index, Identifier $expectedType): bool + { + if (! isset($method->params[$index])) { + return false; + } + + $param = $method->params[$index]; + $currentType = $param->type; + + if ($currentType instanceof Identifier && $currentType->name === $expectedType->name) { + return false; + } + + $param->type = $expectedType; + + return true; + } + + protected function ensureArgsParam(ClassMethod $method): bool + { + if (count($method->params) >= 2) { + return false; + } + + $method->params[] = new Param(new Variable('args'), null, new Identifier('array')); + + return true; + } + + protected function fixObjectParam(ClassMethod $method, int $index, string $expectedClass): bool + { + $param = $method->params[$index]; + $currentType = $param->type; + + if ($currentType instanceof FullyQualified || $currentType instanceof Node\Name) { + $objectType = new ObjectType($currentType->toString()); + $expectedType = new ObjectType($expectedClass); + + if ($expectedType->isSuperTypeOf($objectType)->yes()) { + return false; + } + } + + $param->type = new FullyQualified($expectedClass); + + return true; + } + + protected function normalizeNames(ClassMethod $method): bool + { + if ($this->paramNames === []) { + return false; + } + + $changed = false; + + foreach ($this->paramNames as $index => $name) { + if ($name === null) { + continue; + } + + if (! isset($method->params[$index])) { + continue; + } + + $param = $method->params[$index]; + if (! $param->var instanceof Variable) { + continue; + } + + $oldName = $param->var->name; + if (! is_string($oldName) || $oldName === $name) { + continue; + } + + $param->var = new Variable($name); + $this->variableRenamer->renameVariableInFunctionLike($method, $oldName, $name); + $changed = true; + } + + return $changed; + } +} diff --git a/src/Schema/RootType.php b/src/Schema/RootType.php index 75ca8afc14..1dc54f5816 100644 --- a/src/Schema/RootType.php +++ b/src/Schema/RootType.php @@ -21,4 +21,17 @@ public static function isRootType(string $typeName): bool ], ); } + + /** @return array */ + public static function namespaces(string $rootType): array + { + $key = match ($rootType) { + self::QUERY => 'queries', + self::MUTATION => 'mutations', + self::SUBSCRIPTION => 'subscriptions', + default => throw new \InvalidArgumentException("Invalid root type: {$rootType}."), + }; + + return (array) config("lighthouse.namespaces.{$key}"); + } } diff --git a/src/Schema/Values/FieldValue.php b/src/Schema/Values/FieldValue.php index d2e18ca430..c8497069fd 100644 --- a/src/Schema/Values/FieldValue.php +++ b/src/Schema/Values/FieldValue.php @@ -81,9 +81,9 @@ public function parentNamespaces(): array $parentName = $this->getParentName(); return match ($parentName) { - RootType::QUERY => (array) config('lighthouse.namespaces.queries'), - RootType::MUTATION => (array) config('lighthouse.namespaces.mutations'), - RootType::SUBSCRIPTION => (array) config('lighthouse.namespaces.subscriptions'), + RootType::QUERY, + RootType::MUTATION, + RootType::SUBSCRIPTION => RootType::namespaces($parentName), default => array_map( static fn (string $typesNamespace): string => "{$typesNamespace}\\{$parentName}", (array) config('lighthouse.namespaces.types'), diff --git a/tests/Unit/Rector/RootResolverSignatureRector/Fixture/array_root_with_args.php.inc b/tests/Unit/Rector/RootResolverSignatureRector/Fixture/array_root_with_args.php.inc new file mode 100644 index 0000000000..424fade346 --- /dev/null +++ b/tests/Unit/Rector/RootResolverSignatureRector/Fixture/array_root_with_args.php.inc @@ -0,0 +1,23 @@ + $configuration */ + #[DataProvider('invalidConfigurations')] + public function testRejectsInvalidConfiguration(array $configuration): void + { + $rector = $this->make(RootResolverSignatureRector::class); + + $this->expectException(InvalidConfigurationException::class); + $rector->configure($configuration); + } + + /** @return iterable}> */ + public static function invalidConfigurations(): iterable + { + yield 'too many elements' => [['paramNames' => ['a', 'b', 'c', 'd', 'e']]]; + yield 'non-string element' => [['paramNames' => [42]]]; + yield 'array element' => [['paramNames' => [[]]]]; + yield 'boolean element' => [['paramNames' => [true]]]; + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/tests/Unit/Rector/RootResolverSignatureRector/RootResolverSignatureRectorPartialNamesTest.php b/tests/Unit/Rector/RootResolverSignatureRector/RootResolverSignatureRectorPartialNamesTest.php new file mode 100644 index 0000000000..0ca68f136b --- /dev/null +++ b/tests/Unit/Rector/RootResolverSignatureRector/RootResolverSignatureRectorPartialNamesTest.php @@ -0,0 +1,25 @@ +doTestFile($filePath); + } + + public static function provideData(): \Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/FixturePartialNames'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule_partial_names.php'; + } +} diff --git a/tests/Unit/Rector/RootResolverSignatureRector/RootResolverSignatureRectorTest.php b/tests/Unit/Rector/RootResolverSignatureRector/RootResolverSignatureRectorTest.php new file mode 100644 index 0000000000..83130110f9 --- /dev/null +++ b/tests/Unit/Rector/RootResolverSignatureRector/RootResolverSignatureRectorTest.php @@ -0,0 +1,25 @@ +doTestFile($filePath); + } + + public static function provideData(): \Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/tests/Unit/Rector/RootResolverSignatureRector/RootResolverSignatureRectorWithNamesTest.php b/tests/Unit/Rector/RootResolverSignatureRector/RootResolverSignatureRectorWithNamesTest.php new file mode 100644 index 0000000000..3297c4a2a3 --- /dev/null +++ b/tests/Unit/Rector/RootResolverSignatureRector/RootResolverSignatureRectorWithNamesTest.php @@ -0,0 +1,25 @@ +doTestFile($filePath); + } + + public static function provideData(): \Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/FixtureWithNames'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule_with_names.php'; + } +} diff --git a/tests/Unit/Rector/RootResolverSignatureRector/Source/CustomContext.php b/tests/Unit/Rector/RootResolverSignatureRector/Source/CustomContext.php new file mode 100644 index 0000000000..8d35ba7f00 --- /dev/null +++ b/tests/Unit/Rector/RootResolverSignatureRector/Source/CustomContext.php @@ -0,0 +1,24 @@ +make('config')->set('lighthouse', require __DIR__ . '/../../../../src/lighthouse.php'); diff --git a/tests/Unit/Rector/RootResolverSignatureRector/config/configured_rule.php b/tests/Unit/Rector/RootResolverSignatureRector/config/configured_rule.php new file mode 100644 index 0000000000..04234a8c93 --- /dev/null +++ b/tests/Unit/Rector/RootResolverSignatureRector/config/configured_rule.php @@ -0,0 +1,13 @@ +rule(RootResolverSignatureRector::class); + $rectorConfig->importNames(); + $rectorConfig->bootstrapFiles([ + __DIR__ . '/../bootstrap.php', + __DIR__ . '/../Source/CustomContext.php', + ]); +}; diff --git a/tests/Unit/Rector/RootResolverSignatureRector/config/configured_rule_partial_names.php b/tests/Unit/Rector/RootResolverSignatureRector/config/configured_rule_partial_names.php new file mode 100644 index 0000000000..d5ccfe775c --- /dev/null +++ b/tests/Unit/Rector/RootResolverSignatureRector/config/configured_rule_partial_names.php @@ -0,0 +1,14 @@ +ruleWithConfiguration(RootResolverSignatureRector::class, [ + 'paramNames' => [null, null, 'context', 'resolveInfo'], + ]); + $rectorConfig->bootstrapFiles([ + __DIR__ . '/../bootstrap.php', + __DIR__ . '/../Source/CustomContext.php', + ]); +}; diff --git a/tests/Unit/Rector/RootResolverSignatureRector/config/configured_rule_with_names.php b/tests/Unit/Rector/RootResolverSignatureRector/config/configured_rule_with_names.php new file mode 100644 index 0000000000..d2894bfa9b --- /dev/null +++ b/tests/Unit/Rector/RootResolverSignatureRector/config/configured_rule_with_names.php @@ -0,0 +1,14 @@ +ruleWithConfiguration(RootResolverSignatureRector::class, [ + 'paramNames' => ['_', 'args', 'context', 'resolveInfo'], + ]); + $rectorConfig->bootstrapFiles([ + __DIR__ . '/../bootstrap.php', + __DIR__ . '/../Source/CustomContext.php', + ]); +};