Skip to content

Fix json_decode error return type#1235

Merged
ondrejmirtes merged 2 commits into
phpstan:1.5.xfrom
herndlm:fix-json-decode-error-return-type
Apr 21, 2022
Merged

Fix json_decode error return type#1235
ondrejmirtes merged 2 commits into
phpstan:1.5.xfrom
herndlm:fix-json-decode-error-return-type

Conversation

@herndlm

@herndlm herndlm commented Apr 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Looks like the previous code assumed that both json_encode and json_decode do not return false in case of an error and JSON_THROW_ON_ERROR is set. But actually, the error return value for json_decode is null.

https://www.php.net/manual/en/function.json-decode.php

Returns the value encoded in json in appropriate PHP type. Values true, false and null are returned as true, false and null respectively. null is returned if the json cannot be decoded or if the encoded data is deeper than the nesting limit.

Atm this does not change much, but I'd like to narrow down the return types there even more in 1.6, if possible.

Also: I guess I would have used a private constant, but went with the property for consistency.

@herndlm herndlm changed the base branch from 1.6.x to 1.5.x April 20, 2022 18:49
@herndlm herndlm marked this pull request as draft April 20, 2022 18:59
@ondrejmirtes

ondrejmirtes commented Apr 20, 2022

Copy link
Copy Markdown
Member

Please check this PR and let me know what you think: #993 Also the discussion 😊

@herndlm

herndlm commented Apr 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Haha, reading what I just added in the description a second time, made it clear that json_decode can always return null/false even if that flag is set.
https://3v4l.org/tDHFQ

@herndlm herndlm force-pushed the fix-json-decode-error-return-type branch from f1bd8a4 to a8d00e3 Compare April 20, 2022 19:05
@herndlm herndlm marked this pull request as ready for review April 20, 2022 19:06
@herndlm

herndlm commented Apr 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

That other PR makes sense I guess, feels a bit specific to me maybe, but what it does not do, and what I want to actually see, is something even simpler. As stated also recently in another issue, the return type of json_decode is actually either something like mixed[]|scalar|null or stdClass|scalar|null depending on the value of the associative argument and/or some flags. I'd like to get rid of the mixed. But this for sure will make conflicts with that other PR.

Example test cases https://3v4l.org/CBHAi. But I'm not a JSON pro, maybe I'm missing something obvious..

@herndlm

herndlm commented Apr 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

this should not make ugly conflicts with the PR from Tomas (except potentially failing tests). Maybe we can add the return type behaviour that I described to his PR 🤔 or a follow-up might be even simpler

@ondrejmirtes ondrejmirtes merged commit 820a728 into phpstan:1.5.x Apr 21, 2022
@ondrejmirtes

ondrejmirtes commented Apr 21, 2022

Copy link
Copy Markdown
Member

Thank you!

@herndlm herndlm deleted the fix-json-decode-error-return-type branch April 21, 2022 13:46
@herndlm herndlm mentioned this pull request Apr 25, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

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

@herndlm @ondrejmirtes