diff --git a/UPGRADE.md b/UPGRADE.md index 6291304..6610a34 100644 --- a/UPGRADE.md +++ b/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, ++ 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 diff --git a/lib/Generator/TypeMapper.php b/lib/Generator/TypeMapper.php index 8685d18..edfe1fb 100644 --- a/lib/Generator/TypeMapper.php +++ b/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 $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 === []) { diff --git a/test/Generator/TypeMapperTest.php b/test/Generator/TypeMapperTest.php index a75f834..2a331f9 100644 --- a/test/Generator/TypeMapperTest.php +++ b/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'], 'list with empty items' => [['type' => 'array', 'items' => []], 'array'], + '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', + ], + // 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', @@ -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([ diff --git a/test/Generator/TypesAccessPatternTest.php b/test/Generator/TypesAccessPatternTest.php index 72677dd..bf8b38f 100644 --- a/test/Generator/TypesAccessPatternTest.php +++ b/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(); diff --git a/test/fixtures/blocks/full/assertions.php b/test/fixtures/blocks/full/assertions.php index a94996d..283e2cf 100644 --- a/test/fixtures/blocks/full/assertions.php +++ b/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() ?? []); diff --git a/test/fixtures/blocks/full/expected/BlockCoverage.php b/test/fixtures/blocks/full/expected/BlockCoverage.php index 659a631..8e4d89e 100644 --- a/test/fixtures/blocks/full/expected/BlockCoverage.php +++ b/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, diff --git a/test/fixtures/blocks/full/openapi.json b/test/fixtures/blocks/full/openapi.json index 98e0080..48eb12b 100644 --- a/test/fixtures/blocks/full/openapi.json +++ b/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", diff --git a/test/fixtures/blocks/hero/assertions.php b/test/fixtures/blocks/hero/assertions.php index 79721b2..cd27383 100644 --- a/test/fixtures/blocks/hero/assertions.php +++ b/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() ?? []); diff --git a/test/fixtures/types/site/assertions.php b/test/fixtures/types/site/assertions.php index c07ae81..ecd1418 100644 --- a/test/fixtures/types/site/assertions.php +++ b/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 */ function articleTopicSlugs(EntityArticle $entity): array { diff --git a/test/fixtures/types/site/expected/Entities/EntityArticle.php b/test/fixtures/types/site/expected/Entities/EntityArticle.php index 674ae30..7044a03 100644 --- a/test/fixtures/types/site/expected/Entities/EntityArticle.php +++ b/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{ diff --git a/test/fixtures/types/site/openapi.json b/test/fixtures/types/site/openapi.json index 531c2ec..3d94c49 100644 --- a/test/fixtures/types/site/openapi.json +++ b/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": {