internal/core/dep: handle inline composite literals

When a selector selects a field in a literal struct
(or list), it should be handled separately. Firstly,
the struct (or list) has no path from the root of the
CUE value. Before, not handling this resulted in
nonsensical path values.

Also, because recursive values of literal structs
(or lists) have no path, all references should be
traversed and found.

This adds one more large test case for which the
results are not entirely correct. This will be
fixed in subsequent CLs.

Issue #2247

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I9f4888e9939a7541f2e8f5ae59e7ee7d0f0cb4ed
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/556281
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>
Reviewed-by: Roger Peppe <[email protected]>

internal/core/dep/dep.go

@@ -218,7 +218,45 @@ func (c *visitor) markResolver(env *adt.Environment, r adt.Resolver) {
 	// Note: it is okay to pass an empty CloseInfo{} here as we assume that
 	// all nodes are finalized already and we need neither closedness nor cycle
 	// checks.
-	if ref, _ := c.ctxt.Resolve(adt.MakeConjunct(env, r, adt.CloseInfo{}), r); ref != nil {
+	ref, _ := c.ctxt.Resolve(adt.MakeConjunct(env, r, adt.CloseInfo{}), r)
+
+	// If a vertex is dynamic, for instance the result of {foo: 1}.foo,
+	// then it means there is no valid path to the returned value and
+	// it is pointless to mark dependencies. Instead, we mark dependencies
+	// for the LHS of selector or index references.
+	var expr adt.Expr
+	if ref != nil && isDynamic(ref) {
+		switch x := r.(type) {
+		case *adt.SelectorExpr:
+			expr = x.X
+		case *adt.IndexExpr:
+			expr = x.X
+		}
+	}
+	// TODO: consider the case where an inlined composite literal does not
+	// resolve, but has references. For instance, {a: k, ref}.b would result
+	// in a failure during evaluation if b is not defined within ref. However,
+	// ref might still specialize to allow b.
+	if expr != nil {
+		// Within a dynamic struct, we always process all references,
+		// As these references will otherwise not be visited as part of
+		// a normal traversal as they have no path from the root to reach them.
+
+		// TODO: this probably processes more references than necessary.
+		// Consider:
+		//	x: {
+		//		foo: ref1
+		//		bar: ref2
+		//	}.bar
+		// In this case ref1 should probably not be processed.
+		saved := c.all
+		c.all = true
+		c.markExpr(env, expr)
+		c.all = saved
+		return
+	}
+
+	if ref != nil {
 		// If ref is within a let, we only care about dependencies referred to
 		// by internal expressions. The let expression itself is not considered
 		// a dependency and is considered part of the referring expression.
@@ -267,7 +305,7 @@ func (c *visitor) markResolver(env *adt.Environment, r adt.Resolver) {
 }
 
 // TODO(perf): make this available as a property of vertices to avoid doing
-// these dynamic lookups.
+// work repeatedly.
 func hasLetParent(v *adt.Vertex) bool {
 	for ; v != nil; v = v.Parent {
 		if v.Label.IsLet() {
@@ -277,6 +315,17 @@ func hasLetParent(v *adt.Vertex) bool {
 	return false
 }
 
+// TODO(perf): make this available as a property of vertices to avoid doing
+// work repeatedly.
+func isDynamic(v *adt.Vertex) bool {
+	for ; v != nil; v = v.Parent {
+		if v.IsDynamic {
+			return true
+		}
+	}
+	return false
+}
+
 func (c *visitor) markSubExpr(env *adt.Environment, x adt.Expr) {
 	if c.all {
 		saved := c.top

internal/core/dep/testdata/inline.txtar

@@ -9,6 +9,8 @@ a: b: {
 
 	h1: {out: r: s: string}.out
 	h2: {out: {out: r: c}.out}.out.r[0]
+	// TODO: Should this reference be q or q.x?
+	// q is an approximation, but q.x is more precise, it seems.
 	h3: {out: {out: r: q}.out}.out.r.x
 
 	// TODO: filter these dependencies?
@@ -26,23 +28,18 @@ q: {
 r: {out: k: l: string}.out
 -- out/dependencies/field --
 -- out/dependencies/all --
-[0]
-out
-out
-out
-out
-out.r
-out.r.x
+q
+q
+q
 r.k
 r.k.l
 -- out/dependencies/dynamic --
-a.b[0]
-a.b.out
-a.b.out
-a.b.out
-a.b.out
-a.b.out.r[0]
-a.b.out.r.x
+a.b.c
+a.b.c
+q
+q
+a.b.c
+q
 a.b.h1.r
 a.b.h1.r.s
 r.k

internal/core/dep/testdata/selfref.txtar

@@ -27,7 +27,8 @@ outer: k: l: int
 -- out/dependencies/all --
 outer
 -- out/dependencies/dynamic --
-value
+a.instance
+outer
 a.command.alias.val
 a.#base
 a.#combined

internal/core/export/testdata/selfcontained/issue2247.txtar

@@ -0,0 +1,70 @@
+#inlineImports: true
+-- cue.mod/module.cue --
+module: "example.com"
+-- x.cue --
+import "example.com/t"
+
+f: t.p
+-- t/t.cue --
+package t
+
+p: {
+	c: [int]
+	d: [c][0]
+	e: {out: c}.out
+	f: {out: q}.out
+	g: {out: q}.out
+
+	h: {out: r: s: string}.out
+	i: h.r
+	j: h.r.s
+
+	k: r.k
+	l: r.k.l
+}
+
+q: {
+	x: [...int]
+}
+
+r: {out: k: l: string}.out
+
+-- out/default --
+import "example.com/t"
+
+f: t.p
+-- out/expand_imports --
+f: {
+	c: [int]
+	d: [c][0]
+	e: {
+		out: c
+	}.out
+	f: {
+		out: Q
+	}.out
+	g: {
+		out: Q
+	}.out
+	h: {
+		out: {
+			r: {
+				s: string
+			}
+		}
+	}.out
+	i: h.r
+	j: h.r.s
+	k: K
+	l: K.l
+}
+
+//cue:path: q
+let Q = {
+	x: [...int]
+}
+
+//cue:path: r.k
+let K = {
+	l: string
+}

internal/core/export/testdata/selfcontained/issue2247crash.txtar

@@ -4,7 +4,4 @@ a: _
 b: [a][0]
 -- out/default --
 a: _
-b: Index0
-
-//cue:path: 0
-let Index0 = a
+b: [a][0]