internal/core/compile: fix hidden definition bug

Fixes #533

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

cue/testdata/definitions/issue533.txtar

@@ -0,0 +1,79 @@
+-- in.cue --
+package x
+
+#x: {
+	Name: string
+}
+
+_#x: {
+	Name: string
+}
+
+x1: #x & {
+	Name: "hello"
+	Age:  50
+}
+
+x2: _#x & {
+	Name: "hello"
+	Age:  50
+}
+-- out/eval --
+Errors:
+x1: field `Age` not allowed:
+    ./in.cue:3:5
+    ./in.cue:11:5
+    ./in.cue:13:2
+x2: field `Age` not allowed:
+    ./in.cue:7:6
+    ./in.cue:16:5
+    ./in.cue:18:2
+
+Result:
+(_|_){
+  // [eval]
+  #x: (#struct){
+    Name: (string){ string }
+  }
+  _#x: (#struct){
+    Name: (string){ string }
+  }
+  x1: (_|_){
+    // [eval]
+    Name: (string){ "hello" }
+    Age: (_|_){
+      // [eval] x1: field `Age` not allowed:
+      //     ./in.cue:3:5
+      //     ./in.cue:11:5
+      //     ./in.cue:13:2
+    }
+  }
+  x2: (_|_){
+    // [eval]
+    Name: (string){ "hello" }
+    Age: (_|_){
+      // [eval] x2: field `Age` not allowed:
+      //     ./in.cue:7:6
+      //     ./in.cue:16:5
+      //     ./in.cue:18:2
+    }
+  }
+}
+-- out/compile --
+--- in.cue
+{
+  #x: {
+    Name: string
+  }
+  _#x: {
+    Name: string
+  }
+  x1: (〈0;#x〉 & {
+    Name: "hello"
+    Age: 50
+  })
+  x2: (〈0;_#x〉 & {
+    Name: "hello"
+    Age: 50
+  })
+}

internal/core/adt/feature.go

@@ -116,7 +116,7 @@ func MakeIdentLabel(r StringIndexer, s string) Feature {
 	i := r.StringToIndex(s)
 	t := StringLabel
 	switch {
-	case strings.HasPrefix(s, "#_"):
+	case strings.HasPrefix(s, "_#"):
 		t = HiddenDefinitionLabel
 	case strings.HasPrefix(s, "#"):
 		t = DefinitionLabel

internal/core/adt/feature_test.go

@@ -0,0 +1,116 @@
+// Copyright 2020 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 adt_test
+
+import (
+	"strconv"
+	"testing"
+
+	"cuelang.org/go/internal/core/adt"
+	"cuelang.org/go/internal/core/eval"
+	"cuelang.org/go/internal/core/runtime"
+)
+
+func TestFeatureBool(t *testing.T) {
+	r := runtime.New()
+	u := eval.New(r)
+	ctx := adt.NewContext(r, u, &adt.Vertex{})
+
+	makeInt := func(x int64) adt.Feature {
+		f, _ := adt.MakeLabel(nil, 2, adt.IntLabel)
+		return f
+	}
+
+	testCases := []struct {
+		in           adt.Feature
+		isRegular    bool
+		isDefinition bool
+		isHidden     bool
+		isString     bool
+		isInt        bool
+	}{{
+		in:        ctx.StringLabel("foo"),
+		isRegular: true,
+		isString:  true,
+	}, {
+		in:        ctx.StringLabel("_"),
+		isRegular: true,
+		isString:  true,
+	}, {
+		in:        ctx.StringLabel("_#foo"),
+		isRegular: true,
+		isString:  true,
+	}, {
+		in:        ctx.StringLabel("#foo"),
+		isRegular: true,
+		isString:  true,
+	}, {
+		in:        adt.MakeStringLabel(r, "foo"),
+		isRegular: true,
+		isString:  true,
+	}, {
+		in:        adt.MakeStringLabel(r, "_"),
+		isRegular: true,
+		isString:  true,
+	}, {
+		in:        adt.MakeStringLabel(r, "_#foo"),
+		isRegular: true,
+		isString:  true,
+	}, {
+		in:        adt.MakeStringLabel(r, "#foo"),
+		isRegular: true,
+		isString:  true,
+	}, {
+		in:        makeInt(4),
+		isRegular: true,
+		isInt:     true,
+	}, {
+		in:        adt.MakeIdentLabel(r, "foo"),
+		isRegular: true,
+		isString:  true,
+	}, {
+		in:           adt.MakeIdentLabel(r, "#foo"),
+		isDefinition: true,
+	}, {
+		in:           adt.MakeIdentLabel(r, "_#foo"),
+		isDefinition: true,
+		isHidden:     true,
+	}, {
+		in:       adt.MakeIdentLabel(r, "_foo"),
+		isHidden: true,
+	}}
+	for i, tc := range testCases {
+		t.Run(strconv.Itoa(i), func(t *testing.T) {
+			if got := tc.in.IsRegular(); got != tc.isRegular {
+				t.Errorf("IsRegular: got %v; want %v", got, tc.isRegular)
+			}
+			if got := tc.in.IsString(); got != tc.isString {
+				t.Errorf("IsString: got %v; want %v", got, tc.isString)
+			}
+			if got := tc.in.IsInt(); got != tc.isInt {
+				t.Errorf("IsInt: got %v; want %v", got, tc.isInt)
+			}
+			if got := tc.in.IsDef(); got != tc.isDefinition {
+				t.Errorf("isDefinition: got %v; want %v", got, tc.isDefinition)
+			}
+			if got := tc.in.IsHidden(); got != tc.isHidden {
+				t.Errorf("IsHidden: got %v; want %v", got, tc.isHidden)
+			}
+			if got := tc.in.IsString(); got != tc.isString {
+				t.Errorf("IsString: got %v; want %v", got, tc.isString)
+			}
+		})
+	}
+}

internal/core/compile/label.go

@@ -15,8 +15,6 @@
 package compile
 
 import (
-	"strings"
-
 	"github.com/cockroachdb/apd/v2"
 	"golang.org/x/text/unicode/norm"
 
@@ -31,23 +29,7 @@ func (c *compiler) label(n ast.Node) adt.Feature {
 	index := c.index
 	switch x := n.(type) {
 	case *ast.Ident:
-		s := x.Name
-		i := index.StringToIndex(x.Name)
-		t := adt.StringLabel
-		switch {
-		case strings.HasPrefix(s, "#_"):
-			t = adt.HiddenDefinitionLabel
-		case strings.HasPrefix(s, "#"):
-			t = adt.DefinitionLabel
-		case strings.HasPrefix(s, "_"):
-			t = adt.HiddenLabel
-		}
-		f, err := adt.MakeLabel(n, i, t)
-		if err != nil {
-			c.errf(n, "invalid identifier label: %v", err)
-			return adt.InvalidLabel
-		}
-		return f
+		return adt.MakeIdentLabel(c.index, x.Name)
 
 	case *ast.BasicLit:
 		switch x.Kind {