[CodeQuality] Skip nested value compare in CallbackSingleAssertToSimplerRector (#718)

* [CodeQuality] Skip nested value compare in CallbackSingleAssertToSimplerRector

* handle flipped param order, skip nested compare both orders

Author: TomasVotruba

rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/Fixture/flipped_param_order.php.inc

@@ -0,0 +1,43 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class FlippedParamOrder extends TestCase
+{
+    public function test()
+    {
+        $builder = $this->getMockBuilder('AnyType')->getMock();
+
+        $builder->expects($this->exactly(2))
+            ->method('add')
+            ->with($this->callback(function ($type): bool {
+                $this->assertSame($type, TextType::class);
+
+                return true;
+            }));
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class FlippedParamOrder extends TestCase
+{
+    public function test()
+    {
+        $builder = $this->getMockBuilder('AnyType')->getMock();
+
+        $builder->expects($this->exactly(2))
+            ->method('add')
+            ->with($this->equalTo(TextType::class));
+    }
+}
+
+?>

rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/Fixture/skip_nested_compare.php.inc

@@ -0,0 +1,21 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SkipNestedCompare extends TestCase
+{
+    public function test()
+    {
+        $builder = $this->getMockBuilder('AnyType')->getMock();
+
+        $builder->expects($this->exactly(2))
+            ->method('add')
+            ->with($this->callback(function (array $args): bool {
+                $this->assertSame($args['label'], 'Test Label');
+
+                return true;
+            }));
+    }
+}

rules-tests/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector/Fixture/skip_nested_compare_expected_left.php.inc

@@ -0,0 +1,21 @@
+<?php
+
+namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\CallbackSingleAssertToSimplerRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SkipNestedCompareExpectedLeft extends TestCase
+{
+    public function test()
+    {
+        $builder = $this->getMockBuilder('AnyType')->getMock();
+
+        $builder->expects($this->exactly(2))
+            ->method('add')
+            ->with($this->callback(function (array $args): bool {
+                $this->assertSame('Test Label', $args['label']);
+
+                return true;
+            }));
+    }
+}

rules/CodeQuality/Rector/MethodCall/CallbackSingleAssertToSimplerRector.php

@@ -9,6 +9,7 @@
 use PhpParser\Node\Expr\Closure;
 use PhpParser\Node\Expr\ConstFetch;
 use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Return_;
 use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer;
@@ -153,8 +154,43 @@ private function matchCallbackSoleAssertSameExpected(Expr $expr): ?Expr
             return null;
         }
 
-        return $firstStmtExpr->getArgs()[0]
-            ->value;
+        $assertSameArgs = $firstStmtExpr->getArgs();
+        if (count($assertSameArgs) !== 2) {
+            return null;
+        }
+
+        $firstValue = $assertSameArgs[0]->value;
+        $secondValue = $assertSameArgs[1]->value;
+
+        // one side must be the whole closure parameter; the other side is the expected value
+        // nested access (e.g. $args['label']) is skipped, as equalTo() matches the whole argument
+        if ($this->isClosureSoleParam($innerClosure, $secondValue)) {
+            return $firstValue;
+        }
+
+        if ($this->isClosureSoleParam($innerClosure, $firstValue)) {
+            return $secondValue;
+        }
+
+        return null;
+    }
+
+    private function isClosureSoleParam(Closure $closure, Expr $expr): bool
+    {
+        if (count($closure->params) !== 1) {
+            return false;
+        }
+
+        if (! $expr instanceof Variable) {
+            return false;
+        }
+
+        $soleParam = $closure->params[0];
+        if (! $soleParam->var instanceof Variable) {
+            return false;
+        }
+
+        return $this->nodeComparator->areNodesEqual($soleParam->var, $expr);
     }
 
     private function isTrueReturn(Return_ $return): bool