Skip to content

Do not treat definition of functions and class-likes as unreachable#2263

Merged
ondrejmirtes merged 2 commits intophpstan:1.10.xfrom
takaram:fix-4002
Jun 12, 2023
Merged

Do not treat definition of functions and class-likes as unreachable#2263
ondrejmirtes merged 2 commits intophpstan:1.10.xfrom
takaram:fix-4002

Conversation

@takaram
Copy link
Copy Markdown
Contributor

@takaram takaram commented Feb 26, 2023

PHPStan treats the statement right after always-terminating statement, like return or exit, as unreachable statement.
With this PR, definition of classes, interfaces, traits and functions on top level will not marked as unreachable.
Nop nodes will be ignored as well.

I haven't implemented PHP's early binding mechanism since it is so complicated.
This may cause some false negative on code like this, but I think the advantage of decreasing false positive is larger.

Closes phpstan/phpstan#4002
Closes phpstan/phpstan#8966
Closes phpstan/phpstan#8319 (although the early binding mechanism is not completely compatible with that of PHP)

@antonsacred
Copy link
Copy Markdown

What is the blockers to merge it?

@herndlm
Copy link
Copy Markdown
Contributor

herndlm commented Apr 26, 2023

Oh interesting, this looks super close to what I did in #2279, which explains the conflicts, which makes this not mergable. I wasn't even aware of this PR when I did those changes. It also closed one of the linked issues here.

Most likely it makes sense that somebody checks what's still different / missing and adds those missing changes or adapts my changes to also fix the other issue.

I would gladly do that, but can't because I'm traveling.

@takaram
Copy link
Copy Markdown
Contributor Author

takaram commented May 3, 2023

I've just rebased the branch and merged @herndlm's changes and mine.

Copy link
Copy Markdown
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

And also please add a note in the PR description that this closes #8319, thanks.

]);
$this->analyse([__DIR__ . '/data/bug-4002_class.php'], []);
$this->analyse([__DIR__ . '/data/bug-4002_interface.php'], []);
$this->analyse([__DIR__ . '/data/bug-4002_trait.php'], []);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For each test* method, only a single analyse() call should be made. So these calls should each have their own test method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've split the method.

@ondrejmirtes ondrejmirtes merged commit 6984c5e into phpstan:1.10.x Jun 12, 2023
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nop node hides next unreachable statement "Unreachable statement" for function definition after exit

4 participants