From e286698b25e9248773b6cabab45f251b825b5b47 Mon Sep 17 00:00:00 2001 From: spinillos Date: Thu, 9 Feb 2023 18:30:04 +0100 Subject: [PATCH 1/2] Fix adding duplicate fields when they are comming from an extended class --- generator.go | 138 +++++++++--------- .../imports/extend_without_duplicates.txtar | 43 ++++++ 2 files changed, 115 insertions(+), 66 deletions(-) create mode 100644 testdata/imports/extend_without_duplicates.txtar diff --git a/generator.go b/generator.go index 47c863d..02cca71 100644 --- a/generator.go +++ b/generator.go @@ -462,70 +462,8 @@ func (g *generator) genInterface(name string, v cue.Value) []ts.Decl { return nil } - // Create an empty value, onto which we'll unify fields that need not be - // generated as literals. - nolit := v.Context().CompileString("{...}") - - var extends []ts.Expr - var some bool - - // Recursively walk down Values returned from Expr() and separate - // unified/embedded structs from a struct literal, so that we can make the - // former (if they are also marked with @cuetsy(kind="interface")) show up - // as "extends" instead of writing out their fields directly. - var walkExpr func(wv cue.Value) error - walkExpr = func(wv cue.Value) error { - op, dvals := wv.Expr() - switch op { - case cue.NoOp: - // Simple path - when the field is a plain struct literal decl, the walk function - // will take this branch and return immediately. - - // FIXME this does the struct literal path correctly, but it also - // catches this case, for some reason: - // - // Thing: { - // other.Thing - // } - // - // The saner form - `Thing: other.Thing` - does not go through this path. - return nil - case cue.OrOp: - return valError(wv, "typescript interfaces cannot be constructed from disjunctions") - case cue.SelectorOp: - expr, err := refAsInterface(wv) - if err != nil { - return err - } - - // If we have a string to add to the list of "extends", then also - // add the ref to the list of fields to exclude if subsumed. - if expr != nil { - some = true - extends = append(extends, expr) - nolit = nolit.Unify(cue.Dereference(wv)) - } - return nil - case cue.AndOp: - // First, search the dvals for StructLits. Having more than one is possible, - // but weird, as writing >1 literal and unifying them is the same as just writing - // one containing the unified result - more complicated with no obvious benefit. - for _, dv := range dvals { - if dv.IncompleteKind() != cue.StructKind && dv.IncompleteKind() != cue.TopKind { - panic("impossible? seems like it should be. if this pops, clearly not!") - } - - if err := walkExpr(dv); err != nil { - return err - } - } - return nil - default: - panic(fmt.Sprintf("unhandled op type %s", op.String())) - } - } - - if err := walkExpr(v); err != nil { + extends, nolit, err := findExtends(v) + if err != nil { g.addErr(err) return nil } @@ -559,15 +497,15 @@ func (g *generator) genInterface(name string, v cue.Value) []ts.Decl { // // There's _probably_ a way around this, especially when we move to an // AST rather than dumb string templates. But i'm tired of looking. - if some { + if len(extends) > 0 { // Look up the path of the current field within the nolit value, // then check it for subsumption. sel := iter.Selector() if iter.IsOptional() { sel = sel.Optional() } - sub := nolit.LookupPath(cue.MakePath(sel)) + sub := nolit.LookupPath(cue.MakePath(sel)) // Theoretically, lattice equality can be defined as bijective // subsumption. In practice, Subsume() seems to ignore optional // fields, and Equals() doesn't. So, use Equals(). @@ -636,6 +574,74 @@ func (g *generator) genInterface(name string, v cue.Value) []ts.Decl { return ret } +// Recursively walk down Values returned from Expr() and separate +// unified/embedded structs from a struct literal, so that we can make the +// former (if they are also marked with @cuetsy(kind="interface")) show up +// as "extends" instead of writing out their fields directly. +func findExtends(v cue.Value) ([]ts.Expr, cue.Value, error) { + var extends []ts.Expr + // Create an empty value, onto which we'll unify fields that need not be + // generated as literals. + baseNolit := v.Context().CompileString("") + nolit := v.Context().CompileString("") + var walkExpr func(v cue.Value) error + walkExpr = func(v cue.Value) error { + op, dvals := v.Expr() + switch op { + case cue.NoOp: + // Simple path - when the field is a plain struct literal decl, the walk function + // will take this branch and return immediately. + + // FIXME this does the struct literal path correctly, but it also + // catches this case, for some reason: + // + // Thing: { + // other.Thing + // } + // + // The saner form - `Thing: other.Thing` - does not go through this path. + return nil + case cue.OrOp: + return valError(v, "typescript interfaces cannot be constructed from disjunctions") + case cue.SelectorOp: + expr, err := refAsInterface(v) + if err != nil { + return err + } + + // If we have a string to add to the list of "extends", then also + // add the ref to the list of fields to exclude if subsumed. + if expr != nil { + extends = append(extends, expr) + nolit = baseNolit.Unify(nolit.Unify(cue.Dereference(v))) + } + return nil + case cue.AndOp: + // First, search the dvals for StructLits. Having more than one is possible, + // but weird, as writing >1 literal and unifying them is the same as just writing + // one containing the unified result - more complicated with no obvious benefit. + for _, dv := range dvals { + if dv.IncompleteKind() != cue.StructKind && dv.IncompleteKind() != cue.TopKind { + panic("impossible? seems like it should be. if this pops, clearly not!") + } + + if err := walkExpr(dv); err != nil { + return err + } + } + return nil + default: + panic(fmt.Sprintf("unhandled op type %s", op.String())) + } + } + + if err := walkExpr(v); err != nil { + return nil, nolit, err + } + + return extends, nolit, nil +} + // Generate a typeRef for the cue.Value func (g *generator) genInterfaceField(v cue.Value) (*typeRef, error) { tref := &typeRef{} diff --git a/testdata/imports/extend_without_duplicates.txtar b/testdata/imports/extend_without_duplicates.txtar new file mode 100644 index 0000000..f573604 --- /dev/null +++ b/testdata/imports/extend_without_duplicates.txtar @@ -0,0 +1,43 @@ +-- cue.mod/module.cue -- +module: "example.com" + +-- one.cue -- +package test + +import "example.com/dep" + +#Extended: { + dep.#Base + #AStruct + #BStruct + field: string +} @cuetsy(kind="interface") + +#AStruct: { + anotherField: string +} @cuetsy(kind="interface") + +#BStruct: { + moreField: string +} @cuetsy(kind="interface") + +-- dep/file.cue -- +package dep + +#Base: { + baseField: string +} @cuetsy(kind="interface") + +-- out/gen -- + +export interface Extended extends dep.Base, AStruct, BStruct { + field: string; +} + +export interface AStruct { + anotherField: string; +} + +export interface BStruct { + moreField: string; +} From 63eec989138d6a48eb8beef0826f73dc04f266f8 Mon Sep 17 00:00:00 2001 From: sam boyer Date: Sat, 11 Feb 2023 17:32:39 -0500 Subject: [PATCH 2/2] Update generator.go --- generator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generator.go b/generator.go index 02cca71..a1c2ebc 100644 --- a/generator.go +++ b/generator.go @@ -577,7 +577,7 @@ func (g *generator) genInterface(name string, v cue.Value) []ts.Decl { // Recursively walk down Values returned from Expr() and separate // unified/embedded structs from a struct literal, so that we can make the // former (if they are also marked with @cuetsy(kind="interface")) show up -// as "extends" instead of writing out their fields directly. +// as "extends" instead of inlining their fields. func findExtends(v cue.Value) ([]ts.Expr, cue.Value, error) { var extends []ts.Expr // Create an empty value, onto which we'll unify fields that need not be