GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix: qualify only the model names in types from an imported document by mromaszewicz · Pull Request #2589 · oapi-codegen/oapi-codegen

Conversation

@mromaszewicz

@mromaszewicz mromaszewicz commented Sep 27, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

What was wrong. A type that belongs to an import-mapped document was qualified by putting the package in front of its whole Go spelling (ensureExternalRefsInSchema). That's only right for a single model name. any, *string, []Thing, map[string]Thing, time.Time, and every primitive became externalRef0.any, externalRef0.*string, externalRef0.[]Thing and so on. These either failed gofmt, aborting generation, or didn't compile. Struct literals were skipped instead, so a model named inside one stayed unqualified. Two positions were affected:

  • a reusable response from another document reached from a non-fixed status code, or reused by a second status code;
  • every inline schema (parameters, request body, responses) of an operation in a path item reused by $ref.

In the same position, response headers weren't qualified at all, and neither were the client's typed responses (JSON200 *Thing).

What changed.

  • qualifyExternalTypeNames parses the type expression with go/ast and prefixes only the unqualified, non-builtin type names, leaving the rest of the text (field names, tags, comments) as written. So []Thing becomes []externalRef0.Thing, a struct literal's F *Thing becomes F *externalRef0.Thing, and any, *string, time.Time and externalRef1.Bar stay as they are.
  • ensureExternalRefsInSchema keeps its current result wherever that result compiled: a plain model name is still referred to as externalRef0.<Name> (even when the schema already carries a RefType), and a struct literal that the schema declares as a named type is left alone. Everything else is qualified with the new helper.
  • ensureExternalRefsInResponseDefinitions now qualifies response headers too, except a header that is itself a $ref into another document, which is already qualified with that document's package.
  • The client's typed responses for an inline response of a reused path item are qualified the same way.

One case that compiled before generates differently, on purpose. A struct literal or response header of a reused path item that names a model (Thing *Thing, XThing *Tag) used to compile only when the importing package happened to declare its own type of the same name. It then referred to that local type instead of the imported model the spec names, silently decoding into the wrong shape. It now refers to externalRef0.Thing. That output was broken, so it isn't preserved behind a compatibility option. Handler code that built those structs from the local type needs to use the imported model.

How it's tested.

  • pkg/codegen/externalref_test.go:
    • covers the helper across type shapes, including struct literals with comments, generics, interfaces, arrays and maps keyed by models, qualified names, and unparseable input;
    • checks ensureExternalRefsInSchema's rules, including the preserved ones and the header guard;
    • generates a two-document spec end to end. That test fails on main, where generation aborts.
  • internal/test/references/multipackage/external_types generates an imported package and an API that reuses its reusable responses from default and one of its path items. A client ⇄ strict-server round trip covers any, *string, []Thing, a path parameter, a query parameter, a request body, a response header typed by a model, and the client's typed responses. main fails to generate this fixture (expected 'IDENT', found '[').
  • I compared a binary built from main against one from this branch. The matrix is 143 cross-package cases: the three positions above plus response headers, header $refs into the same and a third document, and path parameters. It covers builtins, date-time/uuid, pointers, x-go-type (local and qualified), arrays and maps of models and builtins, inline objects with and without model fields, enums, unions, additionalProperties, allOf, and $refs to the same and a third document. I ran it as-is, without the client, and with old-aliasing, schema-merging-behavior: v1/v3 and nullable-type. In all six runs, no output that compiles on main changes (the matrix has no same-name collisions; that case is described above). Between 88 and 93 failing cases per run now compile. The only failures left are strict-server receivers for an inline untyped or pointer response with a fixed status code, which fail in the imported package too and are fixed by fix: handle untyped strict response bodies #2514. With fix: handle untyped strict response bodies #2514 merged in, they compile.
  • make generate changes no committed file. make test passes, make lint gives 0 issues, and the new fixture and tests pass under Go 1.25.

Fixes #2588

🤖 Generated with Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

greptile-apps[bot] reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@mromaszewicz @claude
fix: qualify only the model names in types from an imported document
720b95e
A type generated in the context of an import-mapped document was
qualified by prefixing its whole Go spelling with the package, which is
only right when it is a single model name. Every other spelling broke:

    Body externalRef0.any                  // undefined: externalRef0.any
    Body externalRef0.*string              // gofmt: expected 'IDENT', found '*'
    Q *externalRef0.string                 // undefined
    type OpJSONRequestBody = externalRef0.[]Thing
    Body externalRef0.time.Time

This hit a reusable response reached from a non-fixed status code (or
reused by a second status code), and every inline schema of an
operation in a path item reused by $ref: its parameters, request body
and responses. A struct literal was skipped instead, so the models it
named stayed unqualified. The response headers of such a path item were
not qualified at all, nor were the typed responses of its client.

Qualify only the unqualified, non-builtin type names in the expression,
using go/ast to find them and leaving everything else as written:
"[]Thing" becomes "[]externalRef0.Thing", "*string" and "time.Time"
stay as they are. A plain model name, and a struct literal the schema
declares as a named type, are qualified exactly as before. Qualify the
response headers of a reused path item, except a header that is itself
a $ref into another document, and the client's typed responses for it.

One case that compiled before now generates differently. A struct
literal or response header of a reused path item that names a model
used to compile only when the importing package happened to declare a
type of the same name, and then it referred to that local type rather
than the imported model the spec names, so it decoded into the wrong
shape. It now refers to the imported model.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@mromaszewicz
mromaszewicz requested a review from a team as a code owner September 27, 2026 22:55
@mromaszewicz mromaszewicz added the bug Something isn't working label Sep 27, 2026
@mromaszewicz mromaszewicz mentioned this pull request Sep 27, 2026
Keep compiling strict response shapes; wrap untyped text and form bodies Boulea7/oapi-codegen#1
Merged
@greptile-apps

greptile-apps Bot commented Sep 27, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Fixes type qualification for imported OpenAPI schemas.

The PR appears safe to merge based on the reviewed changes.

Summary

The PR changes external-reference handling to qualify model names within Go type expressions rather than prefixing entire expressions. It also qualifies response headers and client typed responses for reused path items, with unit and multi-package round-trip coverage.

Reviews (1) · Last reviewed commit: "fix: qualify only the model names in typ..."

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mromaszewicz
mromaszewicz merged commit 1306ad5 into oapi-codegen:main Sep 27, 2026
16 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@mromaszewicz
mromaszewicz deleted the fix/external-ref-type-qualification branch September 27, 2026 23:14
mromaszewicz added a commit to mromaszewicz/oapi-codegen that referenced this pull request Sep 27, 2026
@mromaszewicz @claude
Merge main into pr-2514-fixes
748b208
Brings in oapi-codegen#2587 (a reusable form response's envelope is marshalled, not
its wrapper) and oapi-codegen#2589 (types from an imported document qualify only
their model names).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
mromaszewicz added a commit to mromaszewicz/oapi-codegen that referenced this pull request Sep 27, 2026
@mromaszewicz @claude
test: regenerate external_types for reusable pointer envelopes
3134315
The fixture oapi-codegen#2589 added has a reusable response whose schema is
`x-go-type: "*string"`, reached only from `default`. Its envelope now
carries a Body field, as a pointer envelope does, so that a
fixed-status $ref to it would compile.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
mromaszewicz added a commit that referenced this pull request Sep 28, 2026
@Boulea7 @mromaszewicz @claude
fix: handle untyped strict response bodies (#2514)
ddbdf0e
* fix: wrap receiver-incompatible strict JSON responses

* test: cover strict JSON response receiver compatibility

* test: cover strict JSON response wire values

* test: cover OpenAPI 3.1 strict JSON responses

* fix: keep strict response shapes that already compile

Wrapping every receiver-incompatible body changed response types that
compiled before, breaking code written against them:

- A components/responses envelope is never a method receiver. Each
  operation that reaches it through $ref embeds it in a struct, and an
  embedded field may be an interface, so an untyped or interface
  envelope compiled as a direct type. Only a pointer, which cannot be
  embedded, needs a Body field there.

- Under schema-merging-behavior v1, an allOf of a single $ref becomes
  a struct that embeds the member, which is a valid receiver, whether
  it is inline or a component of its own. The single-member allOf
  shortcut assumed the member's type is always used directly.

Classify a composition by a referenced member only when the generator
actually emitted that member's type. That also covers a nullable union
collapsed to an untyped member, and an allOf of an untyped $ref with
sibling keywords under v3. Check the declared type of x-go-type-name
as well.

One envelope that compiled before still changes: a pointer envelope
that no operation reaches through a fixed-status $ref, so that it is
never embedded (only reached through default, a status range, or not
at all), goes from `type FooJSONResponse *T` to a struct with a Body
field. The envelope has to match what a fixed-status $ref needs, and
no generated operation uses the envelope in that case.

servers/strict/responsebody_v1 compiles and serves the v1 shapes.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* fix: wrap untyped text and form strict response bodies

A text/plain or application/x-www-form-urlencoded response without a
schema, or with one that lowers to any, generated a method on an
interface type, the same compile error as the JSON case:

    invalid receiver type GetText200TextResponse (pointer or interface type)

The strict templates already write text and form bodies from
response.Body whenever the response has a Body field, so classify
these content types as well.

An operation that reaches a reusable text response through $ref
redeclares its envelope as the receiver type, unlike a JSON or form
envelope, which it embeds. So a text envelope is classified as a
receiver.

That changes one type that compiled before: a reusable text envelope
whose body lowers to any or a pointer, and that no operation reaches
through a fixed-status $ref (only through default, a status range, or
not at all), goes from `type FooTextResponse any` to a struct with a
Body field. A fixed-status $ref to it needs that shape to compile, and
no generated operation uses the envelope otherwise.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* refactor: drop NeedsLocalResponseHeaders

NeedsLocalResponseHeaders only restated `not .IsRef`, and the strict
templates used it for two of their three header branches; the branch
for a response without content still reads $isRef. It changes no
output, so go back to $isRef throughout and keep the templates' diff
to what the fix needs.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* test: describe what the response_cast fixture now covers

The spec headers still described only the #2010 cast. Name the
untyped, pointer and header-bearing responses the fixture now covers,
and cite #2525 alongside #1328.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* test: pin the v1 wire bodies, and spell the nullable fixtures with anyOf

Review follow-ups:

- responsebody_v1 only checked that the direct responses were valid
  JSON, so a visitor that dropped the value would still pass. Assert
  the body. v1 embeds the untyped member, and encoding/json encodes an
  embedded interface as a field named after its type, so the value
  has always gone out as {"Untyped": ...}. The test records that.

- Untyped ({}) already admits null, so oneOf of it and a null branch
  rejects null, the value the fixtures send. Use anyOf, which the
  generator lowers identically, in both fixtures. The unit test keeps
  the oneOf spelling that specs commonly use, and adds the anyOf one.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* test: regenerate external_types for reusable pointer envelopes

The fixture #2589 added has a reusable response whose schema is
`x-go-type: "*string"`, reached only from `default`. Its envelope now
carries a Body field, as a pointer envelope does, so that a
fixed-status $ref to it would compile.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

---------

Co-authored-by: Marcin Romaszewicz <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
@mromaszewicz mromaszewicz mentioned this pull request Sep 29, 2026
refactor: share the Go-type and lowered-type walks the aggregate code grew #2592
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Types from an import-mapped document are qualified as a whole, producing externalRef0.any, externalRef0.*string, externalRef0.[]Thing

Uh oh!

There was an error while loading. Please reload this page.

1 participant

@mromaszewicz
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.