From 342c75f6e5d97c3450f63cd41743ea7e4d4059d2 Mon Sep 17 00:00:00 2001 From: schlndh Date: Fri, 14 Jul 2023 13:20:23 +0200 Subject: [PATCH] report trait access to unititialized property in the trait instead of class --- src/Node/ClassPropertiesNode.php | 3 +- ...singReadOnlyByPhpDocPropertyAssignRule.php | 4 +- .../MissingReadOnlyPropertyAssignRule.php | 4 +- .../Properties/UninitializedPropertyRule.php | 4 +- .../AnalyserTraitsIntegrationTest.php | 47 +++++++++++++++++++ .../PHPStan/Analyser/traits-integration.neon | 2 + .../traits/uninitializedProperty/FooClass.php | 16 +++++++ .../traits/uninitializedProperty/FooTrait.php | 19 ++++++++ 8 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 tests/PHPStan/Analyser/traits-integration.neon create mode 100644 tests/PHPStan/Analyser/traits/uninitializedProperty/FooClass.php create mode 100644 tests/PHPStan/Analyser/traits/uninitializedProperty/FooTrait.php diff --git a/src/Node/ClassPropertiesNode.php b/src/Node/ClassPropertiesNode.php index e9d9a36b80..43acec7002 100644 --- a/src/Node/ClassPropertiesNode.php +++ b/src/Node/ClassPropertiesNode.php @@ -92,7 +92,7 @@ public function getClassReflection(): ClassReflection /** * @param string[] $constructors * @param ReadWritePropertiesExtension[]|null $extensions - * @return array{array, array, array} + * @return array{array, array, array} */ public function getUninitializedProperties( Scope $scope, @@ -212,6 +212,7 @@ public function getUninitializedProperties( $propertyName, $fetch->getLine(), $originalProperties[$propertyName], + $usageScope->getFileDescription(), ]; } } diff --git a/src/Rules/Properties/MissingReadOnlyByPhpDocPropertyAssignRule.php b/src/Rules/Properties/MissingReadOnlyByPhpDocPropertyAssignRule.php index 7523b42c8a..b718d17de0 100644 --- a/src/Rules/Properties/MissingReadOnlyByPhpDocPropertyAssignRule.php +++ b/src/Rules/Properties/MissingReadOnlyByPhpDocPropertyAssignRule.php @@ -44,7 +44,7 @@ public function processNode(Node $node, Scope $scope): array ))->line($propertyNode->getLine())->build(); } - foreach ($prematureAccess as [$propertyName, $line, $propertyNode]) { + foreach ($prematureAccess as [$propertyName, $line, $propertyNode, $file]) { if (!$propertyNode->isReadOnlyByPhpDoc() || $propertyNode->isReadOnly()) { continue; } @@ -52,7 +52,7 @@ public function processNode(Node $node, Scope $scope): array 'Access to an uninitialized @readonly property %s::$%s.', $classReflection->getDisplayName(), $propertyName, - ))->line($line)->build(); + ))->line($line)->file($file)->build(); } foreach ($additionalAssigns as [$propertyName, $line, $propertyNode]) { diff --git a/src/Rules/Properties/MissingReadOnlyPropertyAssignRule.php b/src/Rules/Properties/MissingReadOnlyPropertyAssignRule.php index 2657d2bd9e..e483243228 100644 --- a/src/Rules/Properties/MissingReadOnlyPropertyAssignRule.php +++ b/src/Rules/Properties/MissingReadOnlyPropertyAssignRule.php @@ -44,7 +44,7 @@ public function processNode(Node $node, Scope $scope): array ))->line($propertyNode->getLine())->build(); } - foreach ($prematureAccess as [$propertyName, $line, $propertyNode]) { + foreach ($prematureAccess as [$propertyName, $line, $propertyNode, $file]) { if (!$propertyNode->isReadOnly()) { continue; } @@ -52,7 +52,7 @@ public function processNode(Node $node, Scope $scope): array 'Access to an uninitialized readonly property %s::$%s.', $classReflection->getDisplayName(), $propertyName, - ))->line($line)->build(); + ))->line($line)->file($file)->build(); } foreach ($additionalAssigns as [$propertyName, $line, $propertyNode]) { diff --git a/src/Rules/Properties/UninitializedPropertyRule.php b/src/Rules/Properties/UninitializedPropertyRule.php index 5c4d2a858b..c1f9a2fae1 100644 --- a/src/Rules/Properties/UninitializedPropertyRule.php +++ b/src/Rules/Properties/UninitializedPropertyRule.php @@ -44,7 +44,7 @@ public function processNode(Node $node, Scope $scope): array ))->line($propertyNode->getLine())->build(); } - foreach ($prematureAccess as [$propertyName, $line, $propertyNode]) { + foreach ($prematureAccess as [$propertyName, $line, $propertyNode, $file]) { if ($propertyNode->isReadOnly() || $propertyNode->isReadOnlyByPhpDoc()) { continue; } @@ -52,7 +52,7 @@ public function processNode(Node $node, Scope $scope): array 'Access to an uninitialized property %s::$%s.', $classReflection->getDisplayName(), $propertyName, - ))->line($line)->build(); + ))->line($line)->file($file)->build(); } return $errors; diff --git a/tests/PHPStan/Analyser/AnalyserTraitsIntegrationTest.php b/tests/PHPStan/Analyser/AnalyserTraitsIntegrationTest.php index 83e633f387..023d00e9f4 100644 --- a/tests/PHPStan/Analyser/AnalyserTraitsIntegrationTest.php +++ b/tests/PHPStan/Analyser/AnalyserTraitsIntegrationTest.php @@ -5,7 +5,11 @@ use PHPStan\File\FileHelper; use PHPStan\Testing\PHPStanTestCase; use function array_map; +use function array_merge; +use function array_unique; use function sprintf; +use function usort; +use const PHP_VERSION_ID; class AnalyserTraitsIntegrationTest extends PHPStanTestCase { @@ -165,6 +169,36 @@ public function testMissingReturnInAbstractTraitMethod(): void $this->assertNoErrors($errors); } + public function testUnititializedReadonlyPropertyAccessedInTrait(): void + { + if (PHP_VERSION_ID < 80100) { + $this->markTestSkipped(); + } + + $errors = $this->runAnalyse([ + __DIR__ . '/traits/uninitializedProperty/FooClass.php', + __DIR__ . '/traits/uninitializedProperty/FooTrait.php', + ]); + $this->assertCount(3, $errors); + usort($errors, static fn (Error $a, Error $b) => $a->getLine() <=> $b->getLine()); + $expectedFile = sprintf('%s (in context of class TraitsUnititializedProperty\FooClass)', $this->fileHelper->normalizePath(__DIR__ . '/traits/uninitializedProperty/FooTrait.php')); + + $error = $errors[0]; + $this->assertSame('Access to an uninitialized readonly property TraitsUnititializedProperty\FooClass::$x.', $error->getMessage()); + $this->assertSame(15, $error->getLine()); + $this->assertSame($expectedFile, $error->getFile()); + + $error = $errors[1]; + $this->assertSame('Access to an uninitialized @readonly property TraitsUnititializedProperty\FooClass::$y.', $error->getMessage()); + $this->assertSame(16, $error->getLine()); + $this->assertSame($expectedFile, $error->getFile()); + + $error = $errors[2]; + $this->assertSame('Access to an uninitialized property TraitsUnititializedProperty\FooClass::$z.', $error->getMessage()); + $this->assertSame(17, $error->getLine()); + $this->assertSame($expectedFile, $error->getFile()); + } + /** * @param string[] $files * @return Error[] @@ -178,4 +212,17 @@ private function runAnalyse(array $files): array return $analyser->analyse($files)->getErrors(); } + public static function getAdditionalConfigFiles(): array + { + return array_unique( + array_merge( + parent::getAdditionalConfigFiles(), + [ + __DIR__ . '/../../../conf/bleedingEdge.neon', + __DIR__ . '/traits-integration.neon', + ], + ), + ); + } + } diff --git a/tests/PHPStan/Analyser/traits-integration.neon b/tests/PHPStan/Analyser/traits-integration.neon new file mode 100644 index 0000000000..dcb115730e --- /dev/null +++ b/tests/PHPStan/Analyser/traits-integration.neon @@ -0,0 +1,2 @@ +parameters: + checkUninitializedProperties: true diff --git a/tests/PHPStan/Analyser/traits/uninitializedProperty/FooClass.php b/tests/PHPStan/Analyser/traits/uninitializedProperty/FooClass.php new file mode 100644 index 0000000000..6995d26c7e --- /dev/null +++ b/tests/PHPStan/Analyser/traits/uninitializedProperty/FooClass.php @@ -0,0 +1,16 @@ +foo(); + $this->x = 5; + $this->y = 5; + $this->z = 5; + } +} diff --git a/tests/PHPStan/Analyser/traits/uninitializedProperty/FooTrait.php b/tests/PHPStan/Analyser/traits/uninitializedProperty/FooTrait.php new file mode 100644 index 0000000000..d216f94304 --- /dev/null +++ b/tests/PHPStan/Analyser/traits/uninitializedProperty/FooTrait.php @@ -0,0 +1,19 @@ += 8.1 + +namespace TraitsUnititializedProperty; + +trait FooTrait +{ + protected readonly int $x; + + /** @readonly */ + protected int $y; + protected int $z; + + public function foo(): void + { + echo $this->x; + echo $this->y; + echo $this->z; + } +}