cue: allow hidden fields in Path

This requires users to explicitly specify the package in which
such hidden identifiers should be scoped.

Note:
1) Hid is not called Hidden because this was already
taken. Maybe in a big API rework some renaming would be
handy.

2) We do not allow "" to mean the anonymous packages,
as this will allow using this to mean the "top-level" package
in the future, which will be the common case.

3) When converting from AST identifiers, we could see if it
is bound to a certain package already and then use that
scope. This does not conflict with the current API.

Change-Id: If3438b84b2f6032072342abe58cde1e1bafdc4f3
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/9185
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>

cue/build/instance.go

@@ -131,7 +131,10 @@ type Instance struct {
 // ID returns the package ID unique for this module.
 func (inst *Instance) ID() string {
 	if inst.PkgName == "" {
-		return ""
+		return "_"
+	}
+	if s := inst.ImportPath; s != "" {
+		return s
 	}
 	s := fmt.Sprintf("%s:%s", inst.Module, inst.PkgName)
 	return s

cue/examples_test.go

@@ -0,0 +1,67 @@
+// 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_test
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+
+	"cuelang.org/go/cue"
+	"cuelang.org/go/internal/cuetxtar"
+	"github.com/rogpeppe/go-internal/txtar"
+)
+
+func load(file string) *cue.Instance {
+	dir, _ := ioutil.TempDir("", "*")
+	defer os.RemoveAll(dir)
+
+	inst := cue.Build(cuetxtar.Load(txtar.Parse([]byte(file)), dir))[0]
+	if err := inst.Err; err != nil {
+		panic(err)
+	}
+	return inst
+}
+
+func ExampleHid() {
+	const file = `
+-- cue.mod/module.cue --
+module: "acme.com"
+
+-- main.cue --
+import "acme.com/foo:bar"
+
+bar
+_foo: int // scoped in main (anonymous) package
+baz: _foo
+
+-- foo/bar.cue --
+package bar
+
+_foo: int // scoped within imported package
+bar: _foo
+`
+
+	v := load(file).Value()
+
+	v = v.FillPath(cue.MakePath(cue.Hid("_foo", "acme.com/foo:bar")), 1)
+	v = v.FillPath(cue.MakePath(cue.Hid("_foo", "_")), 2)
+	fmt.Println(v.LookupPath(cue.ParsePath("bar")).Int64())
+	fmt.Println(v.LookupPath(cue.ParsePath("baz")).Int64())
+
+	// Output:
+	// 1 <nil>
+	// 2 <nil>
+}

cue/path.go

@@ -15,6 +15,7 @@
 package cue
 
 import (
+	"fmt"
 	"strconv"
 	"strings"
 
@@ -130,19 +131,37 @@ func toSelectors(expr ast.Expr) []Selector {
 		} else {
 			sel = basicLitSelector(b)
 		}
-		return append(a, sel)
+		return appendSelector(a, sel)
 
 	case *ast.SelectorExpr:
 		a := toSelectors(x.X)
-		return append(a, identSelector(x.Sel))
+		return appendSelector(a, identSelector(x.Sel))
 
 	default:
-		return []Selector{Selector{pathError{
+		return []Selector{{pathError{
 			errors.Newf(token.NoPos, "invalid label %s ", internal.DebugStr(x)),
 		}}}
 	}
 }
 
+// appendSelector is like append(a, sel), except that it collects errors
+// in a one-element slice.
+func appendSelector(a []Selector, sel Selector) []Selector {
+	err, isErr := sel.sel.(pathError)
+	if len(a) == 1 {
+		if p, ok := a[0].sel.(pathError); ok {
+			if isErr {
+				p.Error = errors.Append(p.Error, err.Error)
+			}
+			return a
+		}
+	}
+	if isErr {
+		return []Selector{sel}
+	}
+	return append(a, sel)
+}
+
 func basicLitSelector(b *ast.BasicLit) Selector {
 	switch b.Kind {
 	case token.INT:
@@ -181,10 +200,17 @@ func basicLitSelector(b *ast.BasicLit) Selector {
 func identSelector(label ast.Label) Selector {
 	switch x := label.(type) {
 	case *ast.Ident:
-		if isHiddenOrDefinition(x.Name) {
+		switch s := x.Name; {
+		case strings.HasPrefix(s, "_"):
+			// TODO: extract package from a bound identifier.
+			return Selector{pathError{errors.Newf(token.NoPos,
+				"invalid path: hidden label %s not allowed", s),
+			}}
+		case strings.HasPrefix(s, "#"):
 			return Selector{definitionSelector(x.Name)}
+		default:
+			return Selector{stringSelector(x.Name)}
 		}
-		return Selector{stringSelector(x.Name)}
 
 	case *ast.BasicLit:
 		return basicLitSelector(x)
@@ -211,13 +237,58 @@ func isHiddenOrDefinition(s string) bool {
 	return strings.HasPrefix(s, "#") || strings.HasPrefix(s, "_")
 }
 
+// Hid returns a selector for a hidden field. It panics is pkg is empty.
+// Hidden fields are scoped by package, and pkg indicates for which package
+// the hidden field must apply.For anonymous packages, it must be set to "_".
+func Hid(name, pkg string) Selector {
+	if !ast.IsValidIdent(name) {
+		panic(fmt.Sprintf("invalid identifier %s", name))
+	}
+	if !strings.HasPrefix(name, "_") {
+		panic(fmt.Sprintf("%s is not a hidden field identifier", name))
+	}
+	if pkg == "" {
+		panic(fmt.Sprintf("missing package for hidden identifier %s", name))
+	}
+	return Selector{scopedSelector{name, pkg}}
+}
+
+type scopedSelector struct {
+	name, pkg string
+}
+
+// String returns the CUE representation of the definition.
+func (s scopedSelector) String() string {
+	return s.name
+}
+
+func (s scopedSelector) kind() adt.FeatureType {
+	switch {
+	case strings.HasPrefix(s.name, "#"):
+		return adt.DefinitionLabel
+	case strings.HasPrefix(s.name, "_#"):
+		return adt.HiddenDefinitionLabel
+	case strings.HasPrefix(s.name, "_"):
+		return adt.HiddenLabel
+	default:
+		return adt.StringLabel
+	}
+}
+
+func (s scopedSelector) feature(r adt.Runtime) adt.Feature {
+	return adt.MakeIdentLabel(r, s.name, s.pkg)
+}
+
 // A Def marks a string as a definition label. An # will be added if a string is
-// not prefixed with an # or _# already. Hidden labels are qualified by the
-// package in which they are looked up.
+// not prefixed with a #. It will panic if s cannot be written as a valid
+// identifier.
 func Def(s string) Selector {
-	if !isHiddenOrDefinition(s) {
+	if !strings.HasPrefix(s, "#") {
 		s = "#" + s
 	}
+	if !ast.IsValidIdent(s) {
+		panic(fmt.Sprintf("invalid definition %s", s))
+	}
 	return Selector{definitionSelector(s)}
 }
 
@@ -229,16 +300,7 @@ func (d definitionSelector) String() string {
 }
 
 func (d definitionSelector) kind() adt.FeatureType {
-	switch {
-	case strings.HasPrefix(string(d), "#"):
-		return adt.DefinitionLabel
-	case strings.HasPrefix(string(d), "_#"):
-		return adt.HiddenDefinitionLabel
-	case strings.HasPrefix(string(d), "_"):
-		return adt.HiddenLabel
-	default:
-		panic("invalid definition")
-	}
+	return adt.DefinitionLabel
 }
 
 func (d definitionSelector) feature(r adt.Runtime) adt.Feature {

cue/path_test.go

@@ -20,14 +20,11 @@ import (
 )
 
 func Test(t *testing.T) {
-	p := func(a ...Selector) Path {
-		return Path{path: a}
-	}
-	_ = p
 	var r Runtime
 	inst, _ := r.Compile("", `
 		#Foo:   a: b: 1
 		"#Foo": c: d: 2
+		_foo: b: 5
 		a: 3
 		b: [4, 5, 6]
 		c: "#Foo": 7
@@ -38,7 +35,7 @@ func Test(t *testing.T) {
 		str  string
 		err  bool
 	}{{
-		path: p(Def("#Foo"), Str("a"), Str("b")),
+		path: MakePath(Def("#Foo"), Str("a"), Str("b")),
 		out:  "1",
 		str:  "#Foo.a.b",
 	}, {
@@ -51,17 +48,21 @@ func Test(t *testing.T) {
 		str:  `"#Foo".c.d`,
 	}, {
 		// fallback Def(Foo) -> Def(#Foo)
-		path: p(Def("Foo"), Str("a"), Str("b")),
+		path: MakePath(Def("Foo"), Str("a"), Str("b")),
 		out:  "1",
 		str:  "#Foo.a.b",
 	}, {
-		path: p(Str("b"), Index(2)),
+		path: MakePath(Str("b"), Index(2)),
 		out:  "6",
 		str:  "b[2]", // #Foo.b.2
 	}, {
-		path: p(Str("c"), Str("#Foo")),
+		path: MakePath(Str("c"), Str("#Foo")),
 		out:  "7",
 		str:  `c."#Foo"`,
+	}, {
+		path: MakePath(Hid("_foo", "_"), Str("b")),
+		out:  "5",
+		str:  `_foo.b`,
 	}, {
 		path: ParsePath("#Foo.a.b"),
 		str:  "#Foo.a.b",
@@ -82,7 +83,7 @@ func Test(t *testing.T) {
 		path: ParsePath("foo._foo"),
 		str:  "_|_",
 		err:  true,
-		out:  `_|_ // invalid path: hidden fields not allowed in path foo._foo`,
+		out:  `_|_ // invalid path: hidden label _foo not allowed`,
 	}, {
 		path: ParsePath(`c."#Foo`),
 		str:  "_|_",

cue/testdata/definitions/hidden.txtar

@@ -49,7 +49,7 @@ Result:
     }
   }
   d: (#struct){
-    _val(example.com:pkg): (#struct){
+    _val(example.com/pkg): (#struct){
       f: (int){ 3 }
     }
     _name(:foo): (struct){
@@ -61,7 +61,7 @@ Result:
   }
   e: (_|_){
     // [eval]
-    _val(example.com:pkg): (#struct){
+    _val(example.com/pkg): (#struct){
       f: (int){ 3 }
     }
     _name(:foo): (_|_){
@@ -79,7 +79,7 @@ Result:
     }
   }
   f: (#struct){
-    _val(example.com:pkg): (#struct){
+    _val(example.com/pkg): (#struct){
       f: (int){ 3 }
     }
     _val(:foo): (struct){

cue/types_test.go

@@ -1037,6 +1037,19 @@ func TestFillPath(t *testing.T) {
 		}
 		bar: 3
 		`,
+	}, {
+		in: `
+		foo: _foo: int
+		bar: foo._foo
+		`,
+		x:    3,
+		path: MakePath(Str("foo"), Hid("_foo", "_")),
+		out: `
+		foo: {
+			_foo: 3
+		}
+		bar: 3
+		`,
 	}, {
 		in: `
 		string

internal/core/compile/compile.go

@@ -42,7 +42,7 @@ type Config struct {
 	Imports func(x *ast.Ident) (pkgPath string)
 
 	// pkgPath is used to qualify the scope of hidden fields. The default
-	// scope is "main".
+	// scope is "_".
 	pkgPath string
 }
 
@@ -85,7 +85,7 @@ func newCompiler(cfg *Config, pkgPath string, r adt.Runtime) *compiler {
 		c.Config = *cfg
 	}
 	if pkgPath == "" {
-		pkgPath = "main"
+		pkgPath = "_"
 	}
 	c.Config.pkgPath = pkgPath
 	return c

internal/core/compile/compile_test.go

@@ -53,7 +53,7 @@ func TestCompile(t *testing.T) {
 
 		a := t.ValidInstances()
 
-		v, err := compile.Files(nil, r, "main", a[0].Files...)
+		v, err := compile.Files(nil, r, "", a[0].Files...)
 
 		// Write the results.
 		t.WriteErrors(err)
@@ -98,7 +98,7 @@ func TestX(t *testing.T) {
 	}
 	r := runtime.New()
 
-	arc, err := compile.Files(nil, r, "main", file)
+	arc, err := compile.Files(nil, r, "", file)
 	if err != nil {
 		t.Error(errors.Details(err, nil))
 	}

internal/core/debug/debug.go

@@ -92,7 +92,7 @@ func (w *printer) ident(f adt.Feature) {
 func (w *printer) labelString(f adt.Feature) string {
 	if f.IsHidden() {
 		ident := f.IdentString(w.index)
-		if pkgName := f.PkgID(w.index); pkgName != "main" {
+		if pkgName := f.PkgID(w.index); pkgName != "_" {
 			ident = fmt.Sprintf("%s(%s)", ident, pkgName)
 		}
 		return ident