Skip to content

Report useless return values of function calls#3225

Merged
ondrejmirtes merged 12 commits intophpstan:1.11.xfrom
staabm:useless-return
Jul 12, 2024
Merged

Report useless return values of function calls#3225
ondrejmirtes merged 12 commits intophpstan:1.11.xfrom
staabm:useless-return

Conversation

@staabm
Copy link
Contributor

@staabm staabm commented Jul 11, 2024

@staabm staabm marked this pull request as ready for review July 11, 2024 12:30
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

Copy link
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.

Separate rule please. Same level the void usage is reported (not sure from the top of my head).

@staabm
Copy link
Contributor Author

staabm commented Jul 11, 2024

Same level the void usage is reported (not sure from the top of my head).

its a level 0 error: https://phpstan.org/r/42b76642-82b2-422a-bd7d-f3678eb6e1b0

@clxmstaab clxmstaab force-pushed the useless-return branch 2 times, most recently from 3d864f7 to 51d7783 Compare July 12, 2024 07:10
@staabm
Copy link
Contributor Author

staabm commented Jul 12, 2024

I have no idea why the PHP7.2 build fails with a "risky error". maybe its related that I ran the levels-tests on a windows machine.

Copy link
Member

Choose a reason for hiding this comment

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

This isn't how to make test for >= 80000 only. You need to call markTestSkipped. See many examples in other tests. This is why it's marked as risky.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ouch. great catch, thanks.

Copy link
Member

Choose a reason for hiding this comment

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

Please put () after %s. So that it's print_r() and not just print_r.

@staabm
Copy link
Contributor Author

staabm commented Jul 12, 2024

in phpstorm stubs I found another function alias worth reporting: show_source()

@staabm
Copy link
Contributor Author

staabm commented Jul 12, 2024

just realized the PrestaShop/PrestaShop shows 5 real world errors this rule is targetting, simlar to the one of my initial report 💪

@ondrejmirtes
Copy link
Member

Nice! I really like this addition.

@ondrejmirtes ondrejmirtes merged commit ebce317 into phpstan:1.11.x Jul 12, 2024
@ondrejmirtes
Copy link
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.

3 participants