[10.x] Adding return types flag to MakeController command

[PovilasKorop]

This is a followup on #46166

This PR adds a flag and a prompt to make:controller command to specify what types to return.

php artisan make:controller UserController --resource --return=XXXXX

That XXXXX has 4 different values that I suggest, happy to discuss:

• response - will return Illuminate\Http\Response in methods
• json - will return Illuminate\Http\JsonResponse
• view - will return Illuminate\Contracts\View\View|Illuminate\Contracts\View\Factory
• inertia - will return Inertia\Response

Reason for this: The discussion on the PR above, on Twitter, and my YouTube channel showed that many people do want to return something, and quite a few people suggested it to be managed as a flag in Artisan command. So this is my attempt to implement that. Still a bit opinionated, happy to discuss, or even if this PR is rejected, I felt a weird obligation to at least try :)

A few notices:

• I added the types only on typical GET methods: index/create/edit/show. I suggest to not add types to store/update/destroy, because their return types are probably different and individual.
• I didn't cover the case of JSON returns when using Eloquent API Resources, because they return different types than JsonResponse, and also different types if used like ApiResource::collection() and new ApiResource().
• This PR adds a new case for Laravel stubs where there may or may not be a new line in "use" section. I haven't seen that happening in other stubs in Make command, so I added a workaround for removing a potential empty line: $replace["\n\n\n"] = "\n\n";. Code is not that elegant, any suggestions welcome.

[tontonsb]

Two types in the docblock are already redundant, but : array should also be moved to a native typehint.

[tontonsb]

I added the types only on typical GET methods: index/create/edit/show. I suggest to not add types to store/update/destroy, because their return types are probably different and individual.

I agree and find this to be a good reason against this idea. As you've acknowledged, this is something that would only be applicable on 4 of the methods. For APIs it drops down to 2 as there are no create and edit. Is it worth it to have 2/6 methods return-typed?

I didn't cover the case of JSON returns when using Eloquent API Resources, because they return different types than JsonResponse, and also different types if used like ApiResource::collection() and new ApiResource().

This also seems like a flaw as the feature (especially the CLI prompt) might make people feel that other return types (like arrays or JsonResources) are discouraged. And that consistent return types are encouraged instead of mixing JsonResource and ResourceCollection appropriately.

Btw ResourceCollection is also a subtype of JsonResource. You might also consider the Responsable interface for these.

so I added a workaround for removing a potential empty line

You could delete the additional line break in the stubs and insert the \n if you're actually adding the import.

[timacdonald]

I just don't think this is ever something that we should solve in the framework. Introducing this is gonna be more problematic, IMO, than not.

Too many variations and conflicting types.

[jcsoriano]

Yeah I think Taylor's decision to just have no return types on controller methods at the start is fine.

At the start there's no code within the method anyway, so it makes sense that there's no return type as well. In the same sense that we still need to fill the method with logic for our own case anyway, the return type is just another thing that's pretty much "for our own case" given the number of valid choices and preferences for the return type, each having their own merits.

Maybe in the future the technology, ecosystem, or community will converge on a more clear convention for this but right now too much energy and debate has been spent for too small a thing imo.

In the proposal there are "four ways" but what about those who choose arrays, or strings, or models, or collections, or even return nothing (no content). All of those have their merits. And that the framework intelligently converts those into valid JSON responses are chef's kiss.

It also includes inertia, but what about other packages that might become famous in the future, will we need to add those too? It leads to a slippery slope and the "pain" of having to add the particular typehint we want manually is so small compared to the maintenance burden of this + mental burden of constant debates and additions to the list of choices supported.

[PovilasKorop]

@tontonsb @timacdonald @jcsoriano thank you for taking the time to comment and provide the reasoning.

I slept on it, and you're right: my suggested PR offers a partial solution to a part of the cases, with many more existing cases.

With overwhelming amount of downvotes, community also told their opinion, so decided to close this PR.

Instead will go and shoot a video showing people how to edit stubs.

[timacdonald]

Education is the best solution ❤️