You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Is your feature request related to a problem? Please describe.
httpgen has nine features that customize JSON output (int64_encoding, enum_value, bytes_encoding, timestamp_format, nullable, empty_behavior, flatten, oneof_config, unwrap). Each emits its own complete MarshalJSONSebuf for the message it applies to, so at most one may be used per message.
That restriction has no semantic basis. Eight of the nine generate the identical pipeline and mutate only the key of their own field:
Disjoint keys, same shape. Combining them is concatenating the middle blocks. The exclusivity is an artifact of code layout — one method per feature instead of one method per message.
Notably, the codebase already composes these annotations across nesting levels. From the generated flatten marshaler:
// Forward opts to child's MarshalJSONSebuf when available (annotation composability)
Composability is already an accepted design goal. It just isn't implemented within a message.
That spec is correct and complete. The Go server for the same proto does not compile. We ship an API description that no sebuf-generated server can satisfy — the spec is the artifact consumers integrate against, so the divergence is not cosmetic.
The pipeline starts from opts.Marshal(x), and protojson knows nothing about a child's MarshalJSONSebuf. So a child's annotation is dropped whenever the parent has a marshaler of its own:
This cannot be fixed by hooking into protojson: protojson encodes purely through protoreflect, with a closed registry of well-known-type handlers keyed by full name (wellKnownTypeMarshaler in encoding/protojson/encode.go). It offers no user extension point and will never call a generated method. Delegation therefore has to be an explicit stage that we emit — which is precisely the stage that is missing today.
No error, no guard, no conflict reported. And OpenAPI describes that field as:
The server emits SGVsbG8=, which fails the spec's own pattern. Any spec-validating client rejects a response from our own server. This is wrong data on the wire today, not a missing feature.
3. The workarounds for this gap are the reason the feature set is tangled.
Three features hit the missing delegation step and each improvised differently:
nestsInt64NumberMessage — declared it a conflict and refuses to generate
flatten — hand-rolled child delegation inline
unwrap — abandoned the shared pipeline and hand-rolls the entire output map
One missing stage, three incompatible responses. That is also why unwrap collides with all eight other features and appears in no conflict list.
4. Ordinary API shapes are impossible.
#251's motivating case is string-mapped enums plus a nullable scalar on one message — {"type": "call", "delta": null}. That is a routine payload, not an exotic one. Today it cannot be expressed.
5. The failure mode is unpredictable.
Four different outcomes for what looks to a user like the same thing: some pairs generate correctly, some fail generation with a clear error, 19 of 36 generate and then break go build (#252), and nested cases silently corrupt output. Nothing in the annotation docs predicts which bucket you land in.
6. Extending the system is a landmine.
Adding a tenth feature today means: a new detector, edits to four hand-maintained conflict lists, a filename that must not clash with clientgen (#236), and an UnmarshalJSONSebuf twin (#235) — with nothing failing if any is forgotten. #235, #236, and #252 are three filed symptoms of that one structural gap.
Describe the solution you'd like
One marshaler per message, emitted once, composed from four ordered stages:
func (x*Msg) MarshalJSONSebuf(opts protojson.MarshalOptions) ([]byte, error) {
ifx==nil { return []byte("null"), nil }
// 1. BASE — protojson does the unannotated majoritydata, err:=opts.Marshal(x)
iferr!=nil { returnnil, err }
varrawmap[string]json.RawMessageiferr:=json.Unmarshal(data, &raw); err!=nil { returnnil, err }
// 2. DELEGATE — re-encode message-typed fields whose type has its own// marshaler, so child annotations survive. Fixes "why this matters" #2.ifm, ok:=any(x.Inner).(sebufMarshaler); ok {
ifraw["inner"], err=m.MarshalJSONSebuf(opts); err!=nil { returnnil, err }
}
// 3. FIELD TRANSFORMS — one block per annotated field. Disjoint keys,// order-independent. All eight field-local features live here.ifx.F1==nil { raw["f1"] = []byte("null") } // nullableraw["f2"], _=json.Marshal(x.F2.AsTime().Unix()) // timestamp_format// 4. DOCUMENT TRANSFORM — root unwrap only; replaces the object. Runs last.returnjson.Marshal(raw)
}
Three consequences worth calling out:
The conflict machinery is deleted, not improved. Stage 3 mutations touch only the key of the field they annotate, so they compose by construction. The single genuinely exclusive thing is stage 4, and root unwrap is already a per-message property (annotations.IsRootUnwrap), so at-most-one is structural rather than validated. The four hand-maintained conflict lists and their detectors go away.
unwrap stops being special once split in two. Map-value unwrap rewrites one key's value — that is a stage 3 field transform. Only root unwrap replaces the document (stage 4). Making that distinction is what lets unwrap rejoin the pipeline instead of hand-rolling every sibling field, which is also what makes it silently drop sibling annotations today.
UnmarshalJSONSebuf gets the same treatment. One unmarshaler per message composing each transform's inverse, which closes #235 structurally rather than by remembering to write five more methods.
Describe alternatives you've considered
Harden the exclusion instead — a single marshalJSONFeatures registry replacing the four hand-maintained lists, so every unsupported pair fails generation cleanly. This was filed as #253 and has been closed in favor of this issue. It fixes #252's broken builds, but it is more code written to preserve an accident, it leaves #251 permanently unfixable, and it does nothing about the nested-drop corruption in "why this matters" #2 — which is the most damaging of the three, since it produces wrong bytes with no diagnostic at all.
Do nothing / document the restriction — the nested-drop bug makes this untenable. It is a correctness defect against our own published OpenAPI spec, independent of ergonomics.
Additional context
Suggested sequencing, cheapest and most valuable first:
Stage 2 alone (nested delegation). Independently valuable, fixes the silent corruption, deletes nestsInt64NumberMessage, and is a prerequisite for stages 3 and 4 being correct. Ships without touching the exclusion model at all.
Safety invariant for the migration: single-feature messages must be byte-identical before and after. Golden files will churn broadly; if that invariant holds, the churn is noise and the only behavior changes are (a) combinations that previously failed and (b) nested annotations that previously corrupted. An all-pairs generate-and-compile test over the 36 combinations, in the spirit of internal/urlparamtest, is the natural regression net.
Scope is httpgen. protoc-gen-go-client duplicates much of this surface (#236) and should be evaluated for sharing the composed emitter rather than growing a parallel copy.
Supersedes #253. Implements #251. Closes #252 and #235 structurally.
Is your feature request related to a problem? Please describe.
httpgen has nine features that customize JSON output (
int64_encoding,enum_value,bytes_encoding,timestamp_format,nullable,empty_behavior,flatten,oneof_config,unwrap). Each emits its own completeMarshalJSONSebuffor the message it applies to, so at most one may be used per message.That restriction has no semantic basis. Eight of the nine generate the identical pipeline and mutate only the key of their own field:
Disjoint keys, same shape. Combining them is concatenating the middle blocks. The exclusivity is an artifact of code layout — one method per feature instead of one method per message.
Notably, the codebase already composes these annotations across nesting levels. From the generated
flattenmarshaler:// Forward opts to child's MarshalJSONSebuf when available (annotation composability)Composability is already an accepted design goal. It just isn't implemented within a message.
Why this matters
1. OpenAPI publishes contracts httpgen cannot implement.
protoc-gen-openapiv3handles combinations httpgen rejects. Given a message with bothnullableandtimestamp_format:That spec is correct and complete. The Go server for the same proto does not compile. We ship an API description that no sebuf-generated server can satisfy — the spec is the artifact consumers integrate against, so the divergence is not cosmetic.
2. Nested annotations silently emit spec-violating bytes.
The pipeline starts from
opts.Marshal(x), and protojson knows nothing about a child'sMarshalJSONSebuf. So a child's annotation is dropped whenever the parent has a marshaler of its own:This cannot be fixed by hooking into protojson:
protojsonencodes purely through protoreflect, with a closed registry of well-known-type handlers keyed by full name (wellKnownTypeMarshalerinencoding/protojson/encode.go). It offers no user extension point and will never call a generated method. Delegation therefore has to be an explicit stage that we emit — which is precisely the stage that is missing today.No error, no guard, no conflict reported. And OpenAPI describes that field as:
The server emits
SGVsbG8=, which fails the spec's ownpattern. Any spec-validating client rejects a response from our own server. This is wrong data on the wire today, not a missing feature.3. The workarounds for this gap are the reason the feature set is tangled.
Three features hit the missing delegation step and each improvised differently:
nestsInt64NumberMessage— declared it a conflict and refuses to generateflatten— hand-rolled child delegation inlineunwrap— abandoned the shared pipeline and hand-rolls the entire output mapOne missing stage, three incompatible responses. That is also why
unwrapcollides with all eight other features and appears in no conflict list.4. Ordinary API shapes are impossible.
#251's motivating case is string-mapped enums plus a nullable scalar on one message —
{"type": "call", "delta": null}. That is a routine payload, not an exotic one. Today it cannot be expressed.5. The failure mode is unpredictable.
Four different outcomes for what looks to a user like the same thing: some pairs generate correctly, some fail generation with a clear error, 19 of 36 generate and then break
go build(#252), and nested cases silently corrupt output. Nothing in the annotation docs predicts which bucket you land in.6. Extending the system is a landmine.
Adding a tenth feature today means: a new detector, edits to four hand-maintained conflict lists, a filename that must not clash with clientgen (#236), and an
UnmarshalJSONSebuftwin (#235) — with nothing failing if any is forgotten. #235, #236, and #252 are three filed symptoms of that one structural gap.Describe the solution you'd like
One marshaler per message, emitted once, composed from four ordered stages:
Three consequences worth calling out:
The conflict machinery is deleted, not improved. Stage 3 mutations touch only the key of the field they annotate, so they compose by construction. The single genuinely exclusive thing is stage 4, and root unwrap is already a per-message property (
annotations.IsRootUnwrap), so at-most-one is structural rather than validated. The four hand-maintained conflict lists and their detectors go away.unwrapstops being special once split in two. Map-value unwrap rewrites one key's value — that is a stage 3 field transform. Only root unwrap replaces the document (stage 4). Making that distinction is what letsunwraprejoin the pipeline instead of hand-rolling every sibling field, which is also what makes it silently drop sibling annotations today.UnmarshalJSONSebufgets the same treatment. One unmarshaler per message composing each transform's inverse, which closes #235 structurally rather than by remembering to write five more methods.Describe alternatives you've considered
Harden the exclusion instead — a single
marshalJSONFeaturesregistry replacing the four hand-maintained lists, so every unsupported pair fails generation cleanly. This was filed as #253 and has been closed in favor of this issue. It fixes #252's broken builds, but it is more code written to preserve an accident, it leaves #251 permanently unfixable, and it does nothing about the nested-drop corruption in "why this matters" #2 — which is the most damaging of the three, since it produces wrong bytes with no diagnostic at all.Do nothing / document the restriction — the nested-drop bug makes this untenable. It is a correctness defect against our own published OpenAPI spec, independent of ergonomics.
Additional context
Suggested sequencing, cheapest and most valuable first:
nestsInt64NumberMessage, and is a prerequisite for stages 3 and 4 being correct. Ships without touching the exclusion model at all.unwrap— split map-value (stage 3) from root (stage 4). The only real rewrite.UnmarshalJSONSebuf— closes [BUG] httpgen: five emitters still lack the UnmarshalJSONSebuf twin, so DiscardUnknown is dropped #235.Safety invariant for the migration: single-feature messages must be byte-identical before and after. Golden files will churn broadly; if that invariant holds, the churn is noise and the only behavior changes are (a) combinations that previously failed and (b) nested annotations that previously corrupted. An all-pairs generate-and-compile test over the 36 combinations, in the spirit of
internal/urlparamtest, is the natural regression net.Scope is httpgen.
protoc-gen-go-clientduplicates much of this surface (#236) and should be evaluated for sharing the composed emitter rather than growing a parallel copy.Supersedes #253. Implements #251. Closes #252 and #235 structurally.