[CodeQuality] Fix MatchAssertSameExpectedTypeRector rewriting a passing assert into a failing one (#771)

Author: MizouziE

rules-tests/CodeQuality/Rector/MethodCall/MatchAssertSameExpectedTypeRector/Fixture/skip_stale_narrowed_union_return_type.php.inc

@@ -0,0 +1,19 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+use Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Source\Criteria;
+
+final class SkipStaleNarrowedUnionReturnType extends TestCase
+{
+    public function test()
+    {
+        $criteria = new Criteria();
+        assert(is_string($criteria->getType()));
+
+        $criteria->setTypeId(100);
+
+        $this->assertSame(100, $criteria->getType());
+    }
+}

rules-tests/CodeQuality/Rector/MethodCall/MatchAssertSameExpectedTypeRector/Fixture/skip_stale_narrowed_union_return_type_to_int.php.inc

@@ -0,0 +1,19 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+use Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Source\Criteria;
+
+final class SkipStaleNarrowedUnionReturnTypeToInt extends TestCase
+{
+    public function test()
+    {
+        $criteria = new Criteria();
+        assert(is_int($criteria->getType()));
+
+        $criteria->setAnyType(true);
+
+        $this->assertSame('Any', $criteria->getType());
+    }
+}

rules-tests/CodeQuality/Rector/MethodCall/MatchAssertSameExpectedTypeRector/Fixture/skip_union_return_type.php.inc

@@ -0,0 +1,16 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+use Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Source\Criteria;
+
+final class SkipUnionReturnType extends TestCase
+{
+    public function test()
+    {
+        $criteria = new Criteria();
+
+        $this->assertSame(100, $criteria->getType());
+    }
+}

rules-tests/CodeQuality/Rector/MethodCall/MatchAssertSameExpectedTypeRector/Source/Criteria.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MatchAssertSameExpectedTypeRector\Source;
+
+final class Criteria
+{
+    private ?int $typeId = null;
+
+    private ?bool $anyType = null;
+
+    public function setTypeId(int $typeId): self
+    {
+        $this->typeId = $typeId;
+        $this->anyType = null;
+
+        return $this;
+    }
+
+    public function setAnyType(bool $anyType): self
+    {
+        $this->anyType = $anyType;
+        $this->typeId = null;
+
+        return $this;
+    }
+
+    public function getType(): int|string|null
+    {
+        return $this->anyType ? 'Any' : $this->typeId;
+    }
+}

rules/CodeQuality/Rector/MethodCall/MatchAssertSameExpectedTypeRector.php

@@ -9,7 +9,9 @@
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Scalar\Int_;
 use PhpParser\Node\Scalar\String_;
+use PHPStan\Type\Type;
 use PHPStan\Type\TypeCombinator;
+use Rector\PHPUnit\CodeQuality\Reflection\MethodParametersAndReturnTypesResolver;
 use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer;
 use Rector\Rector\AbstractRector;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -21,7 +23,8 @@
 final class MatchAssertSameExpectedTypeRector extends AbstractRector
 {
     public function __construct(
-        private readonly TestsNodeAnalyzer $testsNodeAnalyzer
+        private readonly TestsNodeAnalyzer $testsNodeAnalyzer,
+        private readonly MethodParametersAndReturnTypesResolver $methodParametersAndReturnTypesResolver
     ) {
     }
 
@@ -103,7 +106,16 @@ public function refactor(Node $node): ?Node
 
         $variableExpr = $node->getArgs()[1]
             ->value;
-        $variableType = $this->nodeTypeResolver->getNativeType($variableExpr);
+
+        if ($variableExpr instanceof MethodCall || $variableExpr instanceof StaticCall) {
+            // an earlier assert can leave the scope type narrowed, even after the caller is mutated
+            $variableType = $this->methodParametersAndReturnTypesResolver->resolveCallReturnType($variableExpr);
+            if (! $variableType instanceof Type) {
+                return null;
+            }
+        } else {
+            $variableType = $this->nodeTypeResolver->getNativeType($variableExpr);
+        }
 
         $directVariableType = TypeCombinator::removeNull($variableType);
 

rules/CodeQuality/Reflection/MethodParametersAndReturnTypesResolver.php

@@ -119,6 +119,39 @@ public function resolveCallParameterTypes(MethodCall|StaticCall|New_ $callLike):
         return $this->resolveParameterTypes($extendedMethodReflection, $classReflection);
     }
 
+    public function resolveCallReturnType(MethodCall|StaticCall $callLike): ?Type
+    {
+        if (! $callLike->name instanceof Identifier) {
+            return null;
+        }
+
+        $methodName = $callLike->name->toString();
+
+        $callerType = $this->resolveObjectType(
+            $this->nodeTypeResolver->getType($callLike instanceof MethodCall ? $callLike->var : $callLike->class)
+        );
+
+        if (! $callerType instanceof ObjectType) {
+            return null;
+        }
+
+        $classReflection = $callerType->getClassReflection();
+        if (! $classReflection instanceof ClassReflection) {
+            return null;
+        }
+
+        if (! $classReflection->hasNativeMethod($methodName)) {
+            return null;
+        }
+
+        $extendedParametersAcceptor = ParametersAcceptorSelector::combineAcceptors(
+            $classReflection->getNativeMethod($methodName)
+                ->getVariants()
+        );
+
+        return $extendedParametersAcceptor->getNativeReturnType();
+    }
+
     /**
      * @return string[]
      */