Qualify external header schema refs in generated response headers#2463
Merged
Conversation
Closes: #2060 When a response header is an external `$ref` (it lives in another file mapped via import-mapping) and that header's schema is itself a `$ref` to a named type, the generated response-headers struct referenced the type by a bare local name (e.g. `ETag ETagSchema`) instead of the imported one (`ETag externalRef0.ETagSchema`). Because the named type is only generated into the imported package, the referencing package failed to compile with an undefined `ETagSchema`. The header's schema `$ref` is written relative to the external file (e.g. `#/components/schemas/ETagSchema`), so `GenerateGoSchema` resolves it against the root spec as a bare local name. Since the header component carries its own `$ref` telling us which file it came from, qualify the schema with that external package via `ensureExternalRefsInSchema`, mirroring how response content schemas are already handled. This is guarded on the schema being a reference so an external header with an inline primitive schema (e.g. `type: string`) is not mangled into `externalRef0.string`. The fix corrects both the strict-server `<Op><Status>ResponseHeaders` struct and the client response wrapper's header fields introduced in #2462. Adds internal/test/references/multipackage/header_ref exercising an external header ref with a ref'd schema across strict-server and client generation; the committed generated files must compile as part of the test module, which guards against the undefined-symbol regression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Greptile SummaryFixes a compile error in generated code when a response header is an external
Confidence Score: 4/5Safe to merge; the change is a narrow, well-scoped fix to a compile-time regression with a compile-time regression test. The fix is minimal, mirrors an identical pattern already in the codebase for response content schemas, and is guarded against mangling inline primitive types. The generated output compiles correctly and the new test directly exercises the previously failing case for both strict-server and client codepaths. Only a style observation about the test file name keeps this from being a clean pass. No files require special attention; the only note is the test file name
|
| Filename | Overview |
|---|---|
| pkg/codegen/operations.go | Targeted fix: qualifies external header schema refs with the imported package name, mirroring the existing pattern for response content schemas. Logic is sound and well-guarded. |
| internal/test/references/multipackage/header_ref/gen/api/api.gen.go | Generated file shows the fix working correctly: both GetThing200ResponseHeaders and GetThingResponse200Headers use *externalRef0.ETagSchema instead of the previously undefined bare *ETagSchema. |
| internal/test/references/multipackage/header_ref/gen/common/common.gen.go | Generated common package correctly emits type ETagSchema = string, which the api package then references via the qualified externalRef0.ETagSchema. |
| internal/test/references/multipackage/header_ref/issue_test.go | Regression test covering both the strict-server response headers struct and the client response wrapper. File is named issue_test.go rather than the conventional header_ref_test.go. |
| internal/test/references/multipackage/header_ref/spec.yaml | New spec exercising an operation whose 200 response carries an ETag header that is an external $ref, exactly the failing scenario from issue-2060. |
| internal/test/references/multipackage/header_ref/common/spec.yaml | Common spec defining the ETag header component whose schema is a $ref to a named type, providing the cross-package reference scenario under test. |
| internal/test/references/multipackage/header_ref/doc.go | Standard doc.go with go:generate directives for both the common and api packages; correctly follows multi-config layout convention. |
| internal/test/references/multipackage/header_ref/config.api.yaml | API generation config enabling chi-server, strict-server, client, and models with the correct import-mapping for the common package. |
| internal/test/references/multipackage/header_ref/config.common.yaml | Common package config generating only models, which is the correct minimal config for the shared schema package. |
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
internal/test/references/multipackage/header_ref/issue_test.go:1
**Test file naming inconsistency**
The repository convention for leaf test directories is `<name>_test.go` (matching the directory's snake_case name). This file is `issue_test.go` rather than the expected `header_ref_test.go`. The neighbouring `response_cast` subdirectory also uses `issue_test.go`, so this isn't unique to this PR, but it makes it harder to correlate test files with their scenario directory at a glance.
Reviews (1): Last reviewed commit: "Qualify external header schema refs in g..." | Re-trigger Greptile
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
1 participant
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.
Closes: #2060
When a response header is an external
$ref(it lives in another file mapped via import-mapping) and that header's schema is itself a$refto a named type, the generated response-headers struct referenced the type by a bare local name (e.g.ETag ETagSchema) instead of the imported one (ETag externalRef0.ETagSchema). Because the named type is only generated into the imported package, the referencing package failed to compile with an undefinedETagSchema.The header's schema
$refis written relative to the external file (e.g.#/components/schemas/ETagSchema), soGenerateGoSchemaresolves it against the root spec as a bare local name. Since the header component carries its own$reftelling us which file it came from, qualify the schema with that external package viaensureExternalRefsInSchema, mirroring how response content schemas are already handled. This is guarded on the schema being a reference so an external header with an inline primitive schema (e.g.type: string) is not mangled intoexternalRef0.string.The fix corrects both the strict-server
<Op><Status>ResponseHeadersstruct and the client response wrapper's header fields introduced in #2462.Adds internal/test/references/multipackage/header_ref exercising an external header ref with a ref'd schema across strict-server and client generation; the committed generated files must compile as part of the test module, which guards against the undefined-symbol regression.