Skip to content

fix: remaining code generation bugs found by regenerating 42 clients (11.4.0) - #163

Merged
tommey merged 25 commits into
masterfrom
feature/fix-remaining-generation-bugs
Sep 28, 2026
Merged

tommey merged 25 commits into
masterfrom
feature/fix-remaining-generation-bugs

Conversation

@tommey

@tommey tommey commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Regenerating 42 real-world clients (every CLIENT_PHP_VERSION they use: 7.2, 7.4, 8.1, 8.3), and running them on PHP 8.5, surfaced 17 more generator bugs after 11.3.0. This PR fixes all of them. Every fix is its own commit on one of four topic branches, merged here, so you can review them commit by commit.

The rule throughout: output stays byte-identical for every input that already worked. Only output that was invalid, threw, or behaved wrongly changes. Every intentional change to working output is listed under Changed in the CHANGELOG.

Generation failures

Bug Before After
Nullable optional path/query/header parameter generator aborts ("Nullable optional parameter is not supported") supported; null = unset
Request body media type without schema generator crashes with a TypeError request sends that Content-Type with no body
Schema named Match/List/Class… class Match doesn't parse on PHP 8 (even for 7.x targets) MatchSchema + warning; enum value class → V_CLASS
Literal body among several 2xx responses generator aborts supported

Runtime errors and deprecations in generated code

Bug Before After
Integer/number/boolean header psr7 ≥ 2.11 E_USER_DEPRECATED sent as strings (1/0 for booleans, like query/cookie)
Primitive / array-of-scalars request body never serializable new copied Request\LiteralRequestBody, unwrapped by the JSON serializer
nullable: true + allOf: [$ref] TypeError on null nullability kept
Optional nullable enum (8.1+) Enum::from(null) TypeError null
oneOf without discriminator, enum-only difference (8.1+) ValueError escapes caught like UnexpectedResponseBodyException
Unknown discriminator value PHP Error (dynamic mapper call) UnexpectedResponseBodyException
Array of arrays of objects left raw; docblock names a missing class mapped to collections
Unlisted 2xx with several 2xx responses (8.0+) UnhandledMatchError RuntimeException, as the pre-8.0 switch already threw
Empty body for a scalar response "Undefined array key" warning + TypeError UnexpectedResponseBodyException (null if nullable)
Inline enum response body (8.1+) return type names a class never generated enum generated
Optional untyped (mixed) property (8.0+) "must not be accessed before initialization" = null; no more ?mixed
Symbol-only enum values (*, =) silently dropped (enum could end up empty) ASTERISK, EQUALS, …
Different schemas deriving one class name silently merged into one class distinct …2 classes + warning; identical ones still share
Array-of-enum response items (8.1+) dead …EnumMapper with empty toSchema() not generated

Verification

  • make test (PHP 8.2 + wiremock, as CI runs it): OK, 614 tests, 1578 assertions, up from 461. Every fix has fixture and/or unit tests, and the new tests fail on 11.3.0.
  • php-cs-fixer dry run: 0 files. PHPStan: no errors.
  • End to end: all 42 clients × (current target + 8.3) = 83 regenerations succeed. Three of them couldn't be generated at all by 11.3.0. 7,400 generated files lint clean on PHP 8.5.10. The fix branches also diffed the full corpus against 11.3.0 output: only the files each fix should touch change. They then exercised every affected client on PHP 8.5 with a runtime harness and targeted probes.
  • One commit syncs the committed example/ clients, whose loginUser literal response gains the empty-body guard, and one declares a new CodeBuilder parameter explicitly nullable.

CHANGELOG: 11.4.0. It's a minor, not a patch, because regenerated clients see the output changes listed under Changed.

Known, out of scope (not hit by any of the 42 specs): array-of-scalars and date-time responses; header parameters that are arrays of non-strings; ambiguity between oneOf alternatives on 7.x targets that differ only by enum value; a colliding definition that is newly added earlier in a spec can shift the numeric suffixes (the warning makes it visible).

🤖 Generated with Claude Code

tommey and others added 25 commits September 28, 2026 13:11
Operations with several 2xx responses are generated as a `match` on the
status code for PHP 8.0+ targets. The match had no default arm, so a 2xx
status code missing from the specification raised an UnhandledMatchError
(an engine Error) instead of an exception.

Add a default arm that throws the same RuntimeException the pre-8.0
switch branch already throws after the switch, so every target behaves
the same.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An optional path/query/header/cookie parameter whose schema (or referenced
schema) is `nullable: true` made RequestGenerator::generateSetters() throw
"Nullable optional parameter is not supported", so the whole generation
aborted. The setter is now generated like for any other optional parameter,
with a nullable type hint (docblock only on PHP 7.0) and null-guarded
constraint checks. An explicit null is treated like an unset parameter:
null values are already filtered out of the query, header and cookie
parameters, exactly as they are for required nullable parameters.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A 2xx response whose schema is a scalar is read from the
`__literalResponseValue` key that the JSON serializer adds. An empty body
unserializes to `[]`, so the generated client raised an "Undefined array
key" warning and then a TypeError from the non-nullable return type.

Treat a missing (or null) literal value like an empty object response:
throw UnexpectedResponseBodyException. PHP 8.0+ targets use a throw
expression (`$response[KEY] ?? throw new ...`), older targets guard with
an `if (!isset(...))` statement. Nullable literal responses return null
(`$response[KEY] ?? null`), which is what they returned before, without
the warning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The OAS 3.0 nullable reference idiom `{ nullable: true, allOf: [ $ref ] }`
lost its nullability in FieldFactory::mergeAllOfAttributes(): cebe gives
every typed schema `nullable: false` by default, so the referenced schema
overrode the outer `nullable: true`. The generated schema and mapper then
treated the property as non-nullable and a null in the response ended in
a TypeError (`<Child>Mapper::toSchema(): Argument #1 ($payload) must be of
type array, null given`).

An explicit `nullable: true` next to allOf is now kept; the merge of the
allOf members is unchanged otherwise.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A requestBody media type object without `schema` (for example
`application/x-www-form-urlencoded: {}`) crashed the generator with a
TypeError from SchemaNaming::getClassName(null) in RequestFactory, and
EXCLUDE_TAGS could not help because tags are filtered after parsing.

In OpenAPI the Media Type Object's `schema` is optional; without it the
content of that media type is simply not described. Such a media type now
still counts as an accepted request content type (it drives the
Content-Type header and the contentType constructor parameter as before),
but contributes no body field of its own:
- if it is the only media type, the request has no body field and sends an
  empty body with that Content-Type;
- next to media types with a schema, the body field comes from those, and
  the schemaless media type no longer trips the "Multiple schemas per
  request" check. That check used to pass or fail depending on the order
  of the media types.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
For PHP 8.1+ targets, an optional property with a nullable enum schema
(inline or $ref) was mapped with `<Enum>::from($payload['x'])` behind an
array_key_exists() guard, so an explicit null in the response ended in a
TypeError (`<Enum>::from(): Argument #1 ($value) must be of type string,
null given`). Wrap it in the same `!== null ? ... : null` ternary that the
required nullable enum, date and object branches already use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
For PHP 8.1+ targets a 2xx response whose schema is an inline enum (for
example a health check returning `type: string, enum: [OK]`) got an
`<Operation>ResponseBodyEnum` return type, but EnumGenerator never
generated that enum and the client returned the raw literal, so every
call ended in a TypeError.

Generate the enum for enum response bodies and convert the literal value
with `<Enum>::from()` (nullable responses map a missing or null value to
null). Targets without native enums keep returning the scalar.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Header parameters were put into getHeaders() with their native PHP type.
guzzlehttp/psr7 >= 2.11 raises E_USER_DEPRECATED for every non-string
header value ("guzzlehttp/psr7 3.0 requires string|string[]"), which shows
up on every request of e.g. the mailer clients (integer X-User-Id), and
nyholm/psr7 rejects boolean header values outright.

RequestGenerator::generateGetHeadersMethod() now casts integer and number
header values (including integer backed enums on PHP 8.1+) with (string),
and turns booleans into '1'/'0', which is how this generator already
serializes booleans in query and cookie parameters (http_build_query).
OpenAPI leaves the textual form of booleans implementation-defined.
Unset optional and null values stay null, so they are still filtered out.
String headers are generated exactly as before.

On the wire integer/number values and `true` are unchanged; `false` is now
sent as '0' instead of an empty header value.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
multiBodyAction() grouped the responses by getPhpClassName(), which
throws for scalar bodies, so an operation with for example a `200`
string response and a `204` aborted the generation with "Call of
getPhpClassName on the non-composite field". Enum bodies only worked on
PHP 8.1+ targets.

Group the responses by getPhpTypeHint() instead. It is the class name for
every body getPhpClassName() accepts, so the output for composite, date
and enum bodies is unchanged, and the scalar type for other literal
bodies, which now get the same handling as a single literal response.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without a discriminator, the mapper tries every oneOf/anyOf alternative in
a try/catch that only caught UnexpectedResponseBodyException. For PHP 8.1+
targets the alternatives map enums with `<Enum>::from()`, which throws a
ValueError for the value of another alternative (e.g. `type: [performer]`
vs `type: [member]`), so every valid payload of a later alternative failed.

Catch ValueError as well for the alternatives whose mapping goes through an
enum (directly or nested); the other catch blocks and the pre-8.1 output
are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A requestBody whose schema is a primitive (e.g. campaign addCampaignMember:
`application/json` integer) or an array of non-object items made getBody()
return the raw value. The body serializers only accept
SerializableInterface, so every call of such an operation failed with a
RequestValidationException wrapping a TypeError, and the getBody() return
type contradicted RequestInterface.

Such a "literal" body is now returned wrapped in the new static
Request\LiteralRequestBody, which exposes the value under
ContentTypeSerializerInterface::LITERAL_VALUE_KEY (DateTimeInterface values
are formatted as RFC 3339, like in schemas). The JSON serializers unwrap
that key again, the inverse of how decode() wraps a literal response, so
the body is sent as the JSON value itself (`123`, `"text"`, `[1,2]`).

LiteralRequestBody is copied into a client only when one of its operations
has a literal body. Object, array-of-object and untyped (mixed) bodies are
generated exactly as before. The copied AbstractJsonContentTypeSerializer
changes for every client; its behaviour is unchanged for all bodies other
than the wrapped literal ones. The committed example clients are synced.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…values

The default arm of the discriminator switch (and the whole dispatch for a
discriminator without mapping) built the setter and mapper names from the
raw value: `$this->{$value . 'Mapper'}` and `set . ucfirst($value)`. A value
outside the mapping, or the schema name itself (the implicit mapping per
OAS), ended in an "Undefined property" warning and an Error (call to an
undefined method, or a member function on null) instead of the mapper's
exception.

Generate explicit cases instead:
- the explicit mapping, as before (the alternative is now looked up by its
  schema name, which is what the mapping references end with);
- for every alternative the mapping does not cover, the implicit mapping
  (the schema name) plus the lcfirst class name that the old value based
  dispatch resolved, so payloads that mapped before still map;
- a default arm that throws UnexpectedResponseBodyException.

The anyOf-with-discriminator fixtures were never run because their data
set names were duplicated in the provider; they now have unique names.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
For CLIENT_PHP_VERSION 8.0 and later, an optional or nullable property with
an empty or untyped schema (`{}`, or only `nullable: true`) is typed `mixed`.
CodeBuilder::localProperty() skipped the `= null` default for `mixed`, so the
property stayed uninitialised and its getter threw "must not be accessed
before initialization" (as did getQueryParameters() for an optional untyped
query parameter). The default is now set; only the `?` prefix is still
omitted, because `mixed` already includes null.

A nullable untyped property also aborted the generation with "mixed type
cannot be nullable" because ParameterBuilder::setType() produced `?mixed`;
it now emits `mixed` for it.

Targets before 8.0 are unchanged (such properties are untyped there).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A property like `data: array<array<ShowSubscriber>>` was documented as
`ShowSubscriberCollection[]`, but the collection class was never generated
(Parser::extractField() did not descend into array items that are arrays)
and the mapper passed the raw payload through, so PHPStan reported an
unknown class and getData() returned raw arrays instead of schemas.

For arrays of arrays (at any depth) of objects:
- Parser extracts the innermost array of objects, so its collection, the
  collection mapper and the container entry are generated;
- the schema mapper injects the collection mapper and maps every innermost
  array with it (array_map with a closure, valid for every PHP target);
- the schema's toArray() serializes the collections back to arrays.

The documented types are unchanged and now name an existing class.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
For CLIENT_PHP_VERSION 8.1 and later, the Parser adds the item of an
array-of-enums response property to the composite response fields, so that
EnumGenerator creates the enum. SchemaMapperGenerator and
ServiceProviderGenerator also iterated those items and emitted an
`{Enum}Mapper` whose `toSchema()` had no body: it was never used (the parent
mapper maps the items with `{Enum}::from()`), threw a TypeError when called,
and failed static analysis. Both generators now skip enum fields; the enum
itself and the array mapping are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…g them

An enum value without letters or digits (e.g. `*`, `=`, `>=` or the empty
string) sanitised to an empty case name, and EnumGenerator silently skipped
it: for CLIENT_PHP_VERSION 8.1 and later the value was missing from the enum
(`Enum::from('=')` threw a ValueError, an enum could even end up with no cases
at all), and before 8.1 a single-symbol value got no constant.

Such values now get a name built from the names of their characters:
`*` -> ASTERISK, `=` -> EQUALS, `>=` -> GREATER_THAN_EQUALS, `€` -> U20AC,
`` -> EMPTY. For the pre-8.1 constants only the values that got no constant
before are affected (e.g. `EMBED_ASTERISK`). A symbolic name that coincides
with the name of another value of the same enum gets a numeric suffix, so the
existing names never change. A parameter default referencing such a value
uses the same name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ration-bugs

# Conflicts:
#	test/suite/unit/Ast/Builder/CodeBuilderTest.php
…ullable

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…words

PhpNameValidator::isValidClassName() only checks the characters, so a
schema named after a keyword or reserved type name (e.g. `Match`, `List`,
`Print`, `Mixed`) produced `class Match`, which is a parse error on PHP 8.x
(for `match`) or on every version (for `list`, `print`, ...). The generation
aborted in the lint step, whatever CLIENT_PHP_VERSION was targeted.

The class of such an object schema now gets a `Schema` suffix (`MatchSchema`,
so its mapper is `MatchSchemaMapper`) and a warning is printed. The names
derived from the schema name are valid and stay as they were (e.g.
`MatchCollection`, or `MatchVideo` for an inline property), and enums are not
affected, their class names already end in `Enum`. The discriminator mapping
of a oneOf/anyOf now also finds an option whose class name differs from its
schema name, so such an option keeps its case.

An enum value `class` produced `case CLASS`, which PHP reserves for the
class name constant; it is now `case V_CLASS`, like values that start with a
digit.

Only output that could not be generated before changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Class names are derived from component, parent, property and parameter
names, so different schemas can get the same one: an enum property
`Call.status` and an enum query parameter `callStatus` are both
`CallStatusEnum`, the same enum parameter can have different values in two
operations, and an inline object can get the fallback name of a component
(`HotDeal`). Only one class was generated per name (the last one registered
won), so the other schema was mapped with the wrong class: valid values threw
a ValueError or UnexpectedResponseBodyException, and a request could not
express the values of its own parameter.

After parsing, a new ClassNameCollisionResolver groups the schema and enum
fields by class name and compares their definitions. Identical definitions
keep sharing their class. Otherwise one definition keeps the name and each
other one gets a numeric suffix (`CallStatus2Enum`, `HotDeal2`), with a
warning naming the operations involved. The name is kept by the component
schema, when one of the definitions is a component referenced by its name;
otherwise by the definition the generators used to emit, so the classes
that were generated before keep their name and content, and only the
references that pointed to a class of another schema change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eneration-bugs

# Conflicts:
#	src/Generator/SchemaMapperGenerator.php
#	test/suite/functional/Generator/EnumGeneratorTest.php
#	test/suite/functional/Generator/RequestGeneratorTest.php
#	test/suite/functional/Generator/SchemaCollectionGeneratorTest.php
#	test/suite/functional/Generator/SchemaGeneratorTest.php
#	test/suite/functional/Generator/SchemaMapperGeneratorTest.php
#	test/suite/functional/Generator/ServiceProviderGeneratorTest.php
The acceptance tests regenerate example/ and example3_1/ PetStoreClient;
loginUser (a literal string response) now guards against an empty body.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tommey
tommey requested review from szhajdu and vsouz4 September 28, 2026 13:07
@tommey tommey self-assigned this Sep 28, 2026
@tommey
tommey merged commit ec50cd8 into master Sep 28, 2026
2 checks passed
@tommey
tommey deleted the feature/fix-remaining-generation-bugs branch September 28, 2026 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant