cue/ast: add back LabelName

Copied from internal, but now with better
semantics and preparing for not allowing
string labels to bind.

Ported most uses of internal.LabelName

Change-Id: I35b178a2c03073a6a6df1f78e0bfe8eae7b882ad
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/3261
Reviewed-by: Marcel van Lohuizen <[email protected]>

cmd/cue/cmd/trim.go

@@ -515,9 +515,9 @@ func (t *trimSet) trimDecls(decls []ast.Decl, rm []ast.Node, m cue.Value, allow
 
 	for _, d := range decls {
 		if f, ok := d.(*ast.Field); ok {
-			label, _ := internal.LabelName(f.Label)
+			label, _, err := ast.LabelName(f.Label)
 			v := m.Lookup(label)
-			if inNodes(rm, f.Value) && (allow || v.Exists()) {
+			if err == nil && inNodes(rm, f.Value) && (allow || v.Exists()) {
 				continue
 			}
 		}

cue/ast/ast.go

@@ -127,7 +127,7 @@ func (*Ellipsis) declNode()      {}
 func (*Package) declNode()      {}
 func (*CommentGroup) declNode() {}
 
-// A Label is any prduction that can be used as a LHS label.
+// A Label is any production that can be used as a LHS label.
 type Label interface {
 	Node
 	labelNode()

cue/ast/astutil/resolve.go

@@ -66,11 +66,16 @@ func newScope(f *ast.File, outer *scope, node ast.Node, decls []ast.Decl) *scope
 	for _, d := range decls {
 		switch x := d.(type) {
 		case *ast.Field:
-			name, _ := internal.LabelName(x.Label)
-			s.insert(name, x.Value)
+			// TODO: switch to ast's implementation
+			name, isIdent := internal.LabelName(x.Label)
+			if isIdent {
+				s.insert(name, x.Value)
+			}
 		case *ast.Alias:
-			name, _ := internal.LabelName(x.Ident)
-			s.insert(name, x)
+			name, isIdent, _ := ast.LabelName(x.Ident)
+			if isIdent {
+				s.insert(name, x)
+			}
 			// Handle imports
 		}
 	}
@@ -149,8 +154,10 @@ func (s *scope) Before(n ast.Node) (w visitor) {
 			walk(s, label)
 		case *ast.TemplateLabel:
 			s := newScope(s.file, s, x, nil)
-			name, _ := internal.LabelName(label)
-			s.insert(name, x.Label) // Field used for entire lambda.
+			name, err := ast.ParseIdent(label.Ident)
+			if err == nil {
+				s.insert(name, x.Label) // Field used for entire lambda.
+			}
 			walk(s, x.Value)
 			return nil
 		}
@@ -180,7 +187,7 @@ func (s *scope) Before(n ast.Node) (w visitor) {
 		return nil
 
 	case *ast.Ident:
-		name, ok := internal.LabelName(x)
+		name, ok, _ := ast.LabelName(x)
 		if !ok {
 			break
 		}
@@ -214,11 +221,15 @@ func scopeClauses(s *scope, clauses []ast.Clause) *scope {
 			walk(s, f.Source)
 			s = newScope(s.file, s, f, nil)
 			if f.Key != nil {
-				name, _ := internal.LabelName(f.Key)
-				s.insert(name, f.Key)
+				name, err := ast.ParseIdent(f.Key)
+				if err == nil {
+					s.insert(name, f.Key)
+				}
+			}
+			name, err := ast.ParseIdent(f.Value)
+			if err == nil {
+				s.insert(name, f.Value)
 			}
-			name, _ := internal.LabelName(f.Value)
-			s.insert(name, f.Value)
 		} else {
 			walk(s, c)
 		}

cue/ast/ident.go

@@ -93,3 +93,53 @@ func ParseIdent(n *Ident) (string, error) {
 
 	return ident, nil
 }
+
+// LabelName reports the name of a label, whether it is an identifier
+// (it binds a value to a scope), and whether it is valid.
+// Keywords that are allowed in label positions are interpreted accordingly.
+//
+// Examples:
+//
+//     Label   Result
+//     foo     "foo"  true   nil
+//     `a-b`   "a-b"  true   nil
+//     true    true   true   nil
+//     "foo"   "foo"  false  nil
+//     `a-b    ""     false  invalid identifier
+//     "foo    ""     false  invalid string
+//     "\(x)"  ""     false  errors.Is(err, ErrIsExpression)
+//     <A>     "A"    false  errors.Is(err, ErrIsExpression)
+//
+func LabelName(l Label) (name string, isIdent bool, err error) {
+	switch n := l.(type) {
+	case *Ident:
+		str, err := ParseIdent(n)
+		if err != nil {
+			return "", false, err
+		}
+		return str, true, nil
+
+	case *BasicLit:
+		switch n.Kind {
+		case token.STRING:
+			// Use strconv to only allow double-quoted, single-line strings.
+			str, err := strconv.Unquote(n.Value)
+			if err != nil {
+				err = errors.Newf(l.Pos(), "invalid")
+			}
+			return str, false, err
+
+		case token.NULL, token.TRUE, token.FALSE:
+			return n.Value, true, nil
+
+			// TODO: allow numbers to be fields?
+		}
+	}
+	// This includes interpolation and template labels.
+	return "", false, errors.Wrapf(ErrIsExpression, l.Pos(),
+		"label is interpolation or template")
+}
+
+// ErrIsExpression reports whether a label is an expression.
+// This error is never returned directly. Use errors.Is or xerrors.Is.
+var ErrIsExpression = errors.New("not a concrete label")

cue/ast/ident_test.go

@@ -0,0 +1,86 @@
+// 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 ast_test
+
+import (
+	"testing"
+
+	"cuelang.org/go/cue/ast"
+	"cuelang.org/go/cue/format"
+	"cuelang.org/go/cue/token"
+	"github.com/stretchr/testify/assert"
+	"golang.org/x/xerrors"
+)
+
+func TestLabelName(t *testing.T) {
+	testCases := []struct {
+		in      ast.Label
+		out     string
+		isIdent bool
+		err     bool
+		expr    bool
+	}{{
+		in:      ast.NewString("foo-bar"),
+		out:     "foo-bar",
+		isIdent: false,
+	}, {
+		in:      ast.NewString("foo bar"),
+		out:     "foo bar",
+		isIdent: false,
+	}, {
+		in:      &ast.Ident{Name: "`foo`"},
+		out:     "foo",
+		isIdent: true,
+	}, {
+		in:      &ast.Ident{Name: "`foo-bar`"},
+		out:     "foo-bar",
+		isIdent: true,
+	}, {
+		in:      &ast.Ident{Name: "`foo-bar\x00`"},
+		out:     "",
+		isIdent: false,
+		err:     true,
+	}, {
+		in:      &ast.Ident{Name: "`foo-bar\x00`"},
+		out:     "",
+		isIdent: false,
+		err:     true,
+	}, {
+		in:      &ast.BasicLit{Kind: token.TRUE, Value: "true"},
+		out:     "true",
+		isIdent: true,
+	}, {
+		in:      &ast.BasicLit{Kind: token.STRING, Value: `"foo`},
+		out:     "",
+		isIdent: false,
+		err:     true,
+	}, {
+		in:      &ast.Interpolation{Elts: []ast.Expr{ast.NewString("foo")}},
+		out:     "",
+		isIdent: false,
+		err:     true,
+		expr:    true,
+	}}
+	for _, tc := range testCases {
+		b, _ := format.Node(tc.in)
+		t.Run(string(b), func(t *testing.T) {
+			str, isIdent, err := ast.LabelName(tc.in)
+			assert.Equal(t, tc.out, str, "value")
+			assert.Equal(t, tc.isIdent, isIdent, "isIdent")
+			assert.Equal(t, tc.err, err != nil, "err")
+			assert.Equal(t, tc.expr, xerrors.Is(err, ast.ErrIsExpression), "expr")
+		})
+	}
+}

cue/build.go

@@ -267,8 +267,8 @@ func (idx *index) strLabel(str string) label {
 func (idx *index) nodeLabel(n ast.Node) (f label, ok bool) {
 	switch x := n.(type) {
 	case *ast.BasicLit:
-		name, ok := internal.LabelName(x)
-		return idx.label(name, false), ok
+		name, _, err := ast.LabelName(x)
+		return idx.label(name, false), err == nil
 	case *ast.Ident:
 		name, err := ast.ParseIdent(x)
 		return idx.label(name, true), err == nil

internal/third_party/yaml/decode.go

@@ -16,7 +16,6 @@ import (
 
 	"cuelang.org/go/cue/ast"
 	"cuelang.org/go/cue/token"
-	"cuelang.org/go/internal"
 )
 
 const (
@@ -697,8 +696,8 @@ outer:
 			key := labelStr(label)
 			for _, decl := range m.Elts {
 				f := decl.(*ast.Field)
-				name, _ := internal.LabelName(f.Label)
-				if name == key {
+				name, _, err := ast.LabelName(f.Label)
+				if err == nil && name == key {
 					f.Value = d.unmarshal(n.children[i+1])
 					continue outer
 				}