Fix ValueByDiscriminator() with external refs#2474
Merged
Conversation
mromaszewicz
force-pushed
the
fix/issue-2470
branch
from
July 16, 2026 17:30
36875aa to
12ce1a8
Compare
Contributor
Greptile SummaryFixes
Confidence Score: 5/5Safe to merge: the change is narrowly scoped to the ValueByDiscriminator() template path and is guarded by a new multi-package regression fixture that would fail to compile if the fix regressed. The core change (resolving mapping values through DiscriminatorCases before template rendering) is correct and well-contained. Local-ref unions produce byte-identical output because UnionElement.Method() equals the raw mapping value for plain Go type names. The generated fixture confirms the external-ref case now compiles and dispatches correctly. No other template files or router backends are affected, and no configuration or public API surface is altered. No files require special attention.
|
| Filename | Overview |
|---|---|
| pkg/codegen/schema.go | Adds DiscriminatorCases() helper that resolves discriminator mapping values to UnionElement.Method() names before template rendering, fixing invalid Go for external-ref types |
| pkg/codegen/templates/union.tmpl | Switches ValueByDiscriminator() generation from iterating the raw Mapping map to using DiscriminatorCases(); also implicitly fixes non-deterministic case ordering (Go map range was random, SortedMapKeys is used in the Go helper) |
| internal/test/references/multipackage/discriminated_union_allof/gen/api/api.gen.go | Generated fixture for the cross-package allOf case; ValueByDiscriminator() correctly calls AsExternalRef0GitDiffFile()/AsExternalRef0AddedImageDiffFile() instead of the previously invalid t.AsexternalRef0.GitDiffFile() |
| internal/test/references/multipackage/discriminated_union_allof/issue_test.go | Runtime regression test exercises ValueByDiscriminator() round-trip for the external-ref case; compilation of the package itself is the primary guard |
| internal/test/references/multipackage/discriminated_union_allof/spec.yaml | New test spec defining PRFile as an allOf of an external discriminated union (DiffFile) and a local object (ViewedStatus) |
| internal/test/references/multipackage/discriminated_union_allof/common/spec.yaml | External spec defining the discriminated oneOf union (DiffFile) with implicit discriminator mapping; no issues |
| internal/test/references/multipackage/discriminated_union_allof/doc.go | Standard doc.go with go:generate directives for both packages; follows existing conventions |
| internal/test/references/multipackage/discriminated_union_allof/config.api.yaml | oapi-codegen config for the api package with import-mapping for the external common spec; correctly structured |
| internal/test/references/multipackage/discriminated_union_allof/config.common.yaml | oapi-codegen config for the common package; straightforward models-only generation |
| internal/test/references/multipackage/discriminated_union_allof/gen/common/common.gen.go | Generated fixture for the common package; local-ref discriminator cases are byte-identical to pre-fix output (Method() == raw mapping value for local types) |
Reviews (2): Last reviewed commit: "Fix ValueByDiscriminator() with external..." | Re-trigger Greptile
Closes: oapi-codegen#2470 When a discriminated `oneOf` union is defined in an external file and pulled into an `allOf` from another package via `import-mapping`, the generated `ValueByDiscriminator()` interpolated the raw discriminator mapping value (e.g. `externalRef0.GitDiffFile`) directly into the method name, producing invalid Go: `t.AsexternalRef0.GitDiffFile()`. The fix computes the switch arms in Go rather than in the template. A new `Schema.DiscriminatorCases()` helper maps each discriminator value to its union element's `Method()` — the same name used by the generated `As*`/`From*`/`Merge*` helpers — so the call becomes `t.AsExternalRef0GitDiffFile()`. `union.tmpl` now just ranges over those precomputed `{Value, Method}` cases, replacing the nested `range UnionElements` / `eq (print $el)` matching logic. Local-ref unions are unaffected: the element `Method` equals the mapping value there, so existing generated code is byte-identical. Adds a regression fixture under `internal/test/references/multipackage/discriminated_union_allof` that reproduces the two-file, import-mapped, discriminated-union-in-allOf case and exercises `ValueByDiscriminator()` end-to-end.
mromaszewicz
force-pushed
the
fix/issue-2470
branch
from
July 16, 2026 17:44
12ce1a8 to
fdc6c6a
Compare
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: #2470
When a discriminated
oneOfunion is defined in an external file and pulled into anallOffrom another package viaimport-mapping, the generatedValueByDiscriminator()interpolated the raw discriminator mapping value (e.g.externalRef0.GitDiffFile) directly into the method name, producing invalid Go:t.AsexternalRef0.GitDiffFile().union.tmplnow resolves each mapping value to its matchingUnionElementand emits{{ .Method }}— the same name used by the generatedAs*/From*/Merge*helpers — so the call becomest.AsExternalRef0GitDiffFile(). Local-ref unions are unaffected: the elementMethodequals the mapping value there, so existing generated code is byte-identical.Adds a regression fixture under
internal/test/references/multipackage/discriminated_union_allofthat reproduces the two-file, import-mapped, discriminated-union-in-allOf case and exercisesValueByDiscriminator()end-to-end.