[DeadCode] Skip RemoveAlwaysTrueIfConditionRector on maybe undefined variable (#8501)

Author: TomasVotruba

rules-tests/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector/Fixture/skip_maybe_undefined_variable.php.inc

@@ -0,0 +1,21 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector\Fixture;
+
+class SkipMaybeUndefinedVariable
+{
+    public function handle(string $username, string $password, object $response): void
+    {
+        if ($username !== 'admin') {
+            $error = true;
+        }
+
+        if ($password !== 'secret') {
+            $error = true;
+        }
+
+        if ($error) {
+            $response->setAttribute('status', 'ERROR');
+        }
+    }
+}

rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php

@@ -19,6 +19,7 @@
 use PhpParser\Node\Stmt\Else_;
 use PhpParser\Node\Stmt\If_;
 use PhpParser\NodeVisitor;
+use PHPStan\Analyser\Scope;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Type\IntersectionType;
 use Rector\DeadCode\NodeAnalyzer\SafeLeftTypeBooleanAndOrAnalyzer;
@@ -130,7 +131,8 @@ public function refactor(Node $node): int|null|array|If_
             return null;
         }
 
-        if ($this->shouldSkipFromVariable($node->cond)) {
+        $scope = ScopeFetcher::fetch($node);
+        if ($this->shouldSkipFromVariable($node->cond, $scope)) {
             return null;
         }
 
@@ -139,7 +141,6 @@ public function refactor(Node $node): int|null|array|If_
             return null;
         }
 
-        $scope = ScopeFetcher::fetch($node);
         $type = $scope->getNativeType($node->cond);
         if (! $type->isTrue()->yes()) {
             return null;
@@ -165,12 +166,18 @@ public function refactor(Node $node): int|null|array|If_
         return $node->stmts;
     }
 
-    private function shouldSkipFromVariable(Expr $expr): bool
+    private function shouldSkipFromVariable(Expr $expr, Scope $scope): bool
     {
         /** @var Variable[] $variables */
         $variables = $this->betterNodeFinder->findInstancesOf($expr, [Variable::class]);
 
         foreach ($variables as $variable) {
+            // maybe undefined variable is treated as null on some code paths, so the condition is not always true
+            $variableName = $this->getName($variable);
+            if (is_string($variableName) && ! $scope->hasVariableType($variableName)->yes()) {
+                return true;
+            }
+
             if ($this->exprAnalyzer->isNonTypedFromParam($variable)) {
                 return true;
             }