Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simple use case for enum #20

Closed
AlexanderAllen opened this issue Apr 19, 2024 · 6 comments · Fixed by #42
Closed

Simple use case for enum #20

AlexanderAllen opened this issue Apr 19, 2024 · 6 comments · Fixed by #42
Assignees
Labels
unit test Unit test needs to be created or updated

Comments

@AlexanderAllen
Copy link
Owner

I think that enum is a pretty popular feature within the OpenAPI spec, and as such an initial release should support it, at least in the most basic form possible.

At the very least scalar types should be supported in the enum, and if it's not unreasonable on time also nullable enums.

@AlexanderAllen AlexanderAllen added the unit test Unit test needs to be created or updated label Apr 19, 2024
@AlexanderAllen AlexanderAllen added this to the 1.0.0-alpha milestone Apr 19, 2024
@AlexanderAllen AlexanderAllen self-assigned this Apr 19, 2024
AlexanderAllen added a commit that referenced this issue Apr 28, 2024
* Keyword `not` completed in #5

* Capture additional todos and requirements in #20, #21.

* Captured comment in #21.

* Note todo for test case allOf, issue #19.

* Make TODOs for #15, #22, #23 in the code.

* Tag #15 re: anyOf.

* Issue #18: Initial boilerplate for file printer.

* Update PanDeAgua boilerplate.

* Promote all MediaNoche code except typematcher function.

* Set Setup as a shared fixture in SRC :/

Sadly, can't seem to be able to leave this fixture under tests.
PHPUnit does not see the namespaced class import, probably due to PHPUnit autoloading.

* OMG: Wrong namespace !

I have been using \Test instead of \Tests all this time?
composer.json autload is the source of thruth here.

* Move all schemas from fixture to schema dir.

* Demote Setup from src to tests where it belongs.

* Make test namespace singular in composer autoload.

* Make test directory consistent with namespace.

* Git ignore only current schema path, but not sub paths.

* Changed tests directory to singular test.

* Update test path for PHPStan.

* Initial print file implementation.

* Disable namespace resolution and specify output directory.

* Promote file printer to class.

* Initial pan de agua comments.

* Assert output file contains source class.

* Swap out default printer for PSR printer.

* Let the user decide both on nullable and readonly attributes.

* Only set default value if present in source schema.

* Address illegal characters for PHP symbols.

* Cleanup some namespaces.

* Update test paths on media noche.

* Change test target to public file.

* Update test path on Github Actions.

* Create tmp output dir on github pipeline.

* Create tmp output dir on github pipeline.
@AlexanderAllen AlexanderAllen removed this from the 1.0.0-alpha milestone May 1, 2024
@AlexanderAllen
Copy link
Owner Author

Removed from alpha, might be slightly bigger change that I want right now.
Will plan to ship for beta or RC.

@AlexanderAllen
Copy link
Owner Author

AlexanderAllen commented May 3, 2024

Some inspiration pasta while I work on this:

From @handrews comment at issue 1900 (Apr, 2021):

nullable: True in the presence of a type field, means that a JSON null is valid in addition to whatever type (plus constraints from other keywords including enum) the schema allows That's it. That's all OAS says.

So if type: string and nullable: true, then the PHP representation is ?string (a nullable string).

Expanding on that:

nullable: true
type: string
default: null

Would equal ?string foo = null, a nullable string with a default value of null. It's still a slightly different meaning when compared to nullable enums, but I think this captures the general vibes.

But maybe not so quick!

enum and default imply different effects in a OAS context.

And as it concerns to OAS you really need both nullable: true and null in the enum value:

type: string
# Representing a TYPE union of string and null, or nullable type.
nullable: true
# Also placing a constraint on the values allowed in the string, which includes a VALUE of null.
enum:
  - arroz
  - habichuelas
  # Sin carne, pfft?
  - null
# The default value of the type is "arroz"
default: arroz

Which in plain-English OAS would mean a string that allows a null along a list of values, with a default value of arroz?

Q: Cool beans. But do PHP enums allow nulls?

A: No (RFC). Only string and int are allowed in backed enums. A PHP enum case can be named "null", but that's not a literal null value, although it could be interpreted as such.

Illegal:
image

Technically legal, but against the spirit of the law (objection!) :
image

I'm fine with interpreting the type as a nullable something, the value however when it comes to enums is still sticky.

Without further internal or external interpretation of the enum (with enum method, or external match, for example), I don't see a way of interpolating the null value in a OAS enum into a PHP enum.

Interpreting that null enum value just feels like a philosophical can of worms, and not one that I'm keen to pop.

@AlexanderAllen
Copy link
Owner Author

The folks at myclabs/php-enum had some thoughts about this a long time ago, way before PHP enums were released...

@AlexanderAllen
Copy link
Owner Author

AlexanderAllen commented May 3, 2024

I just noted this in test/schema/keyword-enum-simple.yml:

enums cases are super interesting in that they are both a property
of their parent object, but they can also be interpreted as a
object by themselves. However unlike object references, enums lack
their own components/schemas entry.

And that is making for a super interesting/fun problem to entertain in the code (sarcasm: false).

With "regular" object schemas a property generator only needs to concern itself with chugging out property types, confident that referenced "class types (objects)" will eventually find themselves in the pipeline and be generated as well. Assuming referenced objects were declared in component/schemas, and with OAS enforcing references.

Enums and inline objects (without their own component/schema entry) then are kinda homeless and need a side action to define a stand-alone type for them.

A property generator is a good place to detect their existence, since you are checking each schema's properties. But a property generator returns properties, not object-like structures, and that's where this gets interesting.

For example, see this enum get intercepted by a property generator/enumerator/parser:

image

The generator is flawless in it's ability to generate both the stand-alone enumType instance and all the rest of the PanettoneEnum properties, which include the enum property itself!

However a prospective caller to a property generator expects only properties in returns, what's up with that random enum object hanging in the response?!

@AlexanderAllen AlexanderAllen linked a pull request May 5, 2024 that will close this issue
@AlexanderAllen
Copy link
Owner Author

Final resolution for the meme solo debate up top:

  • I am not implementing null value case support for PHP enums in any way, shape, or form. At least not until a genius comes flying down from the Internets and suggest a solid way of doing so.
  • Enum properties still have nullable type ?nullableEnumType $enumProp = null support. Meaning printed classes can still assign null to their enum properties. It's just the enum types themselves that don't have a null case.

@handrews
Copy link

handrews commented May 5, 2024

FWIW, while I don't know PHP well at all, ?nullableEnumType as a way to model a JSON Schema "enum" that includes null sounds correct to me. The need to include null in JSON Schema "enum" lists is just an artifact of JSON Schema treating the "null" type like any other type, just one with a single value (null). This was muddled a bit by OpenAPI <3.1's "nullable" keyword breaking that model, which is otherwise consistent with regular JSON Schema's use of "null" as a value for "type". Languages with an alternate way of handling nulls don't need to translate JSON Schema's "enum" literally, whether null values are modeled with "nullable": true (OAS <3.1) or "type": "null" (OAS 3.1 / standard JSON Schema)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unit test Unit test needs to be created or updated
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants