cue/load: relax places where @tag is allowed

Currently @tag can already be arbitrarily nested.
This now also allows embeddings. It now explicitly
disallows fields defined within lists or the scope of
an optional field in the help. It also reports an error
for invalid tag attributes.

These restrictions avoid an injection from being spread
to widely by being generated, which may increase the
ability to analyze a configuration. But if these restrictions
prove to be too cumbersome, they could be removed.

Closes #437

Change-Id: I3af3a49adb20e67fcce7c6693d40bfd14aa8eb0b
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/7082
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cmd/cue/cmd/help.go b/cmd/cue/cmd/help.go
index 399d545..67ec139 100644
--- a/cmd/cue/cmd/help.go
+++ b/cmd/cue/cmd/help.go
@@ -290,7 +290,9 @@
 Injecting values
 
 The injection mechanism allows values to be injected into fields
-that are marked with a "tag" attribute. For any field of the form
+that are not defined within the scope of a comprehension, list, or
+optional field and that are marked with a "tag" attribute. For any
+field of the form
 
    field: x @tag(key)
 
diff --git a/cmd/cue/cmd/testdata/script/inject.txt b/cmd/cue/cmd/testdata/script/inject.txt
new file mode 100644
index 0000000..b2f4403
--- /dev/null
+++ b/cmd/cue/cmd/testdata/script/inject.txt
@@ -0,0 +1,13 @@
+cue eval test.cue -t env=prod
+
+cmp stdout expect-stdout
+
+# TODO: report errors for invalid tags?
+
+-- test.cue --
+{
+    environment: "prod" | "staging" @tag(env,short=prod|staging)
+}
+
+-- expect-stdout --
+environment: "prod"
diff --git a/cmd/cue/cmd/testdata/script/injecterr.txt b/cmd/cue/cmd/testdata/script/injecterr.txt
new file mode 100644
index 0000000..f9bd857
--- /dev/null
+++ b/cmd/cue/cmd/testdata/script/injecterr.txt
@@ -0,0 +1,36 @@
+! cue eval test.cue -t env=prod
+
+cmp stderr expect-stderr
+
+# TODO: report errors for invalid tags?
+
+-- test.cue --
+{
+    environment: "prod" | "staging" @tag(env,short=prod|staging)
+
+    // Don't replace in optional
+    opt?: string @tag(env)
+    bulk: [string]: foo: string @tag(env)
+    bulk: x: {}
+
+    // Don't replace in lists.
+    a: [
+        { no_replace: string @tag(env) }
+    ]
+
+    // Don't allow in comprehensions
+    src: [1, 2]
+    for _ in src {
+        b: string @tag(prod)
+    }
+}
+
+-- expect-stderr --
+@tag not allowed within optional fields:
+    ./test.cue:5:18
+@tag not allowed within optional fields:
+    ./test.cue:6:33
+@tag not allowed within lists:
+    ./test.cue:11:30
+@tag not allowed within comprehension:
+    ./test.cue:17:19
diff --git a/cue/load/tags.go b/cue/load/tags.go
index d30b502..1417c0b 100644
--- a/cue/load/tags.go
+++ b/cue/load/tags.go
@@ -101,20 +101,34 @@
 //
 // TODO: should we limit the depth at which tags may occur?
 func findTags(b *build.Instance) (tags []tag, errs errors.Error) {
+	findInvalidTags := func(x ast.Node, msg string) {
+		ast.Walk(x, nil, func(n ast.Node) {
+			if f, ok := n.(*ast.Field); ok {
+				for _, a := range f.Attrs {
+					if key, _ := a.Split(); key == "tag" {
+						errs = errors.Append(errs, errors.Newf(a.Pos(), msg))
+						// TODO: add position of x.
+					}
+				}
+			}
+		})
+	}
 	for _, f := range b.Files {
 		ast.Walk(f, func(n ast.Node) bool {
-			if b.Err != nil {
-				return false
-			}
-
 			switch x := n.(type) {
-			case *ast.StructLit, *ast.File:
-				return true
+			case *ast.ListLit:
+				findInvalidTags(n, "@tag not allowed within lists")
+				return false
+
+			case *ast.Comprehension:
+				findInvalidTags(n, "@tag not allowed within comprehension")
+				return false
 
 			case *ast.Field:
 				// TODO: allow optional fields?
 				_, _, err := ast.LabelName(x.Label)
 				if err != nil || x.Optional != token.NoPos {
+					findInvalidTags(n, "@tag not allowed within optional fields")
 					return false
 				}
 
@@ -131,9 +145,8 @@
 					t.field = x
 					tags = append(tags, t)
 				}
-				return true
 			}
-			return false
+			return true
 		}, nil)
 	}
 	return tags, errs