From 8ae7755def0e2236d0eec04abec9ad06e50dc405 Mon Sep 17 00:00:00 2001 From: Andrii Date: Fri, 3 Jan 2025 20:17:45 +0100 Subject: [PATCH 1/4] :package: Migrate to phpstan v2, add phpstan-symfony, use max phpstan level, make phpstan happy --- composer.json | 5 +- phpstan.neon | 14 ++-- src/Controller/GraphQLiteController.php | 1 + .../GraphQLiteCompilerPass.php | 65 ++++++++++--------- .../GraphQLiteExtension.php | 17 ++++- src/GraphiQL/EndpointResolver.php | 23 +++---- src/Server/ServerConfig.php | 4 +- src/Types/SymfonyUserInterfaceType.php | 18 +---- 8 files changed, 76 insertions(+), 71 deletions(-) diff --git a/composer.json b/composer.json index 1ad1550..59b6100 100644 --- a/composer.json +++ b/composer.json @@ -36,7 +36,8 @@ "symfony/yaml": "^6.4 || ^7", "beberlei/porpaginas": "^1.2 || ^2.0", "symfony/phpunit-bridge": "^6.4 || ^7", - "phpstan/phpstan": "^1.8", + "phpstan/phpstan": "^2", + "phpstan/phpstan-symfony": "^2.0", "composer/package-versions-deprecated": "^1.8", "composer/semver": "^3.4" }, @@ -47,7 +48,7 @@ "phpdocumentor/type-resolver": "<1.4" }, "scripts": { - "phpstan": "phpstan analyse -c phpstan.neon --level=7 --no-progress" + "phpstan": "phpstan analyse -c phpstan.neon --no-progress" }, "suggest": { "symfony/security-bundle": "To use #[Logged] or #[Right] attributes" diff --git a/phpstan.neon b/phpstan.neon index f4dcf24..53427c0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,20 +1,20 @@ includes: - - vendor/phpstan/phpstan/conf/bleedingEdge.neon + - vendor/phpstan/phpstan/conf/bleedingEdge.neon + - vendor/phpstan/phpstan-symfony/extension.neon + parameters: + level: max tmpDir: .phpstan-cache paths: - - . + - src excludePaths: - vendor - cache - .phpstan-cache - tests - level: max + polluteScopeWithLoopInitialAssignments: false polluteScopeWithAlwaysIterableForeach: false - checkAlwaysTrueCheckTypeFunctionCall: true - checkAlwaysTrueInstanceof: true - checkAlwaysTrueStrictComparison: true checkExplicitMixedMissingReturn: true checkFunctionNameCase: true checkInternalClassCaseSensitivity: true @@ -24,3 +24,5 @@ parameters: ignoreErrors: # Wrong return type hint in Symfony's TreeBuilder - '#Call to an undefined method Symfony\\Component\\Config\\Definition\\Builder\\NodeDefinition::\w+\(\).#' + # PhpStan doesn't know bundle resolved config structure and it's pretty complex to describe it + - '#Parameter \#2 \$value of method Symfony\\Component\\DependencyInjection\\Container::setParameter\(\) expects array\|bool\|float\|int\|string\|UnitEnum\|null, mixed given\.#' diff --git a/src/Controller/GraphQLiteController.php b/src/Controller/GraphQLiteController.php index 9ef3d44..25726ec 100644 --- a/src/Controller/GraphQLiteController.php +++ b/src/Controller/GraphQLiteController.php @@ -94,6 +94,7 @@ public function handleRequest(Request $request): Response if (class_exists(UploadMiddleware::class)) { $uploadMiddleware = new UploadMiddleware(); $psr7Request = $uploadMiddleware->processRequest($psr7Request); + \assert($psr7Request instanceof ServerRequestInterface); } return $this->handlePsr7Request($psr7Request, $request); diff --git a/src/DependencyInjection/GraphQLiteCompilerPass.php b/src/DependencyInjection/GraphQLiteCompilerPass.php index acc29d0..413eed5 100644 --- a/src/DependencyInjection/GraphQLiteCompilerPass.php +++ b/src/DependencyInjection/GraphQLiteCompilerPass.php @@ -49,7 +49,6 @@ use function filter_var; use function ini_get; use function interface_exists; -use function strpos; /** * Detects controllers and types automatically and tag them. @@ -199,52 +198,53 @@ public function process(ContainerBuilder $container): void $container->getDefinition(StaticClassListTypeMapperFactory::class)->setArgument(0, $staticTypes); } - foreach ($container->getDefinitions() as $id => $definition) { + foreach ($container->getDefinitions() as $definition) { if ($definition->isAbstract() || $definition->getClass() === null) { continue; } - /** - * @var class-string $class - */ + + /** @var class-string $class */ $class = $definition->getClass(); -/* foreach ($controllersNamespaces as $controllersNamespace) { - if (strpos($class, $controllersNamespace) === 0) { - $definition->addTag('graphql.annotated.controller'); - } - }*/ foreach ($typesNamespaces as $typesNamespace) { - if (strpos($class, $typesNamespace) === 0) { - //$definition->addTag('graphql.annotated.type'); - // Set the types public - $reflectionClass = new ReflectionClass($class); - $typeAnnotation = $this->getAnnotationReader()->getTypeAnnotation($reflectionClass); - if ($typeAnnotation !== null && $typeAnnotation->isSelfType()) { - continue; - } - if ($typeAnnotation !== null || $this->getAnnotationReader()->getExtendTypeAnnotation($reflectionClass) !== null) { + \assert(\is_string($typesNamespace)); + + if (false === str_starts_with($class, $typesNamespace)) { + continue; + } + + // Set the types public + $reflectionClass = new ReflectionClass($class); + $typeAnnotation = $this->getAnnotationReader()->getTypeAnnotation($reflectionClass); + if ($typeAnnotation !== null && $typeAnnotation->isSelfType()) { + continue; + } + if ($typeAnnotation !== null || $this->getAnnotationReader()->getExtendTypeAnnotation($reflectionClass) !== null) { + $definition->setPublic(true); + } + foreach ($reflectionClass->getMethods(ReflectionMethod::IS_PUBLIC) as $method) { + $factory = $reader->getFactoryAnnotation($method); + if ($factory !== null) { $definition->setPublic(true); } - foreach ($reflectionClass->getMethods(ReflectionMethod::IS_PUBLIC) as $method) { - $factory = $reader->getFactoryAnnotation($method); - if ($factory !== null) { - $definition->setPublic(true); - } - } } } } foreach ($controllersNamespaces as $controllersNamespace) { + \assert(\is_string($controllersNamespace)); + $schemaFactory->addMethodCall('addNamespace', [ $controllersNamespace ]); - foreach ($this->getClassList($controllersNamespace) as $className => $refClass) { + foreach ($this->getClassList($controllersNamespace) as $refClass) { $this->makePublicInjectedServices($refClass, $reader, $container, true); } } foreach ($typesNamespaces as $typeNamespace) { + \assert(\is_string($typeNamespace)); + $schemaFactory->addMethodCall('addNamespace', [ $typeNamespace ]); - foreach ($this->getClassList($typeNamespace) as $className => $refClass) { + foreach ($this->getClassList($typeNamespace) as $refClass) { $this->makePublicInjectedServices($refClass, $reader, $container, false); } } @@ -256,11 +256,14 @@ public function process(ContainerBuilder $container): void $customNotMappedTypes = []; foreach ($taggedServices as $id => $tags) { foreach ($tags as $attributes) { - if (isset($attributes["class"])) { - $phpClass = $attributes["class"]; - if (!class_exists($phpClass)) { + \assert(\is_array($attributes)); + + if (isset($attributes['class']) && is_string($attributes['class'])) { + $phpClass = $attributes['class']; + if (false === class_exists($phpClass)) { throw new \RuntimeException(sprintf('The class attribute of the graphql.output_type annotation of the %s service must point to an existing PHP class. Value passed: %s', $id, $phpClass)); } + $customTypes[$phpClass] = new Reference($id); } else { $customNotMappedTypes[] = new Reference($id); @@ -308,6 +311,8 @@ private function registerController(string $controllerClassName, ContainerBuilde $serviceLocatorMap = []; foreach ($controllersList as $controller) { + \assert(\is_string($controller)); + $serviceLocatorMap[$controller] = new Reference($controller); } diff --git a/src/DependencyInjection/GraphQLiteExtension.php b/src/DependencyInjection/GraphQLiteExtension.php index 7142518..3c0e47b 100644 --- a/src/DependencyInjection/GraphQLiteExtension.php +++ b/src/DependencyInjection/GraphQLiteExtension.php @@ -36,13 +36,17 @@ public function load(array $configs, ContainerBuilder $container): void $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config/container')); + \assert(\is_array($config['namespace'])); if (isset($config['namespace']['controllers'])) { $controllers = $config['namespace']['controllers']; if (!is_array($controllers)) { $controllers = [ $controllers ]; } + $namespaceController = array_map( function($namespace): string { + \assert(\is_string($namespace)); + return rtrim($namespace, '\\'); }, $controllers @@ -50,6 +54,8 @@ function($namespace): string { } else { $namespaceController = []; } + + \assert(\is_array($config['namespace'])); if (isset($config['namespace']['types'])) { $types = $config['namespace']['types']; if (!is_array($types)) { @@ -57,6 +63,8 @@ function($namespace): string { } $namespaceType = array_map( function($namespace): string { + \assert(\is_string($namespace)); + return rtrim($namespace, '\\'); }, $types @@ -65,6 +73,7 @@ function($namespace): string { $namespaceType = []; } + \assert(\is_array($config['security'])); $enableLogin = $config['security']['enable_login'] ?? 'auto'; $enableMe = $config['security']['enable_me'] ?? 'auto'; @@ -80,11 +89,15 @@ function($namespace): string { $loader->load('graphqlite.xml'); $definition = $container->getDefinition(ServerConfig::class); - if (isset($config['debug'])) { - $debugCode = $this->toDebugCode($config['debug']); + if (isset($config['debug']) && \is_array($config['debug'])) { + /** @var array $configDebug */ + $configDebug = $config['debug']; + + $debugCode = $this->toDebugCode($configDebug); } else { $debugCode = DebugFlag::RETHROW_UNSAFE_EXCEPTIONS; } + $definition->addMethodCall('setDebugFlag', [$debugCode]); $container->registerForAutoconfiguration(ObjectType::class) diff --git a/src/GraphiQL/EndpointResolver.php b/src/GraphiQL/EndpointResolver.php index 65bd107..8e32118 100644 --- a/src/GraphiQL/EndpointResolver.php +++ b/src/GraphiQL/EndpointResolver.php @@ -19,25 +19,20 @@ public function __construct(RequestStack $requestStack) $this->requestStack = $requestStack; } - /** - * @return string - */ - public function getBySchema($name) + public function getBySchema($name): string { - if ('default' === $name) { - $request = $this->requestStack->getCurrentRequest(); - assert(!is_null($request)); - - return $request->getBaseUrl().'/graphql'; + if ('default' !== $name) { + /** @phpstan-ignore throw.notThrowable (Missing return type in the library) */ + throw GraphQLEndpointInvalidSchemaException::forSchemaAndResolver($name, self::class); } - throw GraphQLEndpointInvalidSchemaException::forSchemaAndResolver($name, self::class); + $request = $this->requestStack->getCurrentRequest(); + assert(!is_null($request)); + + return $request->getBaseUrl().'/graphql'; } - /** - * @return string - */ - public function getDefault() + public function getDefault(): string { return $this->getBySchema('default'); } diff --git a/src/Server/ServerConfig.php b/src/Server/ServerConfig.php index cdbe5a1..a520535 100644 --- a/src/Server/ServerConfig.php +++ b/src/Server/ServerConfig.php @@ -12,13 +12,15 @@ /** * A slightly modified version of the server config: default validators are added by default when setValidators is called. + * + * @phpstan-type ValidationRulesResolveFn = callable(OperationParams, DocumentNode, string): array */ class ServerConfig extends \GraphQL\Server\ServerConfig { /** * Set validation rules for this server, AND adds by default all the "default" validation rules provided by Webonyx * - * @param ValidationRule[]|callable $validationRules + * @param ValidationRule[]|ValidationRulesResolveFn $validationRules * * @api */ diff --git a/src/Types/SymfonyUserInterfaceType.php b/src/Types/SymfonyUserInterfaceType.php index 26ea2f4..ff8268a 100644 --- a/src/Types/SymfonyUserInterfaceType.php +++ b/src/Types/SymfonyUserInterfaceType.php @@ -15,17 +15,7 @@ class SymfonyUserInterfaceType #[Field] public function getUserName(UserInterface $user): string { - // @phpstan-ignore-next-line Forward Compatibility for Symfony >=5.3 - if (method_exists($user, 'getUserIdentifier')) { - return $user->getUserIdentifier(); - } - - // @phpstan-ignore-next-line Backward Compatibility for Symfony <5.3 - if (method_exists($user, 'getUsername')) { - return $user->getUsername(); - } - - throw FieldNotFoundException::missingField(UserInterface::class, 'userName'); + return $user->getUserIdentifier(); } /** @@ -36,13 +26,9 @@ public function getRoles(UserInterface $user): array { $roles = []; foreach ($user->getRoles() as $role) { - // @phpstan-ignore-next-line BC for Symfony 4 - if (class_exists(Role::class) && $role instanceof Role) { - $role = $role->getRole(); - } - $roles[] = $role; } + return $roles; } } From b3bb280fbd92b54fdbfec8dc4f6ff5fa81d41e62 Mon Sep 17 00:00:00 2001 From: Andrii Date: Fri, 3 Jan 2025 20:24:03 +0100 Subject: [PATCH 2/4] :sparkles: Introduce check for PHP 8.3, 8.4 in CI --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 1882ae9..ff0fdeb 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -12,7 +12,7 @@ jobs: strategy: matrix: install-args: [''] - php-version: ['8.2'] + php-version: ['8.2', '8.3', '8.4'] fail-fast: false steps: # Cancel previous runs of the same branch From 094c5576fc256fb12ee3a264bf53b33dc1ae0b01 Mon Sep 17 00:00:00 2001 From: Andrii Date: Fri, 3 Jan 2025 20:26:14 +0100 Subject: [PATCH 3/4] :package: Fix PHP 8.4 deprecations --- src/Controller/GraphQL/InvalidUserPasswordException.php | 2 +- src/Controller/GraphQLiteController.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Controller/GraphQL/InvalidUserPasswordException.php b/src/Controller/GraphQL/InvalidUserPasswordException.php index e409002..f48a9b0 100644 --- a/src/Controller/GraphQL/InvalidUserPasswordException.php +++ b/src/Controller/GraphQL/InvalidUserPasswordException.php @@ -8,7 +8,7 @@ class InvalidUserPasswordException extends GraphQLException { - public static function create(Exception $previous = null): self + public static function create(?Exception $previous = null): self { return new self('The provided user / password is incorrect.', 401, $previous, ['category' => 'Security']); } diff --git a/src/Controller/GraphQLiteController.php b/src/Controller/GraphQLiteController.php index 25726ec..81bf819 100644 --- a/src/Controller/GraphQLiteController.php +++ b/src/Controller/GraphQLiteController.php @@ -48,7 +48,7 @@ class GraphQLiteController */ private $httpCodeDecider; - public function __construct(ServerConfig $serverConfig, HttpMessageFactoryInterface $httpMessageFactory = null, ?int $debug = null, ?HttpCodeDeciderInterface $httpCodeDecider = null) + public function __construct(ServerConfig $serverConfig, ?HttpMessageFactoryInterface $httpMessageFactory = null, ?int $debug = null, ?HttpCodeDeciderInterface $httpCodeDecider = null) { $this->serverConfig = $serverConfig; $this->httpMessageFactory = $httpMessageFactory ?: new PsrHttpFactory(new ServerRequestFactory(), new StreamFactory(), new UploadedFileFactory(), new ResponseFactory()); From 11deb0c2bb532f6602d3c7aa55b4620463590a19 Mon Sep 17 00:00:00 2001 From: Andrii Date: Fri, 3 Jan 2025 20:31:30 +0100 Subject: [PATCH 4/4] :package: Make IDE happy --- src/Controller/GraphQLiteController.php | 2 +- src/Mappers/RequestParameter.php | 2 +- src/Security/AuthorizationService.php | 2 +- src/Types/SymfonyUserInterfaceType.php | 2 -- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Controller/GraphQLiteController.php b/src/Controller/GraphQLiteController.php index 81bf819..aaa91fa 100644 --- a/src/Controller/GraphQLiteController.php +++ b/src/Controller/GraphQLiteController.php @@ -78,7 +78,7 @@ public function handleRequest(Request $request): Response { $psr7Request = $this->httpMessageFactory->createRequest($request); - if (strtoupper($request->getMethod()) === "POST" && empty($psr7Request->getParsedBody())) { + if (strtoupper($request->getMethod()) === 'POST' && empty($psr7Request->getParsedBody())) { $content = $psr7Request->getBody()->getContents(); $parsedBody = json_decode($content, true); if (json_last_error() !== JSON_ERROR_NONE) { diff --git a/src/Mappers/RequestParameter.php b/src/Mappers/RequestParameter.php index 1f88336..fdba728 100644 --- a/src/Mappers/RequestParameter.php +++ b/src/Mappers/RequestParameter.php @@ -16,7 +16,7 @@ class RequestParameter implements ParameterInterface * @param array $args * @param mixed $context */ - public function resolve(?object $source, array $args, $context, ResolveInfo $info): mixed + public function resolve(?object $source, array $args, mixed $context, ResolveInfo $info): mixed { if (!$context instanceof SymfonyRequestContextInterface) { throw new GraphQLException('Cannot type-hint on a Symfony Request object in your query/mutation/field. The request context must implement SymfonyRequestContextInterface.'); diff --git a/src/Security/AuthorizationService.php b/src/Security/AuthorizationService.php index 9ace90c..83ac407 100644 --- a/src/Security/AuthorizationService.php +++ b/src/Security/AuthorizationService.php @@ -30,7 +30,7 @@ public function __construct(?AuthorizationCheckerInterface $authorizationChecker * * @param mixed $subject The scope this right applies on. $subject is typically an object or a FQCN. Set $subject to "null" if the right is global. */ - public function isAllowed(string $right, $subject = null): bool + public function isAllowed(string $right, mixed $subject = null): bool { if ($this->authorizationChecker === null || $this->tokenStorage === null) { throw new \LogicException('The SecurityBundle is not registered in your application. Try running "composer require symfony/security-bundle".'); diff --git a/src/Types/SymfonyUserInterfaceType.php b/src/Types/SymfonyUserInterfaceType.php index ff8268a..bad1837 100644 --- a/src/Types/SymfonyUserInterfaceType.php +++ b/src/Types/SymfonyUserInterfaceType.php @@ -3,11 +3,9 @@ namespace TheCodingMachine\GraphQLite\Bundle\Types; -use Symfony\Component\Security\Core\Role\Role; use TheCodingMachine\GraphQLite\Annotations\Field; use TheCodingMachine\GraphQLite\Annotations\Type; use Symfony\Component\Security\Core\User\UserInterface; -use TheCodingMachine\GraphQLite\FieldNotFoundException; #[Type(class: UserInterface::class)] class SymfonyUserInterfaceType