internal/core/export: avoid introducing shadowing

The solution is to unconditionally link Ident.Node fields with
their resolution, allowing the sanitizer to unshadow.
The exporter is becoming quite a mess and is due for an
overhaul. The implementation should be notably easier with the
upcoming evaluator overhaul, so it seems a waste to do that now.

Fixes #2311

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I38d2c010814e5d97424d72890da002bb1c6ef2cb
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/552252
Reviewed-by: Roger Peppe <[email protected]>
Unity-Result: CUEcueckoo <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

internal/core/export/adt.go

@@ -297,6 +297,24 @@ func (e *exporter) resolve(env *adt.Environment, r adt.Resolver) ast.Expr {
 	switch x := r.(type) {
 	case *adt.FieldReference:
 		ident, _ := e.newIdentForField(x.Src, x.Label, x.UpCount)
+
+		// Use low-level lookup to bypass structural cycle detection. This is
+		// fine as we do not recurse on the result and it is necessary to detect
+		// shadowing even when a configuration has a structural cycle.
+		for i := 0; i < int(x.UpCount); i++ {
+			env = env.Up
+		}
+
+		// Exclude comprehensions and other temporary/ inlined Vertices, which
+		// cannot be properly resolved, throwing off the sanitize. Also,
+		// comprehensions originate from a single source and do not need to be
+		// handled.
+		if v := env.Vertex; !v.IsDynamic {
+			if v = v.Lookup(x.Label); v != nil {
+				e.linkIdentifier(v, ident)
+			}
+		}
+
 		return ident
 
 	case *adt.ValueReference:
@@ -424,9 +442,32 @@ func (e *exporter) decl(env *adt.Environment, d adt.Decl) ast.Decl {
 		internal.SetConstraint(f, x.ArcType.Token())
 		e.setField(x.Label, f)
 
-		f.Value = e.expr(env, x.Value)
 		f.Attrs = extractFieldAttrs(nil, x)
 
+		st, ok := x.Value.(*adt.StructLit)
+		if !ok {
+			f.Value = e.expr(env, x.Value)
+			return f
+
+		}
+
+		top := e.frame(0)
+		var src *adt.Vertex
+		if top.node != nil {
+			src = top.node.Lookup(x.Label)
+		}
+
+		// Instead of calling e.expr directly, we inline the case for
+		// *adt.StructLit, so that we can pass src.
+		c := adt.MakeRootConjunct(env, st)
+		f.Value = e.mergeValues(adt.InvalidLabel, src, []conjunct{{c: c, up: 0}}, c)
+
+		if top.node != nil {
+			if v := top.node.Lookup(x.Label); v != nil {
+				e.linkField(v, f)
+			}
+		}
+
 		return f
 
 	case *adt.LetField:

internal/core/export/export.go

@@ -247,12 +247,58 @@ type exporter struct {
 	labelAlias  map[adt.Expr]adt.Feature
 	valueAlias  map[*ast.Alias]*ast.Alias
 	letAlias    map[*ast.LetClause]*ast.LetClause
+	references  map[*adt.Vertex]*referenceInfo
 
 	usedHidden map[string]bool
 
 	pivotter *pivotter
 }
 
+// referenceInfo is used to track which Field.Value fields should be linked
+// to Ident.Node fields. The Node field is used by astutil.Resolve to mark
+// the value in the AST to which the respective identifier points.
+// astutil.Sanitize, in turn, uses this information to determine whether
+// a reference is shadowed  and apply fixes accordingly.
+type referenceInfo struct {
+	field      *ast.Field
+	references []*ast.Ident
+}
+
+// linkField reports the Field that represents certain Vertex in the generated
+// output. The Node fields for any references (*ast.Ident) that were already
+// recorded as pointed to this vertex are updated accordingly.
+func (e *exporter) linkField(v *adt.Vertex, f *ast.Field) {
+	if v == nil {
+		return
+	}
+	refs := e.references[v]
+	if refs == nil {
+		// TODO(perf): do a first sweep to only mark referenced arcs or keep
+		// track of that information elsewhere.
+		e.references[v] = &referenceInfo{field: f}
+		return
+	}
+	for _, r := range refs.references {
+		r.Node = f.Value
+	}
+	refs.references = refs.references[:0]
+}
+
+// linkIdentifier reports the Vertex to which indent points. Once the ast.Field
+// for a corresponding Vertex is known, it is linked to ident.
+func (e *exporter) linkIdentifier(v *adt.Vertex, ident *ast.Ident) {
+	refs := e.references[v]
+	if refs == nil {
+		refs = &referenceInfo{}
+		e.references[v] = refs
+	}
+	if refs.field == nil {
+		refs.references = append(refs.references, ident)
+		return
+	}
+	ident.Node = refs.field.Value
+}
+
 // newExporter creates and initializes an exporter.
 func newExporter(p *Profile, r adt.Runtime, pkgID string, v adt.Value) *exporter {
 	n, _ := v.(*adt.Vertex)
@@ -261,6 +307,8 @@ func newExporter(p *Profile, r adt.Runtime, pkgID string, v adt.Value) *exporter
 		ctx:   eval.NewContext(r, n),
 		index: r,
 		pkgID: pkgID,
+
+		references: map[*adt.Vertex]*referenceInfo{},
 	}
 
 	e.markUsedFeatures(v)
@@ -606,8 +654,10 @@ func (e *exporter) popFrame(saved []frame) {
 			setFieldAlias(f.field, f.alias)
 			node = f.field
 		}
-		for _, r := range f.references {
-			r.Node = node
+		if node != nil {
+			for _, r := range f.references {
+				r.Node = node
+			}
 		}
 	}
 

internal/core/export/expr.go

@@ -259,6 +259,8 @@ func (x *exporter) mergeValues(label adt.Feature, src *adt.Vertex, a []conjunct,
 
 		d.Value = e.mergeValues(f, field.arc, c, a...)
 
+		e.linkField(field.arc, d)
+
 		if f.IsDef() {
 			x.inDefinition--
 		}