fix(generator): keep the literals of a nullable enum, type empty lists

UPGRADE.md

@@ -1,5 +1,31 @@
 # Upgrade
 
+## From 3.6 to 3.7
+
+### Generated types: select fields keep their values
+
+A select field's `value` is typed with the values an editor can pick again. The API lists `null`
+among a nullable select's allowed values, and the generator used to drop the whole list because of
+it, falling back to `string`. Its `options`, an empty list instead of an object when nothing is
+selected, is now typed as exactly that:
+
+```diff
+  size: object{
+-     value: string|null,
+-     options: \stdClass|array<int, mixed>,
++     value: 'small'|'medium'|'large'|null,
++     options: \stdClass|array{},
+      label: string|null
+  }|null,
+```
+
+A `match` over `value` can be exhaustive now, and PHPStan reports a comparison against a value the
+field does not have.
+
+This changes the generated file of every block, entity and container that has a select field, so
+`--check` reports them as out of date until you regenerate once. That includes the deprecated
+`flyo-generate-blocks`: keeping its output byte-identical would have meant keeping the bug.
+
 ## From 3.5 to 3.6
 
 Nothing in `lib/Api` or `lib/Model` changed and no runtime behaviour changed. The block generator

lib/Generator/TypeMapper.php

@@ -220,6 +220,10 @@ private function intType(array $schema): string
     /**
      * An `enum` rendered as a union of literal strings, or null when that is not safe.
      *
+     * A `null` member is how OpenAPI 3.0 lets a nullable enum actually hold null (`nullable: true`
+     * alone does not), as a select field's `value` does. It becomes a trailing `null` rather than
+     * disqualifying the whole enum.
+     *
      * @param array<string, mixed> $schema
      */
     private function literalUnion(array $schema): ?string
@@ -230,7 +234,13 @@ private function literalUnion(array $schema): ?string
         }
 
         $literals = [];
+        $hasNull = false;
         foreach ($enum as $value) {
+            if ($value === null) {
+                $hasNull = true;
+                continue;
+            }
+
             // A block id may be a number even though the property is typed string.
             if (!is_scalar($value)) {
                 return null;
@@ -249,6 +259,10 @@ private function literalUnion(array $schema): ?string
             }
         }
 
+        if ($hasNull) {
+            $literals[] = 'null';
+        }
+
         return implode('|', $literals);
     }
 
@@ -316,6 +330,12 @@ private function objectType(
      */
     private function arrayType(array $schema, array $path, int $depth, array $refs): string
     {
+        // A list that can never hold anything: a select field's `options` is an empty list instead
+        // of an object when nothing is selected.
+        if (($schema['maxItems'] ?? null) === 0) {
+            return 'array{}';
+        }
+
         $items = $schema['items'] ?? null;
 
         if (!is_array($items) || $items === []) {

test/Generator/TypeMapperTest.php

@@ -49,6 +49,20 @@ public static function types(): array
                 ['type' => 'string', 'enum' => ["a\nb"]],
                 'string',
             ],
+            // OpenAPI 3.0 needs `null` listed for a nullable enum to accept it.
+            'nullable enum keeps its literals' => [
+                ['type' => 'string', 'nullable' => true, 'enum' => ['red', 'green', null]],
+                "'red'|'green'|null",
+            ],
+            'null member is listed last' => [
+                ['type' => 'string', 'enum' => [null, 'red', 'green']],
+                "'red'|'green'|null",
+            ],
+            'enum of only null' => [['type' => 'string', 'enum' => [null]], 'null'],
+            'enum with an object member falls back to string' => [
+                ['type' => 'string', 'enum' => ['red', ['nested' => true]]],
+                'string',
+            ],
             'object with properties' => [
                 ['type' => 'object', 'properties' => ['a' => ['type' => 'string']]],
                 'object{a: string|null}',
@@ -90,6 +104,24 @@ public static function types(): array
             ],
             'list without items' => [['type' => 'array'], 'array<int, mixed>'],
             'list with empty items' => [['type' => 'array', 'items' => []], 'array<int, mixed>'],
+            'list that is always empty' => [
+                ['type' => 'array', 'maxItems' => 0, 'items' => []],
+                'array{}',
+            ],
+            'list with a positive maxItems is still a list' => [
+                ['type' => 'array', 'maxItems' => 3, 'items' => ['type' => 'string']],
+                'array<int, string>',
+            ],
+            // A select field's `options`: a map of value => label, or `[]` when nothing is selected.
+            'map or empty list' => [
+                [
+                    'oneOf' => [
+                        ['type' => 'object', 'additionalProperties' => ['type' => 'string']],
+                        ['type' => 'array', 'maxItems' => 0, 'items' => []],
+                    ],
+                ],
+                '\stdClass|array{}',
+            ],
             'list of generic blocks' => [
                 ['type' => 'array', 'items' => ['$ref' => '#/components/schemas/block']],
                 'array<int, \Flyo\Model\Block>',
@@ -192,6 +224,18 @@ public function testNullIsNotAppendedTwice(): void
         );
     }
 
+    public function testANullableEnumIsNotSuffixedWithNullTwice(): void
+    {
+        $this->assertSame(
+            "'red'|'green'|null",
+            self::mapper()->propertyType(
+                ['type' => 'string', 'nullable' => true, 'enum' => ['red', 'green', null]],
+                ['a'],
+                false,
+            )
+        );
+    }
+
     public function testRequiredIsHonouredInsideAShape(): void
     {
         $type = self::mapper()->type([

test/Generator/TypesAccessPatternTest.php

@@ -55,10 +55,27 @@ public function testTheModelIsAPlainObjectAllTheWayDown(): void
         $this->assertInstanceOf(\stdClass::class, $model->image);
         $this->assertSame(154311, $model->image->id);
         $this->assertSame('news', $model->category->value);
+        $this->assertInstanceOf(\stdClass::class, $model->category->options);
         $this->assertSame('first-topic', $model->tags->topics[0]->slug);
         $this->assertSame('evt1', $model->events[0]->unique_id);
     }
 
+    /**
+     * With nothing selected, an option's `options` decodes to an empty array instead of an
+     * object: the `array{}` half of its `\stdClass|array{}` type.
+     */
+    public function testAnUnselectedOptionHasAnEmptyListOfOptions(): void
+    {
+        $payload = self::entityPayload();
+        $payload->model->category = self::decode('{ "value": null, "options": [], "label": null }');
+
+        $model = ObjectSerializer::deserialize($payload, self::ENTITY, [])->getModel();
+
+        $this->assertNull($model->category->value);
+        $this->assertSame([], $model->category->options);
+        $this->assertNull($model->category->label);
+    }
+
     public function testTheRestOfTheEntityKeepsTheSdkModels(): void
     {
         $entity = self::entity();

test/fixtures/blocks/full/assertions.php

@@ -76,6 +76,24 @@ function enumIsAnActualUnion(BlockCoverage $block): bool
     return $block->getContent()->multi_enum === 'wide';
 }
 
+/** A `null` member does not cost the enum its literals, so a match over it can be exhaustive. */
+function nullableEnum(BlockCoverage $block): string
+{
+    return match ($block->getContent()->nullable_enum) {
+        'red' => 'Red',
+        'green' => 'Green',
+        null => '',
+    };
+}
+
+/** `maxItems: 0` is a list that can only ever be empty, so its count is known to be zero. */
+function emptyList(BlockCoverage $block): string
+{
+    return match (count($block->getContent()->empty_list ?? [])) {
+        0 => 'empty',
+    };
+}
+
 function deeplyNested(BlockCoverage $block): ?string
 {
     return $block->getContent()->nested_object?->deep?->deeper;
@@ -189,7 +207,7 @@ function itemTypeId(BlockCoverage $block): bool|int|null
 
 function slots(BlockCoverage $block): int
 {
-    $slots = $block->getSlots() ?? [];
+    $slots = $block->getSlots();
     $slot = $slots['main'] ?? null;
 
     return $slot === null ? 0 : count($slot->getContent() ?? []);

test/fixtures/blocks/full/expected/BlockCoverage.php

@@ -42,6 +42,8 @@ class BlockCoverage extends \Flyo\Model\Block
      *     multi_enum: 'wide'|'narrow'|'full'|null,
      *     enum_without_type: 'a'|'b'|null,
      *     enum_with_quote: 'it\'s'|'plain'|null,
+     *     nullable_enum: 'red'|'green'|null,
+     *     empty_list: array{}|null,
      *     nested_object: object{deep: object{deeper: string|null}|null}|null,
      *     open_map: \stdClass|null,
      *     string_map: \stdClass|null,
@@ -82,6 +84,8 @@ public function getContent()
          *     multi_enum: 'wide'|'narrow'|'full'|null,
          *     enum_without_type: 'a'|'b'|null,
          *     enum_with_quote: 'it\'s'|'plain'|null,
+         *     nullable_enum: 'red'|'green'|null,
+         *     empty_list: array{}|null,
          *     nested_object: object{deep: object{deeper: string|null}|null}|null,
          *     open_map: \stdClass|null,
          *     string_map: \stdClass|null,

test/fixtures/blocks/full/openapi.json

@@ -72,6 +72,8 @@
               "multi_enum": { "type": "string", "enum": ["wide", "narrow", "full"] },
               "enum_without_type": { "enum": ["a", "b"] },
               "enum_with_quote": { "type": "string", "enum": ["it's", "plain"] },
+              "nullable_enum": { "type": "string", "nullable": true, "enum": ["red", "green", null] },
+              "empty_list": { "type": "array", "maxItems": 0, "items": {} },
               "nested_object": {
                 "title": "Nested",
                 "type": "object",

test/fixtures/blocks/hero/assertions.php

@@ -97,7 +97,7 @@ function inheritedGetters(BlockHero $block): string
 /** Slots are already real models on the parent, so this is the parent's exact type. */
 function slotChildren(BlockHero $block): int
 {
-    $slots = $block->getSlots() ?? [];
+    $slots = $block->getSlots();
     $slot = $slots['main'] ?? null;
 
     return $slot === null ? 0 : count($slot->getContent() ?? []);

test/fixtures/types/site/assertions.php

@@ -80,6 +80,23 @@ function articleCategory(EntityArticle $entity): string
     };
 }
 
+/**
+ * An option's `options` is a map of value => label, or an empty list when nothing is selected, so
+ * as a list it is known to hold nothing.
+ */
+function articleCategoryHasOptions(EntityArticle $entity): bool
+{
+    $options = $entity->getModel()?->category?->options;
+
+    if (is_array($options)) {
+        return match (count($options)) {
+            0 => false,
+        };
+    }
+
+    return $options !== null;
+}
+
 /** @return list<string> */
 function articleTopicSlugs(EntityArticle $entity): array
 {

test/fixtures/types/site/expected/Entities/EntityArticle.php

@@ -28,7 +28,7 @@ class EntityArticle extends \Flyo\Model\Entity
      *     content: object{html: string|null, json: \stdClass|null}|null,
      *     category: object{
      *         value: 'news'|'report'|null,
-     *         options: \stdClass|null,
+     *         options: \stdClass|array{},
      *         label: string|null
      *     }|null,
      *     tags: object{
@@ -51,7 +51,7 @@ public function getModel()
          *     content: object{html: string|null, json: \stdClass|null}|null,
          *     category: object{
          *         value: 'news'|'report'|null,
-         *         options: \stdClass|null,
+         *         options: \stdClass|array{},
          *         label: string|null
          *     }|null,
          *     tags: object{

test/fixtures/types/site/openapi.json

@@ -120,10 +120,16 @@
           },
           "category": {
             "type": "object",
+            "required": ["value", "options", "label"],
             "properties": {
-              "value": { "enum": ["news", "report"], "type": "string" },
-              "options": { "type": "object" },
-              "label": { "type": "string" }
+              "value": { "enum": ["news", "report", null], "type": "string", "nullable": true },
+              "options": {
+                "oneOf": [
+                  { "type": "object", "additionalProperties": { "type": "string" } },
+                  { "maxItems": 0, "type": "array", "items": {} }
+                ]
+              },
+              "label": { "type": "string", "nullable": true }
             }
           },
           "tags": {