Fix missing detection of dead code in expressions by staabm · Pull Request #4090 · phpstan/phpstan-src

staabm · 2025-07-11T13:27:24Z

closes phpstan/phpstan#13232
closes phpstan/phpstan#11909

staabm · 2025-07-11T14:10:38Z

hmm I wonder whether I should add this $isAlwaysTerminating = $isAlwaysTerminating || $result->isAlwaysTerminating(); handling everywhere, where we have

			$hasYield = $hasYield || $result->hasYield();
			$throwPoints = array_merge($throwPoints, $result->getThrowPoints());
			$impurePoints = array_merge($impurePoints, $result->getImpurePoints());

etc...?

edit: I added it in all places where I could think of a example which I was able to test

staabm · 2025-07-12T05:52:42Z

I think the new behaviour makes sense but the errors are unfortunate

https://github.com/laravel/framework/blob/43993ed92af54aa8620d8e779a7dcd658f44364c/types/Support/Helpers.php#L44-L60

------ ------------------------------------------------------- 
  Line   Support/Helpers.php                                    
 ------ ------------------------------------------------------- 
  47     Unreachable statement - code above always terminates.  
         🪪  deadCode.unreachable                               
  66     Unreachable statement - code above always terminates.  
         🪪  deadCode.unreachable                               
 ------ -------------------------------------------------------

phpstan-bot · 2025-07-12T06:26:36Z

This pull request has been marked as ready for review.

ondrejmirtes

There is StatementResultTest that tests "is always terminating". Please add a similar test for ExpressionResult. Thank you.

src/Analyser/ExpressionResult.php

src/Analyser/NodeScopeResolver.php

staabm · 2025-07-17T16:27:46Z

I think this should be ready to land.

thanks for the feedback

tests/PHPStan/Analyser/ExpressionResultTest.php

ondrejmirtes · 2025-07-21T13:54:51Z

src/Analyser/NodeScopeResolver.php

 				$leftMergedWithRightScope,
 				$leftResult->hasYield() || $rightResult->hasYield(),
-				false,
+				$leftResult->isAlwaysTerminating() || $rightResult->isAlwaysTerminating(),


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

$x && exit(); should be false

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

ondrejmirtes · 2025-07-21T14:06:29Z

I swear GitHub has some kind of caching problem with the new "Files changed" UI. Sorry.

ondrejmirtes · 2025-07-21T14:06:44Z

Thank you!

staabm marked this pull request as ready for review July 12, 2025 06:26

ondrejmirtes requested changes Jul 17, 2025

View reviewed changes

src/Analyser/ExpressionResult.php Outdated Show resolved Hide resolved

staabm force-pushed the bug13232 branch from 0abb8b6 to 06a98e0 Compare July 17, 2025 08:23

ondrejmirtes requested changes Jul 17, 2025

View reviewed changes

src/Analyser/NodeScopeResolver.php Outdated Show resolved Hide resolved

src/Analyser/NodeScopeResolver.php Show resolved Hide resolved

staabm force-pushed the bug13232 branch from 5414391 to 52b172e Compare July 17, 2025 12:31

clxmstaab force-pushed the bug13232 branch from f9fba3b to 367e758 Compare July 17, 2025 14:54

ondrejmirtes requested changes Jul 17, 2025

View reviewed changes

src/Analyser/NodeScopeResolver.php Show resolved Hide resolved

src/Analyser/NodeScopeResolver.php Show resolved Hide resolved

src/Analyser/NodeScopeResolver.php Outdated Show resolved Hide resolved

src/Analyser/NodeScopeResolver.php Show resolved Hide resolved

staabm force-pushed the bug13232 branch from 2a167da to 180c665 Compare July 17, 2025 15:48

ondrejmirtes requested changes Jul 21, 2025

View reviewed changes

clxmstaab force-pushed the bug13232 branch from 5fb5b06 to eea18f2 Compare July 21, 2025 12:38

staabm and others added 15 commits July 21, 2025 15:11

Fix missing detection of dead code in expressions

bda643b

Support method calls / static calls

a3a6a49

fix assignments

c2b3e7b

fix regression

a430885

Fix NullsafeMethodCall

074877a

fix build

1707b28

test nullsafe

6023250

more tests

5bf7349

fix PHP 7.4

82c93d4

better names

0cc9483

fix string interpolation

af69075

fix func args

0610269

fix callables

0b29958

added regression test

0b642df

fix arrays

e7d00d1

staabm and others added 13 commits July 21, 2025 15:11

fix method calls

dfe77ba

ExpressionResult isAlwaysTerminating is consistent with StatementResult

95bb39d

Added ExpressionResultTest

4e0c9c0

Add $isAlwaysTerminating to every node-type if-branch

89d5780

fix build

f18867b

fix arrow-fn and suppress

4e00904

implement more cases

d0d52d9

Update NodeScopeResolver.php

cc65f3b

Update NodeScopeResolver.php

70d67c2

fix nullsafe

24ff8fd

fix expectations

ba52625

simplify ExpressionResultTest

8b2806a

add more tests

15f2b01

staabm force-pushed the bug13232 branch from d8a0a1d to 15f2b01 Compare July 21, 2025 13:11

ondrejmirtes requested changes Jul 21, 2025

View reviewed changes

ondrejmirtes merged commit 89d0610 into phpstan:2.1.x Jul 21, 2025
413 of 419 checks passed

staabm deleted the bug13232 branch July 21, 2025 14:11

mvorisek mentioned this pull request Jul 21, 2025

Conditional type in @param - Call to method Xxx::xxx() with true will always evaluate to true. phpstan/phpstan#10337

Closed

This was referenced Jul 22, 2025

Fix missing detection of dead code in arrow functions #4147

Merged

Fix missing detection of dead code in closures #4148

Merged

Conversation

staabm commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

staabm commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

staabm commented Jul 12, 2025

Uh oh!

phpstan-bot commented Jul 12, 2025

Uh oh!

ondrejmirtes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

staabm commented Jul 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ondrejmirtes Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

ondrejmirtes Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

staabm Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ondrejmirtes commented Jul 21, 2025

Uh oh!

Uh oh!

ondrejmirtes commented Jul 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

staabm commented Jul 11, 2025 •

edited

Loading

staabm commented Jul 11, 2025 •

edited

Loading

staabm Jul 21, 2025 •

edited

Loading