Skip to content

Commit

Permalink
Revert "fix: support inlined or aliased structs as type enum variants (
Browse files Browse the repository at this point in the history
…#1535)"

This reverts commit 21df777.

# Conflicts:
#	go-runtime/compile/schema_test.go
#	go-runtime/compile/testdata/failing/failing.go
  • Loading branch information
matt2e committed May 22, 2024
1 parent 8086136 commit 62d307d
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 140 deletions.
1 change: 1 addition & 0 deletions buildengine/testdata/projects/another/another.go
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
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

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
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

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

98 changes: 43 additions & 55 deletions go-runtime/compile/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,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 +252,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 +671,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 +782,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 +865,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 @@ -1256,14 +1238,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 +1260,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 @@ -1340,7 +1332,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 +1344,32 @@ 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]()
return optional.None[schema.Type](), false
}

// 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()
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]()
return optional.None[schema.Type](), true
} 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]()
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 @@ -1539,7 +1527,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).Get()
aDecl, ok := p.getDeclForTypeName(name)
if !ok {
return optional.None[*schema.Enum](), optional.None[*types.Interface]()
}
Expand All @@ -1558,7 +1546,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 +1555,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
34 changes: 9 additions & 25 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.
export enum ColorInt: Int {
enum ColorInt: Int {
// RedInt is a color.
RedInt = 0
BlueInt = 1
Expand All @@ -92,13 +92,6 @@ 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 @@ -114,9 +107,6 @@ func TestExtractModuleSchema(t *testing.T) {
export data ExportedStruct {
}
export data InlineStruct {
}
export data Nested {
}
Expand Down Expand Up @@ -150,9 +140,6 @@ func TestExtractModuleSchema(t *testing.T) {
data SourceResp {
}
export data UnderlyingStruct {
}
data WithoutDirectiveStruct {
}
Expand Down Expand Up @@ -323,7 +310,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 @@ -452,19 +439,16 @@ func TestErrorReporting(t *testing.T) {
`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`,
`118: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`,
`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`,
`152:2-2: unsupported type ftl/failing/child.ChildStruct from subpackage`,
`152:2-6: unsupported type "ftl/failing/child.ChildStruct" for field "Data"`,
`153:2-2: unsupported type ftl/failing/child.ChildString from subpackage`,
`153:2-8: unsupported type "ftl/failing/child.ChildString" for field "String"`,
`157:1-1: unsupported type ftl/failing/child.ChildStruct from subpackage`,
`157:1-33: unsupported type "struct{Name string}" for type alias`,
}
assert.Equal(t, expected, actual)
}
6 changes: 0 additions & 6 deletions go-runtime/compile/testdata/failing/failing.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,6 @@ type ConformsToTwoTypeEnums string
func (ConformsToTwoTypeEnums) typeEnum1() {}
func (ConformsToTwoTypeEnums) typeEnum2() {}

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

//ftl:enum
type NoMethodsTypeEnum interface{}

//ftl:data
type StructUsingSubpackage struct {
Message string `json:"message"`
Expand Down
Loading

0 comments on commit 62d307d

Please sign in to comment.