Skip to content

[11.x] Add array response type #54160

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

Conversation

MagicalStrangeQuark
Copy link

Update object method annotation in Illuminate\Http\Client\Response to include array as a possible return type from json_decode.

I’m reopening this PR because I have a file using an API that returns an array of objects. The current method assumes only objects are returned, which is causing an error in my pipeline.

For example, if `$this->body() returns something like:

$bar = '[
    {
        "key": "value"
    }
]';

The actual implementation of object allows an array as response

public function object()
{
     return json_decode($this->body(), false);
}

Therefore, the annotation needs to be updated to reflect that an array can also be returned.

@cosmastech
Copy link
Contributor

It looks like I mistakenly removed this in a PR a while ago. You are correct about this possibly returning an array

@MagicalStrangeQuark
Copy link
Author

No problem, I just wanted to know if it would be possible to introduce this annotation here, because I have a file showing an error in my project.

@MagicalStrangeQuark MagicalStrangeQuark changed the title Add array response type [11.x] Add array response type Jan 11, 2025
@NickSdot
Copy link
Contributor

OP is correct here.

However, given the method name is literally object() I am wondering if the cleanest (but breaking) fix here would rather be casting to an object?

https://3v4l.org/7EZqZ

@shaedrich
Copy link
Contributor

OP is correct here.

However, given the method name is literally object() I am wondering if the cleanest (but breaking) fix here would rather be casting to an object?

3v4l.org/7EZqZ

Do you really want

var_dump((object)json_decode('5'));

to end up as

object(stdClass)#1 (1) {
  ["scalar"]=>
  int(5)
}

or

var_dump((object)json_decode('[5]'));

to end up as

object(stdClass)#1 (1) {
  ["0"]=>
  int(5)
}

which would have to accessed as

$obj->{'0'}; // instead of just $arr[0];

I'm not sure if—even if it seems plausible at first thought—manipulating the input is the right way to go. Passing $associative to json_decode() doesn't mean, everything is cast, but rather JSON objects which can be represented by both arrays and objects in PHP due to the versatile nature of PHP arrays. But that means, it's the method name object() that's the problem, not the functionality.

@NickSdot
Copy link
Contributor

Fair, the scalar example results in nonsense. But yeah, I'd totally expect $obj->{0} from a method named object().

As you said, it all boils down to:

it's the method name object() that's the problem, not the functionality

But the method is here now and I believe it should do what it indicates. Tricky.

@shaedrich
Copy link
Contributor

shaedrich commented Jan 12, 2025

That's why I'd say, always returning an object can be the solution. And changing the annotation as it is done in this PR by @MagicalStrangeQuark is the only thing we can do right now about this with the method itself. One additional thing we can do, is adding a warning to the docs

Deprecating the method and creating a new one with the name returnOnlyAsPhpObjectWhenTheJsonObjectHasStringKeys() probably doesn't make sense

@taylorotwell
Copy link
Member

Sorry I can't add a return type of array to a method named "object". Just too embarrassing. 😅

@shaedrich
Copy link
Contributor

@taylorotwell Do you ever read the discussion below a PR? If you did, you wouldn't be saying this.

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.

5 participants