fix return type of jsonDecode method #117
Conversation
|
For some reason the workflows won't trigger. Your change is not sufficient as the static analysis check will fail. For the pull request to be approved and merged you must also provide some tests for the new behaviour. So if you could add some tests for non-array input that would be great. |
|
I'm still not getting the option from GitHub to approve that you can run the workflow. Let me just try to closes the PR, then re-open it to see if that works. |
|
I see your github actions config has |
|
🤦 Thanks for the obvious fix. :) |
| $body = json_decode((string) json_encode($this->getResponseBody()), true); | ||
|
|
||
| if (is_scalar($body) || null === $body) { | ||
| return true; |
There was a problem hiding this comment.
The check for null here, can't that cause false positives? json_decode() will return null if the value can not be decoded. So to allow null we must also make sure that json_last_error() returns JSON_ERROR_NONE.
src/Context/ApiContext.php
Outdated
| $body = json_decode((string) json_encode($this->getResponseBody()), true); | ||
|
|
||
| if (null === $body && JSON_ERROR_NONE === json_last_error()) { |
There was a problem hiding this comment.
This is not correct either as far as I can tell. If $body is null and there is no error, this means that the null value was encoded.
There was a problem hiding this comment.
What's incorrect in the null value being encoded?
There was a problem hiding this comment.
You are checking if json_last_error() returns JSON_ERROR_NONE, but still return false, which means a failure. Is that correct?
There was a problem hiding this comment.
You're right. I changed the condition to check if the error is different from JSON_ERROR_NONE
Fix #116