Skip to content

Commit

Permalink
Set properties autowired with @required as initialized
Browse files Browse the repository at this point in the history
  • Loading branch information
raalderink committed Apr 25, 2023
1 parent db75c81 commit 45e92c3
Show file tree
Hide file tree
Showing 10 changed files with 293 additions and 23 deletions.
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 7 additions & 0 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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
15 changes: 15 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
95 changes: 95 additions & 0 deletions src/Symfony/RequiredAutowiringExtension.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
<?php declare(strict_types = 1);

namespace PHPStan\Symfony;

use PHPStan\BetterReflection\Reflection\Adapter\ReflectionClass;
use PHPStan\Reflection\AdditionalConstructorsExtension;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\Php\PhpPropertyReflection;
use PHPStan\Reflection\PropertyReflection;
use PHPStan\Rules\Properties\ReadWritePropertiesExtension;
use PHPStan\Type\FileTypeMapper;
use Symfony\Contracts\Service\Attribute\Required;
use function class_exists;
use function count;

class RequiredAutowiringExtension implements ReadWritePropertiesExtension, AdditionalConstructorsExtension
{

/** @var FileTypeMapper */
private $fileTypeMapper;

public function __construct(FileTypeMapper $fileTypeMapper)
{
$this->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;
}

}
71 changes: 71 additions & 0 deletions tests/Symfony/RequiredAutowiringExtensionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php declare(strict_types = 1);

namespace PHPStan\Symfony;

use PHPStan\Reflection\AdditionalConstructorsExtension;
use PHPStan\Reflection\ConstructorsHelper;
use PHPStan\Rules\Properties\UninitializedPropertyRule;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use Symfony\Contracts\Service\Attribute\Required;
use function class_exists;

/**
* @extends RuleTestCase<UninitializedPropertyRule>
*/
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/[email protected] 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',
];
}

}
38 changes: 38 additions & 0 deletions tests/Symfony/data/required-annotations.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php // lint >= 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;
}
}
38 changes: 38 additions & 0 deletions tests/Symfony/data/required-attributes.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php // lint >= 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;
}
}
6 changes: 6 additions & 0 deletions tests/Symfony/required-autowiring-config.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
services:
-
class: PHPStan\Symfony\RequiredAutowiringExtension
tags:
- phpstan.properties.readWriteExtension
- phpstan.additionalConstructorsExtension
40 changes: 20 additions & 20 deletions tests/Type/Symfony/ExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/Type/Symfony/ExtensionTestWithoutContainer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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[] */
Expand All @@ -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');
}

/**
Expand Down

0 comments on commit 45e92c3

Please sign in to comment.