Skip to content

Commit bdd623d

Browse files
ThomasK33ThomasK33
committed
refactor: improve code generation with cleaner function signatures and JSON marshaling
Change-Id: Ia6003b4adb006cce9db9e6ab9b0a17294bfcfa44 Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent cd12c48 commit bdd623d

12 files changed

Lines changed: 96 additions & 129 deletions

File tree

‎go/cmd/generate/internal/emit/dispatch.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func WriteDispatchJen(outDir string, schema *load.Schema, meta *load.Meta) error
7979
} else if ir.IsNullResponse(schema.Defs[respName]) {
8080
caseBody = append(caseBody, jCallRequestNoResp(recv, methodName)...)
8181
} else {
82-
caseBody = append(caseBody, jCallRequestWithResp(recv, methodName, respName)...)
82+
caseBody = append(caseBody, jCallRequestWithResp(recv, methodName)...)
8383
}
8484
}
8585
if len(caseBody) > 0 {
@@ -186,7 +186,7 @@ func WriteDispatchJen(outDir string, schema *load.Schema, meta *load.Meta) error
186186
if ir.IsNullResponse(schema.Defs[respName]) {
187187
body = append(body, jCallRequestNoResp(recv, methodName)...)
188188
} else {
189-
body = append(body, jCallRequestWithResp(recv, methodName, respName)...)
189+
body = append(body, jCallRequestWithResp(recv, methodName)...)
190190
}
191191
}
192192
if len(body) > 0 {

‎go/cmd/generate/internal/emit/dispatch_helpers.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func jCallRequestNoResp(recv, methodName string) []Code {
6262
}
6363
}
6464

65-
func jCallRequestWithResp(recv, methodName, respType string) []Code {
65+
func jCallRequestWithResp(recv, methodName string) []Code {
6666
return []Code{
6767
List(Id("resp"), Id("err")).Op(":=").Id(recv).Dot(methodName).Call(Id("ctx"), Id("p")),
6868
If(Id("err").Op("!=").Nil()).Block(jRetToReqErr()),

‎go/cmd/generate/internal/emit/doc.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// Package emit contains helpers to generate the Go SDK from the
2+
// intermediate representation produced by the ACP generator. It
3+
// encapsulates code emission for types, helpers, and dispatch logic.
4+
package emit

‎go/cmd/generate/internal/emit/types.go‎

Lines changed: 76 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func WriteTypesJen(outDir string, schema *load.Schema, meta *load.Meta) error {
8888
if _, ok := req[pk]; !ok {
8989
// Default: omit if empty, except for specific always-present fields
9090
// Ensure InitializeResponse.authMethods is always encoded (even when empty)
91-
if !(name == "InitializeResponse" && pk == "authMethods") {
91+
if name != "InitializeResponse" || pk != "authMethods" {
9292
tag = pk + ",omitempty"
9393
}
9494
}
@@ -583,86 +583,87 @@ func emitUnion(f *File, name string, defs []*load.Definition, exactlyOne bool) {
583583
// Null-only variant encodes to JSON null
584584
if vi.isNull {
585585
gg.Return(Qual("encoding/json", "Marshal").Call(Nil()))
586-
}
587-
// Marshal variant to map for discriminant injection and shaping
588-
gg.Var().Id("m").Map(String()).Any()
589-
gg.List(Id("_b"), Id("_e")).Op(":=").Qual("encoding/json", "Marshal").Call(Op("*").Id("u").Dot(vi.fieldName))
590-
gg.If(Id("_e").Op("!=").Nil()).Block(Return(Index().Byte().Values(), Id("_e")))
591-
gg.If(Qual("encoding/json", "Unmarshal").Call(Id("_b"), Op("&").Id("m")).Op("!=").Nil()).Block(Return(Index().Byte().Values(), Qual("errors", "New").Call(Lit("invalid variant payload"))))
592-
// Inject const discriminants
593-
if len(vi.constPairs) > 0 {
594-
for _, kv := range vi.constPairs {
595-
gg.Id("m").Index(Lit(kv[0])).Op("=").Lit(kv[1])
586+
} else {
587+
// Marshal variant to map for discriminant injection and shaping
588+
gg.Var().Id("m").Map(String()).Any()
589+
gg.List(Id("_b"), Id("_e")).Op(":=").Qual("encoding/json", "Marshal").Call(Op("*").Id("u").Dot(vi.fieldName))
590+
gg.If(Id("_e").Op("!=").Nil()).Block(Return(Index().Byte().Values(), Id("_e")))
591+
gg.If(Qual("encoding/json", "Unmarshal").Call(Id("_b"), Op("&").Id("m")).Op("!=").Nil()).Block(Return(Index().Byte().Values(), Qual("errors", "New").Call(Lit("invalid variant payload"))))
592+
// Inject const discriminants
593+
if len(vi.constPairs) > 0 {
594+
for _, kv := range vi.constPairs {
595+
gg.Id("m").Index(Lit(kv[0])).Op("=").Lit(kv[1])
596+
}
596597
}
597-
}
598-
// Special shaping for ContentBlock variants to preserve exact wire JSON
599-
if name == "ContentBlock" {
600-
switch vi.discValue {
601-
case "text":
602-
gg.Block(
603-
Var().Id("nm").Map(String()).Any(),
604-
Id("nm").Op("=").Make(Map(String()).Any()),
605-
Id("nm").Index(Lit("type")).Op("=").Lit("text"),
606-
Id("nm").Index(Lit("text")).Op("=").Id("m").Index(Lit("text")),
607-
Return(Qual("encoding/json", "Marshal").Call(Id("nm"))),
608-
)
609-
case "image":
610-
gg.Block(
611-
Var().Id("nm").Map(String()).Any(),
612-
Id("nm").Op("=").Make(Map(String()).Any()),
613-
Id("nm").Index(Lit("type")).Op("=").Lit("image"),
614-
Id("nm").Index(Lit("data")).Op("=").Id("m").Index(Lit("data")),
615-
Id("nm").Index(Lit("mimeType")).Op("=").Id("m").Index(Lit("mimeType")),
616-
// Only include uri if present; do not emit null
617-
If(List(Id("_v"), Id("_ok")).Op(":=").Id("m").Index(Lit("uri")), Id("_ok")).Block(
618-
Id("nm").Index(Lit("uri")).Op("=").Id("_v"),
619-
),
620-
Return(Qual("encoding/json", "Marshal").Call(Id("nm"))),
621-
)
622-
case "audio":
623-
gg.Block(
624-
Var().Id("nm").Map(String()).Any(),
625-
Id("nm").Op("=").Make(Map(String()).Any()),
626-
Id("nm").Index(Lit("type")).Op("=").Lit("audio"),
627-
Id("nm").Index(Lit("data")).Op("=").Id("m").Index(Lit("data")),
628-
Id("nm").Index(Lit("mimeType")).Op("=").Id("m").Index(Lit("mimeType")),
629-
Return(Qual("encoding/json", "Marshal").Call(Id("nm"))),
630-
)
631-
case "resource_link":
632-
gg.BlockFunc(func(b *Group) {
633-
b.Var().Id("nm").Map(String()).Any()
634-
b.Id("nm").Op("=").Make(Map(String()).Any())
635-
b.Id("nm").Index(Lit("type")).Op("=").Lit("resource_link")
636-
b.Id("nm").Index(Lit("name")).Op("=").Id("m").Index(Lit("name"))
637-
b.Id("nm").Index(Lit("uri")).Op("=").Id("m").Index(Lit("uri"))
638-
// Only include optional keys if present
639-
b.If(List(Id("v1"), Id("ok1")).Op(":=").Id("m").Index(Lit("description")), Id("ok1")).Block(
640-
Id("nm").Index(Lit("description")).Op("=").Id("v1"),
598+
// Special shaping for ContentBlock variants to preserve exact wire JSON
599+
if name == "ContentBlock" {
600+
switch vi.discValue {
601+
case "text":
602+
gg.Block(
603+
Var().Id("nm").Map(String()).Any(),
604+
Id("nm").Op("=").Make(Map(String()).Any()),
605+
Id("nm").Index(Lit("type")).Op("=").Lit("text"),
606+
Id("nm").Index(Lit("text")).Op("=").Id("m").Index(Lit("text")),
607+
Return(Qual("encoding/json", "Marshal").Call(Id("nm"))),
641608
)
642-
b.If(List(Id("v2"), Id("ok2")).Op(":=").Id("m").Index(Lit("mimeType")), Id("ok2")).Block(
643-
Id("nm").Index(Lit("mimeType")).Op("=").Id("v2"),
609+
case "image":
610+
gg.Block(
611+
Var().Id("nm").Map(String()).Any(),
612+
Id("nm").Op("=").Make(Map(String()).Any()),
613+
Id("nm").Index(Lit("type")).Op("=").Lit("image"),
614+
Id("nm").Index(Lit("data")).Op("=").Id("m").Index(Lit("data")),
615+
Id("nm").Index(Lit("mimeType")).Op("=").Id("m").Index(Lit("mimeType")),
616+
// Only include uri if present; do not emit null
617+
If(List(Id("_v"), Id("_ok")).Op(":=").Id("m").Index(Lit("uri")), Id("_ok")).Block(
618+
Id("nm").Index(Lit("uri")).Op("=").Id("_v"),
619+
),
620+
Return(Qual("encoding/json", "Marshal").Call(Id("nm"))),
644621
)
645-
b.If(List(Id("v3"), Id("ok3")).Op(":=").Id("m").Index(Lit("size")), Id("ok3")).Block(
646-
Id("nm").Index(Lit("size")).Op("=").Id("v3"),
622+
case "audio":
623+
gg.Block(
624+
Var().Id("nm").Map(String()).Any(),
625+
Id("nm").Op("=").Make(Map(String()).Any()),
626+
Id("nm").Index(Lit("type")).Op("=").Lit("audio"),
627+
Id("nm").Index(Lit("data")).Op("=").Id("m").Index(Lit("data")),
628+
Id("nm").Index(Lit("mimeType")).Op("=").Id("m").Index(Lit("mimeType")),
629+
Return(Qual("encoding/json", "Marshal").Call(Id("nm"))),
647630
)
648-
b.If(List(Id("v4"), Id("ok4")).Op(":=").Id("m").Index(Lit("title")), Id("ok4")).Block(
649-
Id("nm").Index(Lit("title")).Op("=").Id("v4"),
631+
case "resource_link":
632+
gg.BlockFunc(func(b *Group) {
633+
b.Var().Id("nm").Map(String()).Any()
634+
b.Id("nm").Op("=").Make(Map(String()).Any())
635+
b.Id("nm").Index(Lit("type")).Op("=").Lit("resource_link")
636+
b.Id("nm").Index(Lit("name")).Op("=").Id("m").Index(Lit("name"))
637+
b.Id("nm").Index(Lit("uri")).Op("=").Id("m").Index(Lit("uri"))
638+
// Only include optional keys if present
639+
b.If(List(Id("v1"), Id("ok1")).Op(":=").Id("m").Index(Lit("description")), Id("ok1")).Block(
640+
Id("nm").Index(Lit("description")).Op("=").Id("v1"),
641+
)
642+
b.If(List(Id("v2"), Id("ok2")).Op(":=").Id("m").Index(Lit("mimeType")), Id("ok2")).Block(
643+
Id("nm").Index(Lit("mimeType")).Op("=").Id("v2"),
644+
)
645+
b.If(List(Id("v3"), Id("ok3")).Op(":=").Id("m").Index(Lit("size")), Id("ok3")).Block(
646+
Id("nm").Index(Lit("size")).Op("=").Id("v3"),
647+
)
648+
b.If(List(Id("v4"), Id("ok4")).Op(":=").Id("m").Index(Lit("title")), Id("ok4")).Block(
649+
Id("nm").Index(Lit("title")).Op("=").Id("v4"),
650+
)
651+
b.Return(Qual("encoding/json", "Marshal").Call(Id("nm")))
652+
})
653+
case "resource":
654+
gg.Block(
655+
Var().Id("nm").Map(String()).Any(),
656+
Id("nm").Op("=").Make(Map(String()).Any()),
657+
Id("nm").Index(Lit("type")).Op("=").Lit("resource"),
658+
Id("nm").Index(Lit("resource")).Op("=").Id("m").Index(Lit("resource")),
659+
Return(Qual("encoding/json", "Marshal").Call(Id("nm"))),
650660
)
651-
b.Return(Qual("encoding/json", "Marshal").Call(Id("nm")))
652-
})
653-
case "resource":
654-
gg.Block(
655-
Var().Id("nm").Map(String()).Any(),
656-
Id("nm").Op("=").Make(Map(String()).Any()),
657-
Id("nm").Index(Lit("type")).Op("=").Lit("resource"),
658-
Id("nm").Index(Lit("resource")).Op("=").Id("m").Index(Lit("resource")),
659-
Return(Qual("encoding/json", "Marshal").Call(Id("nm"))),
660-
)
661+
}
662+
}
663+
// default: remarshal possibly with injected discriminant
664+
if name != "ContentBlock" {
665+
gg.Return(Qual("encoding/json", "Marshal").Call(Id("m")))
661666
}
662-
}
663-
// default: remarshal possibly with injected discriminant
664-
if name != "ContentBlock" {
665-
gg.Return(Qual("encoding/json", "Marshal").Call(Id("m")))
666667
}
667668
})
668669
}

‎go/cmd/generate/internal/ir/ir.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// Package ir defines the intermediate representation used by the Go
2+
// code generator. It organizes methods, bindings, and schema-derived
3+
// types so the emit package can produce helpers, types, and dispatch code.
14
package ir
25

36
import (
@@ -144,14 +147,14 @@ func BuildMethodGroups(schema *load.Schema, meta *load.Meta) Groups {
144147
}
145148
// Post-process bindings and docs-ignore
146149
for _, mi := range groups {
147-
mi.Binding = classifyBinding(schema, meta, mi)
150+
mi.Binding = classifyBinding(schema, mi)
148151
mi.DocsIgnored = isDocsIgnoredMethod(schema, mi)
149152
}
150153
return groups
151154
}
152155

153156
// classifyBinding determines interface binding for each method.
154-
func classifyBinding(schema *load.Schema, meta *load.Meta, mi *MethodInfo) MethodBinding {
157+
func classifyBinding(schema *load.Schema, mi *MethodInfo) MethodBinding {
155158
if mi == nil {
156159
return BindUnknown
157160
}

‎go/cmd/generate/internal/load/load.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// Package load provides utilities to read the ACP JSON schema and
2+
// accompanying metadata into minimal structures used by the generator.
13
package load
24

35
import (

‎go/cmd/generate/internal/util/util.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// Package util contains small string and identifier helpers used by the
2+
// code generator for formatting names and comments.
13
package util
24

35
import (

‎go/example/agent/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (a *exampleAgent) Cancel(ctx context.Context, params acp.CancelNotification
6161
return nil
6262
}
6363

64-
func (a *exampleAgent) Prompt(ctx context.Context, params acp.PromptRequest) (acp.PromptResponse, error) {
64+
func (a *exampleAgent) Prompt(_ context.Context, params acp.PromptRequest) (acp.PromptResponse, error) {
6565
sid := string(params.SessionId)
6666
s, ok := a.sessions[sid]
6767
if !ok {

‎go/example/claude-code/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func (c *replClient) RequestPermission(ctx context.Context, params acp.RequestPe
5555
continue
5656
}
5757
idx := -1
58-
fmt.Sscanf(line, "%d", &idx)
58+
_, _ = fmt.Sscanf(line, "%d", &idx)
5959
idx = idx - 1
6060
if idx >= 0 && idx < len(params.Options) {
6161
return acp.RequestPermissionResponse{Outcome: acp.RequestPermissionOutcome{Selected: &acp.RequestPermissionOutcomeSelected{OptionId: params.Options[idx].OptionId}}}, nil

‎go/example/client/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func (e *exampleClient) RequestPermission(ctx context.Context, params acp.Reques
3939
continue
4040
}
4141
idx := -1
42-
fmt.Sscanf(line, "%d", &idx)
42+
_, _ = fmt.Sscanf(line, "%d", &idx)
4343
idx = idx - 1
4444
if idx >= 0 && idx < len(params.Options) {
4545
return acp.RequestPermissionResponse{Outcome: acp.RequestPermissionOutcome{Selected: &acp.RequestPermissionOutcomeSelected{OptionId: params.Options[idx].OptionId}}}, nil

0 commit comments

Comments
 (0)