cue: resolve identifiers in an ast.Expr for FillPath

The Scope resolution now uses a separate code path,
instead of relying on the fileScope mechanism.
This has the advantage that the parent scopes don't
have to be added for expressions without identifiers
(the majority case) and makes it easier to support
nested scopes.

Change-Id: Iccbfe9c630455ad28ef29ea8177e40be1dc4d6d1
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/9324
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/query.go b/cue/query.go
new file mode 100644
index 0000000..7ffc020
--- /dev/null
+++ b/cue/query.go
@@ -0,0 +1,54 @@
+// Copyright 2021 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 cue
+
+import (
+	"cuelang.org/go/cue/ast"
+	"cuelang.org/go/cue/ast/astutil"
+	"cuelang.org/go/cue/token"
+	"cuelang.org/go/internal/core/adt"
+	"cuelang.org/go/internal/core/compile"
+)
+
+// This file contains query-related code.
+
+// getScopePrefix finds the Vertex that exists in v for the longest prefix of p.
+//
+// It is used to make the parent scopes visible when resolving expressions.
+func getScopePrefix(v Value, p Path) *adt.Vertex {
+	for _, sel := range p.Selectors() {
+		w := v.LookupPath(MakePath(sel))
+		if !w.Exists() {
+			break
+		}
+		v = w
+	}
+	return v.v
+}
+
+func errFn(pos token.Pos, msg string, args ...interface{}) {}
+
+// resolveExpr binds unresolved expressions to values in the expression or v.
+func resolveExpr(ctx *context, v *adt.Vertex, x ast.Expr) adt.Value {
+	cfg := &compile.Config{Scope: v}
+
+	astutil.ResolveExpr(x, errFn)
+
+	c, err := compile.Expr(cfg, ctx.opCtx, pkgID(), x)
+	if err != nil {
+		return &adt.Bottom{Err: err}
+	}
+	return adt.Resolve(ctx.opCtx, c)
+}
diff --git a/cue/types.go b/cue/types.go
index d3f0dc8..da5e1e2 100644
--- a/cue/types.go
+++ b/cue/types.go
@@ -1539,6 +1539,8 @@
 				continue outer
 			}
 		}
+		// TODO: if optional, look up template for name.
+
 		var x *adt.Bottom
 		if err, ok := sel.sel.(pathError); ok {
 			x = &adt.Bottom{Err: err.Error}
@@ -1555,6 +1557,8 @@
 // LookupDef reports the definition with the given name within struct v. The
 // Exists method of the returned value will report false if the definition did
 // not exist. The Err method reports if any error occurred during evaluation.
+//
+// Deprecated: use lookupPath.
 func (v Value) LookupDef(name string) Value {
 	ctx := v.ctx()
 	o, err := v.structValFull(ctx)
@@ -1649,9 +1653,14 @@
 // FillPath creates a new value by unifying v with the value of x at the given
 // path.
 //
-// Values may be any Go value that can be converted to CUE, an ast.Expr or a
-// Value. In the latter case, it will panic if the Value is not from the same
-// Runtime.
+// If x is an cue/ast.Expr, it will be evaluated within the context of the
+// given path: identifiers that are not resolved within the expression are
+// resolved as if they were defined at the path position.
+//
+// If x is a Value, it will be used as is. It panics if x is not created
+// from the same Runtime as v.
+//
+// Otherwise, the given Go value will be converted to CUE.
 //
 // Any reference in v referring to the value at the given path will resolve to x
 // in the newly created value. The resulting value is not validated.
@@ -1659,13 +1668,28 @@
 // List paths are not supported at this time.
 func (v Value) FillPath(p Path, x interface{}) Value {
 	if v.v == nil {
+		// TODO: panic here?
 		return v
 	}
 	ctx := v.ctx()
 	if err := p.Err(); err != nil {
 		return newErrValue(v, ctx.mkErr(nil, 0, "invalid path: %v", err))
 	}
-	var expr adt.Expr = convert.GoValueToValue(ctx.opCtx, x, true)
+	var expr adt.Expr
+	switch x := x.(type) {
+	case Value:
+		if v.idx != x.idx {
+			panic("values are not from the same runtime")
+		}
+		expr = x.v
+	case adt.Node, adt.Feature:
+		panic("cannot set internal Value or Feature type")
+	case ast.Expr:
+		n := getScopePrefix(v, p)
+		expr = resolveExpr(ctx, n, x)
+	default:
+		expr = convert.GoValueToValue(ctx.opCtx, x, true)
+	}
 	for i := len(p.path) - 1; i >= 0; i-- {
 		expr = &adt.StructLit{Decls: []adt.Decl{
 			&adt.Field{
diff --git a/cue/types_test.go b/cue/types_test.go
index f5ab493..4e74180 100644
--- a/cue/types_test.go
+++ b/cue/types_test.go
@@ -1068,6 +1068,67 @@
 		out: `
 		{foo: {bar: "baz"}}
 		`,
+	}, {
+		// Resolve to enclosing
+		in: `
+		foo: _
+		x: 1
+		`,
+		x:    ast.NewIdent("x"),
+		path: ParsePath("foo"),
+		out: `
+		{foo: 1, x: 1}
+		`,
+	}, {
+		in: `
+		foo: {
+			bar: _
+			x: 1
+		}
+		`,
+		x:    ast.NewIdent("x"),
+		path: ParsePath("foo.bar"),
+		out: `
+		{foo: {bar: 1, x: 1}}
+		`,
+	}, {
+		// Resolve one scope up
+		in: `
+		x: 1
+		foo: {
+			bar: _
+		}
+		`,
+		x:    ast.NewIdent("x"),
+		path: ParsePath("foo.bar"),
+		out: `
+		{foo: {bar: 1}, x: 1}
+		`,
+	}, {
+		// Resolve within ast expression
+		in: `
+		foo: {
+			bar: _
+		}
+		`,
+		x: ast.NewStruct(
+			ast.NewIdent("x"), ast.NewString("1"),
+			ast.NewIdent("y"), ast.NewIdent("x"),
+		),
+		path: ParsePath("foo.bar"),
+		out: `
+			{foo: {bar: {x: "1", y: "1"}}}
+			`,
+	}, {
+		// Resolve in non-existing
+		in: `
+		foo: x: 1
+		`,
+		x:    ast.NewIdent("x"),
+		path: ParsePath("foo.bar.baz"),
+		out: `
+		{foo: {x: 1, bar: baz: 1}}
+		`,
 	}}
 
 	for _, tc := range testCases {
diff --git a/internal/core/compile/compile.go b/internal/core/compile/compile.go
index 14ea284..ff30412 100644
--- a/internal/core/compile/compile.go
+++ b/internal/core/compile/compile.go
@@ -30,8 +30,6 @@
 	// Scope specifies a node in which to look up unresolved references. This
 	// is useful for evaluating expressions within an already evaluated
 	// configuration.
-	//
-	// TODO
 	Scope *adt.Vertex
 
 	// Imports allows unresolved identifiers to resolve to imports.
@@ -249,21 +247,6 @@
 		}
 	}
 
-	// TODO: Assume that the other context is unified with the newly compiled
-	// files. This is not the same behavior as the old functionality, but we
-	// wanted to nix this anyway. For instance by allowing pkg_tool to be
-	// treated differently.
-	if v := c.Config.Scope; v != nil {
-		for _, arc := range v.Arcs {
-			if _, ok := c.fileScope[arc.Label]; !ok {
-				c.fileScope[arc.Label] = true
-			}
-		}
-
-		c.pushScope(nil, 0, v.Source()) // File scope
-		defer c.popScope()
-	}
-
 	// TODO: set doc.
 	res := &adt.Vertex{}
 
@@ -281,17 +264,6 @@
 }
 
 func (c *compiler) compileExpr(x ast.Expr) adt.Conjunct {
-	c.fileScope = map[adt.Feature]bool{}
-
-	if v := c.Config.Scope; v != nil {
-		for _, arc := range v.Arcs {
-			c.fileScope[arc.Label] = true
-		}
-
-		c.pushScope(nil, 0, v.Source()) // File scope
-		defer c.popScope()
-	}
-
 	expr := c.expr(x)
 
 	env := &adt.Environment{}
@@ -301,13 +273,6 @@
 		top.Vertex = p
 		top.Up = &adt.Environment{}
 		top = top.Up
-
-		// TODO: do something like this to allow multi-layered scopes.
-		// e := &adt.Environment{Vertex: p}
-		// if env != nil {
-		// 	env.Up = e
-		// }
-		// env = e
 	}
 
 	return adt.MakeRootConjunct(env, expr)
@@ -344,6 +309,18 @@
 				Label:   label,
 			}
 		}
+		for p := c.Scope; p != nil; p = p.Parent {
+			for _, a := range p.Arcs {
+				if a.Label == label {
+					return &adt.FieldReference{
+						Src:     n,
+						UpCount: upCount,
+						Label:   label,
+					}
+				}
+			}
+			upCount++
+		}
 
 		if c.Config.Imports != nil {
 			if pkgPath := c.Config.Imports(n); pkgPath != "" {