From c71f6745235d5d2dc48ad044d93efc50485fdd7a Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Thu, 21 Jul 2022 14:53:27 +0900 Subject: [PATCH] internal/core/export: implement self-contained algo for export The ensures that all references pointing outside an exported value (in Def) are either inlined or included as let, the latter to avoid a possible exponential blowup. This does not yet handle the case where a reference points to an ancestor node of the exported value. Issue #867 Signed-off-by: Marcel van Lohuizen Change-Id: Ie5a93949a68565ea60e954417bb2573fd36b0284 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/541464 Reviewed-by: Paul Jolly Unity-Result: CUEcueckoo TryBot-Result: CUEcueckoo --- internal/core/adt/context.go | 12 + internal/core/export/adt.go | 14 + internal/core/export/export.go | 51 +- internal/core/export/self.go | 440 ++++++++++++++++++ internal/core/export/self_test.go | 197 ++++++++ .../export/testdata/selfcontained/alias.txtar | 88 ++++ .../testdata/selfcontained/alldef.txtar | 19 + .../export/testdata/selfcontained/call.txtar | 24 + .../export/testdata/selfcontained/def.txtar | 16 + .../testdata/selfcontained/errors.txtar | 53 +++ .../testdata/selfcontained/import.txtar | 98 ++++ .../testdata/selfcontained/issue867.txtar | 9 + .../testdata/selfcontained/nochange.txtar | 12 + .../export/testdata/selfcontained/ref.txtar | 49 ++ .../testdata/selfcontained/rootimport.txtar | 42 ++ .../selfcontained/selfcontained.txtar | 53 +++ .../testdata/selfcontained/simplifyexpr.txtar | 23 + 17 files changed, 1194 insertions(+), 6 deletions(-) create mode 100644 internal/core/export/self.go create mode 100644 internal/core/export/self_test.go create mode 100644 internal/core/export/testdata/selfcontained/alias.txtar create mode 100644 internal/core/export/testdata/selfcontained/alldef.txtar create mode 100644 internal/core/export/testdata/selfcontained/call.txtar create mode 100644 internal/core/export/testdata/selfcontained/def.txtar create mode 100644 internal/core/export/testdata/selfcontained/errors.txtar create mode 100644 internal/core/export/testdata/selfcontained/import.txtar create mode 100644 internal/core/export/testdata/selfcontained/issue867.txtar create mode 100644 internal/core/export/testdata/selfcontained/nochange.txtar create mode 100644 internal/core/export/testdata/selfcontained/ref.txtar create mode 100644 internal/core/export/testdata/selfcontained/rootimport.txtar create mode 100644 internal/core/export/testdata/selfcontained/selfcontained.txtar create mode 100644 internal/core/export/testdata/selfcontained/simplifyexpr.txtar diff --git a/internal/core/adt/context.go b/internal/core/adt/context.go index 6b659c1b648..351c46f9e51 100644 --- a/internal/core/adt/context.go +++ b/internal/core/adt/context.go @@ -150,6 +150,18 @@ func (c *OpContext) Logf(v *Vertex, format string, args ...interface{}) { _ = log.Output(2, s) } +// PathToString creates a pretty-printed path of the given list of features. +func (c *OpContext) PathToString(r Runtime, path []Feature) string { + var b strings.Builder + for i, f := range path { + if i > 0 { + b.WriteByte('.') + } + b.WriteString(f.SelectorString(c)) + } + return b.String() +} + // Runtime defines an interface for low-level representation conversion and // lookup. type Runtime interface { diff --git a/internal/core/export/adt.go b/internal/core/export/adt.go index b60e58a448e..b53a5e4c361 100644 --- a/internal/core/export/adt.go +++ b/internal/core/export/adt.go @@ -119,7 +119,12 @@ func (e *exporter) adt(env *adt.Environment, expr adt.Elem) ast.Expr { case adt.Resolver: return e.resolve(env, x) + } + + e.inExpression++ + defer func() { e.inExpression-- }() + switch x := expr.(type) { case *adt.SliceExpr: var lo, hi ast.Expr if x.Lo != nil { @@ -275,6 +280,15 @@ func (e *exporter) adt(env *adt.Environment, expr adt.Elem) ast.Expr { var dummyTop = &ast.Ident{Name: "_"} func (e *exporter) resolve(env *adt.Environment, r adt.Resolver) ast.Expr { + if c := e.pivotter; c != nil { + if alt := c.refExpr(r); alt != nil { + return alt + } + } + + e.inExpression++ + defer func() { e.inExpression-- }() + switch x := r.(type) { case *adt.FieldReference: ident, _ := e.newIdentForField(x.Src, x.Label, x.UpCount) diff --git a/internal/core/export/export.go b/internal/core/export/export.go index ddd37cadeb5..12ccda9d8f0 100644 --- a/internal/core/export/export.go +++ b/internal/core/export/export.go @@ -57,9 +57,11 @@ type Profile struct { // Use unevaluated conjuncts for these error types // IgnoreRecursive - // TODO: recurse over entire tree to determine transitive closure - // of what needs to be printed. - // IncludeDependencies bool + // SelfContained exports a schema such that it does not rely on any imports. + SelfContained bool + + // InlineImports expands references to non-builtin packages. + InlineImports bool } var Simplified = &Profile{ @@ -92,13 +94,16 @@ var All = &Profile{ // Concrete // Def exports v as a definition. +// It resolves references that point outside any of the vertices in v. func Def(r adt.Runtime, pkgID string, v *adt.Vertex) (*ast.File, errors.Error) { return All.Def(r, pkgID, v) } // Def exports v as a definition. -func (p *Profile) Def(r adt.Runtime, pkgID string, v *adt.Vertex) (*ast.File, errors.Error) { +// It resolves references that point outside any of the vertices in v. +func (p *Profile) Def(r adt.Runtime, pkgID string, v *adt.Vertex) (f *ast.File, err errors.Error) { e := newExporter(p, r, pkgID, v) + e.initPivot(v) isDef := v.IsRecursivelyClosed() if isDef { @@ -116,13 +121,18 @@ func (p *Profile) Def(r adt.Runtime, pkgID string, v *adt.Vertex) (*ast.File, er ) } } + return e.finalize(v, expr) } +// Expr exports the given unevaluated expression (schema mode). +// It does not resolve references that point outside the given expession. func Expr(r adt.Runtime, pkgID string, n adt.Expr) (ast.Expr, errors.Error) { return Simplified.Expr(r, pkgID, n) } +// Expr exports the given unevaluated expression (schema mode). +// It does not resolve references that point outside the given expression. func (p *Profile) Expr(r adt.Runtime, pkgID string, n adt.Expr) (ast.Expr, errors.Error) { e := newExporter(p, r, pkgID, nil) @@ -170,21 +180,33 @@ func (e *exporter) toFile(v *adt.Vertex, x ast.Expr) *ast.File { return f } +// Vertex exports evaluated values (data mode). +// It resolves incomplete references that point outside the current context. func Vertex(r adt.Runtime, pkgID string, n *adt.Vertex) (*ast.File, errors.Error) { return Simplified.Vertex(r, pkgID, n) } -func (p *Profile) Vertex(r adt.Runtime, pkgID string, n *adt.Vertex) (*ast.File, errors.Error) { +// Vertex exports evaluated values (data mode). +// It resolves incomplete references that point outside the current context. +func (p *Profile) Vertex(r adt.Runtime, pkgID string, n *adt.Vertex) (f *ast.File, err errors.Error) { e := newExporter(p, r, pkgID, n) + e.initPivot(n) + v := e.value(n, n.Conjuncts...) return e.finalize(n, v) } +// Value exports evaluated values (data mode). +// It does not resolve references that point outside the given Value. func Value(r adt.Runtime, pkgID string, n adt.Value) (ast.Expr, errors.Error) { return Simplified.Value(r, pkgID, n) } -// Should take context. +// Value exports evaluated values (data mode). +// +// It does not resolve references that point outside the given Value. +// +// TODO: Should take context. func (p *Profile) Value(r adt.Runtime, pkgID string, n adt.Value) (ast.Expr, errors.Error) { e := newExporter(p, r, pkgID, n) v := e.value(n) @@ -204,6 +226,7 @@ type exporter struct { stack []frame inDefinition int // for close() wrapping. + inExpression int // for inlining decisions. // hidden label handling pkgID string @@ -217,6 +240,8 @@ type exporter struct { letAlias map[*ast.LetClause]*ast.LetClause usedHidden map[string]bool + + pivotter *pivotter } // newExporter creates and initializes an exporter. @@ -234,11 +259,25 @@ func newExporter(p *Profile, r adt.Runtime, pkgID string, v adt.Value) *exporter return e } +// initPivot initializes the pivotter to allow aligning a configuration around +// a new root, if needed. +func (e *exporter) initPivot(n *adt.Vertex) { + if !e.cfg.InlineImports && + !e.cfg.SelfContained && + n.Parent == nil { + return + } + + e.initPivotter(n) +} + // finalize finalizes the result of an export. It is only needed for use cases // that require conversion to a File, Sanitization, and self containment. func (e *exporter) finalize(n *adt.Vertex, v ast.Expr) (f *ast.File, err errors.Error) { f = e.toFile(n, v) + e.completePivot(f) + if err := astutil.Sanitize(f); err != nil { err := errors.Promote(err, "export") return f, errors.Append(e.errs, err) diff --git a/internal/core/export/self.go b/internal/core/export/self.go new file mode 100644 index 00000000000..6868dc829ca --- /dev/null +++ b/internal/core/export/self.go @@ -0,0 +1,440 @@ +// Copyright 2022 CUE Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package export + +import ( + "fmt" + "strconv" + "strings" + + "cuelang.org/go/cue/ast" + "cuelang.org/go/cue/token" + "cuelang.org/go/internal/core/adt" + "cuelang.org/go/internal/core/dep" +) + +// This file contains the algorithm to contain a self-contained CUE file. + +// TODO: +// - Handle below edge cases where a reference directly references the root +// of the exported main tree. +// - Inline smallish structs that themselves do not have outside +// references. +// - Overall better inlining. +// - Consider a shorthand for the `let X = { #x: foo }` annotation. Possibly +// allow `#{}`, or allow "let definitions", like `let #X = {}`. +// - Make doc comment of root the file comment. + +// initPivotter initializes a selfContainedCloser if either a subtree +// is exported or imports need to be removed. It will not initialize one if +// neither is the case. +func (e *exporter) initPivotter(v *adt.Vertex) { + s := &pivotter{} + e.pivotter = s + s.x = e + + s.depsMap = map[*adt.Vertex]*depData{} + s.refMap = map[adt.Resolver]*refData{} + + s.linkDependencies(v) +} + +func (e *exporter) completePivot(f *ast.File) { + s := e.pivotter + if s == nil || f == nil { + return + } + for _, d := range s.deps { + if !d.isExternalRoot() { + continue + } + s.addExternal(d) + } + f.Decls = append(f.Decls, s.decls...) +} + +// A pivotter pivots a graph around a new root. +// +// Given a Vertex that itself is not the root of a configuration, the pivotter +// recomputes the configuration to have that node as a root instead. +// +// TODO: although this is currently part of Package export, it could be its own +// package down the line, if there is a proven need for it. +type pivotter struct { + x *exporter + + deps []*depData + depsMap map[*adt.Vertex]*depData + + refs []*refData + refMap map[adt.Resolver]*refData + + decls []ast.Decl +} + +type depData struct { + parent *depData + + dstNode *adt.Vertex + dstImport *adt.ImportReference + + ident adt.Feature + path []adt.Feature + useCount int // Other reference using this vertex + included bool + needTopLevel bool +} + +// isExternalRoot reports whether d is an external node (a node referenced +// outside main exported tree) that has no further parent nodes that are +// referenced. +func (d *depData) isExternalRoot() bool { + return d.ident != 0 +} + +func (d *depData) usageCount() int { + return getParent(d).useCount +} + +type refData struct { + dst *depData + ref ast.Expr +} + +func (v *depData) node() *adt.Vertex { + return v.dstNode +} + +func (p *pivotter) linkDependencies(v *adt.Vertex) { + p.markDeps(v) + + // Explicitly add the root of the configuration. + p.markIncluded(v) + + // Link one parent up + for _, d := range p.depsMap { + p.markParentsPass1(d) + } + + // Get transitive closure of parents. + for _, d := range p.depsMap { + if d.parent != nil { + d.parent = getParent(d) + d.parent.useCount++ + } + } + + // Compute the paths for the parent nodes. + for _, d := range p.deps { + if d.parent == nil { + p.makeParentPath(d) + } + } +} + +func getParent(d *depData) *depData { + for ; d.parent != nil; d = d.parent { + } + return d +} + +func (p *pivotter) markDeps(v *adt.Vertex) { + // TODO: sweep all child nodes and mark as no need for recursive checks. + + dep.VisitAll(p.x.ctx, v, func(d dep.Dependency) error { + node := d.Node + + switch { + case + // Already done. + p.refMap[d.Reference] != nil, + + // Only record nodes within import if we want to expand imports. + !p.x.cfg.InlineImports && d.Import() != nil, + + // This may happen for DynamicReferences. + node.Status() == adt.Unprocessed: + + return nil + } + + data, ok := p.depsMap[node] + if !ok { + data = &depData{ + dstNode: node, + dstImport: d.Import(), + } + p.depsMap[node] = data + p.deps = append(p.deps, data) + } + data.useCount++ + + ref := &refData{dst: data} + p.refs = append(p.refs, ref) + p.refMap[d.Reference] = ref + + if !ok { + p.markDeps(node) + } + + return nil + }) +} + +// markIncluded marks all referred nodes that are within the normal tree to be +// exported. +func (p *pivotter) markIncluded(v *adt.Vertex) { + d, ok := p.depsMap[v] + if !ok { + d = &depData{dstNode: v} + p.depsMap[v] = d + } + d.included = true + + for _, a := range v.Arcs { + p.markIncluded(a) + } +} + +// markParentPass1 marks the furthest ancestor node for which there is a +// dependency as its parent. Only dependencies that do not have a parent +// will be assigned to hidden reference. +func (p *pivotter) markParentsPass1(d *depData) { + for n := d.node().Parent; n != nil; n = n.Parent { + if v, ok := p.depsMap[n]; ok { + d.parent = v + } + } +} + +func (p *pivotter) makeParentPath(d *depData) { + if d.parent != nil { + panic("not a parent") + } + + if d.included || d.isExternalRoot() { + return + } + + var f adt.Feature + + if path := d.dstNode.Path(); len(path) > 0 { + f = path[len(path)-1] + } else if imp := d.dstImport; imp != nil { + f = imp.Label + } else { + panic("unexpected zero path length") + } + + str := f.IdentString(p.x.ctx) + str = strings.TrimLeft(str, "_#") + str = strings.ToUpper(str) + uf, _ := p.x.uniqueFeature(str) + + d.path = []adt.Feature{uf} + d.ident = uf + + // Make it a definition if we need it. + if d.dstNode.IsRecursivelyClosed() { + d.path = append(d.path, adt.MakeIdentLabel(p.x.ctx, "#x", "")) + } +} + +// makeAlternativeReference computes the alternative path for the reference. +func (p *pivotter) makeAlternativeReference(ref *refData, r adt.Resolver) ast.Expr { + d := ref.dst + + // Determine if the reference can be inline. + + var path []adt.Feature + if d.parent == nil { + // Get canonical vertexData. + path = d.path + } else { + pathLen, pkgRef := relPathLength(r) + path = d.node().Path() + count := d.stepsToParent() + + switch { + case ref.dst.included: + // Inside main tree. + if count > pathLen { + // Cannot refer to root, so cannot use >= + return nil + } + + case pkgRef: + + default: + // Inside hoisted value. + if count >= pathLen { + return nil + } + } + + path = path[len(path)-count:] + path = append(d.parent.path, path...) + } + + if len(path) == 0 { + path = append(path, p.x.ctx.StringLabel("ROOT")) + } + + var x ast.Expr = p.x.ident(path[0]) + + for _, f := range path[1:] { + if f.IsInt() { + x = &ast.IndexExpr{ + X: x, + Index: ast.NewLit(token.INT, strconv.Itoa(f.Index())), + } + } else { + x = &ast.SelectorExpr{ + X: x, + Sel: p.x.stringLabel(f), + } + } + } + + return x +} + +func (d *depData) stepsToParent() int { + parent := d.parent.node() + count := 0 + for p := d.node(); p != parent; p = p.Parent { + if p == nil { + break + } + count++ + } + return count +} + +func relPathLength(r adt.Resolver) (length int, newRoot bool) { + for { + var expr adt.Expr + switch x := r.(type) { + case *adt.FieldReference, + *adt.DynamicReference, + *adt.LetReference, + *adt.ValueReference: + length++ + case *adt.ImportReference: + // This reference indicates a different vertex as root, but doesn't + // increase the path length. + return length, true + case *adt.SelectorExpr: + length++ + expr = x.X + case *adt.IndexExpr: + length++ + expr = x.X + } + + switch x := expr.(type) { + case nil: + return length, false + case adt.Resolver: + r = x + default: + panic("unreachable") + } + } +} + +// refExpr returns a substituted expression for a given reference, or nil if +// there are no changes. This function implements most of the policy to decide +// when an expression can be inlined. +func (p *pivotter) refExpr(r adt.Resolver) ast.Expr { + ref, ok := p.refMap[r] + if !ok { + return nil + } + + dst := ref.dst + n := dst.node() + + // Inline value, but only when this may not lead to an exponential + // expansion. We allow inlining when a value is only used once, or when + // it is a simple concrete scalar value. + switch { + case dst.included: + // Keep references that point inside the hoisted vertex. + // TODO: force hoisting. This would be akin to taking the interpretation + // that references that initially point outside the included vertex + // are external inputs too, even if they eventually point inside. + case p.x.inDefinition == 0 && n.IsRecursivelyClosed(): + // We need to wrap the value in a definition. + case dst.usageCount() == 0: + // The root value itself. + case dst.usageCount() == 1 && p.x.inExpression == 0: + // Used only once. + fallthrough + case n.IsConcrete() && len(n.Arcs) == 0: + // Simple scalar value. + return p.x.expr(nil, n) + } + + if r := p.makeAlternativeReference(ref, r); r != nil { + dst.needTopLevel = true + return r + } + + return nil +} + +// addExternal converts a vertex for an external reference. +func (p *pivotter) addExternal(d *depData) { + if !d.needTopLevel { + return + } + + expr := p.x.expr(nil, d.node()) + + if len(d.path) > 1 { + expr = ast.NewStruct(&ast.Field{ + Label: p.x.ident(d.path[1]), + Value: expr, + }) + } + let := &ast.LetClause{ + Ident: p.x.ident(d.path[0]), + Expr: expr, + } + + ast.SetRelPos(let, token.NewSection) + + path := p.x.ctx.PathToString(p.x.ctx, d.node().Path()) + var msg string + if d.dstImport == nil { + msg = fmt.Sprintf("//cue:path: %s", path) + } else { + pkg := d.dstImport.ImportPath.SelectorString(p.x.ctx) + if path == "" { + msg = fmt.Sprintf("//cue:path: %s", pkg) + } else { + msg = fmt.Sprintf("//cue:path: %s.%s", pkg, path) + } + } + cg := &ast.CommentGroup{ + Doc: true, + List: []*ast.Comment{{Text: msg}}, + } + ast.SetRelPos(cg, token.NewSection) + ast.AddComment(let, cg) + + p.decls = append(p.decls, let) +} diff --git a/internal/core/export/self_test.go b/internal/core/export/self_test.go new file mode 100644 index 00000000000..68605c4c187 --- /dev/null +++ b/internal/core/export/self_test.go @@ -0,0 +1,197 @@ +// Copyright 2022 CUE Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package export_test + +import ( + "bytes" + "io/ioutil" + "log" + "strings" + "testing" + + "cuelang.org/go/cue" + "cuelang.org/go/cue/ast" + "cuelang.org/go/cue/build" + "cuelang.org/go/cue/cuecontext" + "cuelang.org/go/cue/errors" + "cuelang.org/go/cue/format" + "cuelang.org/go/internal/core/adt" + "cuelang.org/go/internal/core/export" + "cuelang.org/go/internal/cuetest" + "cuelang.org/go/internal/cuetxtar" + "cuelang.org/go/internal/diff" + "cuelang.org/go/internal/types" + "github.com/rogpeppe/go-internal/txtar" +) + +func TestSelfContained(t *testing.T) { + test := cuetxtar.TxTarTest{ + Root: "./testdata/selfcontained", + Update: cuetest.UpdateGoldenFiles, + } + + r := cuecontext.New() + + test.Run(t, func(t *cuetxtar.Test) { + a := t.ValidInstances() + + v := buildFile(t.T, r, a[0]) + + p, ok := t.Value("path") + if ok { + v = v.LookupPath(cue.ParsePath(p)) + } + + var tValue types.Value + v.Core(&tValue) + + self := *export.All + self.SelfContained = true + + w := t.Writer("default") + + test := func() { + file, errs := self.Def(tValue.R, "", tValue.V) + + errors.Print(w, errs, nil) + _, _ = w.Write(formatNode(t.T, file)) + + vf := patch(t.T, r, t.Archive, file) + doDiff(t.T, v, vf) + + v = v.Unify(vf) + doDiff(t.T, v, vf) + } + test() + + if _, ok := t.Value("inlineImports"); ok { + w = t.Writer("expand_imports") + self.InlineImports = true + test() + } + }) +} + +func buildFile(t *testing.T, r *cue.Context, b *build.Instance) cue.Value { + v := r.BuildInstance(b) + if err := v.Err(); err != nil { + t.Fatal(errors.Details(err, nil)) + } + return v +} + +// patch replaces the package at the root of the Archive with the given CUE +// file. +func patch(t *testing.T, r *cue.Context, orig *txtar.Archive, f *ast.File) cue.Value { + a := *orig + a.Files = make([]txtar.File, len(a.Files)) + copy(a.Files, orig.Files) + + k := 0 + for _, f := range a.Files { + if strings.HasSuffix(f.Name, ".cue") && !strings.ContainsRune(f.Name, '/') { + continue + } + a.Files[k] = f + k++ + } + b, err := format.Node(f) + if err != nil { + t.Error(err) + } + + a.Files = append(a.Files, txtar.File{ + Name: "in.cue", + Data: b, + }) + + dir, err := ioutil.TempDir("", "test") + if err != nil { + log.Fatal(err) + } + + instance := cuetxtar.Load(&a, dir)[0] + if instance.Err != nil { + t.Fatal(instance.Err) + } + + vn := buildFile(t, r, instance) + if err := vn.Err(); err != nil { + t.Fatal(err) + } + return vn +} + +func doDiff(t *testing.T, v, w cue.Value) { + var bb bytes.Buffer + p := diff.Schema + p.SkipHidden = true + d, script := p.Diff(v, w) + if d != diff.Identity { + diff.Print(&bb, script) + t.Error(bb.String()) + } +} + +// TestSC is for debugging purposes. Do not delete. +func TestSC(t *testing.T) { + in := ` +-- cue.mod/module.cue -- +module: "example.com/a" + +-- in.cue -- + ` + if strings.HasSuffix(strings.TrimSpace(in), ".cue --") { + t.Skip() + } + + dir, err := ioutil.TempDir("", "test") + if err != nil { + log.Fatal(err) + } + + a := txtar.Parse([]byte(in)) + instance := cuetxtar.Load(a, dir)[0] + if instance.Err != nil { + t.Fatal(instance.Err) + } + + r := cuecontext.New() + + v := buildFile(t, r, instance) + + // v = v.LookupPath(cue.ParsePath("a.b")) + + var tValue types.Value + v.Core(&tValue) + self := export.All + self.SelfContained = true + self.InlineImports = true + + adt.Verbosity = 1 + + file, errs := self.Def(tValue.R, "", tValue.V) + if errs != nil { + t.Fatal(errs) + } + + adt.Verbosity = 0 + + b, _ := format.Node(file) + t.Error(string(b)) + + vf := patch(t, r, a, file) + doDiff(t, v, vf) +} diff --git a/internal/core/export/testdata/selfcontained/alias.txtar b/internal/core/export/testdata/selfcontained/alias.txtar new file mode 100644 index 00000000000..a50b9c81d87 --- /dev/null +++ b/internal/core/export/testdata/selfcontained/alias.txtar @@ -0,0 +1,88 @@ +#path: a.b + +-- in.cue -- +E=ext: one: 1 +E1=("ext1"): two: 2 +E2=("ext2"): { three: 3, a: int} +a: B=b: { + x: string + y: B.x + + // TODO: use with non-concrete value. Right now, this causes the reference + // above to not shorten (it will point to b, instead of x, as x won't be + // reachable). + // Reevaluated after value aliases for embeddings are implemented. + z: "string" + X=(z): 4 + Y=("y"): "foo" + enclosed: { + V=x: string + y: B.enclosed.x + z: V + + labelX: X + labelY: Y + } + hoisted: { + "ext1": E + ext2: E + ext3: E1 + ext4: E2 + ext5: E2.a + } + + other: [INNER=string]: name: INNER +} + +[NAME=string]: b: name: NAME +-- out/default -- + +{ + x: string + y: x + z: "string" + X=(z): 4 + Y="y": "foo" + enclosed: { + x: string + y: enclosed.x + z: x + labelX: X + labelY: Y + } + hoisted: { + ext1: EXT + ext2: EXT + ext3: { + two: 2 + b: { + name: "ext1" + } + } + ext4: EXT2 + ext5: EXT2.a + } + other: { + [INNER=string]: { + name: INNER + } + } +} +name: "a" + +//cue:path: ext +let EXT = { + one: 1 + b: { + name: "ext" + } +} + +//cue:path: ext2 +let EXT2 = { + three: 3 + b: { + name: "ext2" + } + a: int +} diff --git a/internal/core/export/testdata/selfcontained/alldef.txtar b/internal/core/export/testdata/selfcontained/alldef.txtar new file mode 100644 index 00000000000..a97a29b770c --- /dev/null +++ b/internal/core/export/testdata/selfcontained/alldef.txtar @@ -0,0 +1,19 @@ +#path: #a.b + +No need to have an indirection to create a reference if the enclosing value +is already a definition. + +-- in.cue -- +#def: { + a: 1 +} + +#a: b: c: #def +-- out/default -- + +_#def +_#def: { + c: { + a: 1 + } +} diff --git a/internal/core/export/testdata/selfcontained/call.txtar b/internal/core/export/testdata/selfcontained/call.txtar new file mode 100644 index 00000000000..43233457955 --- /dev/null +++ b/internal/core/export/testdata/selfcontained/call.txtar @@ -0,0 +1,24 @@ +#path: a.b +-- in.cue -- +import "struct" + +s: {a: 1, b: 2} + +a: b: { + // Do not inline `s`, as it makes calls less readable. + c: struct.MaxFields(s & d, 3) + d: {} +} + +-- out/default -- +import "struct" + +// Do not inline `s`, as it makes calls less readable. +c: struct.MaxFields(S & d, 3) +d: {} + +//cue:path: s +let S = { + a: 1 + b: 2 +} diff --git a/internal/core/export/testdata/selfcontained/def.txtar b/internal/core/export/testdata/selfcontained/def.txtar new file mode 100644 index 00000000000..52a58d0b3b9 --- /dev/null +++ b/internal/core/export/testdata/selfcontained/def.txtar @@ -0,0 +1,16 @@ +#path: a.b +-- in.cue -- +#def: { + a: 1 +} + +a: b: c: #def +-- out/default -- +c: DEF.#x + +//cue:path: #def +let DEF = { + #x: { + a: 1 + } +} diff --git a/internal/core/export/testdata/selfcontained/errors.txtar b/internal/core/export/testdata/selfcontained/errors.txtar new file mode 100644 index 00000000000..a3cecf56d05 --- /dev/null +++ b/internal/core/export/testdata/selfcontained/errors.txtar @@ -0,0 +1,53 @@ +#path: root + +TODO: do not inline errors +-- in.cue -- +root: { + // comprehensions + t1: {for p in b let x = c if d {}} + + t2: e + t3: f + + // incomplete + t4: [1, 2][x] + t5: m.foo // TODO: do not inline m + t6: {a: 1}[n.foo.bar] +} +b: _ +c: _ +d: _ +e: {for p in _ {}} +f: {if _ {}} + +x: int +y: "b" +z: string +m: {} +n: {foo: {baz: int}} +-- out/default -- +// comprehensions +t1: { + for p in _ let x = _ if _ {} +} +t2: { + for p in _ {} +} +t3: { + if _ {} +} + +// incomplete +t4: [1, 2][X] +t5: {}.foo +t6: { + a: 1 +}[FOO.bar] + +//cue:path: x +let X = int + +//cue:path: n.foo +let FOO = { + baz: int +} diff --git a/internal/core/export/testdata/selfcontained/import.txtar b/internal/core/export/testdata/selfcontained/import.txtar new file mode 100644 index 00000000000..e4fbcfdc7d8 --- /dev/null +++ b/internal/core/export/testdata/selfcontained/import.txtar @@ -0,0 +1,98 @@ +#inlineImports: true + +-- cue.mod/module.cue -- +module: "example.com/a" + +-- in.cue -- +import "example.com/a/pkg" + +// Can be inlined. +v: pkg.v.v + +// Do not simplify because of multiple usages of enclosing struct. +x: pkg.a.b.c +y: pkg.a.b + +// Cannot simplify because of definition. +z: pkg.#Def.f + +// Two references to package, but since the second is a scalar, it can be +// hoisted and only one reference remains. So there is still no need for +// a helper. +// TODO: fix this to eliminate the helper. +w: pkg.w +wa: pkg.w.a + +-- pkg/pkg.cue -- +package pkg + +v: v: { x: 3, y: x } + +a: b: c: { d: int } + +#Def: f: g: int + +w: { + a: 1 + b: a +} + +-- out/default -- +import "example.com/a/pkg" + +// Can be inlined. +v: pkg.v.v + +// Do not simplify because of multiple usages of enclosing struct. +x: pkg.a.b.c +y: pkg.a.b + +// Cannot simplify because of definition. +z: pkg.#Def.f + +// Two references to package, but since the second is a scalar, it can be +// hoisted and only one reference remains. So there is still no need for +// a helper. +// TODO: fix this to eliminate the helper. +w: pkg.w +wa: pkg.w.a +-- out/expand_imports -- +// Can be inlined. +v: { + x: 3 + y: x +} + +// Do not simplify because of multiple usages of enclosing struct. +x: B.c +y: B + +// Cannot simplify because of definition. +z: F.#x + +// Two references to package, but since the second is a scalar, it can be +// hoisted and only one reference remains. So there is still no need for +// a helper. +// TODO: fix this to eliminate the helper. +w: W +wa: 1 + +//cue:path: "example.com/a/pkg".a.b +let B = { + c: { + d: int + } +} + +//cue:path: "example.com/a/pkg".#Def.f +let F = { + #x: { + g: int + } +} + +//cue:path: "example.com/a/pkg".w +let W = { + a: 1 + b: a +} diff --git a/internal/core/export/testdata/selfcontained/issue867.txtar b/internal/core/export/testdata/selfcontained/issue867.txtar new file mode 100644 index 00000000000..90ebb641fac --- /dev/null +++ b/internal/core/export/testdata/selfcontained/issue867.txtar @@ -0,0 +1,9 @@ +#path: output +-- in.cue -- +t: {name: string} +output: [...t] +-- out/default -- + +[...{ + name: string +}] diff --git a/internal/core/export/testdata/selfcontained/nochange.txtar b/internal/core/export/testdata/selfcontained/nochange.txtar new file mode 100644 index 00000000000..486eb2385b9 --- /dev/null +++ b/internal/core/export/testdata/selfcontained/nochange.txtar @@ -0,0 +1,12 @@ +#path: a.b +-- foo.cue -- +a: b: { + c: 1 + d: 2 +} +-- out/selfcontained -- +c: 1 +d: 2 +-- out/default -- +c: 1 +d: 2 diff --git a/internal/core/export/testdata/selfcontained/ref.txtar b/internal/core/export/testdata/selfcontained/ref.txtar new file mode 100644 index 00000000000..913f911f164 --- /dev/null +++ b/internal/core/export/testdata/selfcontained/ref.txtar @@ -0,0 +1,49 @@ +#path: a.b + +TODO: some alternative behaviors to consider here: +- inline small values, even in expressions. +- fail if hoisting results in an impossible expression, + such as: `foo[string]` or `{}.foo`. +- evaluate expressions to completion, if possible. + +-- in.cue -- +x: map: {foo: int} +y: z: map: {bar: int} + +incomplete1: string +incomplete2: string +completeExist: "foo" +completeNotExist: "bar" + +a: b: { + ref1: x.map + ref2: x.map.foo + ref3: x.map[incomplete1] // always an error + ref4: x.map[completeExist] // can be simplified + ref5: x.map[completeNotExist] // always an error + + ref6: y.z.map[incomplete2] // inline the single-use map. +} +-- out/default -- +ref1: MAP +ref2: MAP.foo +ref3: MAP[INCOMPLETE1] +ref4: MAP.foo +ref5: MAP["bar"] +ref6: MAP_1[INCOMPLETE2] + +//cue:path: x.map +let MAP = { + foo: int +} + +//cue:path: incomplete1 +let INCOMPLETE1 = string + +//cue:path: y.z.map +let MAP_1 = { + bar: int +} + +//cue:path: incomplete2 +let INCOMPLETE2 = string diff --git a/internal/core/export/testdata/selfcontained/rootimport.txtar b/internal/core/export/testdata/selfcontained/rootimport.txtar new file mode 100644 index 00000000000..87ec50a2592 --- /dev/null +++ b/internal/core/export/testdata/selfcontained/rootimport.txtar @@ -0,0 +1,42 @@ +Import a package at its root and ensure it comes up with a proper identifier. + +#inlineImports: true +-- cue.mod/module.cue -- +module: "example.com/a" + +-- in.cue -- +import "example.com/a/pkg" + +a: pkg + +// Force no inline. +b: pkg.v.a + +-- pkg/pkg.cue -- +package pkg + +v: { + a: int + b: a +} + +-- out/default -- +import "example.com/a/pkg" + +a: pkg + +// Force no inline. +b: pkg.v.a +-- out/expand_imports -- +a: PKG + +// Force no inline. +b: PKG.v.a + +//cue:path: "example.com/a/pkg" +let PKG = { + v: { + a: int + b: a + } +} diff --git a/internal/core/export/testdata/selfcontained/selfcontained.txtar b/internal/core/export/testdata/selfcontained/selfcontained.txtar new file mode 100644 index 00000000000..c4d822eb8d7 --- /dev/null +++ b/internal/core/export/testdata/selfcontained/selfcontained.txtar @@ -0,0 +1,53 @@ +#path: a.b + +TODO: alternatives to consider +- `b.s` rewrites to `s` here. But we could interpret this as + a "closed" value and write string instead. So + + s: string + t: string + + or + + s: string + t: _s + _s: string + + Theoretically this seems more appropriate. It just may not be + what users expect. Note that `t: s` would still remain `t: s`, + as that reference does not refer to a node outside the closed + node. + +-- in.cue -- +x: "out" +y: { + s: "foo" + t: "bar" +} +let X = "a: " + x + +a: b: { + c: x + // TODO: should most likely resolve to "out", not c, because b points to + // outside the exported value + d: b.c + e: y + f: X + g: "a: " + x // TODO: resolve + + s: string + t: b.s +} +-- out/default -- +c: "out" +// TODO: should most likely resolve to "out", not c, because b points to +// outside the exported value +d: c +e: { + s: "foo" + t: "bar" +} +f: "a: " + "out" +g: "a: " + "out" +s: string +t: s diff --git a/internal/core/export/testdata/selfcontained/simplifyexpr.txtar b/internal/core/export/testdata/selfcontained/simplifyexpr.txtar new file mode 100644 index 00000000000..d9e69564def --- /dev/null +++ b/internal/core/export/testdata/selfcontained/simplifyexpr.txtar @@ -0,0 +1,23 @@ +#path: a.b +-- in.cue -- +import "struct" + +s: {a: 1, b: 2} + +// TODO: this could be resolved because all inputs are fixed. +a: b: { + c: struct.MaxFields(s, 3) + d: {} +} + +-- out/default -- +import "struct" + +c: struct.MaxFields(S, 3) +d: {} + +//cue:path: s +let S = { + a: 1 + b: 2 +}