From 3f74d1c4e86ecada949e7cfed6022b661adde79e Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 20 May 2024 11:06:23 +1000 Subject: [PATCH] fix: do not treat subpackage paths as part of the current package (#1530) fixes https://github.com/TBD54566975/ftl/issues/1529 --- go-runtime/compile/schema.go | 42 +++++++-- go-runtime/compile/schema_test.go | 90 ++++++++++--------- .../compile/testdata/failing/child/child.go | 7 ++ .../compile/testdata/failing/failing.go | 16 ++++ 4 files changed, 107 insertions(+), 48 deletions(-) create mode 100644 go-runtime/compile/testdata/failing/child/child.go diff --git a/go-runtime/compile/schema.go b/go-runtime/compile/schema.go index 5a1a54286f..1012b72f7c 100644 --- a/go-runtime/compile/schema.go +++ b/go-runtime/compile/schema.go @@ -23,6 +23,7 @@ import ( "github.com/TBD54566975/ftl/backend/schema/strcase" "github.com/TBD54566975/ftl/internal/errors" "github.com/TBD54566975/ftl/internal/goast" + islices "github.com/TBD54566975/ftl/internal/slices" ) var ( @@ -1085,6 +1086,10 @@ func visitStruct(pctx *parseContext, pos token.Pos, tnode types.Type, isExported } nodePath := named.Obj().Pkg().Path() if !pctx.isPathInPkg(nodePath) { + if strings.HasPrefix(nodePath, pctx.pkg.PkgPath+"/") { + pctx.errors.add(noEndColumnErrorf(pos, "unsupported struct %s from subpackage", named.Obj().Name())) + return optional.None[*schema.Ref]() + } destModule, ok := ftlModuleFromGoModule(nodePath).Get() if !ok { pctx.errors.add(tokenErrorf(pos, nodePath, "struct declared in non-FTL module %s", nodePath)) @@ -1302,9 +1307,14 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type, isExported b default: nodePath := named.Obj().Pkg().Path() - if !pctx.isPathInPkg(nodePath) && !strings.HasPrefix(nodePath, "ftl/") { - pctx.errors.add(noEndColumnErrorf(pos, "unsupported external type %s", nodePath+"."+named.Obj().Name())) - return optional.None[schema.Type]() + if !pctx.isPathInPkg(nodePath) { + if !strings.HasPrefix(nodePath, "ftl/") { + pctx.errors.add(noEndColumnErrorf(pos, "unsupported external type %s", nodePath+"."+named.Obj().Name())) + return optional.None[schema.Type]() + } else if strings.HasPrefix(nodePath, pctx.pkg.PkgPath+"/") { + pctx.errors.add(noEndColumnErrorf(pos, "unsupported type %s from subpackage", nodePath+"."+named.Obj().Name())) + return optional.None[schema.Type]() + } } if ref, ok := visitStruct(pctx, pos, tnode, isExported).Get(); ok { return optional.Some[schema.Type](ref) @@ -1348,7 +1358,10 @@ func visitNamedRef(pctx *parseContext, pos token.Pos, named *types.Named, isExpo nodePath := named.Obj().Pkg().Path() destModule := pctx.module.Name if !pctx.isPathInPkg(nodePath) { - if !strings.HasPrefix(named.Obj().Pkg().Path(), "ftl/") { + if strings.HasPrefix(nodePath, pctx.pkg.PkgPath+"/") { + pctx.errors.add(noEndColumnErrorf(pos, "unsupported type %s from subpackage", named.Obj().Pkg().Path()+"."+named.Obj().Name())) + return optional.None[schema.Type]() + } else if !strings.HasPrefix(named.Obj().Pkg().Path(), "ftl/") { pctx.errors.add(noEndColumnErrorf(pos, "unsupported external type %q", named.Obj().Pkg().Path()+"."+named.Obj().Name())) return optional.None[schema.Type]() @@ -1495,11 +1508,26 @@ func (p *parseContext) pathEnclosingInterval(start, end token.Pos) (pkg *package return nil, nil, false } +// isPathInPkg checks that the path is within the current package +// +// if it is in a subpackage, it will return false func (p *parseContext) isPathInPkg(path string) bool { - if path == p.pkg.PkgPath { - return true + // find all packages whose path is a prefix of the given path + pkgs := islices.Filter(p.pkgs, func(pkg *packages.Package) bool { + if path == pkg.PkgPath { + return true + } + return strings.HasPrefix(path, pkg.PkgPath+"/") + }) + if len(pkgs) == 0 { + return false } - return strings.HasPrefix(path, p.pkg.PkgPath+"/") + // sort so we know if the current package is the longest prefix + paths := islices.Map(pkgs, func(pkg *packages.Package) string { + return pkg.PkgPath + }) + slices.Sort(paths) + return paths[len(paths)-1] == p.pkg.PkgPath } // getEnumForTypeName returns the enum and interface for a given type name. diff --git a/go-runtime/compile/schema_test.go b/go-runtime/compile/schema_test.go index 0d42e84361..017076a1bb 100644 --- a/go-runtime/compile/schema_test.go +++ b/go-runtime/compile/schema_test.go @@ -411,47 +411,55 @@ func TestErrorReporting(t *testing.T) { filename := filepath.Join(pwd, `testdata/failing/failing.go`) actual := slices.Map(r.MustGet().Errors, func(e *schema.Error) string { return strings.TrimPrefix(e.Error(), filename+":") }) expected := []string{ - `10:13-34: first argument to config and secret declarations must be the name as a string literal`, - `13:18-52: duplicate config declaration at 12:18-52`, - `16:18-52: duplicate secret declaration at 15:18-52`, - `19:14-44: duplicate database declaration at 18:14-44`, - `22:2-10: unsupported type "error" for field "BadParam"`, - `25:2-17: unsupported type "uint64" for field "AnotherBadParam"`, - `28:3-3: unexpected token "export" (expected Directive)`, - `34:36-39: unsupported request type "ftl/failing.Request"`, - `34:50-50: unsupported response type "ftl/failing.Response"`, - `35:16-29: call first argument must be a function but is an unresolved reference to lib.OtherFunc`, - `35:16-29: call first argument must be a function in an ftl module`, - `36:2-46: call must have exactly three arguments`, - `37:16-25: call first argument must be a function in an ftl module`, - `42:1-2: must have at most two parameters (context.Context, struct)`, - `42:69-69: unsupported response type "ftl/failing.Response"`, - `47:22-27: first parameter must be of type context.Context but is ftl/failing.Request`, - `47:37-43: second parameter must be a struct but is string`, - `47:53-53: unsupported response type "ftl/failing.Response"`, - `52:43-47: second parameter must not be ftl.Unit`, - `52:59-59: unsupported response type "ftl/failing.Response"`, - `57:1-2: first parameter must be context.Context`, - `57:18-18: unsupported response type "ftl/failing.Response"`, - `62:1-2: must have at most two results (struct, error)`, - `62:41-44: unsupported request type "ftl/failing.Request"`, - `67:1-2: must at least return an error`, - `67:36-39: unsupported request type "ftl/failing.Request"`, - `71:35-38: unsupported request type "ftl/failing.Request"`, - `71:48-48: must return an error but is ftl/failing.Response`, - `76:41-44: unsupported request type "ftl/failing.Request"`, - `76:55-55: first result must be a struct but is string`, - `76:63-63: must return an error but is string`, - `76:63-63: second result must not be ftl.Unit`, - `83:1-1: duplicate verb name "WrongResponse"`, - `89:2-12: struct field unexported must be exported by starting with an uppercase letter`, - `101:2-24: cannot attach enum value to BadValueEnum because it is a variant of type enum TypeEnum, not a value enum`, - `108:2-41: cannot attach enum value to BadValueEnumOrderDoesntMatter because it is a variant of type enum TypeEnum, not a value enum`, - `121:21-60: config and secret names must be valid identifiers`, - `127:1-26: only one directive expected for type alias`, - `143:1-35: type can not be a variant of more than 1 type enums (TypeEnum1, TypeEnum2)`, - `149:27-27: enum discriminator "TypeEnum3" cannot contain exported methods`, - `152:1-35: enum discriminator "NoMethodsTypeEnum" must define at least one method`, + `11:13-34: first argument to config and secret declarations must be the name as a string literal`, + `14:18-52: duplicate config declaration at 13:18-52`, + `17:18-52: duplicate secret declaration at 16:18-52`, + `20:14-44: duplicate database declaration at 19:14-44`, + `23:2-10: unsupported type "error" for field "BadParam"`, + `26:2-17: unsupported type "uint64" for field "AnotherBadParam"`, + `29:3-3: unexpected token "export" (expected Directive)`, + `35:36-39: unsupported request type "ftl/failing.Request"`, + `35:50-50: unsupported response type "ftl/failing.Response"`, + `36:16-29: call first argument must be a function but is an unresolved reference to lib.OtherFunc`, + `36:16-29: call first argument must be a function in an ftl module`, + `37:2-46: call must have exactly three arguments`, + `38:16-25: call first argument must be a function in an ftl module`, + `43:1-2: must have at most two parameters (context.Context, struct)`, + `43:69-69: unsupported response type "ftl/failing.Response"`, + `48:22-27: first parameter must be of type context.Context but is ftl/failing.Request`, + `48:37-43: second parameter must be a struct but is string`, + `48:53-53: unsupported response type "ftl/failing.Response"`, + `53:43-47: second parameter must not be ftl.Unit`, + `53:59-59: unsupported response type "ftl/failing.Response"`, + `58:1-2: first parameter must be context.Context`, + `58:18-18: unsupported response type "ftl/failing.Response"`, + `63:1-2: must have at most two results (struct, error)`, + `63:41-44: unsupported request type "ftl/failing.Request"`, + `68:1-2: must at least return an error`, + `68:36-39: unsupported request type "ftl/failing.Request"`, + `72:35-38: unsupported request type "ftl/failing.Request"`, + `72:48-48: must return an error but is ftl/failing.Response`, + `77:41-44: unsupported request type "ftl/failing.Request"`, + `77:55-55: first result must be a struct but is string`, + `77:63-63: must return an error but is string`, + `77:63-63: second result must not be ftl.Unit`, + `84:1-1: duplicate verb name "WrongResponse"`, + `90:2-12: struct field unexported must be exported by starting with an uppercase letter`, + `102:2-24: cannot attach enum value to BadValueEnum because it is a variant of type enum TypeEnum, not a value enum`, + `109:2-41: cannot attach enum value to BadValueEnumOrderDoesntMatter because it is a variant of type enum TypeEnum, not a value enum`, + `122:21-60: config and secret names must be valid identifiers`, + `128:1-26: only one directive expected for type alias`, + `144:1-35: type can not be a variant of more than 1 type enums (TypeEnum1, TypeEnum2)`, + `150:27-27: enum discriminator "TypeEnum3" cannot contain exported methods`, + `153:1-35: enum discriminator "NoMethodsTypeEnum" must define at least one method`, + `156:1-2: could not find enum called NoMethodsTypeEnum`, + `158:2-2: unsupported type ftl/failing/child.ChildStruct from subpackage`, + `158:2-6: unsupported type "ftl/failing/child.ChildStruct" for field "Data"`, + `159:2-2: unsupported type ftl/failing/child.ChildString from subpackage`, + `159:2-8: unsupported type "ftl/failing/child.ChildString" for field "String"`, + `163:1-1: unsupported type ftl/failing/child.ChildStruct from subpackage`, + "163:1-33: could not find enum called NoMethodsTypeEnum", + `163:1-33: unsupported type "struct{Name string}" for type alias`, } assert.Equal(t, expected, actual) } diff --git a/go-runtime/compile/testdata/failing/child/child.go b/go-runtime/compile/testdata/failing/child/child.go new file mode 100644 index 0000000000..2754d93352 --- /dev/null +++ b/go-runtime/compile/testdata/failing/child/child.go @@ -0,0 +1,7 @@ +package child + +type ChildStruct struct { + Name string +} + +type ChildString string diff --git a/go-runtime/compile/testdata/failing/failing.go b/go-runtime/compile/testdata/failing/failing.go index 65f6c473bc..976fd3d559 100644 --- a/go-runtime/compile/testdata/failing/failing.go +++ b/go-runtime/compile/testdata/failing/failing.go @@ -2,6 +2,7 @@ package failing import ( "context" + "ftl/failing/child" lib "github.com/TBD54566975/ftl/go-runtime/compile/testdata" "github.com/TBD54566975/ftl/go-runtime/ftl" @@ -150,3 +151,18 @@ type TypeEnum3 interface{ ExportedMethod() } //ftl:enum type NoMethodsTypeEnum interface{} + +//ftl:data +type StructUsingSubpackage struct { + Message string `json:"message"` + Data child.ChildStruct `json:"data"` + String child.ChildString `json:"string"` +} + +//ftl:typealias +type TypeAlias child.ChildStruct + +func aFunc() { + // nothing wrong with using a child struct normally + _ = child.ChildStruct{} +}