janephp / janephp

:seedling: Jane is a set of libraries to generate Models & API Clients based on JSON Schema / OpenAPI specs
https://jane.readthedocs.io/en/latest/
MIT License
602 stars 131 forks source link

Allow to generate models with public & typed properties #525

Open jmsche opened 3 years ago

jmsche commented 3 years ago

Is your feature request related to a problem? Please describe. It would be nice if Jane could generate models with typed & public properties, instead of untyped properties & getters/setters.

Describe the solution you'd like This is one model that is currently generated on my project:

<?php

declare(strict_types=1);

namespace Generated\Model;

class Category
{
    /**
     * @var int
     */
    protected $id;
    /**
     * @var string
     */
    protected $name;
    /**
     * @var string
     */
    protected $slug;

    public function getId(): int
    {
        return $this->id;
    }

    public function setId(int $id): self
    {
        $this->id = $id;

        return $this;
    }

    public function getName(): string
    {
        return $this->name;
    }

    public function setName(string $name): self
    {
        $this->name = $name;

        return $this;
    }

    public function getSlug(): string
    {
        return $this->slug;
    }

    public function setSlug(string $slug): self
    {
        $this->slug = $slug;

        return $this;
    }
}

And here is what could be generated instead:

<?php

declare(strict_types=1);

namespace Generated\Model;

class Category
{
    public int $id;
    public string $name;
    public string $slug;
}
CaptainQuirk commented 3 years ago

Hi !

Isn't it two separate issues ?

What do you think ?

jmsche commented 3 years ago

Hi,

It's clearly two different behaviours to add, but IIRC I added them in the same issue as both impact properties.

Note: typed properties can be implemented for PHP 7.4 as well :)

Korbeil commented 3 years ago

Typed properties will come with Jane v8, I'll release v7 soon-ish ~

CaptainQuirk commented 3 years ago

Note: typed properties can be implemented for PHP 7.4 as well :)

@jmsche Yes you're absolutely right, my mistake ! :smile:

Korbeil commented 3 years ago

Note: typed properties can be implemented for PHP 7.4 as well :)

@jmsche Yes you're absolutely right, my mistake ! smile

It's not exactly true, for typed properties we want to use unions type because it's easier to manage null|string rather than ?string and we often have multiple types. So it will be probably a PHP8 feature.

cjgordon commented 2 years ago

Hi All, Just checking in to see where this if there has been any progress on this issue?

I am using v7.1.5 with PHP and not that types in models are being defined using @var notation instead of being defined in declaration ie. 'private string $words;'

This seems like something I may be able to contribute towards and am more than happy to if this hasn't already been progressed and I'm just missing something simple. :)

Cheers Chris

Korbeil commented 2 years ago

I did not worked on that feature yet, feel free to try it :+1:

cjgordon commented 2 years ago

Okay I will take a look. I can see that one of the places which would need to be changed is: https://github.com/janephp/janephp/blob/next/src/Component/OpenApi3/Generator/Parameter/NonBodyParameterGenerator.php#L148

Will take a poke and note other changes before implementing them.

How would you expect this to work. My thoughts would be if composer declares PHP7.4 or above then use built in typed variables instead of @var annotation. Then also provide an override option in the config, something like 'builtin_types' which if set to false will revert it back to annotations. If using PHP<7.4 this would obviously be ignored and annotations used.

cjgordon commented 2 years ago

Just an update on this I chased it down and the required change is actually this: https://github.com/cjgordon/janephp/commit/ea175c14c960cad3e2f558d5f04d7d983716d045

The issue with this is that the current PHP requirements for the project is 7.2 to 8.0 and class property type hints require 7.4 or greater.

There are two options for dealing with this, increase project minimum PHP version to 7.4 or work out how to turn this off if version is lower than <7.4 in code. Still working out how to implement the later option.

@Korbeil what would you preference be?

Korbeil commented 2 years ago

I would prefer to raise PHP requirements to 7.4.

cjgordon commented 2 years ago

@Korbeil okay will change 7.4 minimum.

I have a bit more work to do as I would like to try improve array type hints a bit more.

Then I will execute test and submit PR in the next couple of days.