From e30face08d57995f024f53c68ceb6c486ba9d977 Mon Sep 17 00:00:00 2001 From: Hicham <53556927+hishamank@users.noreply.github.com> Date: Fri, 3 Jul 2026 14:45:02 +0400 Subject: [PATCH 01/11] feat(ts): render oneofs as discriminated unions by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Oneofs without an explicit sebuf.http oneof_config annotation now render as `$case` discriminated unions instead of flattened optional fields, in both TS generators (protoc-gen-ts-client, protoc-gen-ts-server). Synthetic oneofs (proto3 `optional`) are unaffected. This is the default behavior — there is no opt-in flag. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../golden/oneof_discriminator_client.ts | 7 +++-- internal/tscommon/types.go | 26 ++++++++++++++++++- .../golden/oneof_discriminator_server.ts | 7 +++-- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts b/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts index d8e2568a..1cf80ea9 100644 --- a/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts +++ b/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts @@ -36,10 +36,13 @@ export interface VideoContent { duration: number; } +export type PlainEventContent = + | { $case: "text"; text?: TextContent } + | { $case: "image"; image?: ImageContent }; + export interface PlainEvent { id: string; - text?: TextContent; - image?: ImageContent; + content?: PlainEventContent; } export interface FieldViolation { diff --git a/internal/tscommon/types.go b/internal/tscommon/types.go index 4bdac418..a1fc28dd 100644 --- a/internal/tscommon/types.go +++ b/internal/tscommon/types.go @@ -381,10 +381,15 @@ func GenerateEnumType(p Printer, enum *protogen.Enum) { func GenerateInterface(p Printer, msg *protogen.Message) { name := string(msg.Desc.Name()) - // Collect discriminated oneof info + // Collect discriminated oneof info. Oneofs without an explicit oneof_config + // annotation render as discriminated unions by default; synthetic oneofs + // (proto3 `optional`) are left as plain optional fields. var discriminatedOneofs []*annotations.OneofDiscriminatorInfo for _, oneof := range msg.Oneofs { info := annotations.GetOneofDiscriminatorInfo(oneof) + if info == nil && !oneof.Desc.IsSynthetic() { + info = synthesizeOneofInfo(oneof) + } if info != nil { discriminatedOneofs = append(discriminatedOneofs, info) } @@ -411,6 +416,25 @@ func GenerateInterface(p Printer, msg *protogen.Message) { } } +// synthesizeOneofInfo builds discriminator info for a oneof that has no explicit +// sebuf.http oneof_config annotation, using "$case" as the discriminator and +// each field's JSON name as the discriminator value. +func synthesizeOneofInfo(oneof *protogen.Oneof) *annotations.OneofDiscriminatorInfo { + info := &annotations.OneofDiscriminatorInfo{ + Oneof: oneof, + Discriminator: "$case", + Flatten: false, + } + for _, field := range oneof.Fields { + info.Variants = append(info.Variants, annotations.OneofVariant{ + Field: field, + DiscriminatorVal: field.Desc.JSONName(), + IsMessage: field.Message != nil, + }) + } + return info +} + // GenerateOneofDiscriminatedUnionType generates a TypeScript discriminated union type for a oneof. func GenerateOneofDiscriminatedUnionType(p Printer, msgName string, info *annotations.OneofDiscriminatorInfo) { unionName := msgName + SnakeToUpperCamel(string(info.Oneof.Desc.Name())) diff --git a/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts b/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts index 7bcdabc0..1677842a 100644 --- a/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts +++ b/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts @@ -36,10 +36,13 @@ export interface VideoContent { duration: number; } +export type PlainEventContent = + | { $case: "text"; text?: TextContent } + | { $case: "image"; image?: ImageContent }; + export interface PlainEvent { id: string; - text?: TextContent; - image?: ImageContent; + content?: PlainEventContent; } export interface FieldViolation { From c90bc96e37c6907979bfa784ce3279c3b7edae46 Mon Sep 17 00:00:00 2001 From: Hicham <53556927+hishamank@users.noreply.github.com> Date: Mon, 6 Jul 2026 15:46:02 +0400 Subject: [PATCH 02/11] fix(ts): render un-annotated oneofs as presence-discriminated flat unions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The synthesized $case discriminated union described JSON that never exists on the wire. Un-annotated oneofs serialize as plain protojson, which puts the set member's JSON key directly on the parent object and emits no discriminator field and no property named after the oneof. The generated clients do raw JSON.stringify / resp.json() casts with no conversion layer, so the $case shape was unreachable at runtime in both directions: reads found undefined where the type promised a wrapper property, and writes sent a "$case" key that protojson.Unmarshal rejects as an unknown field. Un-annotated oneofs now render as a union discriminated by key presence, intersected flat into the parent type — assignable to and from the exact JSON the Go server produces and consumes, with no runtime conversion: export type PlainEventContent = | { text: TextContent; image?: never } | { image: ImageContent; text?: never } | { text?: never; image?: never }; export type PlainEvent = PlainEventBase & PlainEventContent; The `?: never` guards keep the exactly-one construction guarantee and let TypeScript narrow on presence (`if (e.text)`); the trailing all-never arm models the unset oneof, which the server's protojson output permits. Set oneof members are non-optional because oneof fields have explicit presence — protojson always emits a set member, even at its zero value. Messages whose fields all belong to oneofs skip the empty Base interface and alias the union directly. Annotated oneofs (oneof_config) are unchanged: their wire format is produced by generated MarshalJSON/UnmarshalJSON pairs, so their TS shape is a separate concern. Co-Authored-By: Claude Fable 5 --- .../oneof_discriminator_consistency_test.go | 38 ++++-- .../golden/oneof_discriminator_client.ts | 10 +- internal/tscommon/types.go | 123 ++++++++++++++---- .../golden/oneof_discriminator_server.ts | 10 +- 4 files changed, 141 insertions(+), 40 deletions(-) diff --git a/internal/httpgen/oneof_discriminator_consistency_test.go b/internal/httpgen/oneof_discriminator_consistency_test.go index 94ae1261..105bd83a 100644 --- a/internal/httpgen/oneof_discriminator_consistency_test.go +++ b/internal/httpgen/oneof_discriminator_consistency_test.go @@ -75,14 +75,28 @@ func TestOneofDiscriminatorTypeScriptTypes(t *testing.T) { t.Error("TypeScript NestedEvent should have vid variant with kind: \"vid\" (custom oneof_value)") } - // PlainEvent: standard interface (no discriminated union) - if !strings.Contains(tsContent, "export interface PlainEvent") { - t.Error("TypeScript PlainEvent should be a standard interface (no discriminated union)") + // PlainEvent: un-annotated oneof renders as a presence-discriminated flat + // union intersected with the base fields, matching plain protojson (the set + // member's key appears directly on the parent; no discriminator on the wire). + if !strings.Contains(tsContent, "export type PlainEvent = PlainEventBase & PlainEventContent") { + t.Error("TypeScript PlainEvent should intersect its base fields with the presence union") } - // PlainEvent should NOT have a discriminator field + if !strings.Contains(tsContent, `{ text: TextContent; image?: never }`) { + t.Error("TypeScript PlainEventContent should have a presence-discriminated text variant") + } + + if !strings.Contains(tsContent, `{ text?: never; image?: never }`) { + t.Error("TypeScript PlainEventContent should include an all-never arm for the unset oneof") + } + + // PlainEvent should NOT have a discriminator field — protojson emits none + // for oneofs without an oneof_config annotation. + if strings.Contains(tsContent, "$case") { + t.Error("TypeScript PlainEvent should NOT have a synthesized discriminator field") + } plainEventPattern := regexp.MustCompile( - `interface PlainEvent \{[^}]*(?:type|kind)\s*:`, + `PlainEventContent =[^;]*(?:type|kind)\s*:`, ) if plainEventPattern.MatchString(tsContent) { t.Error("TypeScript PlainEvent should NOT have a discriminator field (type or kind)") @@ -347,10 +361,16 @@ func verifyOneofDiscriminatorAbsent( t.Errorf("Go should NOT generate MarshalJSON for %s (no discriminator)", message) } - // TypeScript: should be a standard interface - tsInterfacePattern := regexp.MustCompile(`export interface ` + message + ` \{`) - if !tsInterfacePattern.MatchString(tsStr) { - t.Errorf("TypeScript %s should be a standard interface", message) + // TypeScript: presence-discriminated union intersected with base fields, + // with no discriminator field (matches plain protojson serialization). + tsAliasPattern := regexp.MustCompile( + `export type ` + message + ` = ` + message + `Base & ` + message + `\w+;`, + ) + if !tsAliasPattern.MatchString(tsStr) { + t.Errorf("TypeScript %s should intersect its base fields with the presence union", message) + } + if strings.Contains(tsStr, "$case") { + t.Errorf("TypeScript %s should NOT have a synthesized discriminator field", message) } // OpenAPI: should NOT have discriminator diff --git a/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts b/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts index 1cf80ea9..3831e6a3 100644 --- a/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts +++ b/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts @@ -37,14 +37,16 @@ export interface VideoContent { } export type PlainEventContent = - | { $case: "text"; text?: TextContent } - | { $case: "image"; image?: ImageContent }; + | { text: TextContent; image?: never } + | { image: ImageContent; text?: never } + | { text?: never; image?: never }; -export interface PlainEvent { +export interface PlainEventBase { id: string; - content?: PlainEventContent; } +export type PlainEvent = PlainEventBase & PlainEventContent; + export interface FieldViolation { field: string; description: string; diff --git a/internal/tscommon/types.go b/internal/tscommon/types.go index a1fc28dd..dac078b3 100644 --- a/internal/tscommon/types.go +++ b/internal/tscommon/types.go @@ -395,11 +395,13 @@ func GenerateInterface(p Printer, msg *protogen.Message) { } } - // Check if any are flattened (requires type alias with intersection) - hasFlattenedOneof := false + // Flattened and presence-discriminated (un-annotated) oneofs render their + // members flat on the parent object, which requires the intersection type + // alias instead of a wrapper property. + hasIntersectionOneof := false for _, info := range discriminatedOneofs { - if info.Flatten { - hasFlattenedOneof = true + if info.Flatten || info.Discriminator == "" { + hasIntersectionOneof = true break } } @@ -409,20 +411,23 @@ func GenerateInterface(p Printer, msg *protogen.Message) { GenerateOneofDiscriminatedUnionType(p, name, info) } - if hasFlattenedOneof { + if hasIntersectionOneof { GenerateFlattenedOneofInterface(p, msg, name, discriminatedOneofs) } else { GenerateStandardInterface(p, msg, name, discriminatedOneofs) } } -// synthesizeOneofInfo builds discriminator info for a oneof that has no explicit -// sebuf.http oneof_config annotation, using "$case" as the discriminator and -// each field's JSON name as the discriminator value. +// synthesizeOneofInfo builds oneof info for a oneof that has no explicit +// sebuf.http oneof_config annotation. Such oneofs serialize as plain protojson: +// the set member's JSON key appears directly on the parent object and no +// discriminator field exists on the wire. The synthesized info therefore +// carries an empty Discriminator, and the union is discriminated by key +// presence instead of a discriminator field. func synthesizeOneofInfo(oneof *protogen.Oneof) *annotations.OneofDiscriminatorInfo { info := &annotations.OneofDiscriminatorInfo{ Oneof: oneof, - Discriminator: "$case", + Discriminator: "", Flatten: false, } for _, field := range oneof.Fields { @@ -439,6 +444,11 @@ func synthesizeOneofInfo(oneof *protogen.Oneof) *annotations.OneofDiscriminatorI func GenerateOneofDiscriminatedUnionType(p Printer, msgName string, info *annotations.OneofDiscriminatorInfo) { unionName := msgName + SnakeToUpperCamel(string(info.Oneof.Desc.Name())) + if info.Discriminator == "" { + generatePresenceOneofUnionType(p, unionName, info) + return + } + var branches []string for _, variant := range info.Variants { var branch string @@ -491,8 +501,64 @@ func GenerateOneofDiscriminatedUnionType(p Printer, msgName string, info *annota p("") } -// GenerateFlattenedOneofInterface generates a type alias with intersection for messages -// with flattened discriminated oneofs. +// generatePresenceOneofUnionType renders a oneof that has no wire discriminator +// (no oneof_config annotation) as a union discriminated by key presence, matching +// protojson serialization: exactly the set member's JSON key appears on the parent +// object. Sibling members are typed `?: never` so TypeScript narrows on presence +// and rejects construction with more than one member set; a final all-never arm +// models the unset oneof. +func generatePresenceOneofUnionType(p Printer, unionName string, info *annotations.OneofDiscriminatorInfo) { + jsonNames := make([]string, len(info.Variants)) + tsTypes := make([]string, len(info.Variants)) + for i, variant := range info.Variants { + jsonNames[i] = variant.Field.Desc.JSONName() + if variant.IsMessage { + tsTypes[i] = string(variant.Field.Message.Desc.Name()) + } else { + tsTypes[i] = TSScalarTypeForField(variant.Field) + } + } + + var branches []string + for i := range info.Variants { + var sb strings.Builder + fmt.Fprintf(&sb, "{ %s: %s", jsonNames[i], tsTypes[i]) + for j := range info.Variants { + if j == i { + continue + } + fmt.Fprintf(&sb, "; %s?: never", jsonNames[j]) + } + sb.WriteString(" }") + branches = append(branches, sb.String()) + } + + // Unset arm: proto3 oneofs may have no member set at all. + var sb strings.Builder + sb.WriteString("{ ") + for j, jsonName := range jsonNames { + if j > 0 { + sb.WriteString("; ") + } + fmt.Fprintf(&sb, "%s?: never", jsonName) + } + sb.WriteString(" }") + branches = append(branches, sb.String()) + + p("export type %s =", unionName) + for i, branch := range branches { + if i < len(branches)-1 { + p(" | %s", branch) + } else { + p(" | %s;", branch) + } + } + p("") +} + +// GenerateFlattenedOneofInterface generates a type alias with intersection for +// messages whose oneofs render flat on the parent object (flattened annotated +// oneofs and presence-discriminated un-annotated oneofs). func GenerateFlattenedOneofInterface( p Printer, msg *protogen.Message, @@ -502,24 +568,35 @@ func GenerateFlattenedOneofInterface( // Build set of fields that belong to discriminated oneofs oneofFields := BuildOneofFieldSet(discriminatedOneofs) - // Generate base fields interface - p("export interface %sBase {", name) + // Generate base fields interface, unless every field belongs to a oneof. + hasBaseFields := false for _, field := range msg.Fields { - if oneofFields[field] { - continue + if !oneofFields[field] { + hasBaseFields = true + break } - if annotations.IsFlattenField(field) && field.Message != nil { - prefix := annotations.GetFlattenPrefix(field) - GenerateFlattenedFields(p, field.Message, prefix) - continue + } + + var parts []string + if hasBaseFields { + p("export interface %sBase {", name) + for _, field := range msg.Fields { + if oneofFields[field] { + continue + } + if annotations.IsFlattenField(field) && field.Message != nil { + prefix := annotations.GetFlattenPrefix(field) + GenerateFlattenedFields(p, field.Message, prefix) + continue + } + GenerateFieldDeclaration(p, field) } - GenerateFieldDeclaration(p, field) + p("}") + p("") + parts = append(parts, fmt.Sprintf("%sBase", name)) } - p("}") - p("") // Generate type alias as intersection of base and all discriminated union types - parts := []string{fmt.Sprintf("%sBase", name)} for _, info := range discriminatedOneofs { unionName := name + SnakeToUpperCamel(string(info.Oneof.Desc.Name())) parts = append(parts, unionName) diff --git a/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts b/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts index 1677842a..62bf8a8b 100644 --- a/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts +++ b/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts @@ -37,14 +37,16 @@ export interface VideoContent { } export type PlainEventContent = - | { $case: "text"; text?: TextContent } - | { $case: "image"; image?: ImageContent }; + | { text: TextContent; image?: never } + | { image: ImageContent; text?: never } + | { text?: never; image?: never }; -export interface PlainEvent { +export interface PlainEventBase { id: string; - content?: PlainEventContent; } +export type PlainEvent = PlainEventBase & PlainEventContent; + export interface FieldViolation { field: string; description: string; From 4492be5b9426a7dd5c7142e8c8579f5969fa0021 Mon Sep 17 00:00:00 2001 From: Hicham <53556927+hishamank@users.noreply.github.com> Date: Tue, 7 Jul 2026 13:14:04 +0400 Subject: [PATCH 03/11] test(ts): unit-test presence-union oneof emission Add unit coverage for the un-annotated oneof presence-union emitter and interface routing in tscommon, compiling a hand-built proto3 FileDescriptorProto into a protogen.Plugin to exercise real protogen inputs: - synthesizeOneofInfo: empty Discriminator, one variant per field with JSON names, correct IsMessage for scalar vs message members. - generatePresenceOneofUnionType: set member non-optional plus ?: never sibling guards and a final all-never arm, across a 3-variant message oneof and a scalar+message oneof. - GenerateInterface: base field emits XBase + intersection alias; a message whose fields all belong to one oneof aliases the union directly with no XBase interface. Co-Authored-By: Claude Fable 5 --- internal/tscommon/types_test.go | 360 ++++++++++++++++++++++++++++++++ 1 file changed, 360 insertions(+) diff --git a/internal/tscommon/types_test.go b/internal/tscommon/types_test.go index f8ac95e5..94c8e781 100644 --- a/internal/tscommon/types_test.go +++ b/internal/tscommon/types_test.go @@ -1,12 +1,17 @@ package tscommon import ( + "fmt" "os" "path/filepath" "strings" "testing" + "google.golang.org/protobuf/compiler/protogen" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/types/descriptorpb" + "google.golang.org/protobuf/types/pluginpb" ) func TestTSScalarType(t *testing.T) { @@ -150,3 +155,358 @@ func TestTSEnumUnspecifiedValue_ViaGoldenOutput(t *testing.T) { } }) } + +// --- protogen harness ------------------------------------------------------- +// +// The presence-union emitter and interface routing operate on real +// *protogen.Message / *protogen.Oneof values, which cannot be hand-mocked. +// These helpers compile a hand-built proto3 FileDescriptorProto into a +// *protogen.Plugin so the emitters can be exercised on genuine protogen inputs +// without pulling in a proto compiler. + +const testProtoPkg = "test.oneof.v1" + +// msgFieldProto builds a singular proto3 field descriptor. +func msgFieldProto(name string, number int32, typ descriptorpb.FieldDescriptorProto_Type) *descriptorpb.FieldDescriptorProto { + return &descriptorpb.FieldDescriptorProto{ + Name: proto.String(name), + Number: proto.Int32(number), + Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(), + Type: typ.Enum(), + JsonName: proto.String(jsonName(name)), + } +} + +// msgRefFieldProto builds a singular proto3 message-typed field descriptor. +func msgRefFieldProto(name string, number int32, msgName string) *descriptorpb.FieldDescriptorProto { + f := msgFieldProto(name, number, descriptorpb.FieldDescriptorProto_TYPE_MESSAGE) + f.TypeName = proto.String("." + testProtoPkg + "." + msgName) + return f +} + +// jsonName mirrors protoc's default lowerCamelCase json_name derivation. +func jsonName(name string) string { + parts := strings.Split(name, "_") + for i, part := range parts { + if i > 0 && len(part) > 0 { + parts[i] = strings.ToUpper(part[:1]) + part[1:] + } + } + return strings.Join(parts, "") +} + +// oneofIndex returns a pointer to i (fields reference their oneof by index). +func oneofIndex(i int32) *int32 { return proto.Int32(i) } + +// buildTestPlugin compiles the descriptor into a *protogen.Plugin. +func buildTestPlugin(t *testing.T, fd *descriptorpb.FileDescriptorProto) *protogen.Plugin { + t.Helper() + req := &pluginpb.CodeGeneratorRequest{ + FileToGenerate: []string{fd.GetName()}, + ProtoFile: []*descriptorpb.FileDescriptorProto{fd}, + } + plugin, err := protogen.Options{}.New(req) + if err != nil { + t.Fatalf("protogen.Options{}.New: %v", err) + } + return plugin +} + +// findMessage returns the top-level message with the given name. +func findMessage(t *testing.T, plugin *protogen.Plugin, name string) *protogen.Message { + t.Helper() + for _, file := range plugin.Files { + for _, msg := range file.Messages { + if string(msg.Desc.Name()) == name { + return msg + } + } + } + t.Fatalf("message %q not found in plugin", name) + return nil +} + +// findOneof returns the named oneof on a message. +func findOneof(t *testing.T, msg *protogen.Message, name string) *protogen.Oneof { + t.Helper() + for _, oneof := range msg.Oneofs { + if string(oneof.Desc.Name()) == name { + return oneof + } + } + t.Fatalf("oneof %q not found on message %q", name, msg.Desc.Name()) + return nil +} + +// capturePrinter returns a Printer that accumulates emitted lines and the +// buffer holding them. +func capturePrinter() (Printer, *strings.Builder) { + var sb strings.Builder + p := func(format string, args ...interface{}) { + fmt.Fprintf(&sb, format+"\n", args...) + } + return p, &sb +} + +// oneofTestFile builds a proto3 file exercising the un-annotated oneof shapes +// the presence-union emitter targets. +func oneofTestFile() *descriptorpb.FileDescriptorProto { + str := descriptorpb.FieldDescriptorProto_TYPE_STRING + msg := func(name string, fields []*descriptorpb.FieldDescriptorProto, oneofs []*descriptorpb.OneofDescriptorProto) *descriptorpb.DescriptorProto { + return &descriptorpb.DescriptorProto{ + Name: proto.String(name), + Field: fields, + OneofDecl: oneofs, + } + } + oneof := func(name string) *descriptorpb.OneofDescriptorProto { + return &descriptorpb.OneofDescriptorProto{Name: proto.String(name)} + } + + textContent := msg("TextContent", []*descriptorpb.FieldDescriptorProto{ + msgFieldProto("text", 1, str), + }, nil) + imageContent := msg("ImageContent", []*descriptorpb.FieldDescriptorProto{ + msgFieldProto("url", 1, str), + }, nil) + videoContent := msg("VideoContent", []*descriptorpb.FieldDescriptorProto{ + msgFieldProto("url", 1, str), + }, nil) + + // Base field + two-message oneof. + plainEvent := msg("PlainEvent", + []*descriptorpb.FieldDescriptorProto{ + msgFieldProto("id", 1, str), + func() *descriptorpb.FieldDescriptorProto { + f := msgRefFieldProto("text", 2, "TextContent") + f.OneofIndex = oneofIndex(0) + return f + }(), + func() *descriptorpb.FieldDescriptorProto { + f := msgRefFieldProto("image", 3, "ImageContent") + f.OneofIndex = oneofIndex(0) + return f + }(), + }, + []*descriptorpb.OneofDescriptorProto{oneof("content")}, + ) + + // Every field belongs to a single oneof (no base field). + allOneofEvent := msg("AllOneofEvent", + []*descriptorpb.FieldDescriptorProto{ + func() *descriptorpb.FieldDescriptorProto { + f := msgRefFieldProto("text", 1, "TextContent") + f.OneofIndex = oneofIndex(0) + return f + }(), + func() *descriptorpb.FieldDescriptorProto { + f := msgRefFieldProto("image", 2, "ImageContent") + f.OneofIndex = oneofIndex(0) + return f + }(), + }, + []*descriptorpb.OneofDescriptorProto{oneof("body")}, + ) + + // Three-variant message oneof. + threeVariantEvent := msg("ThreeVariantEvent", + []*descriptorpb.FieldDescriptorProto{ + func() *descriptorpb.FieldDescriptorProto { + f := msgRefFieldProto("text", 1, "TextContent") + f.OneofIndex = oneofIndex(0) + return f + }(), + func() *descriptorpb.FieldDescriptorProto { + f := msgRefFieldProto("image", 2, "ImageContent") + f.OneofIndex = oneofIndex(0) + return f + }(), + func() *descriptorpb.FieldDescriptorProto { + f := msgRefFieldProto("video", 3, "VideoContent") + f.OneofIndex = oneofIndex(0) + return f + }(), + }, + []*descriptorpb.OneofDescriptorProto{oneof("pick")}, + ) + + // Oneof mixing a scalar member (with a snake_case name) and a message member. + scalarOneofEvent := msg("ScalarOneofEvent", + []*descriptorpb.FieldDescriptorProto{ + func() *descriptorpb.FieldDescriptorProto { + f := msgFieldProto("raw_text", 1, str) + f.OneofIndex = oneofIndex(0) + return f + }(), + func() *descriptorpb.FieldDescriptorProto { + f := msgRefFieldProto("detail", 2, "TextContent") + f.OneofIndex = oneofIndex(0) + return f + }(), + }, + []*descriptorpb.OneofDescriptorProto{oneof("value")}, + ) + + return &descriptorpb.FileDescriptorProto{ + Name: proto.String("oneof_harness.proto"), + Package: proto.String(testProtoPkg), + Syntax: proto.String("proto3"), + Options: &descriptorpb.FileOptions{ + GoPackage: proto.String("github.com/SebastienMelki/sebuf/internal/tscommon/testoneofv1"), + }, + MessageType: []*descriptorpb.DescriptorProto{ + textContent, imageContent, videoContent, + plainEvent, allOneofEvent, threeVariantEvent, scalarOneofEvent, + }, + } +} + +func TestSynthesizeOneofInfo(t *testing.T) { + plugin := buildTestPlugin(t, oneofTestFile()) + + t.Run("message_variants", func(t *testing.T) { + msg := findMessage(t, plugin, "PlainEvent") + info := synthesizeOneofInfo(findOneof(t, msg, "content")) + + if info.Discriminator != "" { + t.Errorf("Discriminator = %q, want empty", info.Discriminator) + } + if info.Flatten { + t.Error("Flatten = true, want false") + } + if len(info.Variants) != 2 { + t.Fatalf("len(Variants) = %d, want 2", len(info.Variants)) + } + wantJSON := []string{"text", "image"} + for i, v := range info.Variants { + if v.DiscriminatorVal != wantJSON[i] { + t.Errorf("Variants[%d].DiscriminatorVal = %q, want %q", i, v.DiscriminatorVal, wantJSON[i]) + } + if v.Field.Desc.JSONName() != wantJSON[i] { + t.Errorf("Variants[%d] JSONName = %q, want %q", i, v.Field.Desc.JSONName(), wantJSON[i]) + } + if !v.IsMessage { + t.Errorf("Variants[%d].IsMessage = false, want true", i) + } + } + }) + + t.Run("scalar_variant_json_name_and_ismessage", func(t *testing.T) { + msg := findMessage(t, plugin, "ScalarOneofEvent") + info := synthesizeOneofInfo(findOneof(t, msg, "value")) + + if len(info.Variants) != 2 { + t.Fatalf("len(Variants) = %d, want 2", len(info.Variants)) + } + // raw_text is a scalar member; its JSON name is camelCased. + if got := info.Variants[0].DiscriminatorVal; got != "rawText" { + t.Errorf("Variants[0].DiscriminatorVal = %q, want %q", got, "rawText") + } + if info.Variants[0].IsMessage { + t.Error("scalar variant IsMessage = true, want false") + } + // detail is a message member. + if got := info.Variants[1].DiscriminatorVal; got != "detail" { + t.Errorf("Variants[1].DiscriminatorVal = %q, want %q", got, "detail") + } + if !info.Variants[1].IsMessage { + t.Error("message variant IsMessage = false, want true") + } + }) +} + +func TestGeneratePresenceOneofUnionType(t *testing.T) { + plugin := buildTestPlugin(t, oneofTestFile()) + + tests := []struct { + name string + message string + oneof string + unionName string + want string + }{ + { + name: "three_variant_message_oneof", + message: "ThreeVariantEvent", + oneof: "pick", + unionName: "ThreeVariantEventPick", + want: `export type ThreeVariantEventPick = + | { text: TextContent; image?: never; video?: never } + | { image: ImageContent; text?: never; video?: never } + | { video: VideoContent; text?: never; image?: never } + | { text?: never; image?: never; video?: never }; + +`, + }, + { + name: "scalar_and_message_oneof", + message: "ScalarOneofEvent", + oneof: "value", + unionName: "ScalarOneofEventValue", + want: `export type ScalarOneofEventValue = + | { rawText: string; detail?: never } + | { detail: TextContent; rawText?: never } + | { rawText?: never; detail?: never }; + +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msg := findMessage(t, plugin, tt.message) + info := synthesizeOneofInfo(findOneof(t, msg, tt.oneof)) + p, sb := capturePrinter() + generatePresenceOneofUnionType(p, tt.unionName, info) + if sb.String() != tt.want { + t.Errorf("generatePresenceOneofUnionType output mismatch:\n got:\n%s\nwant:\n%s", sb.String(), tt.want) + } + }) + } +} + +func TestGenerateInterface_PresenceOneof(t *testing.T) { + plugin := buildTestPlugin(t, oneofTestFile()) + + t.Run("base_field_emits_base_interface_and_intersection", func(t *testing.T) { + msg := findMessage(t, plugin, "PlainEvent") + p, sb := capturePrinter() + GenerateInterface(p, msg) + out := sb.String() + + want := `export type PlainEventContent = + | { text: TextContent; image?: never } + | { image: ImageContent; text?: never } + | { text?: never; image?: never }; + +export interface PlainEventBase { + id: string; +} + +export type PlainEvent = PlainEventBase & PlainEventContent; + +` + if out != want { + t.Errorf("GenerateInterface(PlainEvent) mismatch:\n got:\n%s\nwant:\n%s", out, want) + } + }) + + t.Run("all_fields_in_oneof_skips_base_interface", func(t *testing.T) { + msg := findMessage(t, plugin, "AllOneofEvent") + p, sb := capturePrinter() + GenerateInterface(p, msg) + out := sb.String() + + // No XBase interface when every field belongs to the oneof. + if strings.Contains(out, "AllOneofEventBase") { + t.Errorf("expected no AllOneofEventBase interface, got:\n%s", out) + } + if strings.Contains(out, "export interface AllOneofEvent") { + t.Errorf("expected no standalone AllOneofEvent interface, got:\n%s", out) + } + // The alias points directly at the union. + if !strings.Contains(out, "export type AllOneofEvent = AllOneofEventBody;") { + t.Errorf("expected direct union alias, got:\n%s", out) + } + }) +} From eee1e1465923b1c8756f2117b059e22fb75f93fd Mon Sep 17 00:00:00 2001 From: Hicham <53556927+hishamank@users.noreply.github.com> Date: Tue, 7 Jul 2026 12:37:42 +0300 Subject: [PATCH 04/11] fix(ts): flatten annotated non-flatten oneofs to match the wire format (#201) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ts): flatten annotated non-flatten oneofs to match the wire format The TS generators rendered annotated non-flatten oneofs as a wrapper property named after the oneof: export interface NestedEvent { id: string; content?: { kind: "text"; text?: TextContent } | ...; } but no such property exists on the wire. The generated MarshalJSON writes the discriminator directly onto the parent object (raw["kind"] = "text") and leaves the variant key where protojson put it — also on the parent — so the actual JSON is {"id": ..., "kind": "text", "text": {...}}. The OpenAPI generator already documents this flat shape; TypeScript was the only generator disagreeing with the Go serialization. Non-flatten annotated oneofs now render flat, intersected with the base fields like flattened ones: export type NestedEventContent = | { kind: "text"; text: TextContent; image?: never; video?: never } | { kind: "image"; image: ImageContent; text?: never; video?: never } | { kind: "vid"; video: VideoContent; text?: never; image?: never } | { kind?: never; text?: never; image?: never; video?: never }; export type NestedEvent = NestedEventBase & NestedEventContent; Payloads are non-optional because oneof fields have explicit presence — a set member is always emitted. Sibling variant keys are `?: never` so narrowing on the discriminator also types the payload keys correctly, and the all-never arm models the unset oneof, which MarshalJSON emits with no discriminator at all. Flattened oneofs gain the same unset arm ({ type?: never }) for that reason. With every oneof now rendering flat on the parent, the wrapper-property branch in GenerateStandardInterface is unreachable and removed — which also removes the bug where the wrapper property kept the oneof's raw snake_case name (e.g. `super_title_image?:`) instead of a camelCase one. Co-Authored-By: Claude Fable 5 * test(ts): cover multi-word oneof names in TS oneof emission Add a TS-only fixture (multi_word_oneof.proto) whose un-annotated oneof has a multi-word name (super_title_image), exercising the presence-union path in tscommon. This locks in the regression fixed on this branch: the oneof name must surface only as the PascalCase union type name (MultiWordEventSuperTitleImage) and never leak into the generated TS as a raw snake_case wrapper property. The fixture is scoped to the ts-client and ts-server generators only, leaving the shared oneof_discriminator.proto byte-identical across all six generators so the Go/OpenAPI/Python goldens and cross-generator consistency tests are untouched. - New golden test cases in tsclientgen/tsservergen; the new proto is added to the tsservergen cross-generator consistency proto list. - Explicit regression assertion (TestMultiWordOneofNameDoesNotLeak): generated TS contains MultiWordEventSuperTitleImage and not super_title_image. - Unit tests for nonFlattenedOneofBranch in tscommon (message, middle, and scalar variants), asserting discriminator/value/payload/never-guards. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- .../oneof_discriminator_consistency_test.go | 13 ++ internal/tsclientgen/golden_test.go | 37 ++++++ .../golden/multi_word_oneof_client.ts | 112 ++++++++++++++++ .../golden/oneof_discriminator_client.ts | 15 ++- .../testdata/proto/multi_word_oneof.proto | 46 +++++++ internal/tscommon/types.go | 124 ++++++++++-------- internal/tscommon/types_test.go | 35 +++++ internal/tsservergen/consistency_test.go | 1 + internal/tsservergen/golden_test.go | 7 + .../golden/multi_word_oneof_server.ts | 123 +++++++++++++++++ .../golden/oneof_discriminator_server.ts | 15 ++- .../testdata/proto/multi_word_oneof.proto | 46 +++++++ 12 files changed, 510 insertions(+), 64 deletions(-) create mode 100644 internal/tsclientgen/testdata/golden/multi_word_oneof_client.ts create mode 100644 internal/tsclientgen/testdata/proto/multi_word_oneof.proto create mode 100644 internal/tsservergen/testdata/golden/multi_word_oneof_server.ts create mode 100644 internal/tsservergen/testdata/proto/multi_word_oneof.proto diff --git a/internal/httpgen/oneof_discriminator_consistency_test.go b/internal/httpgen/oneof_discriminator_consistency_test.go index 105bd83a..aa18d774 100644 --- a/internal/httpgen/oneof_discriminator_consistency_test.go +++ b/internal/httpgen/oneof_discriminator_consistency_test.go @@ -75,6 +75,19 @@ func TestOneofDiscriminatorTypeScriptTypes(t *testing.T) { t.Error("TypeScript NestedEvent should have vid variant with kind: \"vid\" (custom oneof_value)") } + // NestedEvent variants sit flat on the parent, matching the generated + // MarshalJSON: discriminator and the set variant's key are both emitted + // directly on the parent object, with no wrapper property for the oneof. + if !strings.Contains(tsContent, "export type NestedEvent = NestedEventBase & NestedEventContent") { + t.Error("TypeScript NestedEvent should intersect its base fields with the oneof union") + } + if !strings.Contains(tsContent, `{ kind: "text"; text: TextContent; image?: never; video?: never }`) { + t.Error("TypeScript NestedEvent text variant should carry a non-optional payload with never guards") + } + if !strings.Contains(tsContent, `{ kind?: never; text?: never; image?: never; video?: never }`) { + t.Error("TypeScript NestedEventContent should include an all-never arm for the unset oneof") + } + // PlainEvent: un-annotated oneof renders as a presence-discriminated flat // union intersected with the base fields, matching plain protojson (the set // member's key appears directly on the parent; no discriminator on the wire). diff --git a/internal/tsclientgen/golden_test.go b/internal/tsclientgen/golden_test.go index 0520b32d..ed8370fd 100644 --- a/internal/tsclientgen/golden_test.go +++ b/internal/tsclientgen/golden_test.go @@ -116,6 +116,13 @@ func TestTSClientGenGoldenFiles(t *testing.T) { "oneof_discriminator_client.ts", }, }, + { + name: "multi-word oneof name", + protoFile: "multi_word_oneof.proto", + expectedFiles: []string{ + "multi_word_oneof_client.ts", + }, + }, { name: "SSE streaming", protoFile: "sse.proto", @@ -208,6 +215,36 @@ func TestTSClientGenGoldenFiles(t *testing.T) { } } +// TestMultiWordOneofNameDoesNotLeak asserts the regression fixed on this branch: +// a multi-word oneof name (super_title_image) must surface only as the PascalCase +// union type name and never leak into the generated TypeScript as a raw +// snake_case wrapper property. See internal/tscommon/types.go +// (GenerateOneofDiscriminatedUnionType / GenerateStandardInterface). +func TestMultiWordOneofNameDoesNotLeak(t *testing.T) { + wd, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get working directory: %v", err) + } + goldenPath := filepath.Join(wd, "testdata", "golden", "multi_word_oneof_client.ts") + + content, readErr := os.ReadFile(goldenPath) + if readErr != nil { + t.Fatalf("Failed to read golden file %s: %v", goldenPath, readErr) + } + ts := string(content) + + // The oneof name renders as the PascalCase discriminated-union type name. + if !strings.Contains(ts, "MultiWordEventSuperTitleImage") { + t.Error("expected generated TS to contain the PascalCase union type MultiWordEventSuperTitleImage") + } + + // The raw snake_case oneof name must never appear: no wrapper property such + // as `super_title_image?:` leaks onto the message interface. + if strings.Contains(ts, "super_title_image") { + t.Error("generated TS must not contain the raw snake_case oneof name super_title_image") + } +} + func updateGoldenFile(t *testing.T, goldenPath string, content []byte) { t.Helper() writeErr := os.WriteFile(goldenPath, content, 0o644) diff --git a/internal/tsclientgen/testdata/golden/multi_word_oneof_client.ts b/internal/tsclientgen/testdata/golden/multi_word_oneof_client.ts new file mode 100644 index 00000000..b62d656f --- /dev/null +++ b/internal/tsclientgen/testdata/golden/multi_word_oneof_client.ts @@ -0,0 +1,112 @@ +// Code generated by protoc-gen-ts-client. DO NOT EDIT. +// source: multi_word_oneof.proto + +export type MultiWordEventSuperTitleImage = + | { bigText: TextContent; bigImage?: never } + | { bigImage: ImageContent; bigText?: never } + | { bigText?: never; bigImage?: never }; + +export interface MultiWordEventBase { + id: string; +} + +export type MultiWordEvent = MultiWordEventBase & MultiWordEventSuperTitleImage; + +export interface TextContent { + body: string; +} + +export interface ImageContent { + url: string; + width: number; + height: number; +} + +export interface FieldViolation { + field: string; + description: string; +} + +export class ValidationError extends Error { + violations: FieldViolation[]; + + constructor(violations: FieldViolation[]) { + super("Validation failed"); + this.name = "ValidationError"; + this.violations = violations; + } +} + +export class ApiError extends Error { + statusCode: number; + body: string; + + constructor(statusCode: number, message: string, body: string) { + super(message); + this.name = "ApiError"; + this.statusCode = statusCode; + this.body = body; + } +} + +export interface MultiWordOneofServiceClientOptions { + fetch?: typeof fetch; + defaultHeaders?: Record; +} + +export interface MultiWordOneofServiceCallOptions { + headers?: Record; + signal?: AbortSignal; +} + +export class MultiWordOneofServiceClient { + private baseURL: string; + private fetchFn: typeof fetch; + private defaultHeaders: Record; + + constructor(baseURL: string, options?: MultiWordOneofServiceClientOptions) { + this.baseURL = baseURL.replace(/\/+$/, ""); + this.fetchFn = options?.fetch ?? globalThis.fetch; + this.defaultHeaders = { ...options?.defaultHeaders }; + } + + async testMultiWordEvent(req: MultiWordEvent, options?: MultiWordOneofServiceCallOptions): Promise { + let path = "/api/v1/events/multi-word"; + const url = this.baseURL + path; + + const headers: Record = { + "Content-Type": "application/json", + ...this.defaultHeaders, + ...options?.headers, + }; + + const resp = await this.fetchFn(url, { + method: "POST", + headers, + body: JSON.stringify(req), + signal: options?.signal, + }); + + if (!resp.ok) { + return this.handleError(resp); + } + + return await resp.json() as MultiWordEvent; + } + + private async handleError(resp: Response): Promise { + const body = await resp.text(); + if (resp.status === 400) { + try { + const parsed = JSON.parse(body); + if (parsed.violations) { + throw new ValidationError(parsed.violations); + } + } catch (e) { + if (e instanceof ValidationError) throw e; + } + } + throw new ApiError(resp.status, `Request failed with status ${resp.status}`, body); + } +} + diff --git a/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts b/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts index 3831e6a3..7d84f2c4 100644 --- a/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts +++ b/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts @@ -3,7 +3,8 @@ export type FlattenedEventContent = | { type: "text"; body: string } - | { type: "img"; url: string; width: number; height: number }; + | { type: "img"; url: string; width: number; height: number } + | { type?: never }; export interface FlattenedEventBase { id: string; @@ -22,15 +23,17 @@ export interface ImageContent { } export type NestedEventContent = - | { kind: "text"; text?: TextContent } - | { kind: "image"; image?: ImageContent } - | { kind: "vid"; video?: VideoContent }; + | { kind: "text"; text: TextContent; image?: never; video?: never } + | { kind: "image"; image: ImageContent; text?: never; video?: never } + | { kind: "vid"; video: VideoContent; text?: never; image?: never } + | { kind?: never; text?: never; image?: never; video?: never }; -export interface NestedEvent { +export interface NestedEventBase { id: string; - content?: NestedEventContent; } +export type NestedEvent = NestedEventBase & NestedEventContent; + export interface VideoContent { url: string; duration: number; diff --git a/internal/tsclientgen/testdata/proto/multi_word_oneof.proto b/internal/tsclientgen/testdata/proto/multi_word_oneof.proto new file mode 100644 index 00000000..0675197d --- /dev/null +++ b/internal/tsclientgen/testdata/proto/multi_word_oneof.proto @@ -0,0 +1,46 @@ +syntax = "proto3"; + +package testdata.multi_word_oneof; + +option go_package = "github.com/SebastienMelki/sebuf/internal/tsclientgen/testdata/multiwordoneof;multiwordoneof"; + +import "sebuf/http/annotations.proto"; + +// Variant message types +message TextContent { + string body = 1; +} + +message ImageContent { + string url = 1; + int32 width = 2; + int32 height = 3; +} + +// Message with a multi-word oneof name and no oneof_config annotation. +// +// The oneof name (super_title_image) is deliberately multi-word: it must +// surface in the generated TypeScript only as the PascalCase union type name +// (MultiWordEventSuperTitleImage) and must NEVER leak as a raw snake_case +// wrapper property. Because the oneof is un-annotated it takes the +// presence-discriminated flat-union path, matching plain protojson. +message MultiWordEvent { + string id = 1; + oneof super_title_image { + TextContent big_text = 2; + ImageContent big_image = 3; + } +} + +service MultiWordOneofService { + option (sebuf.http.service_config) = { + base_path: "/api/v1" + }; + + rpc TestMultiWordEvent(MultiWordEvent) returns (MultiWordEvent) { + option (sebuf.http.config) = { + path: "/events/multi-word" + method: HTTP_METHOD_POST + }; + } +} diff --git a/internal/tscommon/types.go b/internal/tscommon/types.go index dac078b3..19c35a01 100644 --- a/internal/tscommon/types.go +++ b/internal/tscommon/types.go @@ -395,23 +395,16 @@ func GenerateInterface(p Printer, msg *protogen.Message) { } } - // Flattened and presence-discriminated (un-annotated) oneofs render their - // members flat on the parent object, which requires the intersection type - // alias instead of a wrapper property. - hasIntersectionOneof := false - for _, info := range discriminatedOneofs { - if info.Flatten || info.Discriminator == "" { - hasIntersectionOneof = true - break - } - } - // Generate discriminated union types before the message for _, info := range discriminatedOneofs { GenerateOneofDiscriminatedUnionType(p, name, info) } - if hasIntersectionOneof { + // Every oneof union renders its members flat on the parent object — that is + // where the wire puts them (protojson for un-annotated oneofs, the generated + // MarshalJSON for annotated ones) — so any discriminated oneof requires the + // intersection type alias instead of a wrapper property. + if len(discriminatedOneofs) > 0 { GenerateFlattenedOneofInterface(p, msg, name, discriminatedOneofs) } else { GenerateStandardInterface(p, msg, name, discriminatedOneofs) @@ -449,8 +442,13 @@ func GenerateOneofDiscriminatedUnionType(p Printer, msgName string, info *annota return } + jsonNames := make([]string, len(info.Variants)) + for i, variant := range info.Variants { + jsonNames[i] = variant.Field.Desc.JSONName() + } + var branches []string - for _, variant := range info.Variants { + for i, variant := range info.Variants { var branch string switch { case info.Flatten && variant.IsMessage: @@ -465,31 +463,46 @@ func GenerateOneofDiscriminatedUnionType(p Printer, msgName string, info *annota branch += sb.String() branch += " }" case variant.IsMessage: - // Non-flattened message: { discriminator: "value", fieldName?: MessageType } - fieldJSONName := variant.Field.Desc.JSONName() - msgType := string(variant.Field.Message.Desc.Name()) - branch = fmt.Sprintf( - "{ %s: \"%s\"; %s?: %s }", - info.Discriminator, - variant.DiscriminatorVal, - fieldJSONName, - msgType, + // Non-flattened message. The generated MarshalJSON keeps the variant's + // JSON key where protojson put it — directly on the parent object, + // next to the discriminator — so the arm carries both, with sibling + // variant keys typed `?: never` for presence narrowing. + branch = nonFlattenedOneofBranch( + info, variant.DiscriminatorVal, jsonNames, i, + string(variant.Field.Message.Desc.Name()), ) default: - // Non-flattened scalar: { discriminator: "value", fieldName?: scalarType } - fieldJSONName := variant.Field.Desc.JSONName() - tsType := TSScalarTypeForField(variant.Field) - branch = fmt.Sprintf( - "{ %s: \"%s\"; %s?: %s }", - info.Discriminator, - variant.DiscriminatorVal, - fieldJSONName, - tsType, - ) + // Non-flattened scalar: same flat shape with the scalar's TS type. + if info.Flatten { + branch = fmt.Sprintf( + "{ %s: \"%s\"; %s?: %s }", + info.Discriminator, + variant.DiscriminatorVal, + jsonNames[i], + TSScalarTypeForField(variant.Field), + ) + } else { + branch = nonFlattenedOneofBranch( + info, variant.DiscriminatorVal, jsonNames, i, + TSScalarTypeForField(variant.Field), + ) + } } branches = append(branches, branch) } + // Unset arm: proto3 oneofs may have no member set, in which case the + // generated MarshalJSON omits the discriminator entirely. + var unset strings.Builder + fmt.Fprintf(&unset, "{ %s?: never", info.Discriminator) + if !info.Flatten { + for _, jsonName := range jsonNames { + fmt.Fprintf(&unset, "; %s?: never", jsonName) + } + } + unset.WriteString(" }") + branches = append(branches, unset.String()) + p("export type %s =", unionName) for i, branch := range branches { if i < len(branches)-1 { @@ -501,6 +514,29 @@ func GenerateOneofDiscriminatedUnionType(p Printer, msgName string, info *annota p("") } +// nonFlattenedOneofBranch renders one arm of a non-flattened annotated oneof +// union: discriminator, the set variant's key with a non-optional payload +// (oneof fields have explicit presence, so a set member is always emitted), +// and `?: never` guards for the sibling variant keys. +func nonFlattenedOneofBranch( + info *annotations.OneofDiscriminatorInfo, + discriminatorVal string, + jsonNames []string, + index int, + tsType string, +) string { + var sb strings.Builder + fmt.Fprintf(&sb, "{ %s: \"%s\"; %s: %s", info.Discriminator, discriminatorVal, jsonNames[index], tsType) + for j := range jsonNames { + if j == index { + continue + } + fmt.Fprintf(&sb, "; %s?: never", jsonNames[j]) + } + sb.WriteString(" }") + return sb.String() +} + // generatePresenceOneofUnionType renders a oneof that has no wire discriminator // (no oneof_config annotation) as a union discriminated by key presence, matching // protojson serialization: exactly the set member's JSON key appears on the parent @@ -605,8 +641,10 @@ func GenerateFlattenedOneofInterface( p("") } -// GenerateStandardInterface generates a standard interface, handling non-flattened -// discriminated oneofs as optional union properties. +// GenerateStandardInterface generates a standard interface for messages without +// discriminated oneofs. Fields belonging to discriminated oneofs are skipped; +// messages that have any render through GenerateFlattenedOneofInterface instead, +// since every oneof union sits flat on the parent object. func GenerateStandardInterface( p Printer, msg *protogen.Message, @@ -616,27 +654,9 @@ func GenerateStandardInterface( // Build set of fields that belong to discriminated oneofs oneofFields := BuildOneofFieldSet(discriminatedOneofs) - // Build map of oneof -> union type name for non-flattened - oneofUnionNames := make(map[*protogen.Oneof]string) - for _, info := range discriminatedOneofs { - unionName := name + SnakeToUpperCamel(string(info.Oneof.Desc.Name())) - oneofUnionNames[info.Oneof] = unionName - } - p("export interface %s {", name) - // Track which oneofs we've already emitted - emittedOneofs := make(map[*protogen.Oneof]bool) - for _, field := range msg.Fields { if oneofFields[field] { - // For discriminated oneof fields, emit the union type once for the oneof - if field.Oneof != nil && !emittedOneofs[field.Oneof] { - if unionName, ok := oneofUnionNames[field.Oneof]; ok { - oneofJSONName := string(field.Oneof.Desc.Name()) - p(" %s?: %s;", oneofJSONName, unionName) - emittedOneofs[field.Oneof] = true - } - } continue } diff --git a/internal/tscommon/types_test.go b/internal/tscommon/types_test.go index 94c8e781..482499ff 100644 --- a/internal/tscommon/types_test.go +++ b/internal/tscommon/types_test.go @@ -12,6 +12,8 @@ import ( "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/types/descriptorpb" "google.golang.org/protobuf/types/pluginpb" + + "github.com/SebastienMelki/sebuf/internal/annotations" ) func TestTSScalarType(t *testing.T) { @@ -82,6 +84,39 @@ func TestTSZeroCheck(t *testing.T) { } } +// TestNonFlattenedOneofBranch verifies the arm rendered for a single variant of a +// non-flattened annotated oneof: the discriminator with its value, the set +// variant's key carrying a non-optional payload, and `?: never` guards for every +// sibling variant key. nonFlattenedOneofBranch reads only info.Discriminator, so +// it can be exercised directly without a *protogen.Oneof. +func TestNonFlattenedOneofBranch(t *testing.T) { + // Three message variants keyed by their JSON names; render the "text" arm. + info := &annotations.OneofDiscriminatorInfo{Discriminator: "kind"} + jsonNames := []string{"text", "image", "video"} + + got := nonFlattenedOneofBranch(info, "text", jsonNames, 0, "TextContent") + want := `{ kind: "text"; text: TextContent; image?: never; video?: never }` + if got != want { + t.Errorf("nonFlattenedOneofBranch message variant:\n got: %s\n want: %s", got, want) + } + + // A middle variant guards both its siblings (before and after its index). + gotMid := nonFlattenedOneofBranch(info, "image", jsonNames, 1, "ImageContent") + wantMid := `{ kind: "image"; image: ImageContent; text?: never; video?: never }` + if gotMid != wantMid { + t.Errorf("nonFlattenedOneofBranch middle variant:\n got: %s\n want: %s", gotMid, wantMid) + } + + // A scalar variant: the discriminator value, the field name, and the scalar + // TS type land correctly, with the sibling key guarded. + scalarInfo := &annotations.OneofDiscriminatorInfo{Discriminator: "type"} + gotScalar := nonFlattenedOneofBranch(scalarInfo, "count", []string{"count", "label"}, 0, "number") + wantScalar := `{ type: "count"; count: number; label?: never }` + if gotScalar != wantScalar { + t.Errorf("nonFlattenedOneofBranch scalar variant:\n got: %s\n want: %s", gotScalar, wantScalar) + } +} + // readGoldenFile reads a golden file relative to the project root and returns its content. func readGoldenFile(t *testing.T, projectRoot, relPath string) string { t.Helper() diff --git a/internal/tsservergen/consistency_test.go b/internal/tsservergen/consistency_test.go index 9340c186..b1184ef6 100644 --- a/internal/tsservergen/consistency_test.go +++ b/internal/tsservergen/consistency_test.go @@ -30,6 +30,7 @@ func TestCrossGeneratorTypeConsistency(t *testing.T) { "bytes_encoding.proto", "flatten.proto", "oneof_discriminator.proto", + "multi_word_oneof.proto", } baseDir, err := os.Getwd() diff --git a/internal/tsservergen/golden_test.go b/internal/tsservergen/golden_test.go index 27cc0260..1a7ce015 100644 --- a/internal/tsservergen/golden_test.go +++ b/internal/tsservergen/golden_test.go @@ -123,6 +123,13 @@ func TestTSServerGenGoldenFiles(t *testing.T) { "oneof_discriminator_server.ts", }, }, + { + name: "multi-word oneof name", + protoFile: "multi_word_oneof.proto", + expectedFiles: []string{ + "multi_word_oneof_server.ts", + }, + }, { name: "SSE streaming", protoFile: "sse.proto", diff --git a/internal/tsservergen/testdata/golden/multi_word_oneof_server.ts b/internal/tsservergen/testdata/golden/multi_word_oneof_server.ts new file mode 100644 index 00000000..9c15ec9c --- /dev/null +++ b/internal/tsservergen/testdata/golden/multi_word_oneof_server.ts @@ -0,0 +1,123 @@ +// Code generated by protoc-gen-ts-server. DO NOT EDIT. +// source: multi_word_oneof.proto + +export type MultiWordEventSuperTitleImage = + | { bigText: TextContent; bigImage?: never } + | { bigImage: ImageContent; bigText?: never } + | { bigText?: never; bigImage?: never }; + +export interface MultiWordEventBase { + id: string; +} + +export type MultiWordEvent = MultiWordEventBase & MultiWordEventSuperTitleImage; + +export interface TextContent { + body: string; +} + +export interface ImageContent { + url: string; + width: number; + height: number; +} + +export interface FieldViolation { + field: string; + description: string; +} + +export class ValidationError extends Error { + violations: FieldViolation[]; + + constructor(violations: FieldViolation[]) { + super("Validation failed"); + this.name = "ValidationError"; + this.violations = violations; + } +} + +export class ApiError extends Error { + statusCode: number; + body: string; + + constructor(statusCode: number, message: string, body: string) { + super(message); + this.name = "ApiError"; + this.statusCode = statusCode; + this.body = body; + } +} + +export interface ServerContext { + request: Request; + pathParams: Record; + headers: Record; +} + +export interface ServerOptions { + onError?: (error: unknown, req: Request) => Response | Promise; + validateRequest?: (methodName: string, body: unknown) => FieldViolation[] | undefined; +} + +export interface RouteDescriptor { + method: string; + path: string; + handler: (req: Request) => Promise; +} + +export interface MultiWordOneofServiceHandler { + testMultiWordEvent(ctx: ServerContext, req: MultiWordEvent): Promise; +} + +export function createMultiWordOneofServiceRoutes( + handler: MultiWordOneofServiceHandler, + options?: ServerOptions, +): RouteDescriptor[] { + return [ + { + method: "POST", + path: "/api/v1/events/multi-word", + handler: async (req: Request): Promise => { + try { + const pathParams: Record = {}; + const body = await req.json() as MultiWordEvent; + if (options?.validateRequest) { + const bodyViolations = options.validateRequest("testMultiWordEvent", body); + if (bodyViolations) { + throw new ValidationError(bodyViolations); + } + } + + const ctx: ServerContext = { + request: req, + pathParams, + headers: Object.fromEntries(req.headers.entries()), + }; + + const result = await handler.testMultiWordEvent(ctx, body); + return new Response(JSON.stringify(result as MultiWordEvent), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } catch (err: unknown) { + if (err instanceof ValidationError) { + return new Response(JSON.stringify({ violations: err.violations }), { + status: 400, + headers: { "Content-Type": "application/json" }, + }); + } + if (options?.onError) { + return options.onError(err, req); + } + const message = err instanceof Error ? err.message : String(err); + return new Response(JSON.stringify({ message }), { + status: 500, + headers: { "Content-Type": "application/json" }, + }); + } + }, + }, + ]; +} + diff --git a/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts b/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts index 62bf8a8b..7de9ed7f 100644 --- a/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts +++ b/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts @@ -3,7 +3,8 @@ export type FlattenedEventContent = | { type: "text"; body: string } - | { type: "img"; url: string; width: number; height: number }; + | { type: "img"; url: string; width: number; height: number } + | { type?: never }; export interface FlattenedEventBase { id: string; @@ -22,15 +23,17 @@ export interface ImageContent { } export type NestedEventContent = - | { kind: "text"; text?: TextContent } - | { kind: "image"; image?: ImageContent } - | { kind: "vid"; video?: VideoContent }; + | { kind: "text"; text: TextContent; image?: never; video?: never } + | { kind: "image"; image: ImageContent; text?: never; video?: never } + | { kind: "vid"; video: VideoContent; text?: never; image?: never } + | { kind?: never; text?: never; image?: never; video?: never }; -export interface NestedEvent { +export interface NestedEventBase { id: string; - content?: NestedEventContent; } +export type NestedEvent = NestedEventBase & NestedEventContent; + export interface VideoContent { url: string; duration: number; diff --git a/internal/tsservergen/testdata/proto/multi_word_oneof.proto b/internal/tsservergen/testdata/proto/multi_word_oneof.proto new file mode 100644 index 00000000..0675197d --- /dev/null +++ b/internal/tsservergen/testdata/proto/multi_word_oneof.proto @@ -0,0 +1,46 @@ +syntax = "proto3"; + +package testdata.multi_word_oneof; + +option go_package = "github.com/SebastienMelki/sebuf/internal/tsclientgen/testdata/multiwordoneof;multiwordoneof"; + +import "sebuf/http/annotations.proto"; + +// Variant message types +message TextContent { + string body = 1; +} + +message ImageContent { + string url = 1; + int32 width = 2; + int32 height = 3; +} + +// Message with a multi-word oneof name and no oneof_config annotation. +// +// The oneof name (super_title_image) is deliberately multi-word: it must +// surface in the generated TypeScript only as the PascalCase union type name +// (MultiWordEventSuperTitleImage) and must NEVER leak as a raw snake_case +// wrapper property. Because the oneof is un-annotated it takes the +// presence-discriminated flat-union path, matching plain protojson. +message MultiWordEvent { + string id = 1; + oneof super_title_image { + TextContent big_text = 2; + ImageContent big_image = 3; + } +} + +service MultiWordOneofService { + option (sebuf.http.service_config) = { + base_path: "/api/v1" + }; + + rpc TestMultiWordEvent(MultiWordEvent) returns (MultiWordEvent) { + option (sebuf.http.config) = { + path: "/events/multi-word" + method: HTTP_METHOD_POST + }; + } +} From 3c939f2bbfb9743ba1b08df8d5722cdf29acd452 Mon Sep 17 00:00:00 2001 From: Hicham <53556927+hishamank@users.noreply.github.com> Date: Tue, 7 Jul 2026 16:31:15 +0400 Subject: [PATCH 05/11] style(ts): satisfy golines and unparam in tscommon test harness golangci-lint flagged the protogen test harness: msgFieldProto's signature and the oneofTestFile msg closure exceeded the 120-char golines limit, and oneofIndex's parameter always received 0 (unparam). Wrap the signatures and inline proto.Int32(0) at the call sites. Co-Authored-By: Claude Fable 5 --- internal/tscommon/types_test.go | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/internal/tscommon/types_test.go b/internal/tscommon/types_test.go index 482499ff..bbb4b133 100644 --- a/internal/tscommon/types_test.go +++ b/internal/tscommon/types_test.go @@ -202,7 +202,11 @@ func TestTSEnumUnspecifiedValue_ViaGoldenOutput(t *testing.T) { const testProtoPkg = "test.oneof.v1" // msgFieldProto builds a singular proto3 field descriptor. -func msgFieldProto(name string, number int32, typ descriptorpb.FieldDescriptorProto_Type) *descriptorpb.FieldDescriptorProto { +func msgFieldProto( + name string, + number int32, + typ descriptorpb.FieldDescriptorProto_Type, +) *descriptorpb.FieldDescriptorProto { return &descriptorpb.FieldDescriptorProto{ Name: proto.String(name), Number: proto.Int32(number), @@ -230,9 +234,6 @@ func jsonName(name string) string { return strings.Join(parts, "") } -// oneofIndex returns a pointer to i (fields reference their oneof by index). -func oneofIndex(i int32) *int32 { return proto.Int32(i) } - // buildTestPlugin compiles the descriptor into a *protogen.Plugin. func buildTestPlugin(t *testing.T, fd *descriptorpb.FileDescriptorProto) *protogen.Plugin { t.Helper() @@ -287,7 +288,11 @@ func capturePrinter() (Printer, *strings.Builder) { // the presence-union emitter targets. func oneofTestFile() *descriptorpb.FileDescriptorProto { str := descriptorpb.FieldDescriptorProto_TYPE_STRING - msg := func(name string, fields []*descriptorpb.FieldDescriptorProto, oneofs []*descriptorpb.OneofDescriptorProto) *descriptorpb.DescriptorProto { + msg := func( + name string, + fields []*descriptorpb.FieldDescriptorProto, + oneofs []*descriptorpb.OneofDescriptorProto, + ) *descriptorpb.DescriptorProto { return &descriptorpb.DescriptorProto{ Name: proto.String(name), Field: fields, @@ -314,12 +319,12 @@ func oneofTestFile() *descriptorpb.FileDescriptorProto { msgFieldProto("id", 1, str), func() *descriptorpb.FieldDescriptorProto { f := msgRefFieldProto("text", 2, "TextContent") - f.OneofIndex = oneofIndex(0) + f.OneofIndex = proto.Int32(0) return f }(), func() *descriptorpb.FieldDescriptorProto { f := msgRefFieldProto("image", 3, "ImageContent") - f.OneofIndex = oneofIndex(0) + f.OneofIndex = proto.Int32(0) return f }(), }, @@ -331,12 +336,12 @@ func oneofTestFile() *descriptorpb.FileDescriptorProto { []*descriptorpb.FieldDescriptorProto{ func() *descriptorpb.FieldDescriptorProto { f := msgRefFieldProto("text", 1, "TextContent") - f.OneofIndex = oneofIndex(0) + f.OneofIndex = proto.Int32(0) return f }(), func() *descriptorpb.FieldDescriptorProto { f := msgRefFieldProto("image", 2, "ImageContent") - f.OneofIndex = oneofIndex(0) + f.OneofIndex = proto.Int32(0) return f }(), }, @@ -348,17 +353,17 @@ func oneofTestFile() *descriptorpb.FileDescriptorProto { []*descriptorpb.FieldDescriptorProto{ func() *descriptorpb.FieldDescriptorProto { f := msgRefFieldProto("text", 1, "TextContent") - f.OneofIndex = oneofIndex(0) + f.OneofIndex = proto.Int32(0) return f }(), func() *descriptorpb.FieldDescriptorProto { f := msgRefFieldProto("image", 2, "ImageContent") - f.OneofIndex = oneofIndex(0) + f.OneofIndex = proto.Int32(0) return f }(), func() *descriptorpb.FieldDescriptorProto { f := msgRefFieldProto("video", 3, "VideoContent") - f.OneofIndex = oneofIndex(0) + f.OneofIndex = proto.Int32(0) return f }(), }, @@ -370,12 +375,12 @@ func oneofTestFile() *descriptorpb.FileDescriptorProto { []*descriptorpb.FieldDescriptorProto{ func() *descriptorpb.FieldDescriptorProto { f := msgFieldProto("raw_text", 1, str) - f.OneofIndex = oneofIndex(0) + f.OneofIndex = proto.Int32(0) return f }(), func() *descriptorpb.FieldDescriptorProto { f := msgRefFieldProto("detail", 2, "TextContent") - f.OneofIndex = oneofIndex(0) + f.OneofIndex = proto.Int32(0) return f }(), }, From d2a4aac652756e3202a5be795eb2c86f48801bcc Mon Sep 17 00:00:00 2001 From: Hicham <53556927+hishamank@users.noreply.github.com> Date: Thu, 9 Jul 2026 14:51:10 +0400 Subject: [PATCH 06/11] fix(ts): reject discriminator/variant-key collision on non-flatten oneofs On the non-flatten annotated-oneof path the discriminator and the set variant's JSON key both sit flat on the parent object. A discriminator equal to a variant's JSON name therefore emitted a duplicate TypeScript key (and collided with the sibling `?: never` presence guards). ValidateOneofDiscriminator now rejects, on the flatten=false path only, a discriminator that matches any variant's JSON name. The flatten path is left unchanged: it spreads the variant's child fields and never emits the variant key, so a discriminator equal to a variant name is fine there. Co-Authored-By: Claude Opus 4.8 --- internal/annotations/oneof_discriminator.go | 31 +++- .../annotations/oneof_discriminator_test.go | 166 ++++++++++++++++++ 2 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 internal/annotations/oneof_discriminator_test.go diff --git a/internal/annotations/oneof_discriminator.go b/internal/annotations/oneof_discriminator.go index b8c6c07a..cbf1093e 100644 --- a/internal/annotations/oneof_discriminator.go +++ b/internal/annotations/oneof_discriminator.go @@ -133,6 +133,7 @@ func HasOneofDiscriminator(message *protogen.Message) bool { // 1. Discriminator name collisions with parent message fields // 2. When flatten=true: all variants must be message types // 3. When flatten=true: variant child field names must not collide with parent fields or discriminator. +// 4. When flatten=false: discriminator must not equal any variant's JSON name. func ValidateOneofDiscriminator( message *protogen.Message, oneof *protogen.Oneof, @@ -148,7 +149,11 @@ func ValidateOneofDiscriminator( return validateOneofFlatten(message, oneof, discriminator) } - return nil + // Non-flatten path: the discriminator and each set variant's JSON key both sit + // flat on the parent object, so a discriminator equal to a variant key would + // emit a duplicate object key. The flatten path is exempt — it spreads the + // variant's child fields and never emits the variant key itself. + return validateDiscriminatorVariantCollision(message, oneof, discriminator) } // validateDiscriminatorNameCollision checks discriminator vs parent message fields. @@ -174,6 +179,30 @@ func validateDiscriminatorNameCollision( return nil } +// validateDiscriminatorVariantCollision checks, on the non-flatten path, that the +// discriminator name does not equal any variant's JSON name. On that path the +// discriminator and the set variant's key share the parent object, so a +// discriminator matching a variant key would produce a duplicate TypeScript key +// (and collide with the sibling presence guards emitted for the other variants). +func validateDiscriminatorVariantCollision( + message *protogen.Message, + oneof *protogen.Oneof, + discriminator string, +) error { + for _, field := range oneof.Fields { + if field.Desc.JSONName() == discriminator { + return fmt.Errorf( + "oneof %s.%s (flatten=false): discriminator name %q collides with variant %q (JSON: %q); "+ + "on the non-flatten path the discriminator and the variant key share the parent object", + message.Desc.Name(), oneof.Desc.Name(), discriminator, + field.Desc.Name(), field.Desc.JSONName(), + ) + } + } + + return nil +} + // validateOneofFlatten validates flatten-specific rules for a discriminated oneof. func validateOneofFlatten( message *protogen.Message, diff --git a/internal/annotations/oneof_discriminator_test.go b/internal/annotations/oneof_discriminator_test.go new file mode 100644 index 00000000..feb2b9b0 --- /dev/null +++ b/internal/annotations/oneof_discriminator_test.go @@ -0,0 +1,166 @@ +package annotations + +import ( + "strings" + "testing" + + "google.golang.org/protobuf/compiler/protogen" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/descriptorpb" + "google.golang.org/protobuf/types/pluginpb" + + "github.com/SebastienMelki/sebuf/http" +) + +// --- protogen harness ------------------------------------------------------- +// +// ValidateOneofDiscriminator operates on real *protogen.Message / *protogen.Oneof +// values, which cannot be hand-mocked. These helpers compile a hand-built proto3 +// FileDescriptorProto into a *protogen.Plugin so the validator can be exercised on +// genuine protogen inputs. The oneof_config is passed to the validator directly, so +// the descriptor needs no sebuf.http extension options. + +const validateTestPkg = "test.validate.v1" + +// validateJSONName mirrors protoc's default lowerCamelCase json_name derivation. +func validateJSONName(name string) string { + parts := strings.Split(name, "_") + for i, part := range parts { + if i > 0 && len(part) > 0 { + parts[i] = strings.ToUpper(part[:1]) + part[1:] + } + } + return strings.Join(parts, "") +} + +// scalarField builds a singular proto3 string field descriptor. +func scalarField(name string, number int32) *descriptorpb.FieldDescriptorProto { + return &descriptorpb.FieldDescriptorProto{ + Name: proto.String(name), + Number: proto.Int32(number), + Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(), + Type: descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(), + JsonName: proto.String(validateJSONName(name)), + } +} + +// oneofMsgField builds a singular message-typed field bound to the oneof at index 0. +func oneofMsgField(name string, number int32, msgName string) *descriptorpb.FieldDescriptorProto { + return &descriptorpb.FieldDescriptorProto{ + Name: proto.String(name), + Number: proto.Int32(number), + Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(), + Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(), + TypeName: proto.String("." + validateTestPkg + "." + msgName), + JsonName: proto.String(validateJSONName(name)), + OneofIndex: proto.Int32(0), + } +} + +// validateOneofFile builds a proto3 file with an Event message carrying a base +// field plus a two-variant oneof (text -> TextContent, image -> ImageContent). +func validateOneofFile() *descriptorpb.FileDescriptorProto { + textContent := &descriptorpb.DescriptorProto{ + Name: proto.String("TextContent"), + Field: []*descriptorpb.FieldDescriptorProto{scalarField("body", 1)}, + } + imageContent := &descriptorpb.DescriptorProto{ + Name: proto.String("ImageContent"), + Field: []*descriptorpb.FieldDescriptorProto{scalarField("url", 1)}, + } + event := &descriptorpb.DescriptorProto{ + Name: proto.String("Event"), + Field: []*descriptorpb.FieldDescriptorProto{ + scalarField("id", 1), + oneofMsgField("text", 2, "TextContent"), + oneofMsgField("image", 3, "ImageContent"), + }, + OneofDecl: []*descriptorpb.OneofDescriptorProto{{Name: proto.String("content")}}, + } + + return &descriptorpb.FileDescriptorProto{ + Name: proto.String("validate_oneof.proto"), + Package: proto.String(validateTestPkg), + Syntax: proto.String("proto3"), + Options: &descriptorpb.FileOptions{ + GoPackage: proto.String("github.com/SebastienMelki/sebuf/internal/annotations/validatev1"), + }, + MessageType: []*descriptorpb.DescriptorProto{textContent, imageContent, event}, + } +} + +func buildValidatePlugin(t *testing.T, fd *descriptorpb.FileDescriptorProto) *protogen.Plugin { + t.Helper() + req := &pluginpb.CodeGeneratorRequest{ + FileToGenerate: []string{fd.GetName()}, + ProtoFile: []*descriptorpb.FileDescriptorProto{fd}, + } + plugin, err := protogen.Options{}.New(req) + if err != nil { + t.Fatalf("protogen.Options{}.New: %v", err) + } + return plugin +} + +func findValidateMessage(t *testing.T, plugin *protogen.Plugin, name string) *protogen.Message { + t.Helper() + for _, file := range plugin.Files { + for _, msg := range file.Messages { + if string(msg.Desc.Name()) == name { + return msg + } + } + } + t.Fatalf("message %q not found in plugin", name) + return nil +} + +func findValidateOneof(t *testing.T, msg *protogen.Message, name string) *protogen.Oneof { + t.Helper() + for _, oneof := range msg.Oneofs { + if string(oneof.Desc.Name()) == name { + return oneof + } + } + t.Fatalf("oneof %q not found on message %q", name, msg.Desc.Name()) + return nil +} + +// TestValidateOneofDiscriminator_VariantCollision covers the non-flatten +// discriminator/variant-key collision guard: on the non-flatten path the +// discriminator and the set variant's key share the parent object, so a +// discriminator equal to a variant's JSON name must be rejected. The flatten path +// is exempt because it spreads the variant's child fields and never emits the +// variant key itself. +func TestValidateOneofDiscriminator_VariantCollision(t *testing.T) { + plugin := buildValidatePlugin(t, validateOneofFile()) + msg := findValidateMessage(t, plugin, "Event") + oneof := findValidateOneof(t, msg, "content") + + t.Run("non_flatten_discriminator_equals_variant_name_errors", func(t *testing.T) { + config := &http.OneofConfig{Discriminator: "text", Flatten: false} + err := ValidateOneofDiscriminator(msg, oneof, config) + if err == nil { + t.Fatal("expected error for discriminator colliding with variant JSON name, got nil") + } + for _, want := range []string{"content", "text", "variant"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q missing expected substring %q", err.Error(), want) + } + } + }) + + t.Run("non_flatten_safe_discriminator_no_error", func(t *testing.T) { + config := &http.OneofConfig{Discriminator: "kind", Flatten: false} + if err := ValidateOneofDiscriminator(msg, oneof, config); err != nil { + t.Errorf("expected no error for safe discriminator, got: %v", err) + } + }) + + t.Run("flatten_discriminator_equals_variant_name_allowed", func(t *testing.T) { + config := &http.OneofConfig{Discriminator: "text", Flatten: true} + if err := ValidateOneofDiscriminator(msg, oneof, config); err != nil { + t.Errorf("expected no collision error on flatten path, got: %v", err) + } + }) +} From 512305250e8efb48c15cd444513463b7e68f1f0e Mon Sep 17 00:00:00 2001 From: Hicham <53556927+hishamank@users.noreply.github.com> Date: Thu, 9 Jul 2026 14:51:18 +0400 Subject: [PATCH 07/11] refactor(ts): drop dead params and unreachable scalar oneof branch GenerateStandardInterface only runs for messages with zero discriminated oneofs (GenerateInterface routes any message that has them to GenerateFlattenedOneofInterface). Its discriminatedOneofs parameter, the BuildOneofFieldSet call, and the oneof-field skip were therefore dead; remove them and update the caller. Behavior is identical. In GenerateOneofDiscriminatedUnionType, the scalar default case branched on info.Flatten, but flatten+scalar is rejected upstream by validateOneofFlatten (flatten variants must be messages). The flatten sub-branch was unreachable and emitted an inconsistent optional payload; the scalar case now always renders via nonFlattenedOneofBranch. Co-Authored-By: Claude Opus 4.8 --- internal/tscommon/types.go | 38 +++++++++++--------------------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/internal/tscommon/types.go b/internal/tscommon/types.go index 19c35a01..3d4fc9cc 100644 --- a/internal/tscommon/types.go +++ b/internal/tscommon/types.go @@ -407,7 +407,7 @@ func GenerateInterface(p Printer, msg *protogen.Message) { if len(discriminatedOneofs) > 0 { GenerateFlattenedOneofInterface(p, msg, name, discriminatedOneofs) } else { - GenerateStandardInterface(p, msg, name, discriminatedOneofs) + GenerateStandardInterface(p, msg, name) } } @@ -472,21 +472,13 @@ func GenerateOneofDiscriminatedUnionType(p Printer, msgName string, info *annota string(variant.Field.Message.Desc.Name()), ) default: - // Non-flattened scalar: same flat shape with the scalar's TS type. - if info.Flatten { - branch = fmt.Sprintf( - "{ %s: \"%s\"; %s?: %s }", - info.Discriminator, - variant.DiscriminatorVal, - jsonNames[i], - TSScalarTypeForField(variant.Field), - ) - } else { - branch = nonFlattenedOneofBranch( - info, variant.DiscriminatorVal, jsonNames, i, - TSScalarTypeForField(variant.Field), - ) - } + // Scalar variant: flat shape with the scalar's TS type. Flatten+scalar is + // rejected by validateOneofFlatten (flatten variants must be messages), so + // a scalar variant only ever reaches the non-flatten branch. + branch = nonFlattenedOneofBranch( + info, variant.DiscriminatorVal, jsonNames, i, + TSScalarTypeForField(variant.Field), + ) } branches = append(branches, branch) } @@ -642,24 +634,16 @@ func GenerateFlattenedOneofInterface( } // GenerateStandardInterface generates a standard interface for messages without -// discriminated oneofs. Fields belonging to discriminated oneofs are skipped; -// messages that have any render through GenerateFlattenedOneofInterface instead, -// since every oneof union sits flat on the parent object. +// discriminated oneofs. Messages that have any render through +// GenerateFlattenedOneofInterface instead, since every oneof union sits flat on +// the parent object, so this path never has oneof fields to skip. func GenerateStandardInterface( p Printer, msg *protogen.Message, name string, - discriminatedOneofs []*annotations.OneofDiscriminatorInfo, ) { - // Build set of fields that belong to discriminated oneofs - oneofFields := BuildOneofFieldSet(discriminatedOneofs) - p("export interface %s {", name) for _, field := range msg.Fields { - if oneofFields[field] { - continue - } - if annotations.IsFlattenField(field) && field.Message != nil { prefix := annotations.GetFlattenPrefix(field) GenerateFlattenedFields(p, field.Message, prefix) From ad7f235446457a9e1b1457d34467c2d64a6fc244 Mon Sep 17 00:00:00 2001 From: Hicham <53556927+hishamank@users.noreply.github.com> Date: Thu, 9 Jul 2026 14:51:27 +0400 Subject: [PATCH 08/11] test(ts): cover two un-annotated oneofs in one message Add a two_oneofs fixture with a TwoOneofs message carrying two distinct un-annotated oneofs plus a base field, wired into both the ts-client and ts-server golden testCases. The message renders as the intersection of a base interface and both presence unions (TwoOneofs = TwoOneofsBase & TwoOneofsA & TwoOneofsB), each union carrying its own arms plus an all-never arm and guarding only its own siblings. TestTwoOneofsRenderAsIndependentPresenceUnions asserts the intersection alias, both unions, and that neither union references the other oneof's variant keys (the oneofs are independent). Co-Authored-By: Claude Opus 4.8 --- internal/tsclientgen/golden_test.go | 80 +++++++++++ .../testdata/golden/two_oneofs_client.ts | 123 ++++++++++++++++ .../testdata/proto/two_oneofs.proto | 53 +++++++ internal/tsservergen/golden_test.go | 7 + .../testdata/golden/two_oneofs_server.ts | 134 ++++++++++++++++++ .../testdata/proto/two_oneofs.proto | 53 +++++++ 6 files changed, 450 insertions(+) create mode 100644 internal/tsclientgen/testdata/golden/two_oneofs_client.ts create mode 100644 internal/tsclientgen/testdata/proto/two_oneofs.proto create mode 100644 internal/tsservergen/testdata/golden/two_oneofs_server.ts create mode 100644 internal/tsservergen/testdata/proto/two_oneofs.proto diff --git a/internal/tsclientgen/golden_test.go b/internal/tsclientgen/golden_test.go index ed8370fd..61a41fbf 100644 --- a/internal/tsclientgen/golden_test.go +++ b/internal/tsclientgen/golden_test.go @@ -123,6 +123,13 @@ func TestTSClientGenGoldenFiles(t *testing.T) { "multi_word_oneof_client.ts", }, }, + { + name: "two un-annotated oneofs in one message", + protoFile: "two_oneofs.proto", + expectedFiles: []string{ + "two_oneofs_client.ts", + }, + }, { name: "SSE streaming", protoFile: "sse.proto", @@ -245,6 +252,79 @@ func TestMultiWordOneofNameDoesNotLeak(t *testing.T) { } } +// TestTwoOneofsRenderAsIndependentPresenceUnions asserts that a message with two +// distinct un-annotated oneofs renders as an intersection of a base interface and +// one presence-discriminated union per oneof, and that each union's `?: never` +// presence guards cover only its own siblings — the two oneofs are independent, so +// neither union references the other's variant keys. +func TestTwoOneofsRenderAsIndependentPresenceUnions(t *testing.T) { + wd, err := os.Getwd() + if err != nil { + t.Fatalf("Failed to get working directory: %v", err) + } + goldenPath := filepath.Join(wd, "testdata", "golden", "two_oneofs_client.ts") + + content, readErr := os.ReadFile(goldenPath) + if readErr != nil { + t.Fatalf("Failed to read golden file %s: %v", goldenPath, readErr) + } + ts := string(content) + + // The message type is the intersection of the base and BOTH presence unions. + if !strings.Contains(ts, "export type TwoOneofs = TwoOneofsBase & TwoOneofsA & TwoOneofsB;") { + t.Error("expected TwoOneofs to be an intersection of TwoOneofsBase and both oneof unions") + } + + // Union A: its own arms plus an all-never arm, guarding only its own siblings + // (x, y) — never the other oneof's keys (p, q). + unionA := `export type TwoOneofsA = + | { x: TypeX; y?: never } + | { y: TypeY; x?: never } + | { x?: never; y?: never };` + if !strings.Contains(ts, unionA) { + t.Errorf("expected TwoOneofsA presence union with only its own sibling guards, got:\n%s", ts) + } + + // Union B: independent of A — arms + all-never arm guarding only p, q. + unionB := `export type TwoOneofsB = + | { p: TypeP; q?: never } + | { q: TypeQ; p?: never } + | { p?: never; q?: never };` + if !strings.Contains(ts, unionB) { + t.Errorf("expected TwoOneofsB presence union with only its own sibling guards, got:\n%s", ts) + } + + // The two oneofs are independent: neither union guards against the other's keys. + if strings.Contains(unionAOf(ts), "p?: never") || strings.Contains(unionAOf(ts), "q?: never") { + t.Error("TwoOneofsA must not reference oneof B's variant keys (p, q)") + } + if strings.Contains(unionBOf(ts), "x?: never") || strings.Contains(unionBOf(ts), "y?: never") { + t.Error("TwoOneofsB must not reference oneof A's variant keys (x, y)") + } +} + +// unionAOf / unionBOf extract the TwoOneofsA / TwoOneofsB union declaration text so +// cross-oneof leakage can be asserted without matching against the whole file. +func unionAOf(ts string) string { + return sliceBetween(ts, "export type TwoOneofsA =", "export type TwoOneofsB =") +} + +func unionBOf(ts string) string { + return sliceBetween(ts, "export type TwoOneofsB =", "export interface TwoOneofsBase") +} + +func sliceBetween(s, start, end string) string { + i := strings.Index(s, start) + if i < 0 { + return "" + } + j := strings.Index(s[i:], end) + if j < 0 { + return s[i:] + } + return s[i : i+j] +} + func updateGoldenFile(t *testing.T, goldenPath string, content []byte) { t.Helper() writeErr := os.WriteFile(goldenPath, content, 0o644) diff --git a/internal/tsclientgen/testdata/golden/two_oneofs_client.ts b/internal/tsclientgen/testdata/golden/two_oneofs_client.ts new file mode 100644 index 00000000..f4f7e980 --- /dev/null +++ b/internal/tsclientgen/testdata/golden/two_oneofs_client.ts @@ -0,0 +1,123 @@ +// Code generated by protoc-gen-ts-client. DO NOT EDIT. +// source: two_oneofs.proto + +export type TwoOneofsA = + | { x: TypeX; y?: never } + | { y: TypeY; x?: never } + | { x?: never; y?: never }; + +export type TwoOneofsB = + | { p: TypeP; q?: never } + | { q: TypeQ; p?: never } + | { p?: never; q?: never }; + +export interface TwoOneofsBase { + id: string; +} + +export type TwoOneofs = TwoOneofsBase & TwoOneofsA & TwoOneofsB; + +export interface TypeX { + x: string; +} + +export interface TypeY { + y: string; +} + +export interface TypeP { + p: string; +} + +export interface TypeQ { + q: string; +} + +export interface FieldViolation { + field: string; + description: string; +} + +export class ValidationError extends Error { + violations: FieldViolation[]; + + constructor(violations: FieldViolation[]) { + super("Validation failed"); + this.name = "ValidationError"; + this.violations = violations; + } +} + +export class ApiError extends Error { + statusCode: number; + body: string; + + constructor(statusCode: number, message: string, body: string) { + super(message); + this.name = "ApiError"; + this.statusCode = statusCode; + this.body = body; + } +} + +export interface TwoOneofsServiceClientOptions { + fetch?: typeof fetch; + defaultHeaders?: Record; +} + +export interface TwoOneofsServiceCallOptions { + headers?: Record; + signal?: AbortSignal; +} + +export class TwoOneofsServiceClient { + private baseURL: string; + private fetchFn: typeof fetch; + private defaultHeaders: Record; + + constructor(baseURL: string, options?: TwoOneofsServiceClientOptions) { + this.baseURL = baseURL.replace(/\/+$/, ""); + this.fetchFn = options?.fetch ?? globalThis.fetch; + this.defaultHeaders = { ...options?.defaultHeaders }; + } + + async testTwoOneofs(req: TwoOneofs, options?: TwoOneofsServiceCallOptions): Promise { + let path = "/api/v1/two-oneofs"; + const url = this.baseURL + path; + + const headers: Record = { + "Content-Type": "application/json", + ...this.defaultHeaders, + ...options?.headers, + }; + + const resp = await this.fetchFn(url, { + method: "POST", + headers, + body: JSON.stringify(req), + signal: options?.signal, + }); + + if (!resp.ok) { + return this.handleError(resp); + } + + return await resp.json() as TwoOneofs; + } + + private async handleError(resp: Response): Promise { + const body = await resp.text(); + if (resp.status === 400) { + try { + const parsed = JSON.parse(body); + if (parsed.violations) { + throw new ValidationError(parsed.violations); + } + } catch (e) { + if (e instanceof ValidationError) throw e; + } + } + throw new ApiError(resp.status, `Request failed with status ${resp.status}`, body); + } +} + diff --git a/internal/tsclientgen/testdata/proto/two_oneofs.proto b/internal/tsclientgen/testdata/proto/two_oneofs.proto new file mode 100644 index 00000000..b85ac2ed --- /dev/null +++ b/internal/tsclientgen/testdata/proto/two_oneofs.proto @@ -0,0 +1,53 @@ +syntax = "proto3"; + +package testdata.two_oneofs; + +option go_package = "github.com/SebastienMelki/sebuf/internal/httpgen/testdata/twooneofs;twooneofs"; + +import "sebuf/http/annotations.proto"; + +// Variant message types for the first oneof. +message TypeX { + string x = 1; +} + +message TypeY { + string y = 1; +} + +// Variant message types for the second oneof. +message TypeP { + string p = 1; +} + +message TypeQ { + string q = 1; +} + +// TwoOneofs carries two independent, un-annotated oneofs alongside a base field. +// Each oneof renders as its own presence-discriminated union, and the message +// type is the intersection of the base and both unions. +message TwoOneofs { + string id = 1; + oneof a { + TypeX x = 2; + TypeY y = 3; + } + oneof b { + TypeP p = 4; + TypeQ q = 5; + } +} + +service TwoOneofsService { + option (sebuf.http.service_config) = { + base_path: "/api/v1" + }; + + rpc TestTwoOneofs(TwoOneofs) returns (TwoOneofs) { + option (sebuf.http.config) = { + path: "/two-oneofs" + method: HTTP_METHOD_POST + }; + } +} diff --git a/internal/tsservergen/golden_test.go b/internal/tsservergen/golden_test.go index 1a7ce015..63a1d3f3 100644 --- a/internal/tsservergen/golden_test.go +++ b/internal/tsservergen/golden_test.go @@ -130,6 +130,13 @@ func TestTSServerGenGoldenFiles(t *testing.T) { "multi_word_oneof_server.ts", }, }, + { + name: "two un-annotated oneofs in one message", + protoFile: "two_oneofs.proto", + expectedFiles: []string{ + "two_oneofs_server.ts", + }, + }, { name: "SSE streaming", protoFile: "sse.proto", diff --git a/internal/tsservergen/testdata/golden/two_oneofs_server.ts b/internal/tsservergen/testdata/golden/two_oneofs_server.ts new file mode 100644 index 00000000..f12658c3 --- /dev/null +++ b/internal/tsservergen/testdata/golden/two_oneofs_server.ts @@ -0,0 +1,134 @@ +// Code generated by protoc-gen-ts-server. DO NOT EDIT. +// source: two_oneofs.proto + +export type TwoOneofsA = + | { x: TypeX; y?: never } + | { y: TypeY; x?: never } + | { x?: never; y?: never }; + +export type TwoOneofsB = + | { p: TypeP; q?: never } + | { q: TypeQ; p?: never } + | { p?: never; q?: never }; + +export interface TwoOneofsBase { + id: string; +} + +export type TwoOneofs = TwoOneofsBase & TwoOneofsA & TwoOneofsB; + +export interface TypeX { + x: string; +} + +export interface TypeY { + y: string; +} + +export interface TypeP { + p: string; +} + +export interface TypeQ { + q: string; +} + +export interface FieldViolation { + field: string; + description: string; +} + +export class ValidationError extends Error { + violations: FieldViolation[]; + + constructor(violations: FieldViolation[]) { + super("Validation failed"); + this.name = "ValidationError"; + this.violations = violations; + } +} + +export class ApiError extends Error { + statusCode: number; + body: string; + + constructor(statusCode: number, message: string, body: string) { + super(message); + this.name = "ApiError"; + this.statusCode = statusCode; + this.body = body; + } +} + +export interface ServerContext { + request: Request; + pathParams: Record; + headers: Record; +} + +export interface ServerOptions { + onError?: (error: unknown, req: Request) => Response | Promise; + validateRequest?: (methodName: string, body: unknown) => FieldViolation[] | undefined; +} + +export interface RouteDescriptor { + method: string; + path: string; + handler: (req: Request) => Promise; +} + +export interface TwoOneofsServiceHandler { + testTwoOneofs(ctx: ServerContext, req: TwoOneofs): Promise; +} + +export function createTwoOneofsServiceRoutes( + handler: TwoOneofsServiceHandler, + options?: ServerOptions, +): RouteDescriptor[] { + return [ + { + method: "POST", + path: "/api/v1/two-oneofs", + handler: async (req: Request): Promise => { + try { + const pathParams: Record = {}; + const body = await req.json() as TwoOneofs; + if (options?.validateRequest) { + const bodyViolations = options.validateRequest("testTwoOneofs", body); + if (bodyViolations) { + throw new ValidationError(bodyViolations); + } + } + + const ctx: ServerContext = { + request: req, + pathParams, + headers: Object.fromEntries(req.headers.entries()), + }; + + const result = await handler.testTwoOneofs(ctx, body); + return new Response(JSON.stringify(result as TwoOneofs), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } catch (err: unknown) { + if (err instanceof ValidationError) { + return new Response(JSON.stringify({ violations: err.violations }), { + status: 400, + headers: { "Content-Type": "application/json" }, + }); + } + if (options?.onError) { + return options.onError(err, req); + } + const message = err instanceof Error ? err.message : String(err); + return new Response(JSON.stringify({ message }), { + status: 500, + headers: { "Content-Type": "application/json" }, + }); + } + }, + }, + ]; +} + diff --git a/internal/tsservergen/testdata/proto/two_oneofs.proto b/internal/tsservergen/testdata/proto/two_oneofs.proto new file mode 100644 index 00000000..b85ac2ed --- /dev/null +++ b/internal/tsservergen/testdata/proto/two_oneofs.proto @@ -0,0 +1,53 @@ +syntax = "proto3"; + +package testdata.two_oneofs; + +option go_package = "github.com/SebastienMelki/sebuf/internal/httpgen/testdata/twooneofs;twooneofs"; + +import "sebuf/http/annotations.proto"; + +// Variant message types for the first oneof. +message TypeX { + string x = 1; +} + +message TypeY { + string y = 1; +} + +// Variant message types for the second oneof. +message TypeP { + string p = 1; +} + +message TypeQ { + string q = 1; +} + +// TwoOneofs carries two independent, un-annotated oneofs alongside a base field. +// Each oneof renders as its own presence-discriminated union, and the message +// type is the intersection of the base and both unions. +message TwoOneofs { + string id = 1; + oneof a { + TypeX x = 2; + TypeY y = 3; + } + oneof b { + TypeP p = 4; + TypeQ q = 5; + } +} + +service TwoOneofsService { + option (sebuf.http.service_config) = { + base_path: "/api/v1" + }; + + rpc TestTwoOneofs(TwoOneofs) returns (TwoOneofs) { + option (sebuf.http.config) = { + path: "/two-oneofs" + method: HTTP_METHOD_POST + }; + } +} From c84aec490e813f8bd7ff9c040664c51f322f3a21 Mon Sep 17 00:00:00 2001 From: Hicham <53556927+hishamank@users.noreply.github.com> Date: Thu, 9 Jul 2026 14:51:34 +0400 Subject: [PATCH 09/11] docs(ts): document TypeScript oneof shape and caveats CLAUDE.md gains a section on the default un-annotated oneof behavior and the presence-union TS shape (previously only annotated oneof_config was documented). docs/client-generation.md gains an "Oneofs in TypeScript" subsection covering the un-annotated / flatten:false / flatten:true shapes, a worked construct-and-narrow example, and the five consumer caveats (breaking change, scalar narrowing by presence, construction-time exactly-one enforcement, compile-time-only ?: never guards, and the divergent unset modeling vs OpenAPI). Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 33 +++++++++++++++++++++++++++++++++ docs/client-generation.md | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 410b87a0..803a356e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -507,6 +507,39 @@ message Event { // Not flattened: {"id": "1", "type": "text", "text": {"body": "hello"}} ``` +**Oneofs in TypeScript (default, un-annotated)** — A oneof with *no* `oneof_config` +annotation is not left as a bag of optional fields. Both TS generators render it as a +presence-discriminated union that mirrors protojson: exactly the set member's JSON key +appears on the parent object. Each arm carries one variant key with a non-optional +payload and types every sibling key `?: never`; a final all-`never` arm models the unset +oneof. A message with oneofs becomes the intersection of a base interface and one union +per oneof (independent oneofs each get their own union). The clients are conversion-free +(`JSON.stringify` on send, `as T` on receive), so the TS shape matches the wire exactly. + +```proto +message PlainEvent { + string id = 1; + oneof content { // no oneof_config → presence union + TextContent text = 2; + ImageContent image = 3; + } +} +``` +```typescript +export type PlainEventContent = + | { text: TextContent; image?: never } + | { image: ImageContent; text?: never } + | { text?: never; image?: never }; +export interface PlainEventBase { id: string; } +export type PlainEvent = PlainEventBase & PlainEventContent; +``` + +Annotated oneofs differ: a non-flatten `oneof_config` keeps the discriminator and the +variant key flat on the parent (plus `?: never` sibling guards); `flatten: true` spreads +the variant's child fields onto the parent instead. See +[docs/client-generation.md](docs/client-generation.md#oneofs-in-typescript) for the full +TS shape and consumer caveats. + **flatten / flatten_prefix** - Promote nested message fields to parent (ext 50019, 50020): ```protobuf message Order { diff --git a/docs/client-generation.md b/docs/client-generation.md index 83c3eed2..fec69f13 100644 --- a/docs/client-generation.md +++ b/docs/client-generation.md @@ -613,6 +613,41 @@ try { The proto definition serves as the single source of truth for error shapes — both server and client use the same generated interface for type safety across the wire. +### Oneofs in TypeScript + +Both TypeScript generators render a protobuf `oneof` as a discriminated union that matches protojson exactly. The clients do no conversion — requests are `JSON.stringify`-ed and responses are cast with `as T` — so the generated types are the wire contract. + +- **Un-annotated oneof (default)** — presence-discriminated union: exactly the set member's JSON key appears on the parent. Each arm carries one variant key with a non-optional payload and types every sibling `?: never`, with a final all-`never` arm for the unset oneof. A message becomes the intersection of a base interface and one union per oneof. +- **Annotated `oneof_config`, `flatten: false`** — the discriminator and the set variant's key both sit flat on the parent, with `?: never` guards on the sibling keys. +- **Annotated `oneof_config`, `flatten: true`** — the variant's child fields are spread onto the parent next to the discriminator; the variant key itself is not emitted. + +```typescript +// message Event { string id = 1; oneof content { TextContent text = 2; int32 count = 3; } } +export type EventContent = + | { text: TextContent; count?: never } + | { count: number; text?: never } + | { text?: never; count?: never }; +export type Event = EventBase & EventContent; + +// Construct exactly one variant: +const e: Event = { id: "1", text: { body: "hi" } }; + +// Narrow on presence, NOT truthiness (count can legitimately be 0): +if ("text" in e && e.text !== undefined) { + console.log(e.text.body); +} else if ("count" in e && e.count !== undefined) { + console.log(e.count); // narrowed to number, correct even when 0 +} +``` + +**Caveats:** + +1. **Breaking change.** Consumers written against the old shapes break: the previous un-annotated "flattened bag" of optional fields, and the `event.content` wrapper property for `flatten: false`, are both gone. In particular, setting two members of the same oneof is now a compile error rather than a silently-tolerated object. +2. **Narrow scalar variants by presence, not truthiness.** Use `"count" in e` or `e.count !== undefined` — a truthiness check (`if (e.count)`) misfires on the zero values `0`, `""`, and `false`, which are valid payloads. +3. **Exactly-one is enforced only at object-literal construction.** That is where TypeScript rejects setting more than one member; the TS types are therefore stricter than the Python and OpenAPI outputs for the same proto. +4. **`?: never` is a compile-time guard, not runtime exclusivity.** Under non-strict TypeScript, `{ text: ..., image: undefined }` still type-checks, and reads go through an unchecked `as T` cast — nothing validates at runtime that exactly one member is set. +5. **The unset state is modeled differently across outputs.** TypeScript represents "no member set" with the all-`never` arm of the union, whereas OpenAPI represents it via `oneOf` / a discriminator — so the unset case is not equally visible when comparing the two generated surfaces. + ## See Also - **[HTTP Generation Guide](./http-generation.md)** - Go server-side handler generation From 739983a398e3fb7a4aa6d0ce0e3d15676fce7b6f Mon Sep 17 00:00:00 2001 From: Hicham <53556927+hishamank@users.noreply.github.com> Date: Mon, 20 Jul 2026 13:53:58 +0300 Subject: [PATCH 10/11] docs(ts): exercise an un-annotated oneof end-to-end in ts-client-demo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the remaining (non-blocking) review ask on this PR: no runnable TS example used a oneof, so the presence-union shape was only visible in docs and golden testdata. Add an un-annotated `reminder` oneof (remind_at | remind_in_days) to Note. The Go server seeds note-1 with the timestamp variant and note-2 with the day-offset variant; the TS client narrows it by presence via a `describeReminder` helper (the `in` operator, not truthiness) in the LIST output. Verified end-to-end with `make demo` (note-1 → "reminder at …", note-2 → "reminder in 3 day(s)", others → "no reminder") and `tsc --noEmit` on the client. Document it in docs/examples/README.md (ts-client-demo entry + feature cell). Co-Authored-By: Claude Opus 4.8 --- docs/examples/README.md | 3 ++- examples/ts-client-demo/client/main.ts | 20 +++++++++++++++++++ examples/ts-client-demo/main.go | 4 ++++ .../ts-client-demo/proto/note_service.proto | 11 ++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/docs/examples/README.md b/docs/examples/README.md index cde6d20d..73d626dc 100644 --- a/docs/examples/README.md +++ b/docs/examples/README.md @@ -28,7 +28,7 @@ This starts a working HTTP API with user management, authentication, and OpenAPI | **[nested-resources](../../examples/nested-resources/)** | Organization hierarchy API | Deep path nesting (3 levels), multiple path params per endpoint | | **[multi-service-api](../../examples/multi-service-api/)** | Multi-tenant platform | Multiple services, different auth levels, service/method headers | | **[market-data-unwrap](../../examples/market-data-unwrap/)** | Financial market data API | Unwrap annotation for map values, JSON/protobuf compatibility | -| **[ts-client-demo](../../examples/ts-client-demo/)** | TypeScript client demo | TypeScript HTTP client, CRUD API, query params, headers, error handling | +| **[ts-client-demo](../../examples/ts-client-demo/)** | TypeScript client demo | TypeScript HTTP client, CRUD API, query params, headers, error handling, oneof presence-union | | **[ts-fullstack-demo](../../examples/ts-fullstack-demo/)** | TypeScript full-stack demo | TS client + TS server from same proto, CRUD, unwrap, custom errors | --- @@ -131,6 +131,7 @@ End-to-end TypeScript HTTP client demo with a NoteService CRUD API. - Query parameters: filter by status, limit results - Service-level headers (X-API-Key) and method-level headers (X-Request-ID) - Structured error handling: `ValidationError` and `ApiError` +- Un-annotated oneof (`Note.reminder`) rendered as a TypeScript presence-discriminated union, narrowed by presence in the client (`describeReminder`) - Go server implementing `NoteServiceServer` with in-memory store ```bash diff --git a/examples/ts-client-demo/client/main.ts b/examples/ts-client-demo/client/main.ts index c7469634..16caed91 100644 --- a/examples/ts-client-demo/client/main.ts +++ b/examples/ts-client-demo/client/main.ts @@ -6,6 +6,24 @@ import { type NoteServiceClientOptions, } from "./generated/proto/note_service_client.ts"; +// ---------------------------------------------------------------------------- +// Oneof helper: `Note.reminder` is an un-annotated proto oneof, so the client +// renders it as a presence-discriminated union — exactly one of `remindAt` / +// `remindInDays` is present per note, the other is typed `?: never`. Narrow it +// by PRESENCE (the `in` operator), never by truthiness: `remindInDays` of 0 +// would be dropped by an `if (n.remindInDays)` check. TypeScript flows the +// branch so `n.remindAt` / `n.remindInDays` are non-never inside each guard. +// ---------------------------------------------------------------------------- +function describeReminder(n: Note): string { + if ("remindAt" in n && n.remindAt !== undefined) { + return `reminder at ${n.remindAt}`; + } + if ("remindInDays" in n && n.remindInDays !== undefined) { + return `reminder in ${n.remindInDays} day(s)`; + } + return "no reminder"; +} + // ============================================================================ // Section 1: Client Configuration // ============================================================================ @@ -53,6 +71,8 @@ async function section2_crudOperations(client: NoteServiceClient) { for (const n of allNotes.notes) { const due = n.dueDate ? ` (due: ${n.dueDate})` : ""; console.log(` ${n.id}: "${n.title}" [${n.priority}, ${n.status}]${due}`); + // Un-annotated oneof narrowed by presence (see describeReminder above). + console.log(` ${describeReminder(n)}`); if (n.tags?.length > 0) { console.log(` tags: ${n.tags.map((t) => t.name).join(", ")}`); } diff --git a/examples/ts-client-demo/main.go b/examples/ts-client-demo/main.go index 931d7459..b5ac3f8a 100644 --- a/examples/ts-client-demo/main.go +++ b/examples/ts-client-demo/main.go @@ -46,6 +46,8 @@ func (s *noteService) seedData() { }, Metadata: map[string]string{"sprint": "12", "team": "platform"}, CreatedAt: now.Add(-72 * time.Hour).Format(time.RFC3339), + // Un-annotated oneof: absolute-timestamp reminder variant. + Reminder: &pb.Note_RemindAt{RemindAt: now.Add(24 * time.Hour).Format(time.RFC3339)}, } s.notes["note-2"] = &pb.Note{ Id: "note-2", @@ -60,6 +62,8 @@ func (s *noteService) seedData() { Metadata: map[string]string{"sprint": "12"}, DueDate: strPtr("2025-12-31"), CreatedAt: now.Add(-48 * time.Hour).Format(time.RFC3339), + // Un-annotated oneof: relative day-offset reminder variant. + Reminder: &pb.Note_RemindInDays{RemindInDays: 3}, } s.notes["note-3"] = &pb.Note{ Id: "note-3", diff --git a/examples/ts-client-demo/proto/note_service.proto b/examples/ts-client-demo/proto/note_service.proto index 1bbed616..d3e6b88e 100644 --- a/examples/ts-client-demo/proto/note_service.proto +++ b/examples/ts-client-demo/proto/note_service.proto @@ -50,6 +50,17 @@ message Note { map metadata = 7; optional string due_date = 8; string created_at = 9; + + // Un-annotated oneof: an optional reminder that is EITHER an absolute + // timestamp OR a relative day offset. With no oneof_config annotation, the + // TypeScript client renders this as a presence-discriminated union — exactly + // one member key is present, siblings are `?: never`. Narrow it by presence + // (the `in` operator), not truthiness. See "Oneofs in TypeScript" in + // docs/client-generation.md. + oneof reminder { + string remind_at = 10; + int32 remind_in_days = 11; + } } // --- Unwrap response: returns Note[] directly --- From 595a64a43d90c80ccc68b819b14522840428ad62 Mon Sep 17 00:00:00 2001 From: Hicham <53556927+hishamank@users.noreply.github.com> Date: Tue, 21 Jul 2026 14:04:36 +0300 Subject: [PATCH 11/11] fix(ts): correct oneof unset guard and JSON-aware variant typing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on the TS oneof discriminated-union emission in internal/tscommon/types.go: Finding 1 — flatten unset arm was under-guarded. For a flatten=true oneof the "nothing set" arm only guarded the discriminator ({ type?: never }), so a discriminator-less partial payload like { id: "1", body: "x" } spuriously type-checked as the unset arm. The unset arm now also marks every promoted flattened child key (across all variants, deduped) as ?: never via the new flattenedChildJSONNames helper. Finding 2 — presence-union payload types were computed by hand instead of routing through TSFieldType, so oneof variants lost JSON-aware field typing: enum variants became plain string and google.protobuf.Timestamp variants became a Timestamp message reference. Both the presence-union path and the non-flattened annotated path now type variant payloads with TSFieldType, so enum variants emit the enum union (or number per enum_encoding) and Timestamp variants emit string/number per timestamp_format, matching normal field typing. Message and scalar variants are unchanged. Adds golden fixtures locking in both fixes: - oneof_field_typing.proto: un-annotated oneof with enum, NUMBER-encoded enum, RFC3339 and UNIX_SECONDS Timestamp, message, and scalar variants. - flatten_oneof_unset.proto: flatten=true oneof exercising the corrected unset arm. Regenerated goldens: oneof_discriminator_client.ts and oneof_discriminator_server.ts (flatten unset arm) plus the two new client fixtures. Co-Authored-By: Claude Opus 4.8 --- internal/tsclientgen/golden_test.go | 14 +++ .../golden/flatten_oneof_unset_client.ts | 111 +++++++++++++++++ .../golden/oneof_discriminator_client.ts | 2 +- .../golden/oneof_field_typing_client.ts | 112 ++++++++++++++++++ .../testdata/proto/flatten_oneof_unset.proto | 47 ++++++++ .../testdata/proto/oneof_field_typing.proto | 53 +++++++++ internal/tscommon/types.go | 76 ++++++++---- .../golden/oneof_discriminator_server.ts | 2 +- 8 files changed, 391 insertions(+), 26 deletions(-) create mode 100644 internal/tsclientgen/testdata/golden/flatten_oneof_unset_client.ts create mode 100644 internal/tsclientgen/testdata/golden/oneof_field_typing_client.ts create mode 100644 internal/tsclientgen/testdata/proto/flatten_oneof_unset.proto create mode 100644 internal/tsclientgen/testdata/proto/oneof_field_typing.proto diff --git a/internal/tsclientgen/golden_test.go b/internal/tsclientgen/golden_test.go index 61a41fbf..0453e551 100644 --- a/internal/tsclientgen/golden_test.go +++ b/internal/tsclientgen/golden_test.go @@ -130,6 +130,20 @@ func TestTSClientGenGoldenFiles(t *testing.T) { "two_oneofs_client.ts", }, }, + { + name: "un-annotated oneof with enum and timestamp variants", + protoFile: "oneof_field_typing.proto", + expectedFiles: []string{ + "oneof_field_typing_client.ts", + }, + }, + { + name: "flatten oneof unset arm guards child keys", + protoFile: "flatten_oneof_unset.proto", + expectedFiles: []string{ + "flatten_oneof_unset_client.ts", + }, + }, { name: "SSE streaming", protoFile: "sse.proto", diff --git a/internal/tsclientgen/testdata/golden/flatten_oneof_unset_client.ts b/internal/tsclientgen/testdata/golden/flatten_oneof_unset_client.ts new file mode 100644 index 00000000..c4167a05 --- /dev/null +++ b/internal/tsclientgen/testdata/golden/flatten_oneof_unset_client.ts @@ -0,0 +1,111 @@ +// Code generated by protoc-gen-ts-client. DO NOT EDIT. +// source: flatten_oneof_unset.proto + +export type FlattenUnsetContent = + | { type: "alpha"; title: string; count: number } + | { type: "b"; body: string } + | { type?: never; title?: never; count?: never; body?: never }; + +export interface FlattenUnsetBase { + id: string; +} + +export type FlattenUnset = FlattenUnsetBase & FlattenUnsetContent; + +export interface AlphaPayload { + title: string; + count: number; +} + +export interface BetaPayload { + body: string; +} + +export interface FieldViolation { + field: string; + description: string; +} + +export class ValidationError extends Error { + violations: FieldViolation[]; + + constructor(violations: FieldViolation[]) { + super("Validation failed"); + this.name = "ValidationError"; + this.violations = violations; + } +} + +export class ApiError extends Error { + statusCode: number; + body: string; + + constructor(statusCode: number, message: string, body: string) { + super(message); + this.name = "ApiError"; + this.statusCode = statusCode; + this.body = body; + } +} + +export interface FlattenUnsetServiceClientOptions { + fetch?: typeof fetch; + defaultHeaders?: Record; +} + +export interface FlattenUnsetServiceCallOptions { + headers?: Record; + signal?: AbortSignal; +} + +export class FlattenUnsetServiceClient { + private baseURL: string; + private fetchFn: typeof fetch; + private defaultHeaders: Record; + + constructor(baseURL: string, options?: FlattenUnsetServiceClientOptions) { + this.baseURL = baseURL.replace(/\/+$/, ""); + this.fetchFn = options?.fetch ?? globalThis.fetch; + this.defaultHeaders = { ...options?.defaultHeaders }; + } + + async testFlattenUnset(req: FlattenUnset, options?: FlattenUnsetServiceCallOptions): Promise { + let path = "/api/v1/flatten-unset"; + const url = this.baseURL + path; + + const headers: Record = { + "Content-Type": "application/json", + ...this.defaultHeaders, + ...options?.headers, + }; + + const resp = await this.fetchFn(url, { + method: "POST", + headers, + body: JSON.stringify(req), + signal: options?.signal, + }); + + if (!resp.ok) { + return this.handleError(resp); + } + + return await resp.json() as FlattenUnset; + } + + private async handleError(resp: Response): Promise { + const body = await resp.text(); + if (resp.status === 400) { + try { + const parsed = JSON.parse(body); + if (parsed.violations) { + throw new ValidationError(parsed.violations); + } + } catch (e) { + if (e instanceof ValidationError) throw e; + } + } + throw new ApiError(resp.status, `Request failed with status ${resp.status}`, body); + } +} + diff --git a/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts b/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts index 7d84f2c4..9d4277da 100644 --- a/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts +++ b/internal/tsclientgen/testdata/golden/oneof_discriminator_client.ts @@ -4,7 +4,7 @@ export type FlattenedEventContent = | { type: "text"; body: string } | { type: "img"; url: string; width: number; height: number } - | { type?: never }; + | { type?: never; body?: never; url?: never; width?: never; height?: never }; export interface FlattenedEventBase { id: string; diff --git a/internal/tsclientgen/testdata/golden/oneof_field_typing_client.ts b/internal/tsclientgen/testdata/golden/oneof_field_typing_client.ts new file mode 100644 index 00000000..38ce0efa --- /dev/null +++ b/internal/tsclientgen/testdata/golden/oneof_field_typing_client.ts @@ -0,0 +1,112 @@ +// Code generated by protoc-gen-ts-client. DO NOT EDIT. +// source: oneof_field_typing.proto + +export type OneofFieldTypingValue = + | { color: Color; colorNum?: never; when?: never; whenUnix?: never; text?: never; note?: never } + | { colorNum: number; color?: never; when?: never; whenUnix?: never; text?: never; note?: never } + | { when: string; color?: never; colorNum?: never; whenUnix?: never; text?: never; note?: never } + | { whenUnix: number; color?: never; colorNum?: never; when?: never; text?: never; note?: never } + | { text: TextPayload; color?: never; colorNum?: never; when?: never; whenUnix?: never; note?: never } + | { note: string; color?: never; colorNum?: never; when?: never; whenUnix?: never; text?: never } + | { color?: never; colorNum?: never; when?: never; whenUnix?: never; text?: never; note?: never }; + +export interface OneofFieldTypingBase { + id: string; +} + +export type OneofFieldTyping = OneofFieldTypingBase & OneofFieldTypingValue; + +export interface TextPayload { + body: string; +} + +export type Color = "none" | "red" | "blue"; + +export interface FieldViolation { + field: string; + description: string; +} + +export class ValidationError extends Error { + violations: FieldViolation[]; + + constructor(violations: FieldViolation[]) { + super("Validation failed"); + this.name = "ValidationError"; + this.violations = violations; + } +} + +export class ApiError extends Error { + statusCode: number; + body: string; + + constructor(statusCode: number, message: string, body: string) { + super(message); + this.name = "ApiError"; + this.statusCode = statusCode; + this.body = body; + } +} + +export interface OneofFieldTypingServiceClientOptions { + fetch?: typeof fetch; + defaultHeaders?: Record; +} + +export interface OneofFieldTypingServiceCallOptions { + headers?: Record; + signal?: AbortSignal; +} + +export class OneofFieldTypingServiceClient { + private baseURL: string; + private fetchFn: typeof fetch; + private defaultHeaders: Record; + + constructor(baseURL: string, options?: OneofFieldTypingServiceClientOptions) { + this.baseURL = baseURL.replace(/\/+$/, ""); + this.fetchFn = options?.fetch ?? globalThis.fetch; + this.defaultHeaders = { ...options?.defaultHeaders }; + } + + async testOneofFieldTyping(req: OneofFieldTyping, options?: OneofFieldTypingServiceCallOptions): Promise { + let path = "/api/v1/oneof-field-typing"; + const url = this.baseURL + path; + + const headers: Record = { + "Content-Type": "application/json", + ...this.defaultHeaders, + ...options?.headers, + }; + + const resp = await this.fetchFn(url, { + method: "POST", + headers, + body: JSON.stringify(req), + signal: options?.signal, + }); + + if (!resp.ok) { + return this.handleError(resp); + } + + return await resp.json() as OneofFieldTyping; + } + + private async handleError(resp: Response): Promise { + const body = await resp.text(); + if (resp.status === 400) { + try { + const parsed = JSON.parse(body); + if (parsed.violations) { + throw new ValidationError(parsed.violations); + } + } catch (e) { + if (e instanceof ValidationError) throw e; + } + } + throw new ApiError(resp.status, `Request failed with status ${resp.status}`, body); + } +} + diff --git a/internal/tsclientgen/testdata/proto/flatten_oneof_unset.proto b/internal/tsclientgen/testdata/proto/flatten_oneof_unset.proto new file mode 100644 index 00000000..c4119021 --- /dev/null +++ b/internal/tsclientgen/testdata/proto/flatten_oneof_unset.proto @@ -0,0 +1,47 @@ +syntax = "proto3"; + +package testdata.flatten_oneof_unset; + +option go_package = "github.com/SebastienMelki/sebuf/internal/httpgen/testdata/flattenoneofunset;flattenoneofunset"; + +import "sebuf/http/annotations.proto"; + +// Variant payloads promoted onto the parent by the flattened oneof. +message AlphaPayload { + string title = 1; + int32 count = 2; +} + +message BetaPayload { + string body = 1; +} + +// FlattenUnset exercises a flatten=true discriminated oneof. Because a flattened +// oneof promotes each variant's child fields onto the parent object, the +// generated unset arm must guard the discriminator AND every promoted child key +// (title, count, body). Otherwise a discriminator-less partial payload such as +// { id: "1", body: "x" } would spuriously type-check as "nothing set". +message FlattenUnset { + string id = 1; + oneof content { + option (sebuf.http.oneof_config) = { + discriminator: "type" + flatten: true + }; + AlphaPayload alpha = 2; + BetaPayload beta = 3 [(sebuf.http.oneof_value) = "b"]; + } +} + +service FlattenUnsetService { + option (sebuf.http.service_config) = { + base_path: "/api/v1" + }; + + rpc TestFlattenUnset(FlattenUnset) returns (FlattenUnset) { + option (sebuf.http.config) = { + path: "/flatten-unset" + method: HTTP_METHOD_POST + }; + } +} diff --git a/internal/tsclientgen/testdata/proto/oneof_field_typing.proto b/internal/tsclientgen/testdata/proto/oneof_field_typing.proto new file mode 100644 index 00000000..d7f92688 --- /dev/null +++ b/internal/tsclientgen/testdata/proto/oneof_field_typing.proto @@ -0,0 +1,53 @@ +syntax = "proto3"; + +package testdata.oneof_field_typing; + +option go_package = "github.com/SebastienMelki/sebuf/internal/httpgen/testdata/oneoffieldtyping;oneoffieldtyping"; + +import "google/protobuf/timestamp.proto"; +import "sebuf/http/annotations.proto"; + +// Color enum with custom enum_value mappings. +enum Color { + COLOR_UNSPECIFIED = 0 [(sebuf.http.enum_value) = "none"]; + COLOR_RED = 1 [(sebuf.http.enum_value) = "red"]; + COLOR_BLUE = 2 [(sebuf.http.enum_value) = "blue"]; +} + +// Message variant, to prove message-typed oneof members are unchanged. +message TextPayload { + string body = 1; +} + +// OneofFieldTyping exercises an un-annotated oneof whose variants must pick up +// the same JSON-aware field typing that normal fields use: +// - an enum variant emits the enum union (Color), not plain string; +// - an enum variant with NUMBER encoding emits number; +// - a google.protobuf.Timestamp variant emits string (RFC3339 default), and +// the Timestamp message is skipped from the generated output; +// - a Timestamp variant with UNIX_SECONDS emits number; +// - message and scalar variants keep their existing shapes. +message OneofFieldTyping { + string id = 1; + oneof value { + Color color = 2; + Color color_num = 3 [(sebuf.http.enum_encoding) = ENUM_ENCODING_NUMBER]; + google.protobuf.Timestamp when = 4; + google.protobuf.Timestamp when_unix = 5 [(sebuf.http.timestamp_format) = TIMESTAMP_FORMAT_UNIX_SECONDS]; + TextPayload text = 6; + string note = 7; + } +} + +service OneofFieldTypingService { + option (sebuf.http.service_config) = { + base_path: "/api/v1" + }; + + rpc TestOneofFieldTyping(OneofFieldTyping) returns (OneofFieldTyping) { + option (sebuf.http.config) = { + path: "/oneof-field-typing" + method: HTTP_METHOD_POST + }; + } +} diff --git a/internal/tscommon/types.go b/internal/tscommon/types.go index 3d4fc9cc..b3ae13d3 100644 --- a/internal/tscommon/types.go +++ b/internal/tscommon/types.go @@ -450,8 +450,7 @@ func GenerateOneofDiscriminatedUnionType(p Printer, msgName string, info *annota var branches []string for i, variant := range info.Variants { var branch string - switch { - case info.Flatten && variant.IsMessage: + if info.Flatten && variant.IsMessage { // Flattened: { discriminator: "value", ...variant fields } branch = fmt.Sprintf("{ %s: \"%s\"", info.Discriminator, variant.DiscriminatorVal) var sb strings.Builder @@ -462,35 +461,38 @@ func GenerateOneofDiscriminatedUnionType(p Printer, msgName string, info *annota } branch += sb.String() branch += " }" - case variant.IsMessage: - // Non-flattened message. The generated MarshalJSON keeps the variant's + } else { + // Non-flattened variant. The generated MarshalJSON keeps the variant's // JSON key where protojson put it — directly on the parent object, // next to the discriminator — so the arm carries both, with sibling - // variant keys typed `?: never` for presence narrowing. - branch = nonFlattenedOneofBranch( - info, variant.DiscriminatorVal, jsonNames, i, - string(variant.Field.Message.Desc.Name()), - ) - default: - // Scalar variant: flat shape with the scalar's TS type. Flatten+scalar is - // rejected by validateOneofFlatten (flatten variants must be messages), so - // a scalar variant only ever reaches the non-flatten branch. + // variant keys typed `?: never` for presence narrowing. The payload + // type flows through TSFieldType so enum/timestamp/message/scalar + // variants pick up the same JSON-aware typing as normal fields. + // (Flatten+scalar is rejected by validateOneofFlatten, so a scalar + // variant only ever reaches this non-flatten branch.) branch = nonFlattenedOneofBranch( info, variant.DiscriminatorVal, jsonNames, i, - TSScalarTypeForField(variant.Field), + TSFieldType(variant.Field), ) } branches = append(branches, branch) } // Unset arm: proto3 oneofs may have no member set, in which case the - // generated MarshalJSON omits the discriminator entirely. + // generated MarshalJSON omits the discriminator entirely. The arm must + // guard every key the set arms could promote onto the parent object, or a + // partial payload lacking the discriminator would spuriously type-check as + // "nothing set". For non-flattened oneofs those keys are the variant JSON + // keys; for flattened oneofs they are the promoted child field keys of + // every variant. var unset strings.Builder fmt.Fprintf(&unset, "{ %s?: never", info.Discriminator) - if !info.Flatten { - for _, jsonName := range jsonNames { - fmt.Fprintf(&unset, "; %s?: never", jsonName) - } + guardedKeys := jsonNames + if info.Flatten { + guardedKeys = flattenedChildJSONNames(info) + } + for _, jsonName := range guardedKeys { + fmt.Fprintf(&unset, "; %s?: never", jsonName) } unset.WriteString(" }") branches = append(branches, unset.String()) @@ -529,6 +531,32 @@ func nonFlattenedOneofBranch( return sb.String() } +// flattenedChildJSONNames returns the unique JSON names of every flattened +// variant's child fields, in variant order then field order, deduplicated. +// These are the keys a flattened oneof promotes onto the parent object; the +// unset arm guards them so a discriminator-less partial payload (e.g. +// { body: "x" }) does not spuriously type-check as "nothing set". Only message +// variants contribute keys — flatten variants are always messages, enforced by +// validateOneofFlatten. +func flattenedChildJSONNames(info *annotations.OneofDiscriminatorInfo) []string { + seen := make(map[string]bool) + var names []string + for _, variant := range info.Variants { + if !variant.IsMessage { + continue + } + for _, childField := range variant.Field.Message.Fields { + jsonName := childField.Desc.JSONName() + if seen[jsonName] { + continue + } + seen[jsonName] = true + names = append(names, jsonName) + } + } + return names +} + // generatePresenceOneofUnionType renders a oneof that has no wire discriminator // (no oneof_config annotation) as a union discriminated by key presence, matching // protojson serialization: exactly the set member's JSON key appears on the parent @@ -540,11 +568,11 @@ func generatePresenceOneofUnionType(p Printer, unionName string, info *annotatio tsTypes := make([]string, len(info.Variants)) for i, variant := range info.Variants { jsonNames[i] = variant.Field.Desc.JSONName() - if variant.IsMessage { - tsTypes[i] = string(variant.Field.Message.Desc.Name()) - } else { - tsTypes[i] = TSScalarTypeForField(variant.Field) - } + // Route the payload through the same field-typing path normal fields + // use so enum variants emit the enum union (or numeric encoding) and + // google.protobuf.Timestamp variants emit string/number per + // timestamp_format — not a hand-computed scalar/message-name type. + tsTypes[i] = TSFieldType(variant.Field) } var branches []string diff --git a/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts b/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts index 7de9ed7f..417d346f 100644 --- a/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts +++ b/internal/tsservergen/testdata/golden/oneof_discriminator_server.ts @@ -4,7 +4,7 @@ export type FlattenedEventContent = | { type: "text"; body: string } | { type: "img"; url: string; width: number; height: number } - | { type?: never }; + | { type?: never; body?: never; url?: never; width?: never; height?: never }; export interface FlattenedEventBase { id: string;