chore: bump phpstan version #7239

VincentLanglet · 2025-06-20T15:57:51Z

Q	A
Branch?	main
Tickets	Closes #..., closes #...
License	MIT
Doc PR	api-platform/docs#...

Hi @soyuka,

I wanted to bump PHPStan level to 6 and saw it was still using v1 ; so I started by bumping to v2.
Since there is lot of new errors I used the following strategy

Fix the easy ones
Ignore some of the new errors reported by PHPStan v2

Then I'll provide PR to fix some identifiers error, but I think it's better to split PR for readability.

VincentLanglet · 2025-06-20T15:58:13Z

phpstan.neon.dist

@@ -17,7 +17,6 @@ parameters:
 	excludePaths:
 		# uses larastan
 		- src/Laravel
-		- src/Symfony/Bundle/Command/OpenApiCommand.php


Those files doesn't exists and are reported by PHPStan v2 now.

VincentLanglet · 2025-06-20T15:58:43Z

phpstan.neon.dist

@@ -75,6 +71,15 @@ parameters:
 		# Expected, due to backward compatibility
 		- '#Method GraphQL\\Type\\Definition\\WrappingType::getWrappedType\(\) invoked with 1 parameter, 0 required\.#'
 		- '#Access to an undefined property GraphQL\\Type\\Definition\\NamedType&GraphQL\\Type\\Definition\\Type::\$name\.#'
+		- "#Call to function method_exists\\(\\) with GraphQL\\\\Type\\\\Definition\\\\Type&GraphQL\\\\Type\\\\Definition\\\\WrappingType and 'getInnermostType' will always evaluate to true\\.#"


There is a lot of method_exists call added for BC, so I updated the ignored errors.

VincentLanglet · 2025-06-20T15:59:48Z

phpstan.neon.dist

+		# Allow extra assertions in tests: https://github.com/phpstan/phpstan-strict-rules/issues/130
+		- '#^Call to (static )?method PHPUnit\\Framework\\Assert::.* will always evaluate to true\.$#'
+
+		# TODO For PHPStan 2.0


This is a list of errors which I'd like to fix in following PR.

VincentLanglet · 2025-06-20T16:00:20Z

src/Doctrine/Orm/State/CollectionProvider.php

@@ -56,10 +56,6 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
        $manager = $this->managerRegistry->getManagerForClass($entityClass);

        $repository = $manager->getRepository($entityClass);
-        if (!method_exists($repository, 'createQueryBuilder')) {


$repository is always an EntityRepository according to PHPStan and I think it's right.

yes therefore we can remove that line instead of ignoring ?

This will cause some failure in tests because of testCannotCreateQueryBuilder

public function testCannotCreateQueryBuilder(): void { $this->expectException(RuntimeException::class); $this->expectExceptionMessage('The repository class must have a "createQueryBuilder" method.'); $repositoryProphecy = $this->prophesize(ObjectRepository::class); $managerProphecy = $this->prophesize(ObjectManager::class); $managerProphecy->getRepository(OperationResource::class)->willReturn($repositoryProphecy->reveal())->shouldBeCalled(); $managerRegistryProphecy = $this->prophesize(ManagerRegistry::class); $managerRegistryProphecy->getManagerForClass(OperationResource::class)->willReturn($managerProphecy->reveal())->shouldBeCalled(); $dataProvider = new CollectionProvider($this->prophesize(ResourceMetadataCollectionFactoryInterface::class)->reveal(), $managerRegistryProphecy->reveal()); $operation = (new GetCollection())->withClass(OperationResource::class)->withName('getCollection'); $this->assertEquals([], $dataProvider->provide($operation)); }

The fact is that

An entityManagerINterface always returns an EntityRepository which has the createQueryBuilder

The existing phpdoc say the manager for class is an EntityManagerInterface but it can theorically be a simple ObjectManager (which is the case tested in tests)

/** @var EntityManagerInterface|null $manager */ $manager = $this->managerRegistry->getManagerForClass($entityClass);

So I'm unsure how you want to solve/change this @soyuka

VincentLanglet · 2025-06-20T16:00:33Z

src/Doctrine/Orm/State/ItemProvider.php

@@ -65,10 +65,6 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
        }

        $repository = $manager->getRepository($entityClass);
-        if (!method_exists($repository, 'createQueryBuilder')) {


VincentLanglet · 2025-06-20T16:03:13Z

src/Metadata/Tests/Resource/Factory/ParameterResourceMetadataCollectionFactoryTest.php

@@ -41,6 +41,7 @@ public function testParameterFactory(): void
        $filterLocator->method('get')->willReturn(new class implements FilterInterface {
            public function getDescription(string $resourceClass): array
            {
+                // @phpstan-ignore-next-line return.type


Seems like we're passing volontary a wrong type to check it's handle in the method.

VincentLanglet · 2025-06-20T16:09:01Z

src/Serializer/AbstractCollectionNormalizer.php

@@ -101,7 +101,7 @@ public function normalize(mixed $object, ?string $format = null, array $context
     */
    protected function normalizeRawCollection(iterable $object, ?string $format = null, array $context = []): array|\ArrayObject
    {
-        if (!$object && ($context[Serializer::EMPTY_ARRAY_AS_OBJECT] ?? false) && \is_array($object)) {
+        if ([] === $object && ($context[Serializer::EMPTY_ARRAY_AS_OBJECT] ?? false)) {


!$object already enforce it's [].

But I rewrote the condition to be explicit

VincentLanglet · 2025-06-20T16:10:09Z

src/Serializer/AbstractItemNormalizer.php

@@ -1163,9 +1163,10 @@ private function createAndValidateAttributeValue(string $attribute, mixed $value
            }

            if (
-                ($t instanceof CollectionType && $collectionValueType instanceof ObjectType && (null !== ($className = $collectionValueType->getClassName())))
-                || ($t instanceof LegacyType && $t->isCollection() && null !== $collectionValueType && null !== ($className = $collectionValueType->getClassName()))
+                ($t instanceof CollectionType && $collectionValueType instanceof ObjectType)


In this condition (null !== ($className = $collectionValueType->getClassName())) is always true cause getClassName returns a string.

So we need to remove it from the condition, and it force declaring the variable after the if.

VincentLanglet · 2025-06-20T16:11:06Z

src/Symfony/Bundle/Test/Constraint/MatchesJsonSchema.php

@@ -91,9 +91,6 @@ private function normalizeJson(mixed $document): object|array
        }

        $document = json_encode($document, \JSON_THROW_ON_ERROR);
-        if (!\is_string($document)) {


$document is always a string because of JSON_THROW_ON_ERROR

VincentLanglet · 2025-06-20T16:11:20Z

tests/Symfony/Bundle/Test/ClientTest.php

@@ -47,7 +47,6 @@ public static function getResources(): array
    public function testRequest(): void
    {
        $client = self::createClient();
-        $client->getKernelBrowser();


Useless call

soyuka

Most of these are fine I think it'd be a good start, just a few comments.

Also any reason we don't do this on the 4.1 branch? Can you also rebase on the latest main branch?

soyuka · 2025-06-26T07:42:12Z

src/Doctrine/Orm/State/CollectionProvider.php

@@ -56,10 +56,6 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
        $manager = $this->managerRegistry->getManagerForClass($entityClass);

        $repository = $manager->getRepository($entityClass);
-        if (!method_exists($repository, 'createQueryBuilder')) {


yes therefore we can remove that line instead of ignoring ?

composer.json

VincentLanglet · 2025-06-26T08:23:45Z

Also any reason we don't do this on the 4.1 branch?

There is some extra changes in main like

New feature/simplification
Added @phpstan-ignore tags cf conflict when cherry-picking this on 4.1

        both modified:   src/JsonSchema/Tests/Metadata/Property/Factory/SchemaPropertyMetadataFactoryTest.php
        both modified:   src/JsonSchema/Tests/SchemaFactoryTest.php
        both modified:   src/Metadata/Tests/Property/Factory/SerializerPropertyMetadataFactoryTest.php
        both modified:   src/Metadata/Tests/Util/PropertyInfoToTypeInfoHelperTest.php
        both modified:   src/Serializer/AbstractItemNormalizer.php
        both modified:   src/Serializer/Tests/AbstractItemNormalizerTest.php

Which means that bumping phpstan on 4.1, will give failure on main branch and require extra work. Especially if we do more PR to solve more PHPStan errors.

Wouldn't it be ok to plan a 4.2 release instead ?

soyuka · 2025-06-27T13:58:39Z

Thanks @VincentLanglet this is a good starting point!

VincentLanglet force-pushed the bumpPhpstan branch from 12d82b6 to 739f51c Compare June 20, 2025 15:59

VincentLanglet commented Jun 20, 2025

View reviewed changes

VincentLanglet force-pushed the bumpPhpstan branch 3 times, most recently from 299f4cb to 6837010 Compare June 20, 2025 16:56

VincentLanglet mentioned this pull request Jun 20, 2025

chore: phpstan:^2.0 #7154

Closed

soyuka requested changes Jun 26, 2025

View reviewed changes

VincentLanglet force-pushed the bumpPhpstan branch from 5afe28a to b008fb4 Compare June 26, 2025 08:15

VincentLanglet added 2 commits June 26, 2025 10:16

chore: bump phpstan version

ebf549a

chore: bump

ac5b05c

VincentLanglet force-pushed the bumpPhpstan branch from b008fb4 to ac5b05c Compare June 26, 2025 08:16

VincentLanglet requested a review from soyuka June 26, 2025 08:24

soyuka merged commit b48501a into api-platform:main Jun 27, 2025
111 of 114 checks passed

VincentLanglet mentioned this pull request Jun 27, 2025

chore: solve some phpstan issues #7249

Merged

Uh oh!

chore: bump phpstan version #7239

chore: bump phpstan version #7239

Uh oh!

Conversation

VincentLanglet commented Jun 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soyuka left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

VincentLanglet commented Jun 26, 2025

Uh oh!

Uh oh!

soyuka commented Jun 27, 2025

Uh oh!

Uh oh!

soyuka left a comment •

edited

Loading