From 98f1498b7dd6a86e781f682f9bd61a7ce32ba5a3 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 20 Jun 2026 15:25:53 +0700 Subject: [PATCH 1/7] [DeadCode] Avoid scan whole new stmts on RemoveAlwaysTrueIfConditionRector --- .../If_/RemoveAlwaysTrueIfConditionRector.php | 21 ++----------------- src/NodeAnalyzer/ExprAnalyzer.php | 2 +- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php b/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php index 37dc380a6e7..35c1fe6e1b2 100644 --- a/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php +++ b/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php @@ -149,15 +149,9 @@ private function shouldSkipFromVariable(Expr $expr): bool { /** @var Variable[] $variables */ $variables = $this->betterNodeFinder->findInstancesOf($expr, [Variable::class]); - - $hasPlainVariable = false; foreach ($variables as $variable) { - if ($this->exprAnalyzer->isNonTypedFromParam($variable)) { - return true; - } - if (! $variable->name instanceof Expr) { - $hasPlainVariable = true; + return true; } $type = $this->nodeTypeResolver->getNativeType($variable); @@ -170,18 +164,7 @@ private function shouldSkipFromVariable(Expr $expr): bool } } - // a dynamic variable assignment, e.g. ${$name} = ..., is invisible to native type inference, so the - // condition variable may be overwritten at runtime and is not safe to evaluate as always true - return $hasPlainVariable && $this->hasDynamicVariable(); - } - - private function hasDynamicVariable(): bool - { - return (bool) $this->betterNodeFinder->findFirst( - $this->getFile() - ->getNewStmts(), - static fn (Node $node): bool => $node instanceof Variable && $node->name instanceof Expr - ); + return false; } private function shouldSkipExpr(Expr $expr): bool diff --git a/src/NodeAnalyzer/ExprAnalyzer.php b/src/NodeAnalyzer/ExprAnalyzer.php index a71a77f35a9..285ce980b55 100644 --- a/src/NodeAnalyzer/ExprAnalyzer.php +++ b/src/NodeAnalyzer/ExprAnalyzer.php @@ -141,7 +141,7 @@ public function isExprWithExprPropertyWrappable(Node $node): bool public function isNonTypedFromParam(Expr $expr): bool { if (! $expr instanceof Variable) { - return false; + return true; } $scope = $expr->getAttribute(AttributeKey::SCOPE); From ffc1be480fa42e6a77682542d6a64c76f034403b Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 20 Jun 2026 16:37:25 +0700 Subject: [PATCH 2/7] Fix --- .../If_/RemoveAlwaysTrueIfConditionRector.php | 19 ++++++++++++++++++- src/NodeAnalyzer/ExprAnalyzer.php | 2 +- .../NodeVisitor/ContextNodeVisitor.php | 5 +++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php b/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php index 35c1fe6e1b2..4f44ef8d66f 100644 --- a/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php +++ b/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php @@ -19,6 +19,7 @@ use PhpParser\Node\Stmt\If_; use PhpParser\NodeVisitor; use PHPStan\Reflection\ClassReflection; +use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\IntersectionType; use Rector\DeadCode\NodeAnalyzer\SafeLeftTypeBooleanAndOrAnalyzer; use Rector\NodeAnalyzer\ExprAnalyzer; @@ -149,8 +150,9 @@ private function shouldSkipFromVariable(Expr $expr): bool { /** @var Variable[] $variables */ $variables = $this->betterNodeFinder->findInstancesOf($expr, [Variable::class]); + foreach ($variables as $variable) { - if (! $variable->name instanceof Expr) { + if ($this->exprAnalyzer->isNonTypedFromParam($variable)) { return true; } @@ -164,6 +166,21 @@ private function shouldSkipFromVariable(Expr $expr): bool } } + $scope = ScopeFetcher::fetch($expr); + $definedVariables = $scope->getDefinedVariables(); + + foreach ($definedVariables as $definedVariable) { + if (! $scope->hasVariableType($definedVariable)->yes()) { + continue; + } + + $variableType = $scope->getVariableType($definedVariable); + if ($variableType instanceof ConstantStringType + && in_array($variableType->getValue(), $definedVariables, true)) { + return true; + } + } + return false; } diff --git a/src/NodeAnalyzer/ExprAnalyzer.php b/src/NodeAnalyzer/ExprAnalyzer.php index 285ce980b55..a71a77f35a9 100644 --- a/src/NodeAnalyzer/ExprAnalyzer.php +++ b/src/NodeAnalyzer/ExprAnalyzer.php @@ -141,7 +141,7 @@ public function isExprWithExprPropertyWrappable(Node $node): bool public function isNonTypedFromParam(Expr $expr): bool { if (! $expr instanceof Variable) { - return true; + return false; } $scope = $expr->getAttribute(AttributeKey::SCOPE); diff --git a/src/PhpParser/NodeVisitor/ContextNodeVisitor.php b/src/PhpParser/NodeVisitor/ContextNodeVisitor.php index 4d147b6a9cb..e2f8c9266a9 100644 --- a/src/PhpParser/NodeVisitor/ContextNodeVisitor.php +++ b/src/PhpParser/NodeVisitor/ContextNodeVisitor.php @@ -112,6 +112,11 @@ public function enterNode(Node $node): ?Node return null; } + if ($node instanceof Variable && $node->name instanceof Expr) { + $node->setAttribute(AttributeKey::IS_DYNAMIC_VARIABLE, true); + return null; + } + $this->processContextInClass($node); return null; } From a3e47acf37045803811c075c87a705b81d33dde6 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 20 Jun 2026 16:38:11 +0700 Subject: [PATCH 3/7] Fix --- src/PhpParser/NodeVisitor/ContextNodeVisitor.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/PhpParser/NodeVisitor/ContextNodeVisitor.php b/src/PhpParser/NodeVisitor/ContextNodeVisitor.php index e2f8c9266a9..4d147b6a9cb 100644 --- a/src/PhpParser/NodeVisitor/ContextNodeVisitor.php +++ b/src/PhpParser/NodeVisitor/ContextNodeVisitor.php @@ -112,11 +112,6 @@ public function enterNode(Node $node): ?Node return null; } - if ($node instanceof Variable && $node->name instanceof Expr) { - $node->setAttribute(AttributeKey::IS_DYNAMIC_VARIABLE, true); - return null; - } - $this->processContextInClass($node); return null; } From 40420b6400356b5d81eb2938d3d89f2322a9bac4 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 20 Jun 2026 16:46:47 +0700 Subject: [PATCH 4/7] final touch: move to ExprAnalyzer --- .../If_/RemoveAlwaysTrueIfConditionRector.php | 16 ---------------- src/NodeAnalyzer/ExprAnalyzer.php | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php b/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php index 4f44ef8d66f..b17a1b2678e 100644 --- a/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php +++ b/rules/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector.php @@ -19,7 +19,6 @@ use PhpParser\Node\Stmt\If_; use PhpParser\NodeVisitor; use PHPStan\Reflection\ClassReflection; -use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\IntersectionType; use Rector\DeadCode\NodeAnalyzer\SafeLeftTypeBooleanAndOrAnalyzer; use Rector\NodeAnalyzer\ExprAnalyzer; @@ -166,21 +165,6 @@ private function shouldSkipFromVariable(Expr $expr): bool } } - $scope = ScopeFetcher::fetch($expr); - $definedVariables = $scope->getDefinedVariables(); - - foreach ($definedVariables as $definedVariable) { - if (! $scope->hasVariableType($definedVariable)->yes()) { - continue; - } - - $variableType = $scope->getVariableType($definedVariable); - if ($variableType instanceof ConstantStringType - && in_array($variableType->getValue(), $definedVariables, true)) { - return true; - } - } - return false; } diff --git a/src/NodeAnalyzer/ExprAnalyzer.php b/src/NodeAnalyzer/ExprAnalyzer.php index a71a77f35a9..94831c3d02e 100644 --- a/src/NodeAnalyzer/ExprAnalyzer.php +++ b/src/NodeAnalyzer/ExprAnalyzer.php @@ -50,6 +50,7 @@ use PhpParser\Node\Scalar\InterpolatedString; use PhpParser\Node\Scalar\String_; use PHPStan\Analyser\Scope; +use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\MixedType; use PHPStan\Type\ObjectWithoutClassType; use PHPStan\Type\UnionType; @@ -168,6 +169,19 @@ public function isNonTypedFromParam(Expr $expr): bool return true; } + $definedVariables = $scope->getDefinedVariables(); + foreach ($definedVariables as $definedVariable) { + if (! $scope->hasVariableType($definedVariable)->yes()) { + continue; + } + + $variableType = $scope->getVariableType($definedVariable); + if ($variableType instanceof ConstantStringType + && in_array($variableType->getValue(), $definedVariables, true)) { + return true; + } + } + if ($nativeType instanceof UnionType) { return ! $nativeType->equals($type); } From 0190a2fc35e3d96dc3c593073978b0d4b313dce1 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 20 Jun 2026 17:15:42 +0700 Subject: [PATCH 5/7] clean up $scope->hasVariableType() check inside loop defined variables --- src/NodeAnalyzer/ExprAnalyzer.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/NodeAnalyzer/ExprAnalyzer.php b/src/NodeAnalyzer/ExprAnalyzer.php index 94831c3d02e..ba20619a1e7 100644 --- a/src/NodeAnalyzer/ExprAnalyzer.php +++ b/src/NodeAnalyzer/ExprAnalyzer.php @@ -171,10 +171,6 @@ public function isNonTypedFromParam(Expr $expr): bool $definedVariables = $scope->getDefinedVariables(); foreach ($definedVariables as $definedVariable) { - if (! $scope->hasVariableType($definedVariable)->yes()) { - continue; - } - $variableType = $scope->getVariableType($definedVariable); if ($variableType instanceof ConstantStringType && in_array($variableType->getValue(), $definedVariables, true)) { From e7015fac01b35b7186f6a753c7bd0f68d0323745 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sat, 20 Jun 2026 17:19:06 +0700 Subject: [PATCH 6/7] move check on defined variables on last resort after other check --- src/NodeAnalyzer/ExprAnalyzer.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/NodeAnalyzer/ExprAnalyzer.php b/src/NodeAnalyzer/ExprAnalyzer.php index ba20619a1e7..841bac67f6f 100644 --- a/src/NodeAnalyzer/ExprAnalyzer.php +++ b/src/NodeAnalyzer/ExprAnalyzer.php @@ -169,6 +169,15 @@ public function isNonTypedFromParam(Expr $expr): bool return true; } + if ($nativeType instanceof UnionType && ! $nativeType->equals($type)) { + return true; + } + + if (! $nativeType->isSuperTypeOf($type) + ->yes()) { + return true; + } + $definedVariables = $scope->getDefinedVariables(); foreach ($definedVariables as $definedVariable) { $variableType = $scope->getVariableType($definedVariable); @@ -178,12 +187,7 @@ public function isNonTypedFromParam(Expr $expr): bool } } - if ($nativeType instanceof UnionType) { - return ! $nativeType->equals($type); - } - - return ! $nativeType->isSuperTypeOf($type) - ->yes(); + return false; } public function isDynamicExpr(Expr $expr): bool From ff19731b87950e1417af908cc8b083b9b3215237 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Sun, 21 Jun 2026 04:49:24 +0700 Subject: [PATCH 7/7] add fixture to proof scoped dynamic variable --- .../Fixture/scoped_dynamic_variable.php.inc | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 rules-tests/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector/Fixture/scoped_dynamic_variable.php.inc diff --git a/rules-tests/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector/Fixture/scoped_dynamic_variable.php.inc b/rules-tests/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector/Fixture/scoped_dynamic_variable.php.inc new file mode 100644 index 00000000000..56c7a23c996 --- /dev/null +++ b/rules-tests/DeadCode/Rector/If_/RemoveAlwaysTrueIfConditionRector/Fixture/scoped_dynamic_variable.php.inc @@ -0,0 +1,65 @@ + +----- + \ No newline at end of file