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.

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: support inlined or aliased structs as type enum variants #1535

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 0 additions & 1 deletion buildengine/testdata/projects/another/another.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,21 @@ type MyTime time.Time

func (MyTime) tag() {}

type List []string
type MyList []string

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

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

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

type MyString string

func (MyString) tag() {}

type Struct struct{}
type MyStruct struct{}

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

type MyOption ftl.Option[string]

Expand Down
22 changes: 11 additions & 11 deletions buildengine/testdata/type_registry_main.go

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

13 changes: 13 additions & 0 deletions go-runtime/compile/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,19 @@ 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

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

96 changes: 54 additions & 42 deletions go-runtime/compile/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func extractTypeDeclsForNode(pctx *parseContext, node *ast.GenDecl) {
switch dir := dir.(type) {
case *directiveEnum:
typ := pctx.pkg.TypesInfo.TypeOf(t.Type)
switch typ.Underlying().(type) {
switch underlying := typ.Underlying().(type) {
case *types.Basic:
enum := &schema.Enum{
Pos: goPosToSchemaPos(node.Pos()),
Expand All @@ -251,6 +251,23 @@ 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 @@ -670,7 +687,7 @@ func visitGenDecl(pctx *parseContext, node *ast.GenDecl) {
}
}
} else if _, ok := dir.(*directiveTypeAlias); ok {
decl, ok := pctx.getDeclForTypeName(t.Name.Name)
decl, ok := pctx.getDeclForTypeName(t.Name.Name).Get()
if !ok {
pctx.errors.add(errorf(node, "could not find type alias declaration for %q", t.Name.Name))
return
Expand Down Expand Up @@ -781,10 +798,7 @@ func maybeVisitTypeEnumVariant(pctx *parseContext, node *ast.GenDecl, directives
isExported := enum.IsExported()
for _, dir := range directives {
if exportableDir, ok := dir.(exportable); ok {
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()
isExported = exportableDir.IsExported() || isExported
}
}
vType, ok := visitTypeValue(pctx, named, t.Type, nil, isExported).Get()
Expand Down Expand Up @@ -864,7 +878,11 @@ func visitTypeValue(pctx *parseContext, named *types.Named, tnode ast.Expr, inde
}

default:
if typ, ok := visitType(pctx, tnode.Pos(), named, isExported).Get(); ok {
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 {
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 @@ -1226,20 +1244,14 @@ 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())
decl, ok := pctx.getDeclForTypeName(named.Obj().Name()).Get()
if ok {
switch decl.(type) {
case *schema.TypeAlias, *schema.Enum:
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()),
})
return visitNamedRef(pctx, pos, named, isExported)
case *schema.Data, *schema.Verb, *schema.Config, *schema.Secret, *schema.Database, *schema.FSM:
}
}
Expand All @@ -1248,12 +1260,8 @@ 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 {
ref, doneWithVisit := visitNamedRef(pctx, pos, named)
if doneWithVisit {
return ref
}
return visitNamedRef(pctx, pos, named, isExported)
}

switch underlying.Kind() {
case types.String:
return optional.Some[schema.Type](&schema.String{Pos: goPosToSchemaPos(pos)})
Expand Down Expand Up @@ -1315,10 +1323,7 @@ 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 {
ref, doneWithVisit := visitNamedRef(pctx, pos, named)
if doneWithVisit {
return ref
}
return visitNamedRef(pctx, pos, named, isExported)
}
return optional.None[schema.Type]()

Expand All @@ -1327,29 +1332,36 @@ func visitType(pctx *parseContext, pos token.Pos, tnode types.Type, isExported b
}
}

func visitNamedRef(pctx *parseContext, pos token.Pos, named *types.Named) (optional.Option[schema.Type], bool) {
func visitNamedRef(pctx *parseContext, pos token.Pos, named *types.Named, isExported bool) optional.Option[schema.Type] {
if named.Obj().Pkg() == nil {
return optional.None[schema.Type](), false
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)
}
}

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

ref := &schema.Ref{
Pos: goPosToSchemaPos(pos),
Module: destModule,
Name: strcase.ToUpperCamel(named.Obj().Name()),
}
return optional.Some[schema.Type](ref)
}

func visitMap(pctx *parseContext, pos token.Pos, tnode *types.Map, isExported bool) optional.Option[schema.Type] {
Expand Down Expand Up @@ -1492,7 +1504,7 @@ func (p *parseContext) isPathInPkg(path string) bool {

// 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)
aDecl, ok := p.getDeclForTypeName(name).Get()
if !ok {
return optional.None[*schema.Enum](), optional.None[*types.Interface]()
}
Expand All @@ -1511,7 +1523,7 @@ func (p *parseContext) getEnumForTypeName(name string) (optional.Option[*schema.
return optional.Some(decl), optional.None[*types.Interface]()
}

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

func (p *parseContext) markAsExported(node schema.Node) {
Expand Down
20 changes: 17 additions & 3 deletions go-runtime/compile/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func TestExtractModuleSchema(t *testing.T) {
}

// Comments about ColorInt.
enum ColorInt: Int {
export enum ColorInt: Int {
// RedInt is a color.
RedInt = 0
BlueInt = 1
Expand All @@ -92,6 +92,13 @@ func TestExtractModuleSchema(t *testing.T) {
Two = 2
}

export enum TypeEnum {
Option String?
InlineStruct one.InlineStruct
AliasedStruct one.UnderlyingStruct
ValueEnum one.ColorInt
}

data Config {
field String
}
Expand All @@ -107,6 +114,9 @@ func TestExtractModuleSchema(t *testing.T) {
export data ExportedStruct {
}

export data InlineStruct {
}

export data Nested {
}

Expand Down Expand Up @@ -140,6 +150,9 @@ func TestExtractModuleSchema(t *testing.T) {
data SourceResp {
}

export data UnderlyingStruct {
}

data WithoutDirectiveStruct {
}

Expand Down Expand Up @@ -305,7 +318,7 @@ func TestExtractModuleSchemaNamedTypes(t *testing.T) {
assert.Equal(t, normaliseString(expected), normaliseString(actual.String()))
}

func TestParseDirectives(t *testing.T) {
func TestParsedirectives(t *testing.T) {
tests := []struct {
name string
input string
Expand Down Expand Up @@ -434,10 +447,11 @@ func TestErrorReporting(t *testing.T) {
`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`,
`117:1-26: parent enum "ExportedTypeEnum" is exported, but directive "ftl:data" on "PrivateData" is not: all variants of exported enums that have a directive must be explicitly exported as well`,
`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`,
}
assert.Equal(t, expected, actual)
}
6 changes: 6 additions & 0 deletions go-runtime/compile/testdata/failing/failing.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,9 @@ type ConformsToTwoTypeEnums string

func (ConformsToTwoTypeEnums) typeEnum1() {}
func (ConformsToTwoTypeEnums) typeEnum2() {}

//ftl:enum
type TypeEnum3 interface{ ExportedMethod() }

//ftl:enum
type NoMethodsTypeEnum interface{}
23 changes: 23 additions & 0 deletions go-runtime/compile/testdata/one/one.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,29 @@ func (List) blobOrList() {}
type Nested struct {
}

//ftl:enum
type TypeEnum interface {
tag()
}

type Option ftl.Option[string]

func (Option) tag() {}

type InlineStruct struct{}

func (InlineStruct) tag() {}

type AliasedStruct UnderlyingStruct

func (AliasedStruct) tag() {}

type UnderlyingStruct struct{}

type ValueEnum ColorInt

func (ValueEnum) tag() {}

//ftl:enum
type PrivateEnum interface{ privateEnum() }

Expand Down
Loading