Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow subpackage types #1558

Merged
merged 2 commits into from
May 23, 2024
Merged

fix: allow subpackage types #1558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62d307d
Revert "fix: support inlined or aliased structs as type enum variants…
matt2e May 22, 2024
c04c7c3
Revert "fix: do not treat subpackage paths as part of the current pac…
matt2e May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions buildengine/testdata/projects/another/another.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type One int

func (One) typeEnum() {}

//ftl:typealias export
type Two string

func (Two) typeEnum() {}
Expand Down
12 changes: 6 additions & 6 deletions buildengine/testdata/projects/other/other.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,21 @@ type MyTime time.Time

func (MyTime) tag() {}

type MyList []string
type List []string

func (MyList) tag() {}
func (List) tag() {}

type MyMap map[string]string
type Map map[string]string

func (MyMap) tag() {}
func (Map) tag() {}

type MyString string

func (MyString) tag() {}

type MyStruct struct{}
type Struct struct{}

func (MyStruct) tag() {}
func (Struct) tag() {}

type MyOption ftl.Option[string]

Expand Down
22 changes: 11 additions & 11 deletions buildengine/testdata/type_registry_main.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 0 additions & 13 deletions go-runtime/compile/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,19 +350,6 @@ var scaffoldFuncs = scaffolder.FuncMap{
return out
},
"schemaType": schemaType,
// A standalone enum variant is one that is purely an alias to a type and does not appear
// elsewhere in the schema.
"isStandaloneEnumVariant": func(v schema.EnumVariant) bool {
tv, ok := v.Value.(*schema.TypeValue)
if !ok {
return false
}
if ref, ok := tv.Value.(*schema.Ref); ok {
return ref.Name != v.Name
}

return false
},
}

func schemaType(t schema.Type) string {
Expand Down
2 changes: 1 addition & 1 deletion ..._ftl/go/modules/{{ range .NonMainModules }}{{ push .Name . }}{{ end }}/external_module.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

138 changes: 49 additions & 89 deletions go-runtime/compile/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ 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 (
Expand Down Expand Up @@ -240,7 +239,7 @@ func extractTypeDeclsForNode(pctx *parseContext, node *ast.GenDecl) {
switch dir := dir.(type) {
case *directiveEnum:
typ := pctx.pkg.TypesInfo.TypeOf(t.Type)
switch underlying := typ.Underlying().(type) {
switch typ.Underlying().(type) {
case *types.Basic:
enum := &schema.Enum{
Pos: goPosToSchemaPos(node.Pos()),
Expand All @@ -252,23 +251,6 @@ func extractTypeDeclsForNode(pctx *parseContext, node *ast.GenDecl) {
pctx.module.Decls = append(pctx.module.Decls, enum)
pctx.nativeNames[enum] = nativeName
case *types.Interface:
if underlying.NumMethods() == 0 {
pctx.errors.add(errorf(node, "enum discriminator %q must define at least one method", t.Name.Name))
break
}

hasExportedMethod := false
for i, n := 0, underlying.NumMethods(); i < n; i++ {
if underlying.Method(i).Exported() {
pctx.errors.add(noEndColumnErrorf(underlying.Method(i).Pos(), "enum discriminator %q cannot "+
"contain exported methods", t.Name.Name))
hasExportedMethod = true
}
}
if hasExportedMethod {
break
}

enum := &schema.Enum{
Pos: goPosToSchemaPos(node.Pos()),
Comments: visitComments(node.Doc),
Expand Down Expand Up @@ -688,7 +670,7 @@ func visitGenDecl(pctx *parseContext, node *ast.GenDecl) {
}
}
} else if _, ok := dir.(*directiveTypeAlias); ok {
decl, ok := pctx.getDeclForTypeName(t.Name.Name).Get()
decl, ok := pctx.getDeclForTypeName(t.Name.Name)
if !ok {
pctx.errors.add(errorf(node, "could not find type alias declaration for %q", t.Name.Name))
return
Expand Down Expand Up @@ -799,7 +781,10 @@ func maybeVisitTypeEnumVariant(pctx *parseContext, node *ast.GenDecl, directives
isExported := enum.IsExported()
for _, dir := range directives {
if exportableDir, ok := dir.(exportable); ok {
isExported = exportableDir.IsExported() || isExported
if enum.Export && !exportableDir.IsExported() {
pctx.errors.add(errorf(node, "parent enum %q is exported, but directive %q on %q is not: all variants of exported enums that have a directive must be explicitly exported as well", enumName, exportableDir, t.Name.Name))
}
isExported = exportableDir.IsExported()
}
}
vType, ok := visitTypeValue(pctx, named, t.Type, nil, isExported).Get()
Expand Down Expand Up @@ -879,11 +864,7 @@ func visitTypeValue(pctx *parseContext, named *types.Named, tnode ast.Expr, inde
}

default:
variantNode := pctx.pkg.TypesInfo.TypeOf(tnode)
if _, ok := variantNode.(*types.Struct); ok {
variantNode = named
}
if typ, ok := visitType(pctx, tnode.Pos(), variantNode, isExported).Get(); ok {
if typ, ok := visitType(pctx, tnode.Pos(), named, isExported).Get(); ok {
return optional.Some(&schema.TypeValue{Value: typ})
} else {
pctx.errors.add(errorf(tnode, "unsupported type %q for type enum variant", named))
Expand Down Expand Up @@ -1093,10 +1074,6 @@ 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))
Expand Down Expand Up @@ -1256,14 +1233,20 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type, isExported b
if tparam, ok := tnode.(*types.TypeParam); ok {
return optional.Some[schema.Type](&schema.Ref{Pos: goPosToSchemaPos(pos), Name: tparam.Obj().Id()})
}

if named, ok := tnode.(*types.Named); ok {
// Handle refs to type aliases and enums, rather than the underlying type.
decl, ok := pctx.getDeclForTypeName(named.Obj().Name()).Get()
decl, ok := pctx.getDeclForTypeName(named.Obj().Name())
if ok {
switch decl.(type) {
case *schema.TypeAlias, *schema.Enum:
return visitNamedRef(pctx, pos, named, isExported)
if isExported {
pctx.markAsExported(decl)
}
return optional.Some[schema.Type](&schema.Ref{
Pos: goPosToSchemaPos(pos),
Module: pctx.module.Name,
Name: strcase.ToUpperCamel(named.Obj().Name()),
})
case *schema.Data, *schema.Verb, *schema.Config, *schema.Secret, *schema.Database, *schema.FSM:
}
}
Expand All @@ -1272,8 +1255,12 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type, isExported b
switch underlying := tnode.Underlying().(type) {
case *types.Basic:
if named, ok := tnode.(*types.Named); ok {
return visitNamedRef(pctx, pos, named, isExported)
ref, doneWithVisit := visitNamedRef(pctx, pos, named)
if doneWithVisit {
return ref
}
}

switch underlying.Kind() {
case types.String:
return optional.Some[schema.Type](&schema.String{Pos: goPosToSchemaPos(pos)})
Expand Down Expand Up @@ -1314,14 +1301,9 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type, isExported b

default:
nodePath := named.Obj().Pkg().Path()
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 !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 ref, ok := visitStruct(pctx, pos, tnode, isExported).Get(); ok {
return optional.Some[schema.Type](ref)
Expand All @@ -1340,7 +1322,10 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type, isExported b
return optional.Some[schema.Type](&schema.Any{Pos: goPosToSchemaPos(pos)})
}
if named, ok := tnode.(*types.Named); ok {
return visitNamedRef(pctx, pos, named, isExported)
ref, doneWithVisit := visitNamedRef(pctx, pos, named)
if doneWithVisit {
return ref
}
}
return optional.None[schema.Type]()

Expand All @@ -1349,39 +1334,29 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type, isExported b
}
}

func visitNamedRef(pctx *parseContext, pos token.Pos, named *types.Named, isExported bool) optional.Option[schema.Type] {
func visitNamedRef(pctx *parseContext, pos token.Pos, named *types.Named) (optional.Option[schema.Type], bool) {
if named.Obj().Pkg() == nil {
return optional.None[schema.Type]()
}

// Update the visibility of the reference if the referencer is exported (ensuring refs are transitively
// exported as needed).
if isExported {
if decl, ok := pctx.getDeclForTypeName(named.Obj().Name()).Get(); ok {
pctx.markAsExported(decl)
}
return optional.None[schema.Type](), false
}

nodePath := named.Obj().Pkg().Path()
destModule := pctx.module.Name
var ref *schema.Ref
if !pctx.isPathInPkg(nodePath) {
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/") {
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]()
return optional.None[schema.Type](), true
}
base := path.Dir(pctx.pkg.PkgPath)
destModule = path.Base(strings.TrimPrefix(nodePath, base+"/"))
}
ref := &schema.Ref{
Pos: goPosToSchemaPos(pos),
Module: destModule,
Name: strcase.ToUpperCamel(named.Obj().Name()),
destModule := path.Base(strings.TrimPrefix(nodePath, base+"/"))
ref = &schema.Ref{
Pos: goPosToSchemaPos(pos),
Module: destModule,
Name: named.Obj().Name(),
}
return optional.Some[schema.Type](ref), true
}
return optional.Some[schema.Type](ref)
return optional.None[schema.Type](), false

}

func visitMap(pctx *parseContext, pos token.Pos, tnode *types.Map, isExported bool) optional.Option[schema.Type] {
Expand Down Expand Up @@ -1515,31 +1490,16 @@ 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 {
// 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
if path == p.pkg.PkgPath {
return true
}
// 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
return strings.HasPrefix(path, p.pkg.PkgPath+"/")
}

// getEnumForTypeName returns the enum and interface for a given type name.
func (p *parseContext) getEnumForTypeName(name string) (optional.Option[*schema.Enum], optional.Option[*types.Interface]) {
aDecl, ok := p.getDeclForTypeName(name).Get()
aDecl, ok := p.getDeclForTypeName(name)
if !ok {
return optional.None[*schema.Enum](), optional.None[*types.Interface]()
}
Expand All @@ -1558,7 +1518,7 @@ func (p *parseContext) getEnumForTypeName(name string) (optional.Option[*schema.
return optional.Some(decl), optional.None[*types.Interface]()
}

func (p *parseContext) getDeclForTypeName(name string) optional.Option[schema.Decl] {
func (p *parseContext) getDeclForTypeName(name string) (enum schema.Decl, ok bool) {
for _, decl := range p.module.Decls {
nativeName, ok := p.nativeNames[decl]
if !ok {
Expand All @@ -1567,9 +1527,9 @@ func (p *parseContext) getDeclForTypeName(name string) optional.Option[schema.De
if nativeName != p.pkg.Name+"."+name {
continue
}
return optional.Some(decl)
return decl, true
}
return optional.None[schema.Decl]()
return nil, false
}

func (p *parseContext) markAsExported(node schema.Node) {
Expand Down
Loading
Loading