Allow attributes' enums to be specified with an enum class string

[k2tzumi]

Proposal

When using Attribute to describe specifications, we would like to synchronize the enum property with the Enum class.

I thought it would be a good idea to have the enum value automatically expanded from the Enum class name.

#[Property('status', type: 'string', enum: StatusEnum::class, example: StatusEnum::DRAFT)]
public StatusEnum status;

What we dare not do

• Allowing UnitEnum arrays to be specified.
  In PHP8.1, IDE and linter warn when calling EnumClass::cases() in Attribute, so it was decided not to support it at this time.
  For example, here are some warnings

  • Constant expression contains invalid operations
  • Cannot call abstract static method UnitEnum::cases().

Could be a problem with the PHP version, but do you have any knowledge of this issue?

[DerManoMann]

This looks quite good and the fact that there is a test makes it even better :)

The only thing I'd prefer done differently is the use of the Util class.
I've been trying to move away from using it and/or adding to it. There is a lot of logic spread across a lot of files and in additrion I would suspect that this doesn't work for annotations?

As an alternative the convertEnum code should be added to the ExpandEnums processor which would cover all cases in a single place. Looks like $enum is coming from either Schema or ServerVariable, so adding some new processing loop like this should deal with all annotations/attributes:

            /** @var AnnotationSchema[] $schemas */
            $schemas = $analysis->getAnnotationsOfType([AnnotationSchema::class, AttributeSchema::class, ServerVariable::class]);

            foreach ($schemas as $schema) {
            }
        }

[k2tzumi]

Thank you.

There is a lot of logic spread across a lot of files and in additrion I would suspect that this doesn't work for annotations?

Yes.
We wanted to use Attrube only, since Enum itself is supported by PHP 8.1 and above.

But it would be nice if it could be used for annotations as well.
Since annotations and attributes are treated as similar constructs in the source, it was necessary to review some of the annotation sources as well.

As an alternative the convertEnum code should be added to the ExpandEnums processor which would cover all cases in a single place. Looks like $enum is coming from either Schema or ServerVariable, so adding some new processing loop like this should deal with all annotations/attributes:

I am very happy to learn that there is an expansion point!

I understood the source line to be as follows
https://github.com/zircote/swagger-php/blob/master/src/Processors/ExpandEnums.php#L29-L34

I haven't caught up with understanding the source yet, but is it like rewriting the context enum value in the above loop?

I have one question.
Will the context enum property feel like only strings are allowed and not the original array coming across?

https://github.com/zircote/swagger-php/blob/master/src/Context.php#L28

[DerManoMann]

I understood the source line to be as follows https://github.com/zircote/swagger-php/blob/master/src/Processors/ExpandEnums.php#L29-L34

I haven't caught up with understanding the source yet, but is it like rewriting the context enum value in the above loop?

Yes, you'd just need a separate loop in order to catch also ServerVariable which does not extend from Schema.
It might be an idea to break down the ExpandEnums code into separate methods - one for the current loop and a second one for the loop to be added.

I have one question. Will the context enum property feel like only strings are allowed and not the original array coming across?

https://github.com/zircote/swagger-php/blob/master/src/Context.php#L28

The context enum is different. That will contain the enum class name in case the parsed file contains an enum, just like the class property contains the class name.
See: https://github.com/zircote/swagger-php/blob/master/src/Analysers/ReflectionAnalyser.php#L86

[DerManoMann]

Could you please use use statements and import those enums rather than using FQDN?

[k2tzumi]

I have been following the implementation and feel that it can be handled with the following description.

#[Schema(description: "backend status", example: StatusEnumBacked::DRAFT)]
enum StatusEnumBacked: int
{
    case DRAFT = 1;
    case PUBLISHED = 2;
    case ARCHIVED = 3;
}

#[Property('status', refs: '#/components/schemas/StatusEnumBacked')]
public StatusEnumBacked status;

Is it universal usage to refer to it by ref as in this code?

The only problem with the way I wrote this is that I could not overwrite the nullable value.

#[Property('status', refs: '#/components/schemas/StatusEnumBacked', nullable: true)]
public ?StatusEnumBacked status;

Would it be preferable to eliminate this problem?
If we were to have it override by the referenced property, would it be difficult to handle because of the large impact?

[k2tzumi]

The only problem with the way I wrote this is that I could not overwrite the nullable value.

There was a related Issue.
#918

Is there any chance that the following PR will be merge?

#921
#1039

It would be helpful to know if there are any factors that are blocking you.

[DerManoMann]

The only problem with the way I wrote this is that I could not overwrite the nullable value.

Yes, that is an known issue that is not easy to solve.

Is there any chance that the following PR will be merge?

#921 #1039

Not as they are. The 3.0.0 spec does not allow any other keys next to $ref and 3.1.0 only summary and description.
Setting the OpenApi version to 3.1.0 should deal with nullable but I suspect that doesn't work in combination with ref.
I suggest opening a new issue for that.

It would be helpful to know if there are any factors that are blocking you.

Time and money :)

Still waiting for the updates to the PR as requested.

[DerManoMann]

On the topic of using ref for an enum - that is not neccesary as they enum typehint is enough.

<?php

use OpenApi\Attributes as OAT;

#[OAT\Schema(description: "backend status", example: StatusEnumBacked::DRAFT)]
enum StatusEnumBacked: int
{
    case DRAFT = 1;
    case PUBLISHED = 2;
    case ARCHIVED = 3;
}

#[OAT\Schema]
class Model
{
    #[OAT\Property(nullable: true)]
    public ?StatusEnumBacked $status;
}

will generate this YAML:

openapi: 3.0.0
components:
  schemas:
    StatusEnumBacked:
      description: 'backend status'
      type: integer
      enum:
        - DRAFT
        - PUBLISHED
        - ARCHIVED
      example: 1
    Model:
      properties:
        status:
          $ref: '#/components/schemas/StatusEnumBacked'
      type: object

[k2tzumi]

Not as they are. The 3.0.0 spec does not allow any other keys next to $ref and 3.1.0 only summary and description.
Setting the OpenApi version to 3.1.0 should deal with nullable but I suspect that doesn't work in combination with ref.
I suggest opening a new issue for that.

I understood that the OpenAPI specification makes it difficult to achieve this in the first place.

Shall we resume consideration when we have more time and money.. 😅

Still waiting for the updates to the PR as requested.

I understand that this modification will be an effective way to define nullable enums, so I would like to proceed.
Please give me some more time.

[k2tzumi]

It might be an idea to break down the ExpandEnums code into separate methods - one for the current loop and a second one for the loop to be added.

Should I separate the Processor class( ExpandEnums ) itself?

With a class name like ExpandEnumClasssString

I am a little unsure if it will work with the Processor dependencies, but I am thinking of running ExpandEnumClasssString after ExpandEnums.

[DerManoMann]

It might be an idea to break down the ExpandEnums code into separate methods - one for the current loop and a second one for the loop to be added.

Should I separate the Processor class( ExpandEnums ) itself?

With a class name like ExpandEnumClasssString

I am a little unsure if it will work with the Processor dependencies, but I am thinking of running ExpandEnumClasssString after ExpandEnums.

Nah, just leave it all in the same processor, just split into two separate methods.

[k2tzumi]

I took your advice and modified ExpandEnums.

820b159

I haven't been able to implement the test, but is this what it looks like?

[k2tzumi]

Tests have been implemented.
e480ebe

[DerManoMann]

you can run composer cs to make php-cs-fixer fix the code style for you.. You'll need 8.1 for that though + latest deps

[DerManoMann]

Some nitpicking but good enough to merge :)

[DerManoMann]

Thanks @k2tzumi