[8.x] Fix the wrong of response::make() function's parameter type

[hmingv]

‘’‘
$content = [
'code' => 1
];
\Illuminate\Support\Facades\Response::make($content);
’‘’

The above syntax prompt is incorrect
Expected parameter of type 'string|void', 'array' provided

However, $content can actually pass array, but the annotation is wrong.

[driesvints]

This is incorrect. It's okay to widen a param type in the implementation. The facade change is okay though.

[hmingv]

Do I need to undo this change?
I think the scope of implementation classes should not exceed interfaces.

[driesvints]

Actually, the facade change is incorrect as well as it's mixin the contract, not the concrete implementation. No change is needed here and this PR should be closed.

If you want to do what you want you should type-hint the concrete factory class or use new Response directly.

[hmingv]

@driesvints

In fact, the $content parameter type of the make method of src/illuminate/routing/ResponseFactory.php is mixed

However, the method annotation in src/illuminate/support/facades/Response.php is of type string.

As you said, if the $content parameter type of src/illuminate/contracts/routing/ResponseFactory.php make method is string, but its implementation class is indeed mixed, don't you think this is wrong?

I think that if I call it successfully, the editor should not prompt an error.

So my point is to correct the wrong annotation.

[driesvints]

In fact, the $content parameter type of the make method of src/illuminate/routing/ResponseFactory.php is mixed

However, the method annotation in src/illuminate/support/facades/Response.php is of type string.

Yes, you can widen parameter types in implementations. As I said before: the facade mixes the concrete, not the factory implementation so it should follow the types of the contract.

As you said, if the $content parameter type of src/illuminate/contracts/routing/ResponseFactory.php make method is string, but its implementation class is indeed mixed, don't you think this is wrong?

No, again: you can widen types in implementations: https://www.php.net/manual/en/migration72.new-features.php#migration72.new-features.param-type-widening

I think that if I call it successfully, the editor should not prompt an error.

That's because you're using the facade which is linked to the contract, not the factory class. If you need the factory class itself you should resolve that instead of using the facade.

We could update the Facade mixin to the concrete response class but that would be a breaking change and not something we can do in the current stable release.

[taylorotwell]

I don't see any practical harm in updating both of these to match the reality of how they are being used.

[hmingv]

I don't see any practical harm in updating both of these to match the reality of how they are being used.

Yes, this is just a Annotation.

Thanks, at the same time, thank @driesvints very much.

The information he mentioned was very helpful to me.

[driesvints]

Yeah the merged solution is a good middle ground 👍