cmd/cue: rewrite quoted identifiers

- also fixes formatting of label aliasing
- add ast.IsValidIdent
- deprecate ast.QuoteIdent and ast.ParseIdent

Change-Id: Id058b37bcbd480ba016d61df1caac54956e31346
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/3870
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cmd/cue/cmd/fix.go b/cmd/cue/cmd/fix.go
index 92a859e..e4391fc 100644
--- a/cmd/cue/cmd/fix.go
+++ b/cmd/cue/cmd/fix.go
@@ -15,6 +15,7 @@
 package cmd
 
 import (
+	"fmt"
 	"strconv"
 	"strings"
 
@@ -58,7 +59,11 @@
 		return true
 	}, nil)
 
+	// Referred nodes and used identifiers.
 	referred := map[ast.Node]string{}
+	used := map[string]bool{}
+	replacement := map[ast.Node]string{}
+
 	ast.Walk(f, func(n ast.Node) bool {
 		if i, ok := n.(*ast.Ident); ok {
 			str, err := ast.ParseIdent(i)
@@ -66,10 +71,22 @@
 				return false
 			}
 			referred[i.Node] = str
+			used[str] = true
 		}
 		return true
 	}, nil)
 
+	num := 0
+	newIdent := func() string {
+		for num++; ; num++ {
+			str := fmt.Sprintf("X%d", num)
+			if !used[str] {
+				used[str] = true
+				return str
+			}
+		}
+	}
+
 	// Rewrite TemplateLabel to ListLit.
 	// Note: there is a chance that the name will clash with the
 	// scope in which it is defined. We drop the alias if it is not
@@ -99,23 +116,61 @@
 			if !ok {
 				break
 			}
-			b, ok := x.Label.(*ast.BasicLit)
-			if !ok || b.Kind != token.STRING {
-				break
+			switch b := x.Label.(type) {
+			case *ast.BasicLit:
+				if b.Kind != token.STRING {
+					return true
+				}
+				str, err := strconv.Unquote(b.Value)
+				if err != nil || str != m {
+					return true
+				}
+
+			case *ast.Ident:
+				str, err := ast.ParseIdent(b)
+				if err != nil || str != m || str == b.Name {
+					return true
+				}
+				if ast.IsValidIdent(str) {
+					x.Label = astutil.CopyMeta(ast.NewIdent(str), x.Label).(ast.Label)
+					return true
+				}
 			}
-			str, err := strconv.Unquote(b.Value)
-			if err != nil || str != m {
-				break
-			}
-			str, err = ast.QuoteIdent(str)
-			if err != nil {
-				return false
-			}
-			x.Label = astutil.CopyMeta(ast.NewIdent(str), x.Label).(ast.Label)
+
+			ident := newIdent()
+			replacement[x.Value] = ident
+			expr := x.Label.(ast.Expr)
+			expr = &ast.Alias{Ident: ast.NewIdent(ident), Expr: expr}
+			ast.SetRelPos(x.Label, token.NoRelPos)
+			x.Label = astutil.CopyMeta(expr, x.Label).(ast.Label)
 		}
 		return true
 	}, nil).(*ast.File)
 
+	// Replace quoted references with their alias identifier.
+	astutil.Apply(f, func(c astutil.Cursor) bool {
+		n := c.Node()
+		switch x := n.(type) {
+		case *ast.Ident:
+			if r, ok := replacement[x.Node]; ok {
+				c.Replace(astutil.CopyMeta(ast.NewIdent(r), n))
+				break
+			}
+			str, err := ast.ParseIdent(x)
+			if err != nil || str == x.Name {
+				break
+			}
+			// Either the identifier is valid, in which can be replaced simply
+			// as here, or it is a complicated identifier and the original
+			// destination must have been quoted, in which case it is handled
+			// above.
+			if ast.IsValidIdent(str) {
+				c.Replace(astutil.CopyMeta(ast.NewIdent(str), n))
+			}
+		}
+		return true
+	}, nil)
+
 	// TODO: we are probably reintroducing slices. Disable for now.
 	//
 	// Rewrite slice expression.
diff --git a/cmd/cue/cmd/fix_test.go b/cmd/cue/cmd/fix_test.go
index 4ddeca2..49019e3 100644
--- a/cmd/cue/cmd/fix_test.go
+++ b/cmd/cue/cmd/fix_test.go
@@ -35,21 +35,47 @@
 "baz": ` + "`foo-bar`" + `
 
 a: {
-	"qux": 3
+	// qux
+	"qux": 3 // qux line
+	// qux-quux
 	"qux-quux": qux
 	"qaz": ` + "`qux-quux`" + `
+	qax:  qux
+
+	fiz: 4
+	faz: ` + "`fiz`" + `
+	// fuz
+	fuz: ` + "`qux-quux`" + ` // fuz
+
+	// biz
+	` + "`biz`" + `: 5 // biz
+	buz: ` + "`biz`" + `
+	baz: ` + "`qux`" + `
 }
 `,
 		out: `package foo
 
-"foo":     3
-` + "`foo-bar`" + `: 2
-"baz":     ` + "`foo-bar`" + `
+"foo":        3
+X1="foo-bar": 2
+"baz":        X1
 
 a: {
-	qux:        3
-	` + "`qux-quux`" + `: qux
-	"qaz":      ` + "`qux-quux`" + `
+	// qux
+	X2="qux": 3 // qux line
+	// qux-quux
+	X3="qux-quux": X2
+	"qaz":         X3
+	qax:           X2
+
+	fiz: 4
+	faz: fiz
+	// fuz
+	fuz: X3 // fuz
+
+	// biz
+	biz: 5 // biz
+	buz: biz
+	baz: X2
 }
 `,
 	}, {
diff --git a/cue/ast/ident.go b/cue/ast/ident.go
index a626ba2..35d8838 100644
--- a/cue/ast/ident.go
+++ b/cue/ast/ident.go
@@ -32,8 +32,24 @@
 	return '0' <= ch && ch <= '9' || ch >= utf8.RuneSelf && unicode.IsDigit(ch)
 }
 
+// IsValidIdent reports whether str is a valid identifier.
+func IsValidIdent(ident string) bool {
+	for i, r := range ident {
+		if isLetter(r) || r == '_' {
+			continue
+		}
+		if i > 0 && isDigit(r) {
+			continue
+		}
+		return false
+	}
+	return true
+}
+
 // QuoteIdent quotes an identifier, if needed, and reports
 // an error if the identifier is invalid.
+//
+// Deprecated: quoted identifiers are deprecated. Use aliases.
 func QuoteIdent(ident string) (string, error) {
 	if ident != "" && ident[0] == '`' {
 		if _, err := strconv.Unquote(ident); err != nil {
@@ -66,6 +82,8 @@
 
 // ParseIdent unquotes a possibly quoted identifier and validates
 // if the result is valid.
+//
+// Deprecated: quoted identifiers are deprecated. Use aliases.
 func ParseIdent(n *Ident) (string, error) {
 	ident := n.Name
 	if ident == "" {
diff --git a/cue/format/node.go b/cue/format/node.go
index e2572d6..c0a3d4c 100644
--- a/cue/format/node.go
+++ b/cue/format/node.go
@@ -384,6 +384,9 @@
 	f.before(l)
 	defer f.after(l)
 	switch n := l.(type) {
+	case *ast.Alias:
+		f.expr(n)
+
 	case *ast.Ident:
 		// Escape an identifier that has invalid characters. This may happen,
 		// if the AST is not generated by the parser.