tools/trim: fix spurious removal of top-level declarations

Previously, the following
  ["aFoo"]: 3
  aFoo:     _

resulted in:
  ["aFoo"]: 3

This only applied at the top-level.

Change-Id: I9756e24510583ab416c9a4827a74f78be0aa92f5
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/4202
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/tools/trim/trim.go b/tools/trim/trim.go
index d3a5a53..55c22e7 100644
--- a/tools/trim/trim.go
+++ b/tools/trim/trim.go
@@ -96,7 +96,7 @@
 	rm := gen.trim("root", root, cue.Value{}, root)
 
 	for _, f := range files {
-		f.Decls = gen.trimDecls(f.Decls, rm, root, false)
+		f.Decls = gen.trimDecls(f.Decls, rm, cue.Value{}, true)
 	}
 	return nil
 }
@@ -267,24 +267,32 @@
 		gen = append(gen, v.Source())
 	}
 
+	// Identify generated components and unify them with the mixin value.
+	exists := false
+	for _, v := range vSplit {
+		if src := v.Source(); t.alwaysGen[src] || inNodes(gen, src) {
+			if !v.IsConcrete() {
+				// The template has an expression that cannot be fully
+				// resolved. Attempt to complete the expression by
+				// evaluting it within the struct to which the template
+				// is applied.
+				expr := v.Syntax()
+				// TODO: this only resolves references contained in scope.
+				v = internal.EvalExpr(scope, expr).(cue.Value)
+			}
+
+			if w := in.Unify(v); w.Err() == nil {
+				in = w
+			}
+			// One of the sources of this struct is generated. That means
+			// we can safely delete a non-generated version.
+			exists = true
+			gen = append(gen, src)
+		}
+	}
+
 	switch v.Kind() {
 	case cue.StructKind:
-		// TODO: merge optional field preprocessing with that of fields.
-
-		// Identify generated components and unify them with the mixin value.
-		exists := false
-		for _, v := range v.Split() {
-			if src := v.Source(); t.alwaysGen[src] {
-				if w := in.Unify(v); w.Err() == nil {
-					in = w
-				}
-				// One of the sources of this struct is generated. That means
-				// we can safely delete a non-generated version.
-				exists = true
-				gen = append(gen, src)
-			}
-		}
-
 		// Build map of mixin fields.
 		valueMap := map[key]cue.Value{}
 		for mIter, _ := in.Fields(cue.All(), cue.Optional(false)); mIter.Next(); {
@@ -392,28 +400,12 @@
 		fallthrough
 
 	default:
-		for _, v := range vSplit {
-			src := v.Source()
-			if t.alwaysGen[src] || inNodes(gen, src) {
-				if v.IsIncomplete() {
-					// The template has an expression that cannot be fully
-					// resolved. Attempt to complete the expression by
-					// evaluting it within the struct to which the template
-					// is applied.
-					expr := v.Syntax()
-					// TODO: this only resolves references contained in scope.
-					v = internal.EvalExpr(scope, expr).(cue.Value)
-				}
-				in = in.Unify(v)
-				gen = append(gen, src)
-			}
-		}
-
 		// Mark any subsumed part that is covered by generated config.
 		if in.Err() == nil && v.Subsumes(in) {
 			for _, v := range vSplit {
 				src := v.Source()
-				if t.canRemove(src) && !inNodes(gen, src) {
+				if t.canRemove(src) && !inNodes(gen, src) &&
+					exists {
 					rmSet = append(rmSet, src)
 				}
 			}
diff --git a/tools/trim/trim_test.go b/tools/trim/trim_test.go
new file mode 100644
index 0000000..558fa72
--- /dev/null
+++ b/tools/trim/trim_test.go
@@ -0,0 +1,135 @@
+// Copyright 2019 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 trim
+
+import (
+	"testing"
+
+	"cuelang.org/go/cue"
+	"cuelang.org/go/cue/ast"
+	"cuelang.org/go/cue/format"
+	"cuelang.org/go/cue/parser"
+)
+
+func TestFiles(t *testing.T) {
+	const trace = false
+	testCases := []struct {
+		name string
+		in   string
+		out  string
+	}{{
+		name: "optional does not remove required",
+		in: `
+		a: ["aFoo"]: 3
+		a: aFoo:     _
+
+		a: {
+			["aFoo"]: 3
+			aFoo:     _
+		}
+
+		["aFoo"]: 3
+		aFoo:     _
+		`,
+		out: `a: ["aFoo"]: 3
+a: aFoo:     _
+
+a: {
+	["aFoo"]: 3
+	aFoo:     _
+}
+
+["aFoo"]: 3
+aFoo:     _
+`,
+	}, {
+		// TODO: make this optional
+		name: "defaults can remove non-defaults",
+		in: `
+		foo: [string]: a: *1 | int
+		foo: b: a: 1
+		`,
+		out: `foo: [string]: a: *1 | int
+foo: b: {
+}
+`,
+	}, {
+		name: "remove top-level struct",
+		in: `
+		a: b: 3
+		for k, v in a {
+			c: "\(k)": v
+		}
+		c: b: 3
+
+		z: {
+			a: b: 3
+			for k, v in a {
+				c: "\(k)": v
+			}
+			c: b: 3
+		}
+		`,
+		out: `a: b: 3
+for k, v in a {
+	c: "\(k)": v
+}
+
+z: {
+	a: b: 3
+	for k, v in a {
+		c: "\(k)": v
+	}
+}
+`,
+		// TODO: This used to work.
+		// 	name: "remove implied interpolations",
+		// 	in: `
+		// 	foo: [string]: {
+		// 		a: string
+		// 		b: "--\(a)--"
+		// 	}
+		// 	foo: entry: {
+		// 		a: "insert"
+		// 		b: "--insert--"
+		// 	}
+		// 	`,
+		// 	out: ``,
+	}}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			f, err := parser.ParseFile("test", tc.in)
+			if err != nil {
+				t.Fatal(err)
+			}
+			r := cue.Runtime{}
+			inst, err := r.CompileFile(f)
+			if err != nil {
+				t.Fatal(err)
+			}
+			err = Files([]*ast.File{f}, inst, &Config{Trace: trace})
+			if err != nil {
+				t.Fatal(err)
+			}
+			out, err := format.Node(f)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if got := string(out); got != tc.out {
+				t.Errorf("\ngot:\n%s\nwant:\n%s", got, tc.out)
+			}
+		})
+	}
+}