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 authored and ondrejmirtes committed Sep 29, 2023
1 parent abc7682 commit 3838559
Show file tree
Hide file tree
Showing 8 changed files with 258 additions and 2 deletions.
5 changes: 3 additions & 2 deletions 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.36"
},
"conflict": {
"symfony/framework-bundle": "<3.0"
Expand All @@ -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
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
10 changes: 10 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,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
Expand Down
91 changes: 91 additions & 0 deletions src/Symfony/RequiredAutowiringExtension.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
<?php declare(strict_types = 1);

namespace PHPStan\Symfony;

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 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 ($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;
}

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

namespace PHPStan\Symfony;

use PHPStan\Reflection\AdditionalConstructorsExtension;
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 $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',
];
}

}
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

0 comments on commit 3838559

Please sign in to comment.