Repository navigation
fix: remaining code generation bugs found by regenerating 42 clients (11.4.0) - #163
Merged
Merged
Conversation
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>
…g-generation-bugs
…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>
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.
Regenerating 42 real-world clients (every
CLIENT_PHP_VERSIONthey 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
null= unsetschemaContent-Typewith no bodyMatch/List/Class…class Matchdoesn't parse on PHP 8 (even for 7.x targets)MatchSchema+ warning; enum valueclass→V_CLASSRuntime errors and deprecations in generated code
E_USER_DEPRECATED1/0for booleans, like query/cookie)Request\LiteralRequestBody, unwrapped by the JSON serializernullable: true+allOf: [$ref]TypeErroron nullEnum::from(null)TypeErrornullValueErrorescapesUnexpectedResponseBodyExceptionError(dynamic mapper call)UnexpectedResponseBodyExceptionUnhandledMatchErrorRuntimeException, as the pre-8.0switchalready threwUnexpectedResponseBodyException(nullif nullable)mixed) property (8.0+)= null; no more?mixed*,=)ASTERISK,EQUALS, ……2classes + warning; identical ones still share…EnumMapperwith emptytoSchema()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.example/clients, whoseloginUserliteral response gains the empty-body guard, and one declares a newCodeBuilderparameter 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-timeresponses; 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