Fix union-type handling in PublicStaticPropertyFetchCollector (#121)

* Fix union-type handling in PublicStaticPropertyFetchCollector * another test * fix namespace * Update UnusedPublicPropertyRuleTest.php
TomasVotruba · Jul 15, 2024 · 5da0ca6 · 5da0ca6
1 parent da62379
commit 5da0ca6
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 9 deletions.
diff --git a/src/Collectors/PublicStaticPropertyFetchCollector.php b/src/Collectors/PublicStaticPropertyFetchCollector.php
@@ -10,6 +10,8 @@
 use PhpParser\Node\Name;
 use PHPStan\Analyser\Scope;
 use PHPStan\Collectors\Collector;
+use PHPStan\Reflection\ClassReflection;
+use TomasVotruba\UnusedPublic\ClassTypeDetector;
 use TomasVotruba\UnusedPublic\Configuration;
 
 /**
@@ -18,7 +20,8 @@
 final readonly class PublicStaticPropertyFetchCollector implements Collector
 {
     public function __construct(
-        private Configuration $configuration
+        private Configuration $configuration,
+        private ClassTypeDetector $classTypeDetector,
     ) {
     }
 
@@ -33,26 +36,38 @@ public function getNodeType(): string
      */
     public function processNode(Node $node, Scope $scope): ?array
     {
-        if (! $this->configuration->isUnusedPropertyEnabled()) {
+        if (!$this->configuration->isUnusedPropertyEnabled()) {
             return null;
         }
 
-        if (! $node->class instanceof Name) {
+        if (!$node->name instanceof Identifier) {
             return null;
         }
 
-        if (! $node->name instanceof Identifier) {
+        if (
+            $node->class instanceof Name
+            && ($node->class->toString() === 'self' || $node->class->toString() === 'static')
+        ) {
+            // self fetch is allowed
             return null;
         }
 
-        if ($node->class->toString() === 'self') {
-            // self fetch is allowed
+        $classReflection = $scope->getClassReflection();
+        if ($classReflection instanceof ClassReflection && $this->classTypeDetector->isTestClass($classReflection)) {
             return null;
         }
 
-        $className = $node->class->toString();
-        $propertyName = $node->name->toString();
+        if ($node->class instanceof Name) {
+            $classType = $scope->resolveTypeByName($node->class);
+        } else {
+            $classType = $scope->getType($node->class);
+        }
+        $result = [];
+        foreach($classType->getObjectClassNames() as $className) {
+            $propertyName = $node->name->toString();
+            $result[] = $className . '::' . $propertyName;
+        }
 
-        return [$className . '::' . $propertyName];
+        return $result;
     }
 }
diff --git a/tests/Rules/UnusedPublicPropertyRule/Fixture/StaticUsedInTestCaseOnly.php b/tests/Rules/UnusedPublicPropertyRule/Fixture/StaticUsedInTestCaseOnly.php
@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TomasVotruba\UnusedPublic\Tests\Rules\UnusedPublicPropertyRule\Fixture;
+
+final class StaticUsedInTestCaseOnly
+{
+    static public $property = 'public static';
+}
diff --git a/tests/Rules/UnusedPublicPropertyRule/Fixture/StaticUsedInUnionA.php b/tests/Rules/UnusedPublicPropertyRule/Fixture/StaticUsedInUnionA.php
@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rules\UnusedPublicPropertyRule\Fixture;
+
+class StaticUsedInUnionA {
+    static public float $amount;
+}
diff --git a/tests/Rules/UnusedPublicPropertyRule/Fixture/StaticUsedInUnionB.php b/tests/Rules/UnusedPublicPropertyRule/Fixture/StaticUsedInUnionB.php
@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rules\UnusedPublicPropertyRule\Fixture;
+
+class StaticUsedInUnionB {
+    static public float $amount;
+}
diff --git a/tests/Rules/UnusedPublicPropertyRule/Source/StaticUsedInUnion.php b/tests/Rules/UnusedPublicPropertyRule/Source/StaticUsedInUnion.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TomasVotruba\UnusedPublic\Tests\Rules\UnusedPublicPropertyRule\Source;
+
+use Rules\UnusedPublicPropertyRule\Fixture\StaticUsedInUnionA;
+use Rules\UnusedPublicPropertyRule\Fixture\StaticUsedInUnionB;
+
+function doFooBar(StaticUsedInUnionA|StaticUsedInUnionB $aOrB): void {
+    echo $aOrB::$amount;
+}
+
diff --git a/tests/Rules/UnusedPublicPropertyRule/Source/TestCaseUser.php b/tests/Rules/UnusedPublicPropertyRule/Source/TestCaseUser.php
@@ -5,6 +5,7 @@
 namespace TomasVotruba\UnusedPublic\Tests\Rules\UnusedPublicPropertyRule\Source;
 
 use PHPUnit\Framework\TestCase;
+use Rules\UnusedPublicPropertyRule\Fixture\StaticUsedInTestCaseOnly;
 use TomasVotruba\UnusedPublic\Tests\Rules\UnusedPublicPropertyRule\Fixture\UsedInTestCaseOnly;
 
 final class TestCaseUser extends TestCase
@@ -13,5 +14,8 @@ private function go()
     {
         $o = new UsedInTestCaseOnly();
         $o->property = 'a value';
+
+        $o = new StaticUsedInTestCaseOnly();
+        $o::$property = 'a value';
     }
 }
diff --git a/tests/Rules/UnusedPublicPropertyRule/UnusedPublicPropertyRuleTest.php b/tests/Rules/UnusedPublicPropertyRule/UnusedPublicPropertyRuleTest.php
@@ -19,6 +19,7 @@
 use TomasVotruba\UnusedPublic\Tests\Rules\UnusedPublicPropertyRule\Fixture\LocallyUsedStaticPropertyViaStatic;
 use TomasVotruba\UnusedPublic\Tests\Rules\UnusedPublicPropertyRule\Fixture\LocalyUsedPublicProperty;
 use TomasVotruba\UnusedPublic\Tests\Rules\UnusedPublicPropertyRule\Fixture\UsedInTestCaseOnly;
+use TomasVotruba\UnusedPublic\Tests\Rules\UnusedPublicPropertyRule\Fixture\StaticUsedInTestCaseOnly;
 
 final class UnusedPublicPropertyRuleTest extends RuleTestCase
 {
@@ -99,6 +100,12 @@ public static function provideData(): Iterator
             [[$errorMessage1, 7, RuleTips::SOLUTION_MESSAGE]],
         ];
 
+        $errorMessage1 = sprintf(UnusedPublicPropertyRule::ERROR_MESSAGE, StaticUsedInTestCaseOnly::class, 'property');
+        yield [
+            [__DIR__ . '/Fixture/StaticUsedInTestCaseOnly.php', __DIR__ . '/Source/TestCaseUser.php'],
+            [[$errorMessage1, 7, RuleTips::SOLUTION_MESSAGE]],
+        ];
+
         yield [[__DIR__ . '/Fixture/plain.php', __DIR__ . '/Source/PublicPropertyClass.php'], []];
 
         yield [
@@ -112,6 +119,7 @@ public static function provideData(): Iterator
 
         yield [[__DIR__ . '/Fixture/UsedInUnionA.php', __DIR__ . '/Fixture/UsedInUnionB.php', __DIR__ . '/Source/UsedInUnion.php'], []];
         yield [[__DIR__ . '/Fixture/UsedInUnionA.php', __DIR__ . '/Fixture/UsedInUnionB.php', __DIR__ . '/Source/UsedInUnionPhpdoc.php'], []];
+        yield [[__DIR__ . '/Fixture/StaticUsedInUnionA.php', __DIR__ . '/Fixture/StaticUsedInUnionB.php', __DIR__ . '/Source/StaticUsedInUnion.php'], []];
 
     }