Skip to content

fix(HybridBridge): unwrap a Header's JsonContent/XmlContent in hybrid mode - #2204

Merged
DerManoMann merged 2 commits into
zircote:masterfrom
DerManoMann:fix/hybrid-header-content
Sep 16, 2026
Merged

DerManoMann merged 2 commits into
zircote:masterfrom
DerManoMann:fix/hybrid-header-content

Conversation

@DerManoMann

@DerManoMann DerManoMann commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Overview

Hybrid mode silently dropped a Header's content when that content was written as a
JsonContent or XmlContent. The header itself came out, description and all, simply
without its content — and with no warning or error to say so. Classic and spec both
render it.

Header gained content in #2191, which also taught the merge processors to accept it.
Hybrid runs with an empty processor pipeline, so MergeJsonContent never fires: the
annotation stays parked in _unmerged while $header->content is still the undefined
sentinel. HybridBridge::convertHeader() read that property directly, found the sentinel,
and emitted the header with nothing in it.

The second half of this is documentation. Writing the fixture raised a question the suite
had never answered in writing — when a new case earns a new Scratch fixture rather than a
case in an existing one — so that rule is now written down, together with a marker for the
test scaffolding that exists only to serve classic and hybrid.

Changes

  • HybridBridge::resolveContent() takes OA\Header in its union, and convertHeader()
    calls it rather than mapping over the raw property. The unwrapping it already does for
    Response, RequestBody and Parameter is exactly what Header was missing.
  • Clears the two argument.type errors on convertHeader() that chore(Types): clear phpstan level 7 outside classic #2203 left standing
    deliberately; level 7 on src/HybridBridge.php goes from 17 errors to 15.
  • Scratch/HeaderObject carries two more headers, one reaching content through
    JsonContent and one through XmlContent. It pinned content in all three modes
    already, but only via an explicit MediaType list — the one shape that never reaches the
    merge processors, which is why this went unseen.
  • docs/dev/testing.md gains a rule for what belongs in a Scratch fixture: the unit is the
    field family with the attribute as the default, a new case belongs in the existing fixture
    unless it needs expected logs the rest of the file must not inherit or cannot be written in
    the spec attribute API, and a fixture is named for the shape it holds rather than the bug
    that prompted it. Plus the trap that Scratch fixtures are deliberately not autoloadable,
    which makes bin/openapi report one as an unknown class instead of running it.
  • ROADMAP.md defines a [classic] / [hybrid] marker for documentation and test structure
    that @deprecated cannot reach, so the v8 removal is a grep rather than an untangling. Its
    first use is the new testing.md section.

… mode

Header gained `content` in zircote#2191, which also taught the merge processors to
accept it, but the bridge was never extended to match. Hybrid runs with an
empty processor pipeline, so MergeJsonContent never fires and a header written
as `content: new JsonContent(...)` leaves $header->content undefined with the
annotation parked in _unmerged. convertHeader() read the property directly,
found the sentinel and emitted the header with no content at all: no warning,
no error, while classic and spec both render it.

resolveContent() already does exactly this unwrapping for Response, RequestBody
and Parameter, so Header joins its union and convertHeader() calls it instead
of mapping over the raw property.

Scratch/HeaderObject pinned Header content in all three modes already, but only
through an explicit MediaType list -- the one shape that never reaches the merge
processors. It now carries the other two.
…ith classic

tests/Fixtures/Scratch is 50 fixtures across 263 files, and nothing said when a
new case earns a new file -- so every fixture arriving alongside a bug fix
re-decided it, and the single-case fixtures in the tree are all bug-driven.
Write the rule down: the unit is the field family with the attribute as the
default, a new case belongs in the existing fixture unless it needs expected
logs the rest of the file must not inherit or cannot be expressed in the spec
attribute API, and a fixture is named for the shape it holds rather than the bug
that prompted it. SchemaKeywords is the worked example. Includes the gotcha that
Scratch fixtures are deliberately not autoloadable, which makes bin/openapi
report one as an unknown class rather than running it.

Separately, mark the parts of the test setup that exist only to serve classic or
hybrid so v8 can delete them rather than untangle them. Code carries @deprecated
from v7; prose, fixture layout and test-matrix structure cannot, so they carry
[classic] / [hybrid] instead. ROADMAP.md defines the marker next to the line
saying classic is removed, which is where that work starts.
@DerManoMann
DerManoMann force-pushed the fix/hybrid-header-content branch from 472bbd5 to ece4dda Compare September 16, 2026 00:52
@DerManoMann
DerManoMann merged commit b3c0026 into zircote:master Sep 16, 2026
18 checks passed
@DerManoMann
DerManoMann deleted the fix/hybrid-header-content branch September 16, 2026 00:57
DerManoMann added a commit to DerManoMann/swagger-php that referenced this pull request Sep 16, 2026
array_map() preserves keys, so every classic keyed map converted into a
Spec list came out as array<X> against a declared list<X>: examples on
Parameter/MediaType/Header/Components, and allOf/anyOf/oneOf/prefixItems
on Schema. array_values() is safe for all of them — the compiler re-keys
examples off Spec\Example::$example (compileKeyedMap(..., 'example', ...)),
and the four Schema keywords are positional in OpenAPI. patternProperties
and dependentSchemas are genuinely keyed and are left alone.

Also types convertSecurityRequirements()/convertCallbacks()/filterType().

**One error is left standing on purpose**, and it is not a typing gap:
Spec\ServerVariable::$enum is list<string> but classic's is
list<string|int|float|bool|UnitEnum>|class-string. Whether a server
variable enum may hold non-strings is an OpenAPI compliance question
(the spec says string), not something to cast away here.

Two more stood in convertHeader()'s $content array_map until zircote#2204,
which is the behaviour fix they were waiting on: a Header whose content
is a JsonContent was silently dropped in hybrid, and an is_array() guard
here would have cleared the errors while making the drop look deliberate.
resolveContent() now accepts Header and does the unwrapping, so both go
without a guard. It keeps the guard it already had, for the non-array
shapes the declared type still permits.

Full suite green (2265 tests). composer lint and analyse clean.
Takes HybridBridge from 15 level-7 errors on master to 1.
DerManoMann added a commit to DerManoMann/swagger-php that referenced this pull request Sep 16, 2026
Each has a reason that only existed in a commit message until now, which
is the wrong place for it: the next person to run phpstan at level 7 sees
an unexplained error and the obvious fix — a guard or a cast — buries the
thing the error is pointing at.

- HybridBridge::convertServer(): Spec\ServerVariable::$enum is
  list<string> per the spec while classic's accepts more.
- TypeInfoTypeResolver::applyToAnnotation(): an explicit
  additionalProperties: false is dereferenced as an object.

A third sat in HybridBridge::convertHeader(), where a Header whose content
is a JsonContent was silently dropped. zircote#2204 has since fixed that, so the
error and the comment explaining it both go with it.
DerManoMann added a commit to DerManoMann/swagger-php that referenced this pull request Sep 17, 2026
array_map() preserves keys, so every classic keyed map converted into a
Spec list came out as array<X> against a declared list<X>: examples on
Parameter/MediaType/Header/Components, and allOf/anyOf/oneOf/prefixItems
on Schema. array_values() is safe for all of them — the compiler re-keys
examples off Spec\Example::$example (compileKeyedMap(..., 'example', ...)),
and the four Schema keywords are positional in OpenAPI. patternProperties
and dependentSchemas are genuinely keyed and are left alone.

Also types convertSecurityRequirements()/convertCallbacks()/filterType().

**One error is left standing on purpose**, and it is not a typing gap:
Spec\ServerVariable::$enum is list<string> but classic's is
list<string|int|float|bool|UnitEnum>|class-string. Whether a server
variable enum may hold non-strings is an OpenAPI compliance question
(the spec says string), not something to cast away here.

Two more stood in convertHeader()'s $content array_map until zircote#2204,
which is the behaviour fix they were waiting on: a Header whose content
is a JsonContent was silently dropped in hybrid, and an is_array() guard
here would have cleared the errors while making the drop look deliberate.
resolveContent() now accepts Header and does the unwrapping, so both go
without a guard. It keeps the guard it already had, for the non-array
shapes the declared type still permits.

Full suite green (2265 tests). composer lint and analyse clean.
Takes HybridBridge from 15 level-7 errors on master to 1.
DerManoMann added a commit to DerManoMann/swagger-php that referenced this pull request Sep 17, 2026
Each has a reason that only existed in a commit message until now, which
is the wrong place for it: the next person to run phpstan at level 7 sees
an unexplained error and the obvious fix — a guard or a cast — buries the
thing the error is pointing at.

- HybridBridge::convertServer(): Spec\ServerVariable::$enum is
  list<string> per the spec while classic's accepts more.
- TypeInfoTypeResolver::applyToAnnotation(): an explicit
  additionalProperties: false is dereferenced as an object.

A third sat in HybridBridge::convertHeader(), where a Header whose content
is a JsonContent was silently dropped. zircote#2204 has since fixed that, so the
error and the comment explaining it both go with it.
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