[Rules] Skip Property fetch on NoIssetOnObjectRule

rectorphp · Jun 18, 2024 · d225589 · d225589
1 parent 9388c31
commit d225589
Show file tree

Hide file tree

Showing 7 changed files with 95 additions and 55 deletions.
diff --git a/config/extension.neon b/config/extension.neon
@@ -54,6 +54,7 @@ services:
     - Rector\TypePerfect\Matcher\Collector\PublicClassMethodMatcher
     - Rector\TypePerfect\Matcher\ClassMethodCallReferenceResolver
     - Rector\TypePerfect\Printer\CollectorMetadataPrinter
+    - Rector\TypePerfect\Guard\EmptyIssetGuard
 
     # for NarrowPublicClassMethodParamTypeByCallerTypeRule
     -

diff --git a/src/Guard/EmptyIssetGuard.php b/src/Guard/EmptyIssetGuard.php
@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\TypePerfect\Guard;
+
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\ArrayDimFetch;
+use PhpParser\Node\Expr\Variable;
+use PHPStan\Analyser\Scope;
+use PHPStan\Type\TypeCombinator;
+
+final class EmptyIssetGuard
+{
+    public function isLegal(Expr $expr, Scope $scope): bool
+    {
+        if ($expr instanceof ArrayDimFetch) {
+            return true;
+        }
+
+        if (! $expr instanceof Variable) {
+            return true;
+        }
+
+        if ($expr->name instanceof Expr) {
+            return true;
+        }
+
+        if (! $scope->hasVariableType($expr->name)->yes()) {
+            return true;
+        }
+
+        $varType = $scope->getType($expr);
+        $varType = TypeCombinator::removeNull($varType);
+        return $varType->getObjectClassNames() === [];
+    }
+}
diff --git a/src/Rules/NoEmptyOnObjectRule.php b/src/Rules/NoEmptyOnObjectRule.php
@@ -5,24 +5,26 @@
 namespace Rector\TypePerfect\Rules;
 
 use PhpParser\Node;
-use PhpParser\Node\Expr;
-use PhpParser\Node\Expr\ArrayDimFetch;
 use PhpParser\Node\Expr\Empty_;
-use PhpParser\Node\Expr\Variable;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
-use PHPStan\Type\TypeCombinator;
+use Rector\TypePerfect\Guard\EmptyIssetGuard;
 
 /**
  * @implements Rule<Empty_>
  */
-final class NoEmptyOnObjectRule implements Rule
+final readonly class NoEmptyOnObjectRule implements Rule
 {
     /**
      * @var string
      */
     public const ERROR_MESSAGE = 'Use instanceof instead of empty() on object';
 
+    public function __construct(
+        private EmptyIssetGuard $emptyIssetGuard
+    ) {
+    }
+
     public function getNodeType(): string
     {
         return Empty_::class;
@@ -34,34 +36,12 @@ public function getNodeType(): string
      */
     public function processNode(Node $node, Scope $scope): array
     {
-        $expr = $node->expr;
-        if ($this->shouldSkipVariable($expr, $scope)) {
+        if ($this->emptyIssetGuard->isLegal($node->expr, $scope)) {
             return [];
         }
 
         return [
             self::ERROR_MESSAGE,
         ];
     }
-
-    private function shouldSkipVariable(Expr $expr, Scope $scope): bool
-    {
-        if ($expr instanceof ArrayDimFetch) {
-            return true;
-        }
-
-        if ($expr instanceof Variable) {
-            if ($expr->name instanceof Expr) {
-                return true;
-            }
-
-            if (! $scope->hasVariableType($expr->name)->yes()) {
-                return true;
-            }
-        }
-
-        $varType = $scope->getType($expr);
-        $varType = TypeCombinator::removeNull($varType);
-        return $varType->getObjectClassNames() === [];
-    }
 }
diff --git a/src/Rules/NoIssetOnObjectRule.php b/src/Rules/NoIssetOnObjectRule.php
@@ -5,25 +5,27 @@
 namespace Rector\TypePerfect\Rules;
 
 use PhpParser\Node;
-use PhpParser\Node\Expr;
-use PhpParser\Node\Expr\ArrayDimFetch;
 use PhpParser\Node\Expr\Isset_;
-use PhpParser\Node\Expr\Variable;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
-use PHPStan\Type\TypeCombinator;
+use Rector\TypePerfect\Guard\EmptyIssetGuard;
 
 /**
  * @see \Rector\TypePerfect\Tests\Rules\NoIssetOnObjectRule\NoIssetOnObjectRuleTest
  * @implements Rule<Isset_>
  */
-final class NoIssetOnObjectRule implements Rule
+final readonly class NoIssetOnObjectRule implements Rule
 {
     /**
      * @var string
      */
     public const ERROR_MESSAGE = 'Use instanceof instead of isset() on object';
 
+    public function __construct(
+        private EmptyIssetGuard $emptyIssetGuard
+    ) {
+    }
+
     public function getNodeType(): string
     {
         return Isset_::class;
@@ -37,7 +39,7 @@ public function getNodeType(): string
     public function processNode(Node $node, Scope $scope): array
     {
         foreach ($node->vars as $var) {
-            if ($this->shouldSkipVariable($var, $scope)) {
+            if ($this->emptyIssetGuard->isLegal($var, $scope)) {
                 continue;
             }
 
@@ -48,25 +50,4 @@ public function processNode(Node $node, Scope $scope): array
 
         return [];
     }
-
-    private function shouldSkipVariable(Expr $expr, Scope $scope): bool
-    {
-        if ($expr instanceof ArrayDimFetch) {
-            return true;
-        }
-
-        if ($expr instanceof Variable) {
-            if ($expr->name instanceof Expr) {
-                return true;
-            }
-
-            if (! $scope->hasVariableType($expr->name)->yes()) {
-                return true;
-            }
-        }
-
-        $varType = $scope->getType($expr);
-        $varType = TypeCombinator::removeNull($varType);
-        return $varType->getObjectClassNames() === [];
-    }
 }
diff --git a/tests/Rules/NoIssetOnObjectRule/Fixture/SkipIssetOnPropertyFetch.php b/tests/Rules/NoIssetOnObjectRule/Fixture/SkipIssetOnPropertyFetch.php
@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\TypePerfect\Tests\Rules\NoIssetOnObjectRule\Fixture;
+
+use Rector\TypePerfect\Tests\Rules\NoIssetOnObjectRule\Source\Foo;
+
+class SkipIssetOnPropertyFetch
+{
+	private Foo $foo;
+
+	public function sayHello(): void
+	{
+		$this->initializeFoo();
+
+		$this->foo->sayHello();
+	}
+
+	private function initializeFoo(): void
+	{
+		if (!isset($this->foo))
+		{
+			$this->foo = new Foo();
+		}
+	}
+}
diff --git a/tests/Rules/NoIssetOnObjectRule/NoIssetOnObjectRuleTest.php b/tests/Rules/NoIssetOnObjectRule/NoIssetOnObjectRuleTest.php
@@ -28,6 +28,7 @@ public static function provideData(): Iterator
         yield [__DIR__ . '/Fixture/SkipIssetOnArray.php', []];
         yield [__DIR__ . '/Fixture/SkipIssetOnArrayNestedOnObject.php', []];
         yield [__DIR__ . '/Fixture/SkipPossibleUndefinedVariable.php', []];
+        yield [__DIR__ . '/Fixture/SkipIssetOnPropertyFetch.php', []];
     }
 
     /**

diff --git a/tests/Rules/NoIssetOnObjectRule/Source/Foo.php b/tests/Rules/NoIssetOnObjectRule/Source/Foo.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\TypePerfect\Tests\Rules\NoIssetOnObjectRule\Source;
+
+class Foo
+{
+	public function sayHello(): void
+	{
+		echo 'Hello';
+	}
+}