Skip to content

Fix missing detection of dead code in expressions #4090

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
Jul 21, 2025
Merged

Fix missing detection of dead code in expressions #4090

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
bda643b
Fix missing detection of dead code in expressions
staabm Jul 11, 2025
a3a6a49
Support method calls / static calls
staabm Jul 11, 2025
c2b3e7b
fix assignments
staabm Jul 11, 2025
a430885
fix regression
staabm Jul 11, 2025
074877a
Fix NullsafeMethodCall
staabm Jul 11, 2025
1707b28
fix build
staabm Jul 11, 2025
6023250
test nullsafe
staabm Jul 11, 2025
5bf7349
more tests
staabm Jul 11, 2025
82c93d4
fix PHP 7.4
staabm Jul 11, 2025
0cc9483
better names
staabm Jul 11, 2025
af69075
fix string interpolation
staabm Jul 12, 2025
0610269
fix func args
staabm Jul 12, 2025
0b29958
fix callables
staabm Jul 12, 2025
0b642df
added regression test
staabm Jul 12, 2025
e7d00d1
fix arrays
staabm Jul 12, 2025
dfe77ba
fix method calls
staabm Jul 12, 2025
95bb39d
ExpressionResult isAlwaysTerminating is consistent with StatementResult
staabm Jul 17, 2025
4e0c9c0
Added ExpressionResultTest
staabm Jul 17, 2025
89d5780
Add $isAlwaysTerminating to every node-type if-branch
staabm Jul 17, 2025
f18867b
fix build
staabm Jul 17, 2025
4e00904
fix arrow-fn and suppress
staabm Jul 17, 2025
d0d52d9
implement more cases
staabm Jul 17, 2025
cc65f3b
Update NodeScopeResolver.php
staabm Jul 17, 2025
70d67c2
Update NodeScopeResolver.php
staabm Jul 17, 2025
24ff8fd
fix nullsafe
staabm Jul 17, 2025
ba52625
fix expectations
staabm Jul 21, 2025
8b2806a
simplify ExpressionResultTest
staabm Jul 21, 2025
15f2b01
add more tests
staabm Jul 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/Analyser/NodeScopeResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3289,7 +3289,7 @@ static function (): void {
return new ExpressionResult(
$leftMergedWithRightScope,
$leftResult->hasYield() || $rightResult->hasYield(),
false,
$leftResult->isAlwaysTerminating() || $rightResult->isAlwaysTerminating(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The right side might never be executed so this should only mention the left side.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$x && exit(); should be false

Copy link
Contributor Author

@staabm staabm Jul 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is false, see https://github.com/phpstan/phpstan-src/pull/4090/files#diff-fa85cdb8d6b73f343f042bbbe433ecc95119bf3dc5e76cc7291563d1c49cbd0dR87

I guess you was looking at an outdated version of the PR while commenting

array_merge($leftResult->getThrowPoints(), $rightResult->getThrowPoints()),
array_merge($leftResult->getImpurePoints(), $rightResult->getImpurePoints()),
static fn (): MutatingScope => $rightResult->getScope()->filterByTruthyValue($expr),
Expand All @@ -3310,7 +3310,7 @@ static function (): void {
return new ExpressionResult(
$leftMergedWithRightScope,
$leftResult->hasYield() || $rightResult->hasYield(),
false,
$leftResult->isAlwaysTerminating(),
array_merge($leftResult->getThrowPoints(), $rightResult->getThrowPoints()),
array_merge($leftResult->getImpurePoints(), $rightResult->getImpurePoints()),
static fn (): MutatingScope => $leftMergedWithRightScope->filterByTruthyValue($expr),
Expand All @@ -3335,7 +3335,7 @@ static function (): void {
$hasYield = $condResult->hasYield() || $rightResult->hasYield();
$throwPoints = array_merge($condResult->getThrowPoints(), $rightResult->getThrowPoints());
$impurePoints = array_merge($condResult->getImpurePoints(), $rightResult->getImpurePoints());
$isAlwaysTerminating = false;
$isAlwaysTerminating = $condResult->isAlwaysTerminating();
} elseif ($expr instanceof BinaryOp) {
$result = $this->processExprNode($stmt, $expr->left, $scope, $nodeCallback, $context->enterDeep());
$scope = $result->getScope();
Expand Down Expand Up @@ -3368,7 +3368,7 @@ static function (): void {
true,
);
$hasYield = $result->hasYield();
$isAlwaysTerminating = false;
$isAlwaysTerminating = $result->isAlwaysTerminating();
$scope = $result->getScope()->afterExtractCall();
} elseif ($expr instanceof Expr\Print_) {
$result = $this->processExprNode($stmt, $expr->expr, $scope, $nodeCallback, $context->enterDeep());
Expand Down
24 changes: 24 additions & 0 deletions tests/PHPStan/Analyser/ExpressionResultTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ public static function dataIsAlwaysTerminating(): array
'(string) $x;',
false,
],
[
'$x || exit();',
false,
],
[
'$x ?? exit();',
false,
],
[
'sprintf("hello %s", exit());',
true,
Expand Down Expand Up @@ -78,6 +86,22 @@ public static function dataIsAlwaysTerminating(): array
'@exit();',
true,
],
[
'$x && exit();',
true,
],
[
'exit() && $x;',
true,
],
[
'exit() || $x;',
true,
],
[
'exit() ?? $x;',
true,
],
];
}

Expand Down