glideapps / quicktype

Generate types and converters from JSON, Schema, and GraphQL
https://app.quicktype.io
Apache License 2.0
11.93k stars 1.05k forks source link

Better PHP Support #2432

Open HighLiuk opened 9 months ago

HighLiuk commented 9 months ago

Hey there. First of all, I'd like to thank you all for this tool, it's awesome.

I'd like to use quicktype a lot on PHP, but I've tried it and IMHO I'd never use the generated code like this. So I've cloned it locally and in a week or so I've managed to write a PHP implementation that suits my needs. Is there any chance I can contribute to enhance PHP language support? Also, is there any rule to follow in order to have it approved?

Thank you in advance.

comod commented 8 months ago

Please explain what youve customized. I am currently analyzing quicktype especially for php as well

HighLiuk commented 8 months ago

@comod here you go if you want to test it yourself > https://github.com/HighLiuk/quicktype

I've added support for a lot of different features ranging through PHP 7.3 (or older) to PHP 8.2 which has far way less boilerplate than previous versions of PHP especially for OOP.

I've added a dozen options as shown in this local version of the web app I've scaffolded to play with it. As you can see from the screenshot, choosing which PHP version to target is an option. This internally guarantees that you cannot opt in for features that are not available for that PHP version (e.g. readonly attributes are introduced in PHP 8.1. These may be turned off with a flag (defaults to true for simplicity) but are auto-disabled if the PHP version is lower than 8.1)

It is not 100% accurate (I recently noticed a few inconsistencies and so) and I've not run unit tests against it, but if a PR is accepted I would add the required tests.

Also, I've revised how the validation works in order to keep it simpler.

Let me know what do you think about. Would be nice if a maintainer read this too (perhaps liking the thread would help?)

comod commented 8 months ago

At first glance, this looks exactly like what I need as well. I have already started building my own version as well. Things I've noticed or changed: enums (BackedEnum), validations which are currently completely pointless. From/To-Array instead of stdClass, Multiple-Files with namespaces and imports instead of a god-class. Let's go into the details in the new year and then we simply create a pull request.

HighLiuk commented 8 months ago

I saw that other languages (like Go) have support for output multiple files but I didn't figure out how to implement it. In PHP this is necessary when you leverage autoloads and if you have to do that manually over several generated classes, it easily becomes messy.

dvdsgl commented 7 months ago

Hello! You can make a PR and we can review it there.

comod commented 7 months ago

i figured out that it needs to remove this line to get it work @HighLiuk: image The generated code looks good so far!

I was able to generate multiple files with some few changes like this: image

but i dont know how to add namespaces and imports. should this be inferred by additional properties in the json schema or should this be inferred by the original structure of the json-schema source. i only get fileName as parameter and not the whole filePath. so this is kind of tricky - any ideas?

HighLiuk commented 7 months ago

Hey there @comod thanks in advance

but i dont know how to add namespaces and imports. should this be inferred by additional properties in the json schema or should this be inferred by the original structure of the json-schema source. i only get fileName as parameter and not the whole filePath. so this is kind of tricky - any ideas?

probably you should look for the property types to get which classes / enums need to be imported at the top of the file.

But a clever option would be to add a "namespace" option to add a namespace on the top of the file. Like --namespace=Glideapps\Quicktype so for instance class Foo would have namespace Glideapps\Quicktype; on top of the file. This way according to PSR-4 you don't even need to add use Glideapps\Quicktype\Foo; if you add them to the same directory with the same namespace.

comod commented 7 months ago

I wasnt clear enough. What i try to achive is multiple files in different folders either according to the source structure (which is already organized in different folders) or to any additional information from the json schema (like a property named "package" or "namespace") if this makes any sense. Maybe this is out of scope and should be done as extra step

comod commented 7 months ago

I have now delved even deeper into the subject matter and have a few basic questions for the authors:

In the code, for example, I found this:

    private _globalForbiddenNamespace: Namespace | undefined;
    private _otherForbiddenNamespaces: Map<string, Namespace> | undefined;
    private _globalNamespace: Namespace | undefined;

However, I could not understand what this is used for. I believe this has nothing to do with the namespace I mean.

Finding a solution outside is no less challenging, as not only namespaces have to be added, but all references also have to be adjusted. Additionally, the sources would have to be read and mapped, etc. It would simply be more elegant to get a grip on this with this one tool on the spot.

If it is to remain with just the one global namespace, then what @HighLiuk suggested is indeed sufficient.

HighLiuk commented 7 months ago

Not sure if I understand everything @comod, but I agree that it should be simpler to use QuickType's internal API to easily print on different files.