Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set properties autowired with @required as initialized #348

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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/[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