Skip to content

Fix fputcsv fields parameter. Should be string[].#448

Merged
ondrejmirtes merged 1 commit intophpstan:masterfrom
DaveLiddament:patch-1
Feb 15, 2021
Merged

Fix fputcsv fields parameter. Should be string[].#448
ondrejmirtes merged 1 commit intophpstan:masterfrom
DaveLiddament:patch-1

Conversation

@DaveLiddament
Copy link
Copy Markdown
Contributor

@DaveLiddament DaveLiddament commented Feb 15, 2021

The fields parameter for fputcsv should be an array of strings. See PHP manual.

This PR should fix analysis of this snippet by correctly reporting an error on line 22.

Running the code in the snippet on PHP 8.0 (PHP 8.0.0 (cli) (built: Dec 6 2020 06:56:11) ( NTS )) results in this error:

PHP Fatal error:  Uncaught Error: Object of class Person could not be converted to string in /home/vagrant/example/csv.php:24
Stack trace:
#0 /home/vagrant/example/csv.php(24): fputcsv()
#1 {main}
  thrown in /home/vagrant/example/csv.php on line 24

NOTE: I've seen a similar error with PHP 7.3 too.

The `fields` parameter for `fputcsv` should be an array of strings. See [PHP manual](https://www.php.net/manual/en/function.fputcsv.php).

This should fix [this snippet](https://phpstan.org/r/65420d60-d1fd-4eef-9ba3-e057672082ee) by correctly reporting an error on line `22`.

Running the code in the snippet on PHP 8.0 (`PHP 8.0.0 (cli) (built: Dec  6 2020 06:56:11) ( NTS )`) results in this error:
```
PHP Fatal error:  Uncaught Error: Object of class Person could not be converted to string in /home/vagrant/example/csv.php:24
Stack trace:
#0 /home/vagrant/example/csv.php(24): fputcsv()
phpstan#1 {main}
  thrown in /home/vagrant/example/csv.php on line 24
```

NOTE: I've seen  a similar error with PHP 7.3 too.
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you!

@DaveLiddament
Copy link
Copy Markdown
Contributor Author

@ondrejmirtes I raised a similar issue with Psalm. It has just been pointed out that 'int|float|bool' also work for fputcsv on PHP. Perhaps my suggestion is too strict? Sorry if that's the case.

See updated snippet

@ondrejmirtes
Copy link
Copy Markdown
Member

Yeah, please send an update that also works with scalars, preferably as array<int, int|float|bool|string>.

@DaveLiddament
Copy link
Copy Markdown
Contributor Author

@ondrejmirtes I might have caused you trouble today with this PR....

I've created a more extended snippet that covers off all the edge cases I can find: https://phpstan.org/r/c279f6fb-9183-48da-9c34-541370818fce

I think we actually need: array<array-key, int|float|bool|string|\Stringable|null>

Not sure if this is a BC break when using PHP <8.0 without \Stringable?

Possibly needs a few tests to check that everything is correct on all versions?

I'll update with a proposed fix.

@ondrejmirtes
Copy link
Copy Markdown
Member

Stringable won't work but look for a special handling somewhere in the codebase of 'sprintf', it has a similar problem...

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.

2 participants