all: implement hidden identifiers scoped per-package

This has been part of the spec for a long time, but never implemented.

Note that all files with the same package name within a module
belong to the same package. A hidden identifier is thus uniqued by
module+name.

Closes #65
Fixes #503

Change-Id: I6d97ca1dbcf4ccc5730fde2cf8193c8e667787ad
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/7361
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>

cue/build.go

@@ -262,7 +262,7 @@ func (idx *index) loadInstance(p *build.Instance) *Instance {
 		err := runtime.ResolveFiles(idx.Index, p, isBuiltin)
 		errs = errors.Append(errs, err)
 
-		v, err := compile.Files(nil, idx.Runtime, p.Files...)
+		v, err := compile.Files(nil, idx.Runtime, p.ID(), p.Files...)
 		errs = errors.Append(errs, err)
 
 		inst := newInstance(idx, p, v)

cue/build/instance.go

@@ -15,6 +15,7 @@
 package build
 
 import (
+	"fmt"
 	pathpkg "path"
 	"path/filepath"
 	"strings"
@@ -127,6 +128,15 @@ type Instance struct {
 	Match      []string
 }
 
+// ID returns the package ID unique for this module.
+func (inst *Instance) ID() string {
+	if inst.PkgName == "" {
+		return ""
+	}
+	s := fmt.Sprintf("%s:%s", inst.Module, inst.PkgName)
+	return s
+}
+
 // Dependencies reports all Instances on which this instance depends.
 func (inst *Instance) Dependencies() []*Instance {
 	// TODO: as cyclic dependencies are not allowed, we could just not check.

cue/instance.go

@@ -127,6 +127,11 @@ func init() {
 	}
 }
 
+// pkgID reports a package path that can never resolve to a valid package.
+func pkgID() string {
+	return "_"
+}
+
 // evalExpr evaluates expr within scope.
 func evalExpr(ctx *context, scope *adt.Vertex, expr ast.Expr) evaluated {
 	cfg := &compile.Config{
@@ -139,7 +144,7 @@ func evalExpr(ctx *context, scope *adt.Vertex, expr ast.Expr) evaluated {
 		},
 	}
 
-	c, err := compile.Expr(cfg, ctx.opCtx, expr)
+	c, err := compile.Expr(cfg, ctx.opCtx, pkgID(), expr)
 	if err != nil {
 		return &adt.Bottom{Err: err}
 	}
@@ -182,6 +187,15 @@ func evalExpr(ctx *context, scope *adt.Vertex, expr ast.Expr) evaluated {
 	// return c.NewErrf("could not evaluate %s", c.Str(x))
 }
 
+// ID returns the package identifier that uniquely qualifies module and
+// package name.
+func (inst *Instance) ID() string {
+	if inst == nil || inst.inst == nil {
+		return ""
+	}
+	return inst.inst.ID()
+}
+
 // Doc returns the package comments for this instance.
 func (inst *Instance) Doc() []*ast.CommentGroup {
 	var docs []*ast.CommentGroup
@@ -253,7 +267,8 @@ func (inst *Instance) Build(p *build.Instance) *Instance {
 
 	rErr := runtime.ResolveFiles(idx.Index, p, isBuiltin)
 
-	v, err := compile.Files(&compile.Config{Scope: inst.root}, r, p.Files...)
+	cfg := &compile.Config{Scope: inst.root}
+	v, err := compile.Files(cfg, r, p.ID(), p.Files...)
 
 	v.AddConjunct(adt.MakeRootConjunct(nil, inst.root))
 

cue/load/config.go

@@ -528,7 +528,7 @@ func (c Config) complete() (cfg *Config, err error) {
 		}
 
 		r := runtime.New()
-		v, err := compile.Files(nil, r, file)
+		v, err := compile.Files(nil, r, "_", file)
 		if err != nil {
 			return nil, errors.Wrapf(err, token.NoPos, "invalid cue.mod file")
 		}

cue/marshal.go

@@ -142,7 +142,7 @@ func (r *Runtime) Marshal(instances ...*Instance) (b []byte, err error) {
 			return p
 		}
 		// TODO: support exporting instance
-		file, _ := export.Def(r.idx.Runtime, i.root)
+		file, _ := export.Def(r.idx.Runtime, i.inst.ID(), i.root)
 		imports := []string{}
 		file.VisitImports(func(i *ast.ImportDecl) {
 			for _, spec := range i.Specs {

cue/testdata/comprehensions/nested.txtar

@@ -88,7 +88,7 @@ for k, v in deployment {
                   0: (struct){
                     containerPort: (int){ 9100 }
                     name: (string){ "scrape" }
-                    _export: (bool){ |(*(bool){ true }, (bool){ false }) }
+                    _export(:kube): (bool){ |(*(bool){ true }, (bool){ false }) }
                   }
                 }
                 name: (string){ string }
@@ -97,11 +97,11 @@ for k, v in deployment {
           }
         }
       }
-      _name: (string){ string }
+      _name(:kube): (string){ string }
     }
   }
-  _spec: (struct){
-    _name: (string){ string }
+  _spec(:kube): (struct){
+    _name(:kube): (string){ string }
     spec: (struct){
       template: (struct){
         spec: (struct){

cue/testdata/definitions/hidden.txtar

@@ -12,29 +12,41 @@ import "example.com/pkg"
   _name: d: int
 }
 
-d: pkg.#D & { _name: d: int }
+d: pkg.#D & { _name: d: int, _val: 4 }
 
 // TODO: this should fail, as the _name restricting it is in this
 // package.
-// e: pkg.#D & #def & { _name: e: int }
+e: pkg.#D & #def & { _name: e: int, _val: int }
+f: e._val
 
 -- pkg/bar.cue --
 package pkg
 
-#D: {}
+#D: { _val: 3 }
 
 -- out/eval --
 (struct){
   #def: (#struct){
-    _name: (#struct){
+    _name(:foo): (#struct){
       d: (int){ int }
     }
   }
   d: (#struct){
-    _name: (#struct){
+    _val(example.com:pkg): (int){ 3 }
+    _name(:foo): (#struct){
       d: (int){ int }
     }
+    _val(:foo): (int){ 4 }
   }
+  e: (#struct){
+    _val(example.com:pkg): (int){ 3 }
+    _name(:foo): (#struct){
+      d: (int){ int }
+      e: (int){ int }
+    }
+    _val(:foo): (int){ int }
+  }
+  f: (int){ int }
 }
 -- out/compile --
 --- in.cue
@@ -48,5 +60,13 @@ package pkg
     _name: {
       d: int
     }
+    _val: 4
+  })
+  e: ((〈import;"example.com/pkg"〉.#D & 〈0;#def〉) & {
+    _name: {
+      e: int
+    }
+    _val: int
   })
+  f: 〈0;e〉._val
 }

cue/testdata/definitions/issue533.txtar

@@ -35,7 +35,7 @@ Result:
   #x: (#struct){
     Name: (string){ string }
   }
-  _#x: (#struct){
+  _#x(:x): (#struct){
     Name: (string){ string }
   }
   x1: (_|_){

cue/testdata/fulleval/051_detectIncompleteYAML.txtar

@@ -69,7 +69,7 @@ Val:
 -- out/eval --
 (struct){
   #Spec: (#struct){
-    _vars: (#struct){
+    _vars(:foobar): (#struct){
       something: (string){ string }
     }
     data: (#struct){
@@ -87,7 +87,7 @@ Val:
     }
   }
   Val: (#struct){
-    _vars: (#struct){
+    _vars(:foobar): (#struct){
       something: (string){ "var-string" }
     }
     data: (#struct){

cue/testdata/fulleval/052_detectIncompleteJSON.txtar

@@ -61,7 +61,7 @@ Val:
 -- out/eval --
 (struct){
   #Spec: (#struct){
-    _vars: (#struct){
+    _vars(:foobar): (#struct){
       something: (string){ string }
     }
     data: (#struct){
@@ -78,7 +78,7 @@ Val:
     }
   }
   Val: (#struct){
-    _vars: (#struct){
+    _vars(:foobar): (#struct){
       something: (string){ "var-string" }
     }
     data: (#struct){

cue/types.go

@@ -937,13 +937,15 @@ func (v Value) Syntax(opts ...Option) ast.Node {
 		ShowDocs:        o.docs,
 	}
 
+	pkgID := v.instance().ID()
+
 	// var expr ast.Expr
 	var err error
 	var f *ast.File
 	if o.concrete || o.final {
 		// inst = v.instance()
 		var expr ast.Expr
-		expr, err = p.Value(v.idx.Runtime, v.v)
+		expr, err = p.Value(v.idx.Runtime, pkgID, v.v)
 		if err != nil {
 			return nil
 		}
@@ -955,7 +957,7 @@ func (v Value) Syntax(opts ...Option) ast.Node {
 		}
 		// return expr
 	} else {
-		f, err = p.Def(v.idx.Runtime, v.v)
+		f, err = p.Def(v.idx.Runtime, pkgID, v.v)
 		if err != nil {
 			panic(err)
 		}
@@ -1718,7 +1720,7 @@ func (v Value) Format(state fmt.State, verb rune) {
 	case state.Flag('+'):
 		_, _ = io.WriteString(state, ctx.opCtx.Str(v.v))
 	default:
-		n, _ := export.Raw.Expr(v.idx.Runtime, v.v)
+		n, _ := export.Raw.Expr(v.idx.Runtime, v.instance().ID(), v.v)
 		b, _ := format.Node(n)
 		_, _ = state.Write(b)
 	}

internal/core/adt/feature.go

@@ -15,6 +15,7 @@
 package adt
 
 import (
+	"fmt"
 	"strconv"
 	"strings"
 
@@ -68,10 +69,36 @@ func (f Feature) SelectorString(index StringIndexer) string {
 		}
 		return literal.String.Quote(s)
 	default:
-		return index.IndexToString(int64(x))
+		return f.IdentString(index)
 	}
 }
 
+// IdentString reports the identifier of f. The result is undefined if f
+// is not an identifier label.
+func (f Feature) IdentString(index StringIndexer) string {
+	s := index.IndexToString(int64(f.Index()))
+	if f.IsHidden() {
+		if p := strings.IndexByte(s, '\x00'); p >= 0 {
+			s = s[:p]
+		}
+	}
+	return s
+}
+
+// PkgID returns the package identifier, composed of the module and package
+// name, associated with this identifier. It will return "" if this is not
+// a hidden label.
+func (f Feature) PkgID(index StringIndexer) string {
+	if !f.IsHidden() {
+		return ""
+	}
+	s := index.IndexToString(int64(f.Index()))
+	if p := strings.IndexByte(s, '\x00'); p >= 0 {
+		return s[p+1:]
+	}
+	return s
+}
+
 // StringValue reports the string value of f, which must be a string label.
 func (f Feature) StringValue(index StringIndexer) string {
 	if !f.IsString() {
@@ -113,17 +140,19 @@ func MakeStringLabel(r StringIndexer, s string) Feature {
 }
 
 // MakeIdentLabel creates a label for the given identifier.
-func MakeIdentLabel(r StringIndexer, s string) Feature {
-	i := r.StringToIndex(s)
+func MakeIdentLabel(r StringIndexer, s, pkgpath string) Feature {
 	t := StringLabel
 	switch {
 	case strings.HasPrefix(s, "_#"):
 		t = HiddenDefinitionLabel
+		s = fmt.Sprintf("%s\x00%s", s, pkgpath)
 	case strings.HasPrefix(s, "#"):
 		t = DefinitionLabel
 	case strings.HasPrefix(s, "_"):
+		s = fmt.Sprintf("%s\x00%s", s, pkgpath)
 		t = HiddenLabel
 	}
+	i := r.StringToIndex(s)
 	f, err := MakeLabel(nil, i, t)
 	if err != nil {
 		panic("out of free string slots")

internal/core/adt/feature_test.go

@@ -77,18 +77,18 @@ func TestFeatureBool(t *testing.T) {
 		isRegular: true,
 		isInt:     true,
 	}, {
-		in:        adt.MakeIdentLabel(r, "foo"),
+		in:        adt.MakeIdentLabel(r, "foo", "main"),
 		isRegular: true,
 		isString:  true,
 	}, {
-		in:           adt.MakeIdentLabel(r, "#foo"),
+		in:           adt.MakeIdentLabel(r, "#foo", "main"),
 		isDefinition: true,
 	}, {
-		in:           adt.MakeIdentLabel(r, "_#foo"),
+		in:           adt.MakeIdentLabel(r, "_#foo", "main"),
 		isDefinition: true,
 		isHidden:     true,
 	}, {
-		in:       adt.MakeIdentLabel(r, "_foo"),
+		in:       adt.MakeIdentLabel(r, "_foo", "main"),
 		isHidden: true,
 	}}
 	for i, tc := range testCases {