From 45e92c38f477496a5ca713bf7730edb988489508 Mon Sep 17 00:00:00 2001 From: raalderink Date: Fri, 21 Apr 2023 13:37:56 +0200 Subject: [PATCH] Set properties autowired with @required as initialized --- composer.json | 2 +- extension.neon | 7 ++ phpstan-baseline.neon | 15 +++ src/Symfony/RequiredAutowiringExtension.php | 95 +++++++++++++++++++ .../RequiredAutowiringExtensionTest.php | 71 ++++++++++++++ tests/Symfony/data/required-annotations.php | 38 ++++++++ tests/Symfony/data/required-attributes.php | 38 ++++++++ tests/Symfony/required-autowiring-config.neon | 6 ++ tests/Type/Symfony/ExtensionTest.php | 40 ++++---- .../Symfony/ExtensionTestWithoutContainer.php | 4 +- 10 files changed, 293 insertions(+), 23 deletions(-) create mode 100644 src/Symfony/RequiredAutowiringExtension.php create mode 100644 tests/Symfony/RequiredAutowiringExtensionTest.php create mode 100644 tests/Symfony/data/required-annotations.php create mode 100644 tests/Symfony/data/required-attributes.php create mode 100644 tests/Symfony/required-autowiring-config.neon diff --git a/composer.json b/composer.json index 22501953..d0edc5bc 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,7 @@ "require": { "php": "^7.2 || ^8.0", "ext-simplexml": "*", - "phpstan/phpstan": "^1.9.18" + "phpstan/phpstan": "^1.10.14" }, "conflict": { "symfony/framework-bundle": "<3.0" diff --git a/extension.neon b/extension.neon index 40165b39..fdfbfdcf 100644 --- a/extension.neon +++ b/extension.neon @@ -329,3 +329,10 @@ services: - factory: PHPStan\Type\Symfony\InputBagTypeSpecifyingExtension tags: [phpstan.typeSpecifier.methodTypeSpecifyingExtension] + + # Additional constructors and initialization checks for @required autowiring + - + class: PHPStan\Symfony\RequiredAutowiringExtension + tags: + - phpstan.properties.readWriteExtension + - phpstan.additionalConstructorsExtension diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 4abdaa0a..969bd7dc 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,5 +1,10 @@ parameters: ignoreErrors: + - + message: "#^Although PHPStan\\\\Reflection\\\\Php\\\\PhpPropertyReflection is covered by backward compatibility promise, this instanceof assumption might break because it's not guaranteed to always stay the same\\.$#" + count: 1 + path: src/Symfony/RequiredAutowiringExtension.php + - message: "#^Call to function method_exists\\(\\) with Symfony\\\\Component\\\\Console\\\\Input\\\\InputOption and 'isNegatable' will always evaluate to true\\.$#" count: 1 @@ -10,6 +15,16 @@ parameters: count: 1 path: tests/Rules/NonexistentInputBagClassTest.php + - + message: "#^Creating new PHPStan\\\\Reflection\\\\ConstructorsHelper is not covered by backward compatibility promise\\. The class might change in a minor PHPStan version\\.$#" + count: 1 + path: tests/Symfony/RequiredAutowiringExtensionTest.php + + - + message: "#^Creating new PHPStan\\\\Rules\\\\Properties\\\\UninitializedPropertyRule is not covered by backward compatibility promise\\. The class might change in a minor PHPStan version\\.$#" + count: 1 + path: tests/Symfony/RequiredAutowiringExtensionTest.php + - message: "#^Accessing PHPStan\\\\Rules\\\\Comparison\\\\ImpossibleCheckTypeMethodCallRule\\:\\:class is not covered by backward compatibility promise\\. The class might change in a minor PHPStan version\\.$#" count: 1 diff --git a/src/Symfony/RequiredAutowiringExtension.php b/src/Symfony/RequiredAutowiringExtension.php new file mode 100644 index 00000000..14552730 --- /dev/null +++ b/src/Symfony/RequiredAutowiringExtension.php @@ -0,0 +1,95 @@ +fileTypeMapper = $fileTypeMapper; + } + + public function isAlwaysRead(PropertyReflection $property, string $propertyName): bool + { + return false; + } + + public function isAlwaysWritten(PropertyReflection $property, string $propertyName): bool + { + return false; + } + + public function isInitialized(PropertyReflection $property, string $propertyName): bool + { + // If the property is public, check for @required on the property itself + if (!$property->isPublic()) { + return false; + } + + if ($property->getDocComment() !== null && $this->isRequiredFromDocComment($property->getDocComment())) { + return true; + } + + // Check for the attribute version + if (class_exists(Required::class) && $property instanceof PhpPropertyReflection && count($property->getNativeReflection()->getAttributes(Required::class)) > 0) { + return true; + } + + return false; + } + + public function getAdditionalConstructors(ClassReflection $classReflection): array + { + $additionalConstructors = []; + /** @var ReflectionClass $nativeReflection */ + $nativeReflection = $classReflection->getNativeReflection(); + + foreach ($nativeReflection->getMethods() as $method) { + if (!$method->isPublic()) { + continue; + } + + if ($method->getDocComment() !== false && $this->isRequiredFromDocComment($method->getDocComment())) { + $additionalConstructors[] = $method->getName(); + } + + if (!class_exists(Required::class) || count($method->getAttributes(Required::class)) === 0) { + continue; + } + + $additionalConstructors[] = $method->getName(); + } + + return $additionalConstructors; + } + + private function isRequiredFromDocComment(string $docComment): bool + { + $phpDoc = $this->fileTypeMapper->getResolvedPhpDoc(null, null, null, null, $docComment); + + foreach ($phpDoc->getPhpDocNodes() as $node) { + // @required tag is available, meaning this property is always initialized + if (count($node->getTagsByName('@required')) > 0) { + return true; + } + } + + return false; + } + +} diff --git a/tests/Symfony/RequiredAutowiringExtensionTest.php b/tests/Symfony/RequiredAutowiringExtensionTest.php new file mode 100644 index 00000000..06615b24 --- /dev/null +++ b/tests/Symfony/RequiredAutowiringExtensionTest.php @@ -0,0 +1,71 @@ + + */ +final class RequiredAutowiringExtensionTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + $container = self::getContainer(); + $container->getServicesByTag(AdditionalConstructorsExtension::EXTENSION_TAG); + + return new UninitializedPropertyRule( + new ConstructorsHelper( + $container, + [] + ) + ); + } + + public function testRequiredAnnotations(): void + { + $this->analyse([__DIR__ . '/data/required-annotations.php'], [ + [ + 'Class RequiredAnnotationTest\TestAnnotations has an uninitialized property $three. Give it default value or assign it in the constructor.', + 12, + ], + [ + 'Class RequiredAnnotationTest\TestAnnotations has an uninitialized property $four. Give it default value or assign it in the constructor.', + 14, + ], + ]); + } + + public function testRequiredAttributes(): void + { + if (!class_exists(Required::class)) { + self::markTestSkipped('Required symfony/service-contracts@3.2.1 or higher is not installed'); + } + + $this->analyse([__DIR__ . '/data/required-attributes.php'], [ + [ + 'Class RequiredAttributesTest\TestAttributes has an uninitialized property $three. Give it default value or assign it in the constructor.', + 14, + ], + [ + 'Class RequiredAttributesTest\TestAttributes has an uninitialized property $four. Give it default value or assign it in the constructor.', + 16, + ], + ]); + } + + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/required-autowiring-config.neon', + ]; + } + +} diff --git a/tests/Symfony/data/required-annotations.php b/tests/Symfony/data/required-annotations.php new file mode 100644 index 00000000..e2085ab3 --- /dev/null +++ b/tests/Symfony/data/required-annotations.php @@ -0,0 +1,38 @@ += 7.4 + +namespace RequiredAnnotationTest; + +class TestAnnotations +{ + /** @required */ + public string $one; + + private string $two; + + public string $three; + + private string $four; + + /** + * @required + */ + public function setTwo(int $two): void + { + $this->two = $two; + } + + public function getTwo(): int + { + return $this->two; + } + + public function setFour(int $four): void + { + $this->four = $four; + } + + public function getFour(): int + { + return $this->four; + } +} diff --git a/tests/Symfony/data/required-attributes.php b/tests/Symfony/data/required-attributes.php new file mode 100644 index 00000000..d847d276 --- /dev/null +++ b/tests/Symfony/data/required-attributes.php @@ -0,0 +1,38 @@ += 8.0 + +namespace RequiredAttributesTest; + +use Symfony\Contracts\Service\Attribute\Required; + +class TestAttributes +{ + #[Required] + public string $one; + + private string $two; + + public string $three; + + private string $four; + + #[Required] + public function setTwo(int $two): void + { + $this->two = $two; + } + + public function getTwo(): int + { + return $this->two; + } + + public function setFour(int $four): void + { + $this->four = $four; + } + + public function getFour(): int + { + return $this->four; + } +} diff --git a/tests/Symfony/required-autowiring-config.neon b/tests/Symfony/required-autowiring-config.neon new file mode 100644 index 00000000..3ff4183c --- /dev/null +++ b/tests/Symfony/required-autowiring-config.neon @@ -0,0 +1,6 @@ +services: + - + class: PHPStan\Symfony\RequiredAutowiringExtension + tags: + - phpstan.properties.readWriteExtension + - phpstan.additionalConstructorsExtension diff --git a/tests/Type/Symfony/ExtensionTest.php b/tests/Type/Symfony/ExtensionTest.php index 879abb5d..44ea88e2 100644 --- a/tests/Type/Symfony/ExtensionTest.php +++ b/tests/Type/Symfony/ExtensionTest.php @@ -14,49 +14,49 @@ class ExtensionTest extends TypeInferenceTestCase /** @return mixed[] */ public function dataFileAsserts(): iterable { - yield from $this->gatherAssertTypes(__DIR__ . '/data/envelope_all.php'); - yield from $this->gatherAssertTypes(__DIR__ . '/data/header_bag_get.php'); - yield from $this->gatherAssertTypes(__DIR__ . '/data/response_header_bag_get_cookies.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/envelope_all.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/header_bag_get.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/response_header_bag_get_cookies.php'); if (class_exists('Symfony\Component\HttpFoundation\InputBag')) { - yield from $this->gatherAssertTypes(__DIR__ . '/data/input_bag.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/input_bag.php'); } - yield from $this->gatherAssertTypes(__DIR__ . '/data/tree_builder.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/tree_builder.php'); - yield from $this->gatherAssertTypes(__DIR__ . '/data/ExampleBaseCommand.php'); - yield from $this->gatherAssertTypes(__DIR__ . '/data/ExampleOptionCommand.php'); - yield from $this->gatherAssertTypes(__DIR__ . '/data/ExampleOptionLazyCommand.php'); - yield from $this->gatherAssertTypes(__DIR__ . '/data/kernel_interface.php'); - yield from $this->gatherAssertTypes(__DIR__ . '/data/request_get_content.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/ExampleBaseCommand.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/ExampleOptionCommand.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/ExampleOptionLazyCommand.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/kernel_interface.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/request_get_content.php'); $ref = new ReflectionMethod(Request::class, 'getSession'); $doc = (string) $ref->getDocComment(); if (strpos($doc, '@return SessionInterface|null') !== false) { - yield from $this->gatherAssertTypes(__DIR__ . '/data/request_get_session_null.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/request_get_session_null.php'); } else { - yield from $this->gatherAssertTypes(__DIR__ . '/data/request_get_session.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/request_get_session.php'); } if (class_exists('Symfony\Bundle\FrameworkBundle\Controller\Controller')) { - yield from $this->gatherAssertTypes(__DIR__ . '/data/ExampleController.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/ExampleController.php'); } if (class_exists('Symfony\Bundle\FrameworkBundle\Controller\AbstractController')) { - yield from $this->gatherAssertTypes(__DIR__ . '/data/ExampleAbstractController.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/ExampleAbstractController.php'); } - yield from $this->gatherAssertTypes(__DIR__ . '/data/serializer.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/serializer.php'); if (class_exists('Symfony\Component\HttpFoundation\InputBag')) { - yield from $this->gatherAssertTypes(__DIR__ . '/data/input_bag_from_request.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/input_bag_from_request.php'); } - yield from $this->gatherAssertTypes(__DIR__ . '/data/denormalizer.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/denormalizer.php'); - yield from $this->gatherAssertTypes(__DIR__ . '/data/FormInterface_getErrors.php'); - yield from $this->gatherAssertTypes(__DIR__ . '/data/cache.php'); - yield from $this->gatherAssertTypes(__DIR__ . '/data/form_data_type.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/FormInterface_getErrors.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/cache.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/form_data_type.php'); } /** diff --git a/tests/Type/Symfony/ExtensionTestWithoutContainer.php b/tests/Type/Symfony/ExtensionTestWithoutContainer.php index fd1785c7..e07c2397 100644 --- a/tests/Type/Symfony/ExtensionTestWithoutContainer.php +++ b/tests/Type/Symfony/ExtensionTestWithoutContainer.php @@ -15,7 +15,7 @@ public function dataExampleController(): iterable return; } - yield from $this->gatherAssertTypes(__DIR__ . '/data/ExampleController.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/ExampleController.php'); } /** @return mixed[] */ @@ -25,7 +25,7 @@ public function dataAbstractController(): iterable return; } - yield from $this->gatherAssertTypes(__DIR__ . '/data/ExampleAbstractController.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/ExampleAbstractController.php'); } /**