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.

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

Update $data argument types of file_put_contents #6954

Merged
merged 1 commit into from Nov 21, 2021

Conversation

ciaranmcnulty
Copy link
Contributor

The main aim is to exclude boolean false, which would be coerced into '' in the following cases:

file_put_contents('outfile', file_get_contents('invalid_file')); // false instead of string
file_put_contents('outfile', fopen('invalid_file', 'r')); // false instead of resource
file_put_contents('outfile', file('invalid_file')); // false instead of array

The main aim is to exclude boolean false, which could be coerced into '' in the following cases:

```
file_put_contents('outfile', file_get_contents('invalid_file')); // false instead of string
file_put_contents('outfile', fopen('invalid_file', 'r')); // false instead of resource
file_put_contents('outfile', file('invalid_file')); // false instead of array
```
@ciaranmcnulty ciaranmcnulty changed the title Update data types of file_put_contents Update $data argument types of file_put_contents Nov 21, 2021
@orklah orklah added the release:fix The PR will be included in 'Fixes' section of the release notes label Nov 21, 2021
@orklah
Copy link
Collaborator

orklah commented Nov 21, 2021

Thanks! Congrats for your first PR!

Note that with Psalm's default, there still won't be an error from false due to https://psalm.dev/docs/running_psalm/configuration/#ignoreinternalfunctionfalsereturn

Make sure to put that to false in order to get Psalm to emit errors here!

@orklah orklah merged commit 912079e into vimeo:master Nov 21, 2021
@ciaranmcnulty
Copy link
Contributor Author

I'm not sure those functions 'rarely return false' TBH

@orklah
Copy link
Collaborator

orklah commented Nov 21, 2021

Yeah, it's a pretty opinionated take that Psalm took, but as long as it's behind a config, I'm fine with it.

Ideally, Psalm would have made the difference between false values returned because of wrong parameters and false values returned because runtime errors (missing files is a good example here). This way, it would have paved the way for PHP8 TypeErrors and left other false/null values as error by default, but that would have been a big work...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants