[CodeQuality] Fix infinite loop on trait on AddInstanceofAssertForNullableInstanceRector (#818)

Author: samsonasik

rules-tests/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector/Fixture/in_trait.php.inc

@@ -0,0 +1,30 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AddInstanceofAssertForNullableInstanceRector\Fixture;
+
+trait InTrait
+{
+    public function test(\Throwable $e)
+    {
+        $p = $e->getPrevious();
+        $p->getMessage();
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\ClassMethod\AddInstanceofAssertForNullableInstanceRector\Fixture;
+
+trait InTrait
+{
+    public function test(\Throwable $e)
+    {
+        $p = $e->getPrevious();
+        $this->assertInstanceOf(\Throwable::class, $p);
+        $p->getMessage();
+    }
+}
+
+?>

rules/CodeQuality/Rector/ClassMethod/AddInstanceofAssertForNullableInstanceRector.php

@@ -6,9 +6,11 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Foreach_;
 use Rector\PHPUnit\CodeQuality\NodeAnalyser\NullableObjectAssignCollector;
 use Rector\PHPUnit\CodeQuality\NodeFactory\AssertMethodCallFactory;
@@ -116,6 +118,19 @@ public function refactor(Node $node): ?Node
 
         $next = 0;
         foreach ($node->stmts as $key => $stmt) {
+            // already asserted, e.g. in trait, where $this is not known as TestCase so the type is not narrowed
+            $assertedVariableName = $this->matchAssertInstanceOfVariableName($stmt);
+            if ($assertedVariableName !== null) {
+                $assertedVariableNameToType = $variableNameToTypeCollection->matchByVariableName(
+                    $assertedVariableName
+                );
+                if ($assertedVariableNameToType instanceof VariableNameToType) {
+                    $variableNameToTypeCollection->remove($assertedVariableNameToType);
+                }
+
+                continue;
+            }
+
             // has callable on nullable variables of already collected names?
             // a single statement can use multiple nullable variables at once
             $matchedNullableVariableNameToTypes = $this->matchNullableVariableNameToTypes(
@@ -146,6 +161,33 @@ public function refactor(Node $node): ?Node
         return $node;
     }
 
+    private function matchAssertInstanceOfVariableName(Stmt $stmt): ?string
+    {
+        if (! $stmt instanceof Expression) {
+            return null;
+        }
+
+        $call = $stmt->expr;
+        if (! $call instanceof MethodCall && ! $call instanceof StaticCall) {
+            return null;
+        }
+
+        if (! $this->isName($call->name, 'assertInstanceOf')) {
+            return null;
+        }
+
+        if ($call->isFirstClassCallable()) {
+            return null;
+        }
+
+        $args = $call->getArgs();
+        if (! isset($args[1]) || ! $args[1]->value instanceof Variable) {
+            return null;
+        }
+
+        return $this->getName($args[1]->value);
+    }
+
     /**
      * @return VariableNameToType[]
      */