From 383855999db6a7d65d0bf580ce2762e17188c2a5 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 | 5 +- extension.neon | 7 ++ phpstan-baseline.neon | 10 ++ src/Symfony/RequiredAutowiringExtension.php | 91 +++++++++++++++++++ .../RequiredAutowiringExtensionTest.php | 65 +++++++++++++ tests/Symfony/data/required-annotations.php | 38 ++++++++ tests/Symfony/data/required-attributes.php | 38 ++++++++ tests/Symfony/required-autowiring-config.neon | 6 ++ 8 files changed, 258 insertions(+), 2 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 ca18861b..b8f33962 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.36" }, "conflict": { "symfony/framework-bundle": "<3.0" @@ -35,7 +35,8 @@ "symfony/http-foundation": "^5.4 || ^6.1", "symfony/messenger": "^5.4", "symfony/polyfill-php80": "^1.24", - "symfony/serializer": "^5.4" + "symfony/serializer": "^5.4", + "symfony/service-contracts": "^2.2.0" }, "config": { "sort-packages": true 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..0f6edd5c 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,11 @@ parameters: count: 1 path: tests/Rules/NonexistentInputBagClassTest.php + - + message: "#^Accessing PHPStan\\\\Rules\\\\Properties\\\\UninitializedPropertyRule\\:\\:class 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..a3cc6a31 --- /dev/null +++ b/src/Symfony/RequiredAutowiringExtension.php @@ -0,0 +1,91 @@ +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 ($property instanceof PhpPropertyReflection && count($property->getNativeReflection()->getAttributes('Symfony\Contracts\Service\Attribute\Required')) > 0) { + return true; + } + + return false; + } + + public function getAdditionalConstructors(ClassReflection $classReflection): array + { + $additionalConstructors = []; + $nativeReflection = $classReflection->getNativeReflection(); + + foreach ($nativeReflection->getMethods() as $method) { + if (!$method->isPublic()) { + continue; + } + + if ($method->getDocComment() !== false && $this->isRequiredFromDocComment($method->getDocComment())) { + $additionalConstructors[] = $method->getName(); + } + + if (count($method->getAttributes('Symfony\Contracts\Service\Attribute\Required')) === 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..93fb3822 --- /dev/null +++ b/tests/Symfony/RequiredAutowiringExtensionTest.php @@ -0,0 +1,65 @@ + + */ +final class RequiredAutowiringExtensionTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + $container = self::getContainer(); + $container->getServicesByTag(AdditionalConstructorsExtension::EXTENSION_TAG); + + return $container->getByType(UninitializedPropertyRule::class); + } + + 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