From 1bb12683b4e57aad4702bbb259c551c0578e936f Mon Sep 17 00:00:00 2001 From: worstell Date: Thu, 8 Aug 2024 13:26:38 -0700 Subject: [PATCH] fix: value enum bugs (#2300) fixes value enum type validation const block variants with the same underlying type are correctly associated to their parent enum object fixes #2254 --- backend/schema/validate.go | 20 ++++++++++++++++++- go-runtime/schema/common/fact.go | 6 ++++-- go-runtime/schema/enum/analyzer.go | 6 +++--- go-runtime/schema/schema_test.go | 15 ++++++++++++++ go-runtime/schema/testdata/two/two.go | 20 +++++++++++++++++++ go-runtime/schema/transitive/analyzer.go | 4 ++-- .../schema/valueenumvariant/analyzer.go | 4 +++- 7 files changed, 66 insertions(+), 9 deletions(-) diff --git a/backend/schema/validate.go b/backend/schema/validate.go index b5dfeb6131..94feeb8e61 100644 --- a/backend/schema/validate.go +++ b/backend/schema/validate.go @@ -178,7 +178,9 @@ func ValidateModuleInSchema(schema *Schema, m optional.Option[*Module]) (*Schema case *Enum: if n.IsValueEnum() { for _, v := range n.Variants { - if reflect.TypeOf(v.Value.schemaValueType()) != reflect.TypeOf(n.Type) { + expected := resolveType(schema, v.Value.schemaValueType()) + actual := resolveType(schema, n.Type) + if reflect.TypeOf(expected) != reflect.TypeOf(actual) { merr = append(merr, errorf(v, "enum variant %q of type %s cannot have a value of "+ "type %q", v.Name, n.Type, v.Value.schemaValueType())) } @@ -805,3 +807,19 @@ func validateRetries(module *Module, retry *MetadataRetry, requestType optional. func typeName(v any) string { return strings.ToLower(reflect.Indirect(reflect.ValueOf(v)).Type().Name()) } + +func resolveType(sch *Schema, typ Type) Type { + ref, ok := typ.(*Ref) + if !ok { + return typ + } + resolved, ok := sch.Resolve(ref).Get() + if !ok { + return typ + } + ta, ok := resolved.(*TypeAlias) + if !ok { + return typ + } + return resolveType(sch, ta.Type) +} diff --git a/go-runtime/schema/common/fact.go b/go-runtime/schema/common/fact.go index 4375942847..2ac4109b3a 100644 --- a/go-runtime/schema/common/fact.go +++ b/go-runtime/schema/common/fact.go @@ -71,6 +71,8 @@ func (*MaybeTypeEnumVariant) schemaFactValue() {} type MaybeValueEnumVariant struct { // this variant Variant *schema.EnumVariant + // type of the variant + Type types.Object } func (*MaybeValueEnumVariant) schemaFactValue() {} @@ -152,9 +154,9 @@ func MarkMaybeTypeEnumVariant(pass *analysis.Pass, obj types.Object, variant *sc } // MarkMaybeValueEnumVariant marks the given object as a possible value enum variant. -func MarkMaybeValueEnumVariant(pass *analysis.Pass, obj types.Object, variant *schema.EnumVariant) { +func MarkMaybeValueEnumVariant(pass *analysis.Pass, obj types.Object, variant *schema.EnumVariant, typ types.Object) { fact := newFact(pass, obj) - fact.Add(&MaybeValueEnumVariant{Variant: variant}) + fact.Add(&MaybeValueEnumVariant{Variant: variant, Type: typ}) pass.ExportObjectFact(obj, fact) } diff --git a/go-runtime/schema/enum/analyzer.go b/go-runtime/schema/enum/analyzer.go index dbdf0b0552..71e1ab575f 100644 --- a/go-runtime/schema/enum/analyzer.go +++ b/go-runtime/schema/enum/analyzer.go @@ -14,8 +14,8 @@ import ( "github.com/TBD54566975/ftl/go-runtime/schema/common" ) -// Extractor extracts type aliases to the module schema. -var Extractor = common.NewDeclExtractor[*schema.Enum, *ast.TypeSpec]("typealias", Extract) +// Extractor extracts enums to the module schema. +var Extractor = common.NewDeclExtractor[*schema.Enum, *ast.TypeSpec]("enums", Extract) func Extract(pass *analysis.Pass, node *ast.TypeSpec, obj types.Object) optional.Option[*schema.Enum] { valueVariants := findValueEnumVariants(pass, obj) @@ -68,7 +68,7 @@ func Extract(pass *analysis.Pass, node *ast.TypeSpec, obj types.Object) optional func findValueEnumVariants(pass *analysis.Pass, obj types.Object) []*schema.EnumVariant { var variants []*schema.EnumVariant for o, fact := range common.GetAllFactsOfType[*common.MaybeValueEnumVariant](pass) { - if o.Type() == obj.Type() && validateVariant(pass, o, fact.Variant) { + if fact.Type == obj && validateVariant(pass, o, fact.Variant) { variants = append(variants, fact.Variant) } } diff --git a/go-runtime/schema/schema_test.go b/go-runtime/schema/schema_test.go index 83fac5161f..6e33bdaa94 100644 --- a/go-runtime/schema/schema_test.go +++ b/go-runtime/schema/schema_test.go @@ -202,12 +202,22 @@ func TestExtractModuleSchemaTwo(t *testing.T) { +typemap kotlin "com.foo.bar.NonFTLType" +typemap go "github.com/TBD54566975/ftl/go-runtime/schema/testdata.lib.NonFTLType" + typealias PaymentState String + typealias TransitiveAliasAlias Any +typemap go "github.com/TBD54566975/ftl/go-runtime/schema/testdata.lib.NonFTLType" typealias TransitiveAliasType Any +typemap go "github.com/TBD54566975/ftl/go-runtime/schema/testdata.lib.NonFTLType" + enum PayinState: two.PaymentState { + PayinPending = "PAYIN_PENDING" + } + + enum PayoutState: two.PaymentState { + PayoutPending = "PAYOUT_PENDING" + } + export enum TwoEnum: String { Blue = "Blue" Green = "Green" @@ -235,6 +245,11 @@ func TestExtractModuleSchemaTwo(t *testing.T) { body T } + data Payment { + in two.PayinState + out two.PayoutState + } + export data User { name String } diff --git a/go-runtime/schema/testdata/two/two.go b/go-runtime/schema/testdata/two/two.go index 25ec35c6de..92a31f17bb 100644 --- a/go-runtime/schema/testdata/two/two.go +++ b/go-runtime/schema/testdata/two/two.go @@ -114,3 +114,23 @@ func superTransitiveVerbCall(ctx context.Context, req Payload[string]) error { _, err := ftl.Call(ctx, Three, req) return err } + +type PaymentState string + +type PayinState PaymentState + +const ( + PayinPending PayinState = "PAYIN_PENDING" +) + +type PayoutState PaymentState + +const ( + PayoutPending PayoutState = "PAYOUT_PENDING" +) + +//ftl:data +type Payment struct { + In PayinState + Out PayoutState +} diff --git a/go-runtime/schema/transitive/analyzer.go b/go-runtime/schema/transitive/analyzer.go index 1fcbbc7660..7544d7f1a4 100644 --- a/go-runtime/schema/transitive/analyzer.go +++ b/go-runtime/schema/transitive/analyzer.go @@ -116,8 +116,8 @@ func inferDeclType(pass *analysis.Pass, node ast.Node, obj types.Object) optiona } if !common.IsSelfReference(pass, obj, t) { // if this is a type alias and it has enum variants, infer to be a value enum - for o := range common.GetAllFactsOfType[*common.MaybeValueEnumVariant](pass) { - if o.Type() == obj.Type() { + for _, fact := range common.GetAllFactsOfType[*common.MaybeValueEnumVariant](pass) { + if fact.Type == obj { return optional.Some[schema.Decl](&schema.Enum{}) } } diff --git a/go-runtime/schema/valueenumvariant/analyzer.go b/go-runtime/schema/valueenumvariant/analyzer.go index c0d7ce1a1f..6a1bc19728 100644 --- a/go-runtime/schema/valueenumvariant/analyzer.go +++ b/go-runtime/schema/valueenumvariant/analyzer.go @@ -82,7 +82,9 @@ func extractEnumVariant(pass *analysis.Pass, node *ast.ValueSpec) { if md, ok := common.GetFactForObject[*common.ExtractedMetadata](pass, obj).Get(); ok { variant.Comments = md.Comments } - common.MarkMaybeValueEnumVariant(pass, obj, variant) + if typ, ok := common.GetObjectForNode(pass.TypesInfo, node.Type).Get(); ok { + common.MarkMaybeValueEnumVariant(pass, obj, variant, typ) + } } func extractValue(pass *analysis.Pass, cnode *types.Const) optional.Option[schema.Value] {