[PHP] Proposal: php-nextgen

[thomasphansen]

Description

Hello, I've been chatting with @wing328 about some PR's I'd like to submit to modernize the php generator, as well as targeting some of its limitations and enhancing some features. My original idea was to:

• add parameter and return types as much as possible
• reorganize the code: use smaller functions, reduce nesting, use early returns.
• make the code safer by checking if a class implements an interface before trying to call its methods; introduce EnumInterface and enhance ModelInterface with a method to get the discriminator value.
• create traits to reduce code duplication between Model classes
• implement a solution for OneOf.

(i've already written the first 3 items in a feature branch - haven't created a PR yet, since our talk evolved to different directions)

During our conversation, William suggested the creation of a new generator, called php-nextgen: this generator would eventually be swapped with the current one, when releasing openapi-generator 7.0.0. He also suggested that the OneOf implementation could follow the ones used in other clients (C#, R, Powershell).

Based on these talks, I'd like to offer myself to develop this new php-nextgen generator, using the following guidelines:

• should be based on php 8.1;
• the first two items of my original suggestion (as well as parts for the third, and the traits creation) would be reused in the new generator;
• the Enum model would be rewritten to use Enumerations instead - superseeds PR #11602
• bring support for OneOf, using the R client as a model.

This last part has brought me some considerations that I'd like to discuss, as I wrote to @wing328:

1. Today all the fields in Model objects are properties inside a set of arrays. I was thinking if a better approach wouldn't be to use "real" properties instead, using annotations to provide proper metadata (regarding nullable fields, for example). What do you think?
2. The R client uses mustache to branch between object creation and simple attribution (checking "isPrimitiveType" and "isContainer" - pretty clever, BTW, way better than the php solution). I'd like to use this approach as well, but the AbstractPhpCodegen.java class defines "\DateType" and "\SplFileObject" as primitives. I haven't investigated yet, but I guess changing that would break all php clients, which I "guess" we don't want. What do you suggest to be done? (we could, for example, add another abstract codegen between the current one and its descendant classes, and move the current "primitives" definition to it).

(actually, while looking at the existing PRs and Issues for php, I found #10817, which seems to be caused exactly because we add \SplFileObject to the primitives list... maybe we should remove it?)

Let me know your thoughts! :smile: I have some time available for this project, and it would be a pleasure to contribute with this client! :smile:

openapi-generator version

7.0.0

OpenAPI declaration file content or url

not relevant

Command line used for generation

not relevant

Steps to reproduce

not a bug

Related issues/PRs

12331, #12721, #10817, #11485

Suggest a fix/enhancement

:smile:

[thomasphansen]

Forgot to mark you guys! :smile: sorry! @jebentier @dkarlovi @mandrean @jfastnacht @ybelenko, @renepardon

[ybelenko]

Today all the fields in Model objects are properties inside a set of arrays. I was thinking if a better approach wouldn't be to use "real" properties instead, using annotations to provide proper metadata (regarding nullable fields, for example). What do you think?

I tried that in php Slim codegen at first, but ended with the same $container prop for all values. Don’t actually remember why, but it becomes really complicated since model can be primitive type. Describe string enum via typical php class is very difficult using logic less templates. Anyway, you can try, I would definitely write minimal unit tests to be sure that approach with class props fits for all OpenApi types.

[thomasphansen]

I tried that in php Slim codegen at first, but ended with the same $container prop for all values. Don’t actually remember why, but it becomes really complicated since model can be primitive type. Describe string enum via typical php class is very difficult using logic less templates. Anyway, you can try, I would definitely write minimal unit tests to be sure that approach with class props fits for all OpenApi types.

I'll start in the TDD way, so there will (hopefully) be enough tests to cover all scenarios. As I said before, I think I need to change the primitives list in order to make this work. I'll try to do it without disturbing the other clients. Is it fine if I bother you and/or @wing328 now and then in openapi-generator Slack channel, to get some input on how it is going? Otherwise, I'll just bring pull requests...

[ybelenko]

I tried that in php Slim codegen at first, but ended with the same $container prop for all values. Don’t actually remember why, but it becomes really complicated since model can be primitive type. Describe string enum via typical php class is very difficult using logic less templates. Anyway, you can try, I would definitely write minimal unit tests to be sure that approach with class props fits for all OpenApi types.

I'll start in the TDD way, so there will (hopefully) be enough tests to cover all scenarios. As I said before, I think I need to change the primitives list in order to make this work. I'll try to do it without disturbing the other clients. Is it fine if I bother you and/or @wing328 now and then in openapi-generator Slack channel, to get some input on how it is going? Otherwise, I'll just bring pull requests...

I would love to help with PHP client. Few years ago I was truly inspired to enhance ALL PHP GENERATORS AS FAR AS POSSIBLE(🤦‍♂️ 🤦‍♂️ 🤦‍♂️). I was really naive at the moment, it's huge work for a single person. Current OpenAPI spec is so flexible and far beyond in features, it's just too exhausting to add even one tiny thing(was fighting against query parameter serialization #11225 (pipes, form, object, nesting etc.) for almost two weeks, crazy). And then we have polymorphism which doesn't end with allOf, blows my mind. I have to complain about mustaches also, they cool for a flat basic data, but when we use deeply nested schemas the output becomes unpredictable.

Regarding models, that's my own implementation of those in PHP Slim: https://github.com/OpenAPITools/openapi-generator/blob/01a9c55b6efd2149fc1bacc61f808c393ff0eb6f/samples/server/petstore/php-slim4/lib/BaseModel.php I don't like it very much, looks ugly, but it works and passes all the tests.

P.S. Btw, try to imagine PHP class which is ready to be anyOf: [object, string and boolean] 😅 It's 100% legal schema. It might be even allOf, not anyOf and we have to deal with it.

[thomasphansen]

Hi @ybelenko, Yes, I've or course seen many of your commits and your efforts to make things work better, thank you so much! :smile:

Just looked at your BaseModel, I can see that your approach is to move away from mustache and resolve things as much as possible in the php side. Nice one! I'll look more carefully at it later, and see what I can learn from it! :smile:

I know this is a hard task. I've been around it for some years, doing one or another contribution to help it fit better to our needs, but I feel like it's time to give more - in return to all the time I've saved thanks to this project and the hard work of all you guys! :wink:

I'll use the free time I have this week to do some tests on small changes to the AbstractPhpCodegen, build the base skeleton for the new generator, and hopefully break down the task in smaller parts, so that it's easier to plan and code. Feel free to reach me in Slack, if there's anything you'd like to suggest/discuss, ok? :smile:

Cheers!