From 9cc1ae5efa93d26127c558081bd66dea72d5b26c Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 25 Mar 2024 11:31:47 +1100 Subject: [PATCH 1/3] fix: external_module.go with sinks/sources build failures --- backend/schema/visit.go | 11 ++++++++--- buildengine/build_go_test.go | 1 - buildengine/build_kotlin.go | 2 +- go-runtime/compile/build.go | 12 ++++++++++-- go-runtime/compile/schema_test.go | 2 +- go-runtime/compile/testdata/two/two.go | 6 +++--- 6 files changed, 23 insertions(+), 11 deletions(-) diff --git a/backend/schema/visit.go b/backend/schema/visit.go index c3430329ca..49b9450b4b 100644 --- a/backend/schema/visit.go +++ b/backend/schema/visit.go @@ -15,11 +15,16 @@ func Visit(n Node, visit func(n Node, next func() error) error) error { // VisitExcludingMetadataChildren visits all nodes in the schema except the children of metadata nodes. // This is used when generating external modules to avoid adding imports only referenced in the bodies of // stubbed verbs. -func VisitExcludingMetadataChildren(n Node, visit func(n Node, next func() error) error) error { - return visit(n, func() error { +func VisitExcludingMetadataChildren(n Node, visit func(n Node, parents []Node, next func() error) error) error { + return innerVisitExcludingMetadataChildren(n, []Node{}, visit) +} + +func innerVisitExcludingMetadataChildren(n Node, parents []Node, visit func(n Node, parents []Node, next func() error) error) error { + return visit(n, parents, func() error { if _, ok := n.(Metadata); !ok { + parents = append(parents, n) for _, child := range n.schemaChildren() { - if err := VisitExcludingMetadataChildren(child, visit); err != nil { + if err := innerVisitExcludingMetadataChildren(child, parents, visit); err != nil { return err } } diff --git a/buildengine/build_go_test.go b/buildengine/build_go_test.go index 13007f4b21..ecc380dec7 100644 --- a/buildengine/build_go_test.go +++ b/buildengine/build_go_test.go @@ -67,7 +67,6 @@ package other import ( "context" - "github.com/TBD54566975/ftl/go-runtime/ftl" ) var _ = context.Background diff --git a/buildengine/build_kotlin.go b/buildengine/build_kotlin.go index 2bdb685a64..a00e878b08 100644 --- a/buildengine/build_kotlin.go +++ b/buildengine/build_kotlin.go @@ -168,7 +168,7 @@ var scaffoldFuncs = scaffolder.FuncMap{ }, "imports": func(m *schema.Module) []string { imports := sets.NewSet[string]() - _ = schema.VisitExcludingMetadataChildren(m, func(n schema.Node, next func() error) error { + _ = schema.VisitExcludingMetadataChildren(m, func(n schema.Node, parents []schema.Node, next func() error) error { switch n.(type) { case *schema.Data: imports.Add("xyz.block.ftl.Export") diff --git a/go-runtime/compile/build.go b/go-runtime/compile/build.go index 769cbec520..91df9f68aa 100644 --- a/go-runtime/compile/build.go +++ b/go-runtime/compile/build.go @@ -222,7 +222,7 @@ var scaffoldFuncs = scaffolder.FuncMap{ }, "imports": func(m *schema.Module) map[string]string { imports := map[string]string{} - _ = schema.VisitExcludingMetadataChildren(m, func(n schema.Node, next func() error) error { + _ = schema.VisitExcludingMetadataChildren(m, func(n schema.Node, parents []schema.Node, next func() error) error { switch n := n.(type) { case *schema.Ref: if n.Module == "" || n.Module == m.Name { @@ -243,9 +243,17 @@ var scaffoldFuncs = scaffolder.FuncMap{ case *schema.Time: imports["time"] = "stdtime" - case *schema.Optional, *schema.Unit: + case *schema.Optional: imports["github.com/TBD54566975/ftl/go-runtime/ftl"] = "" + case *schema.Unit: + switch parents[len(parents)-1].(type) { + case *schema.Verb: + // Go verbs do not include Unit parameters in the signature, so should not cause an ftl import + break + default: + imports["github.com/TBD54566975/ftl/go-runtime/ftl"] = "" + } default: } return next() diff --git a/go-runtime/compile/schema_test.go b/go-runtime/compile/schema_test.go index 2719e4c431..78edb16888 100644 --- a/go-runtime/compile/schema_test.go +++ b/go-runtime/compile/schema_test.go @@ -148,7 +148,7 @@ func TestExtractModuleSchemaTwo(t *testing.T) { verb callsTwo(two.Payload) two.Payload +calls two.two - verb returnsUser(Unit) two.UserResponse? + verb returnsUser(Unit) two.UserResponse verb two(two.Payload) two.Payload } diff --git a/go-runtime/compile/testdata/two/two.go b/go-runtime/compile/testdata/two/two.go index c823117ca8..c33fa3d644 100644 --- a/go-runtime/compile/testdata/two/two.go +++ b/go-runtime/compile/testdata/two/two.go @@ -42,10 +42,10 @@ func CallsTwo(ctx context.Context, req Payload[string]) (Payload[string], error) } //ftl:export -func ReturnsUser(ctx context.Context) (ftl.Option[UserResponse], error) { - return ftl.Some[UserResponse](UserResponse{ +func ReturnsUser(ctx context.Context) (UserResponse, error) { + return UserResponse{ User: User{ Name: "John Doe", }, - }), nil + }, nil } From 52e85e8acc06b5edf39d3213366cf94a7fef85a0 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 25 Mar 2024 13:11:03 +1100 Subject: [PATCH 2/3] move verb unit logic to VisitExcludingMetadataChildren --- backend/schema/visit.go | 17 +++++++++-------- buildengine/build_kotlin.go | 2 +- go-runtime/compile/build.go | 13 ++----------- 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/backend/schema/visit.go b/backend/schema/visit.go index 49b9450b4b..c3aa4ab292 100644 --- a/backend/schema/visit.go +++ b/backend/schema/visit.go @@ -15,16 +15,17 @@ func Visit(n Node, visit func(n Node, next func() error) error) error { // VisitExcludingMetadataChildren visits all nodes in the schema except the children of metadata nodes. // This is used when generating external modules to avoid adding imports only referenced in the bodies of // stubbed verbs. -func VisitExcludingMetadataChildren(n Node, visit func(n Node, parents []Node, next func() error) error) error { - return innerVisitExcludingMetadataChildren(n, []Node{}, visit) -} - -func innerVisitExcludingMetadataChildren(n Node, parents []Node, visit func(n Node, parents []Node, next func() error) error) error { - return visit(n, parents, func() error { +func VisitExcludingMetadataChildren(n Node, visit func(n Node, next func() error) error) error { + return visit(n, func() error { if _, ok := n.(Metadata); !ok { - parents = append(parents, n) for _, child := range n.schemaChildren() { - if err := innerVisitExcludingMetadataChildren(child, parents, visit); err != nil { + _, isParentVerb := n.(*Verb) + _, isChildUnit := child.(*Unit) + if isParentVerb && isChildUnit { + // Skip visiting children of a verb that are units as the scaffolded code will not inclue them + continue + } + if err := VisitExcludingMetadataChildren(child, visit); err != nil { return err } } diff --git a/buildengine/build_kotlin.go b/buildengine/build_kotlin.go index a00e878b08..2bdb685a64 100644 --- a/buildengine/build_kotlin.go +++ b/buildengine/build_kotlin.go @@ -168,7 +168,7 @@ var scaffoldFuncs = scaffolder.FuncMap{ }, "imports": func(m *schema.Module) []string { imports := sets.NewSet[string]() - _ = schema.VisitExcludingMetadataChildren(m, func(n schema.Node, parents []schema.Node, next func() error) error { + _ = schema.VisitExcludingMetadataChildren(m, func(n schema.Node, next func() error) error { switch n.(type) { case *schema.Data: imports.Add("xyz.block.ftl.Export") diff --git a/go-runtime/compile/build.go b/go-runtime/compile/build.go index 91df9f68aa..584b13c339 100644 --- a/go-runtime/compile/build.go +++ b/go-runtime/compile/build.go @@ -222,7 +222,7 @@ var scaffoldFuncs = scaffolder.FuncMap{ }, "imports": func(m *schema.Module) map[string]string { imports := map[string]string{} - _ = schema.VisitExcludingMetadataChildren(m, func(n schema.Node, parents []schema.Node, next func() error) error { + _ = schema.VisitExcludingMetadataChildren(m, func(n schema.Node, next func() error) error { switch n := n.(type) { case *schema.Ref: if n.Module == "" || n.Module == m.Name { @@ -243,17 +243,8 @@ var scaffoldFuncs = scaffolder.FuncMap{ case *schema.Time: imports["time"] = "stdtime" - case *schema.Optional: + case *schema.Optional, *schema.Unit: imports["github.com/TBD54566975/ftl/go-runtime/ftl"] = "" - - case *schema.Unit: - switch parents[len(parents)-1].(type) { - case *schema.Verb: - // Go verbs do not include Unit parameters in the signature, so should not cause an ftl import - break - default: - imports["github.com/TBD54566975/ftl/go-runtime/ftl"] = "" - } default: } return next() From 2bf952c2d6bd2f26554d76f6fb5c46c5aeda5bff Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 25 Mar 2024 13:12:18 +1100 Subject: [PATCH 3/3] undo whitespace change --- go-runtime/compile/build.go | 1 + 1 file changed, 1 insertion(+) diff --git a/go-runtime/compile/build.go b/go-runtime/compile/build.go index 584b13c339..769cbec520 100644 --- a/go-runtime/compile/build.go +++ b/go-runtime/compile/build.go @@ -245,6 +245,7 @@ var scaffoldFuncs = scaffolder.FuncMap{ case *schema.Optional, *schema.Unit: imports["github.com/TBD54566975/ftl/go-runtime/ftl"] = "" + default: } return next()