[CodeQuality] Skip SimplifyUselessVariableRector on commented-out code and inline concat

rules-tests/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector/Fixture/skip_commented_code_between.php.inc

@@ -0,0 +1,18 @@
+<?php
+
+namespace Rector\Tests\CodeQuality\Rector\FunctionLike\SimplifyUselessVariableRector\Fixture;
+
+class SkipCommentedCodeBetween
+{
+    public function run($ch)
+    {
+        $pdf = curl_exec($ch);
+
+        // $url = "https://example.com" . "?id=" . $id;
+        // curl_setopt($ch, CURLOPT_URL, $url);
+        // $pdf = curl_exec($ch);
+        // curl_close($ch);
+
+        return $pdf;
+    }
+}

rules/CodeQuality/Rector/FunctionLike/SimplifyUselessVariableRector.php

@@ -7,56 +7,36 @@
 use PhpParser\Comment\Doc;
 use PhpParser\Node;
 use PhpParser\Node\Expr\Assign;
-use PhpParser\Node\Expr\AssignOp;
-use PhpParser\Node\Expr\Ternary;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Return_;
 use PHPStan\Type\MixedType;
 use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
-use Rector\Contract\Rector\ConfigurableRectorInterface;
 use Rector\NodeAnalyzer\CallAnalyzer;
 use Rector\NodeAnalyzer\VariableAnalyzer;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\PhpParser\Enum\NodeGroup;
-use Rector\PhpParser\Node\AssignAndBinaryMap;
 use Rector\Rector\AbstractRector;
-use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
 /**
  * @see \Rector\Tests\CodeQuality\Rector\FunctionLike\SimplifyUselessVariableRector\SimplifyUselessVariableRectorTest
  */
-final class SimplifyUselessVariableRector extends AbstractRector implements ConfigurableRectorInterface
+final class SimplifyUselessVariableRector extends AbstractRector
 {
-    /**
-     * @api
-     */
-    public const string ONLY_DIRECT_ASSIGN = 'only_direct_assign';
-
-    private bool $onlyDirectAssign = false;
-
     public function __construct(
-        private readonly AssignAndBinaryMap $assignAndBinaryMap,
         private readonly VariableAnalyzer $variableAnalyzer,
         private readonly CallAnalyzer $callAnalyzer,
         private readonly PhpDocInfoFactory $phpDocInfoFactory
     ) {
     }
 
-    /**
-     * @param array<string, mixed> $configuration
-     */
-    public function configure(array $configuration): void
-    {
-        $this->onlyDirectAssign = $configuration[self::ONLY_DIRECT_ASSIGN] ?? false;
-    }
-
     public function getRuleDefinition(): RuleDefinition
     {
         return new RuleDefinition('Remove useless variable assigns', [
-            new ConfiguredCodeSample(
+            new CodeSample(
                 <<<'CODE_SAMPLE'
 function () {
     $a = true;
@@ -69,33 +49,6 @@ function () {
     return true;
 };
 CODE_SAMPLE
-                ,
-                // default
-                [
-                    self::ONLY_DIRECT_ASSIGN => true,
-                ]
-            ),
-            new ConfiguredCodeSample(
-                <<<'CODE_SAMPLE'
-function () {
-    $a = 'Hello, ';
-    $a .= 'World!';
-
-    return $a;
-};
-CODE_SAMPLE
-                ,
-                <<<'CODE_SAMPLE'
-function () {
-    $a = 'Hello, ';
-
-    return $a . 'World!';
-};
-CODE_SAMPLE
-                ,
-                [
-                    self::ONLY_DIRECT_ASSIGN => false,
-                ]
             ),
         ]);
     }
@@ -137,42 +90,31 @@ public function refactor(Node $node): ?Node
                 return null;
             }
 
-            if ($this->isReturnWithVarAnnotation($stmt)) {
+            // the variable might be used in commented-out code between assign and return
+            if ($this->isVariableMentionedInComment($stmt)) {
                 return null;
             }
 
-            /** @var Expression<Assign|AssignOp> $previousStmt */
-            $assign = $previousStmt->expr;
-
-            return $this->processSimplifyUselessVariable($node, $stmt, $assign, $key);
-        }
+            if ($this->isReturnWithVarAnnotation($stmt)) {
+                return null;
+            }
 
-        return null;
-    }
+            if (! $previousStmt instanceof Expression) {
+                return null;
+            }
 
-    /**
-     * @param StmtsAware $stmtsAware
-     * @return StmtsAware|null
-     */
-    private function processSimplifyUselessVariable(
-        Node $stmtsAware,
-        Return_ $return,
-        Assign|AssignOp $assign,
-        int $key
-    ): ?Node {
-        if (! $assign instanceof Assign) {
-            $binaryClass = $this->assignAndBinaryMap->getAlternative($assign);
-            if ($binaryClass === null) {
+            $assign = $previousStmt->expr;
+            if (! $assign instanceof Assign) {
                 return null;
             }
 
-            $return->expr = new $binaryClass($assign->var, $assign->expr);
-        } else {
-            $return->expr = $assign->expr;
+            $stmt->expr = $assign->expr;
+            unset($node->stmts[$key - 1]);
+
+            return $node;
         }
 
-        unset($stmtsAware->stmts[$key - 1]);
-        return $stmtsAware;
+        return null;
     }
 
     private function shouldSkipStmt(Return_ $return, Stmt $previousStmt): bool
@@ -192,15 +134,7 @@ private function shouldSkipStmt(Return_ $return, Stmt $previousStmt): bool
         // is variable part of single assign
         $previousNode = $previousStmt->expr;
 
-        if (! $previousNode instanceof AssignOp && ! $previousNode instanceof Assign) {
-            return true;
-        }
-
-        if ($this->onlyDirectAssign && $previousNode instanceof AssignOp) {
-            return true;
-        }
-
-        if ($previousNode instanceof AssignOp && $previousNode->expr instanceof Ternary) {
+        if (! $previousNode instanceof Assign) {
             return true;
         }
 
@@ -223,6 +157,25 @@ private function shouldSkipStmt(Return_ $return, Stmt $previousStmt): bool
         return $this->variableAnalyzer->isUsedByReference($variable);
     }
 
+    private function isVariableMentionedInComment(Return_ $return): bool
+    {
+        $comments = $return->getComments();
+        if ($comments === []) {
+            return false;
+        }
+
+        if (! $return->expr instanceof Variable) {
+            return false;
+        }
+
+        $variableName = $return->expr->name;
+        if (! is_string($variableName)) {
+            return false;
+        }
+
+        return array_any($comments, fn ($comment): bool => str_contains($comment->getText(), '$' . $variableName));
+    }
+
     private function hasSomeComment(Stmt $stmt): bool
     {
         if ($stmt->getComments() !== []) {

src/PhpParser/Node/AssignAndBinaryMap.php

@@ -4,7 +4,6 @@
 
 namespace Rector\PhpParser\Node;
 
-use PhpParser\Node;
 use PhpParser\Node\Expr\AssignOp;
 use PhpParser\Node\Expr\AssignOp\BitwiseAnd as AssignBitwiseAnd;
 use PhpParser\Node\Expr\AssignOp\BitwiseOr as AssignBitwiseOr;
@@ -87,21 +86,11 @@ public function __construct()
     }
 
     /**
-     * @return class-string<BinaryOp|AssignOp>|null
+     * @return class-string<AssignOp>|null
      */
-    public function getAlternative(Node $node): ?string
+    public function getAlternative(BinaryOp $binaryOp): ?string
     {
-        $nodeClass = $node::class;
-
-        if ($node instanceof AssignOp) {
-            return self::ASSIGN_OP_TO_BINARY_OP_CLASSES[$nodeClass] ?? null;
-        }
-
-        if ($node instanceof BinaryOp) {
-            return $this->binaryOpToAssignClasses[$nodeClass] ?? null;
-        }
-
-        return null;
+        return $this->binaryOpToAssignClasses[$binaryOp::class] ?? null;
     }
 
     /**