fix(php): support non-nullable unions#2942
Merged
Merged
Conversation
PHP generation failed with "union are not supported" for any union that
wasn't just nullable — a heterogeneous array like [1, "two", {...}] made
the whole render throw.
Unions are now rendered inline as PHP 8.0 union type declarations
(e.g. `MixedClass|int|string`), so no named union class is emitted. The
from/to/validate converters dispatch on the runtime type of the value
with an if/elseif chain, testing the most specific checks first
(instanceof for classes, enums and DateTime, then is_int before the
float check, which also accepts integers) and throwing on unmatched
values. Samples use the first member. On the JSON side classes and
maps are stdClass and enums and dates are strings, with textual
duplicates deduplicated in the declaration.
This also adds the PHP reserved words to the renderer's forbidden
names: the motivating fixture has a property named "mixed", which used
to name its class `Mixed` — a compile error in PHP 8 (reserved type
name), now renamed to `MixedClass`. Previously no reserved word was
protected at all, so a JSON key like "class" or "print" with an object
value generated uncompilable PHP.
Generated output for the fixtures parses cleanly as PHP 8.0.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Running the union-heavy JSON inputs and the JSON Schema fixture against the PHP renderer for the first time exposed several bugs in generated code: - `any`- and `null`-typed properties were declared as `Object` (and the converter parameters as the nonexistent types `any`/`null`), which fatals as soon as a null or scalar flows through. They are now `mixed`, matching the PHP 8.0 baseline the inline union declarations already require. - The `any` validation called `defined()`, which tests whether a *constant* of the given name exists and throws for non-string arguments. Any value is a valid `any`, so no check is emitted. - Double validation used a bare `is_float`, rejecting whole numbers, which json_decode hands over as ints. It now accepts ints, like the union member checks already did. - Nullable DateTime/UUID properties lost their `?` in type declarations (`optionalize` was not applied), so `fromX` fataled returning null. - The nullable branch of the `from` converters emitted a hardcoded `return null;`, which is wrong when the conversion is nested inside a map's foreach; it now uses the caller's lhs like the `to` side does. - Nested array samples closed with `);` in expression position, and map samples emitted statements in expression position; array closings now respect the caller's suffix and map samples are immediately-invoked closures. - Classes without properties generated a bodyless non-void `validate(): bool`, a TypeError when called; it now returns true. - A property named "this" produced an illegal `$this` constructor parameter; "this" is now a forbidden property name. - A union-member check for an enum on the JSON side only tested `is_string`, shadowing plain-string members (e.g. a UUID sharing a union with an enum); it now checks membership in the enum's values. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The PHP fixture previously ran only a whitelist of easy JSON samples and no schema fixture at all, so the new union support was covered by unit tests alone. quicktype's primary testing method is the end-to-end JSON / JSON Schema fixture tests, so: - Enable the union-heavy JSON inputs for PHP: unions.json, union-constructor-clash.json, combinations1-4.json, nst-test-suite.json, kitchen-sink.json, list.json and bug427.json. - Add test/inputs/json/priority/php-mixed-union.json, the motivating repro: a heterogeneous array (int, string, bool, null and an object) under a property named "mixed", which is a PHP reserved word, so it exercises the reserved-word class renaming and the union runtime dispatch together. The input is skipped for the languages that already skip unions.json (Ruby, Scala 3, Smithy4s, Kotlin and Kotlin Jackson), since it is a subset of that input's shapes. - Register the JSON Schema fixture for PHP and run it in CI as schema-php. 61 of the 65 schemas run; the four skips are keyword-unions.schema (PHP class names are case-insensitive but the namer dedups case-sensitively — the same reason Java and Python skip it), recursive-union-flattening.schema (a top-level union produces no named TopLevel class now that unions are inlined), top-level-enum.schema (generated top-level enum code is incompatible with the driver, as for C# and Ruby) and union.schema (the driver does not support top-level arrays). Not enabled: optional-union.json and the identifiers/keywords inputs. Top-level arrays are unsupported by the PHP driver, identifiers.json needs comment/string escaping of raw JSON names, and keywords.json hits the case-insensitive class-name collision above — all pre-existing limitations independent of union support. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Any change affecting generated output must be covered by the JSON / JSON Schema fixture tests; unit tests complement them but are never a substitute. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PHP generation failed with
Error: union are not supported.for any union that isn't just nullable — a heterogeneous array like"mixed": [1, "two", true, null, {"nested": "object"}]made the whole render throw. Nullable unions (?string) already worked; everything else hit seven unimplementedmatchTypecases.What this does
private MixedClass|int|string $v;, its converter parameter asstdClass|int|string(the decoded-JSON side, where classes/maps arestdClassand enums/dates are strings, deduplicated).unionNeedsNameis nowfalse— no union class is ever emitted.from/to/validateconverters: anif/elseifchain over the members, most specific check first (instanceoffor classes/enums/DateTime, enum-value membership before the plain-string check,is_intbefore the float check — which also accepts integers, since PHP ints pass wherever floats do), throwing on unmatched values.sampleuses the first member;anymembers act as the catch-all branch.mixed, whose class used to be emitted asclass Mixed— a fatal error in PHP 8 (reserved type name); it's nowMixedClass. Previously the renderer protected no reserved word at all, so keys likeclassorprintwith object values generated uncompilable PHP. A property namedthisno longer produces an illegal$thisconstructor parameter.any/nulltyped properties aremixedinstead of the fatalObject/any/nulldeclarations, the bogusdefined()"validation" foranyis gone, double validation accepts ints (whatjson_decodeyields for whole numbers), nullableDateTime/UUID declarations keep their?, nested array/mapsamplemethods are now syntactically valid expressions, empty classes'validate(): boolreturnstrueinstead of nothing, and the nullablefromconverter no longer emits a strayreturn null;inside map loops.Testing
quicktype's primary testing method is end-to-end JSON / JSON Schema fixture tests, and this PR is now covered by them (the principle is written down in a new
## Testingsection inCLAUDE.md):test/languages.tsgrows from 7 easy samples to 18, adding the union-heavy inputsunions.json,union-constructor-clash.json,combinations1–4.json,nst-test-suite.json,kitchen-sink.json,list.jsonandbug427.json, plus a newtest/inputs/json/priority/php-mixed-union.json— the motivating repro ("mixed": [1, "two", true, null, {"nested": "object"}]), which exercises the reserved-word class renaming and union dispatch together. The new input is skipped for the five languages that already skipunions.json(Ruby, Scala 3, Smithy4s, Kotlin, Kotlin Jackson).schema-phpis now registered intest/fixtures.tsand runs in CI. 61 of 65 schemas pass; the four skips (with reasons intest/languages.ts) arekeyword-unions.schema(PHP class names are case-insensitive, the namer dedups case-sensitively — Java and Python skip it for the same reason),recursive-union-flattening.schema(a top-level union has no namedTopLevelclass now that unions are inline),top-level-enum.schema(driver-incompatible, as for C#/Ruby) andunion.schema(the driver doesn't support top-level arrays).optional-union.json(top-level array),identifiers.json(raw JSON names need comment/string escaping) andkeywords.json(the case-insensitive class-name collision above).test/unit/php-unions.test.tsstay as a complement: they assert things fixtures can't (e.g. that no union class is emitted) and run without a PHP toolchain.Both fixtures pass locally end-to-end under a PHP 8.4 CLI (
FIXTURE=php script/test: 18/18,FIXTURE=schema-php script/test: 61 schemas), andnpm run test:unitpasses.🤖 Generated with Claude Code