Skip to content
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

Jump to bottom

Add notFalse #164

Merged
merged 1 commit into from
Jan 5, 2020
Merged

Add notFalse #164

merged 1 commit into from
Jan 5, 2020

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Dec 28, 2019

A lot of PHP functions return a value or false in case of error. Therefore, notFalse makes it easier to assert that no error occured. This also makes Psalm/PHPStan happy.

@ruudk
Copy link
Contributor Author

ruudk commented Dec 28, 2019

Not sure why the test is failing, it appears that it installs webmozart/assert (1.6.0) during Psalm analysis. It should always pick master, right?

@ruudk
Copy link
Contributor Author

ruudk commented Dec 28, 2019

Proof that it works: https://psalm.dev/r/dfa38a60dc

@BackEndTea
Copy link
Collaborator

Not sure why the test is failing, it appears that it installs webmozart/assert (1.6.0) during Psalm analysis. It should always pick master, right?

pslam uses this library internally, however that is not an issue as it doesn't use that for its static analysis.

BackEndTea
BackEndTea requested changes Dec 28, 2019
View reviewed changes
Copy link
Collaborator

@BackEndTea BackEndTea left a comment

Choose a reason for hiding this comment

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

This looks good, but it does need tests.

@ruudk
Add notFalse
0456a36 
A lot of PHP functions return a value or false in case of error. Therefore, `notFalse` makes it easier to assert that no error occured. This also makes Psalm/PHPStan happy.
@ruudk
Copy link
Contributor Author

ruudk commented Dec 29, 2019

@BackEndTea Thanks for reviewing. Added the tests.

@ruudk
Copy link
Contributor Author

ruudk commented Jan 3, 2020

@BackEndTea What do you think? Is shippable? Or do you want me to make adjustments? Thanks.

@BackEndTea
Copy link
Collaborator

I'll take a look at this within the week

@BackEndTea BackEndTea merged commit f2a787b into webmozarts:master Jan 5, 2020
@BackEndTea
Copy link
Collaborator

Thank you for the work on this @ruudk

@ruudk
Copy link
Contributor Author

ruudk commented Jan 5, 2020

Thanks for merging. Do you think this can be tagged?

@ruudk ruudk deleted the not-false branch January 5, 2020 13:15
@ruudk ruudk mentioned this pull request Jan 17, 2020
Closed
@ruudk
Copy link
Contributor Author

ruudk commented Jan 30, 2020

@BackEndTea As this feature is extremely useful in cases where PHPStan complains about something (from php) returning false|other.... can this please be tagged 🙏 ?

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

@BackEndTea BackEndTea BackEndTea requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ruudk @BackEndTea