cue/parser: allow keywords in more places

Now also as aliases and nested fields.
This was made possible by removing support for
old-style comprehensions.

Issue #339

Change-Id: I3b556b4150f52054f52dc8a78e8a42e44dee1bd4
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/5761
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/parser/parser.go b/cue/parser/parser.go
index e7409da..b576e4a 100644
--- a/cue/parser/parser.go
+++ b/cue/parser/parser.go
@@ -689,6 +689,14 @@
 		Rparen: rparen}
 }
 
+// TODO: inline this function in parseFieldList once we no longer user comment
+// position information in parsing.
+func (p *parser) consumeDeclComma() {
+	if p.atComma("struct literal", token.RBRACE, token.EOF) {
+		p.next()
+	}
+}
+
 func (p *parser) parseFieldList() (list []ast.Decl) {
 	if p.trace {
 		defer un(trace(p, "FieldList"))
@@ -698,23 +706,16 @@
 
 	for p.tok != token.RBRACE && p.tok != token.ELLIPSIS && p.tok != token.EOF {
 		switch p.tok {
-		case token.FOR, token.IF:
-			list = append(list, p.parseComprehension())
-
-		case token.LET:
-			list = append(list, p.parseLetDecl())
-
 		case token.ATTRIBUTE:
 			list = append(list, p.parseAttribute())
-			if p.atComma("struct literal", token.RBRACE) { // TODO: may be EOF
-				p.next()
-			}
+			p.consumeDeclComma()
 
 		default:
 			list = append(list, p.parseField())
 		}
 
-		// TODO: handle next comma here, after disallowing non-colon separator.
+		// TODO: handle next comma here, after disallowing non-colon separator
+		// and we have eliminated the need comment positions.
 	}
 
 	if p.tok == token.ELLIPSIS {
@@ -727,36 +728,38 @@
 	return
 }
 
-func (p *parser) parseLetDecl() (decl ast.Decl) {
+func (p *parser) parseLetDecl() (decl ast.Decl, ident *ast.Ident) {
 	if p.trace {
 		defer un(trace(p, "Field"))
 	}
 
 	c := p.openComments()
-	defer func() { c.closeNode(p, decl) }()
 
 	letPos := p.expect(token.LET)
 	if p.tok != token.IDENT {
-		return &ast.Ident{
+		c.closeNode(p, ident)
+		return nil, &ast.Ident{
 			NamePos: letPos,
 			Name:    "let",
 		}
 	}
+	defer func() { c.closeNode(p, decl) }()
 
-	ident := p.parseIdent()
+	ident = p.parseIdent()
 	assign := p.expect(token.BIND)
 	expr := p.parseRHS()
-	p.expectComma()
+
+	p.consumeDeclComma()
 
 	return &ast.LetClause{
 		Let:   letPos,
 		Ident: ident,
 		Equal: assign,
 		Expr:  expr,
-	}
+	}, nil
 }
 
-func (p *parser) parseComprehension() (decl ast.Decl) {
+func (p *parser) parseComprehension() (decl ast.Decl, ident *ast.Ident) {
 	if p.trace {
 		defer un(trace(p, "Comprehension"))
 	}
@@ -768,53 +771,12 @@
 	pos := p.pos
 	clauses, fc := p.parseComprehensionClauses(true)
 	if fc != nil {
-		lab := &ast.Ident{
+		ident = &ast.Ident{
 			NamePos: pos,
 			Name:    tok.String(),
 		}
-		opt := token.NoPos
-		tok = p.tok
-		pos = p.pos
-		switch p.tok {
-		case token.COMMA, token.EOF:
-			decl = &ast.EmbedDecl{Expr: lab}
-
-		case token.OPTION:
-			opt = pos
-			p.next()
-			tok = p.tok
-			pos = p.pos
-
-			fallthrough
-
-		default:
-			p.next()
-			rhs := p.parseRHS()
-
-			// Field or alias.
-			switch tok {
-			case token.BIND:
-				decl = &ast.Alias{Ident: lab, Equal: pos, Expr: rhs}
-			case token.ISA, token.COLON:
-				decl = &ast.Field{
-					Label:    lab,
-					Optional: opt,
-					TokenPos: pos,
-					Token:    tok,
-					Value:    rhs,
-					Attrs:    p.parseAttributes(),
-				}
-			default:
-				decl = &ast.BadDecl{From: lab.Pos(), To: rhs.End()}
-			}
-		}
-
-		fc.closeNode(p, decl)
-		if p.atComma("struct literal", token.RBRACE, token.EOF) {
-			p.next()
-		}
-
-		return decl
+		fc.closeNode(p, ident)
+		return nil, ident
 	}
 
 	sc := p.openComments()
@@ -828,7 +790,7 @@
 	return &ast.Comprehension{
 		Clauses: clauses,
 		Value:   expr,
-	}
+	}, nil
 }
 
 func (p *parser) parseField() (decl ast.Decl) {
@@ -847,7 +809,10 @@
 	for i := 0; ; i++ {
 		tok := p.tok
 
-		label, expr, ok := p.parseLabel(false)
+		label, expr, decl, ok := p.parseLabel(false)
+		if decl != nil {
+			return decl
+		}
 		m.Label = label
 
 		if !ok {
@@ -859,15 +824,11 @@
 					p.errorExpected(p.pos, "label or ':'")
 					return &ast.BadDecl{From: pos, To: p.pos}
 				}
-				if p.atComma("struct literal", token.RBRACE) {
-					p.next()
-				}
+				p.consumeDeclComma()
 				return a
 			}
 			e := &ast.EmbedDecl{Expr: expr}
-			if p.atComma("struct literal", token.RBRACE) {
-				p.next()
-			}
+			p.consumeDeclComma()
 			return e
 		}
 
@@ -926,7 +887,7 @@
 		}
 
 		tok := p.tok
-		label, expr, ok := p.parseLabel(true)
+		label, expr, _, ok := p.parseLabel(true)
 		if !ok || (p.tok != token.COLON && p.tok != token.ISA && p.tok != token.OPTION) {
 			if expr == nil {
 				expr = p.parseRHS()
@@ -963,9 +924,7 @@
 		m.Attrs = attrs
 	}
 
-	if p.atComma("struct literal", token.RBRACE) { // TODO: may be EOF
-		p.next()
-	}
+	p.consumeDeclComma()
 
 	return this
 }
@@ -979,16 +938,6 @@
 	return attrs
 }
 
-func (p *parser) parseAttributeDecls() (a []ast.Decl) {
-	for p.tok == token.ATTRIBUTE {
-		a = append(a, p.parseAttribute())
-		if p.atComma("struct literal", token.RBRACE) { // TODO: may be EOF
-			p.next()
-		}
-	}
-	return a
-}
-
 func (p *parser) parseAttribute() *ast.Attribute {
 	c := p.openComments()
 	a := &ast.Attribute{At: p.pos, Text: p.lit}
@@ -997,46 +946,31 @@
 	return a
 }
 
-func (p *parser) parseLabel(rhs bool) (label ast.Label, expr ast.Expr, ok bool) {
+func (p *parser) parseLabel(rhs bool) (label ast.Label, expr ast.Expr, decl ast.Decl, ok bool) {
 	tok := p.tok
 	switch tok {
-	case token.IDENT, token.STRING, token.INTERPOLATION,
-		token.NULL, token.TRUE, token.FALSE,
-		token.FOR, token.IF, token.LET, token.IN:
-		expr = p.parseExpr()
 
-		switch x := expr.(type) {
-		case *ast.BasicLit:
-			switch x.Kind {
-			case token.STRING, token.NULL, token.TRUE, token.FALSE:
-				// Keywords that represent operands.
-
-				// Allowing keywords to be used as a labels should not interfere with
-				// generating good errors: any keyword can only appear on the RHS of a
-				// field (after a ':'), whereas labels always appear on the LHS.
-
-				label, ok = x, true
-			}
-
-		case *ast.Ident:
-			if strings.HasPrefix(x.Name, "__") {
-				p.errf(x.NamePos, "identifiers starting with '__' are reserved")
-			}
-
-			expr = p.parseAlias(x)
-			if a, ok := expr.(*ast.Alias); ok {
-				if _, ok = a.Expr.(ast.Label); !ok {
-					break
-				}
-				label = a
-			} else {
-				label = x
-			}
-			ok = true
-
-		case ast.Label:
-			label, ok = x, true
+	case token.FOR, token.IF:
+		if rhs {
+			expr = p.parseExpr()
+			break
 		}
+		comp, ident := p.parseComprehension()
+		if comp != nil {
+			return nil, nil, comp, false
+		}
+		expr = ident
+
+	case token.LET:
+		let, ident := p.parseLetDecl()
+		if let != nil {
+			return nil, nil, let, false
+		}
+		expr = ident
+
+	case token.IDENT, token.STRING, token.INTERPOLATION,
+		token.NULL, token.TRUE, token.FALSE, token.IN:
+		expr = p.parseExpr()
 
 	case token.LBRACK:
 		expr = p.parseRHS()
@@ -1046,7 +980,40 @@
 			label, ok = x, true
 		}
 	}
-	return label, expr, ok
+
+	switch x := expr.(type) {
+	case *ast.BasicLit:
+		switch x.Kind {
+		case token.STRING, token.NULL, token.TRUE, token.FALSE:
+			// Keywords that represent operands.
+
+			// Allowing keywords to be used as a labels should not interfere with
+			// generating good errors: any keyword can only appear on the RHS of a
+			// field (after a ':'), whereas labels always appear on the LHS.
+
+			label, ok = x, true
+		}
+
+	case *ast.Ident:
+		if strings.HasPrefix(x.Name, "__") {
+			p.errf(x.NamePos, "identifiers starting with '__' are reserved")
+		}
+
+		expr = p.parseAlias(x)
+		if a, ok := expr.(*ast.Alias); ok {
+			if _, ok = a.Expr.(ast.Label); !ok {
+				break
+			}
+			label = a
+		} else {
+			label = x
+		}
+		ok = true
+
+	case ast.Label:
+		label, ok = x, true
+	}
+	return label, expr, nil, ok
 }
 
 func (p *parser) parseStruct() (expr ast.Expr) {
@@ -1243,7 +1210,7 @@
 			expr := p.parseStruct()
 			sc.closeExpr(p, expr)
 
-			if p.atComma("struct literal", token.RBRACK) { // TODO: may be EOF
+			if p.atComma("list literal", token.RBRACK) { // TODO: may be EOF
 				p.next()
 			}
 
diff --git a/cue/parser/parser_test.go b/cue/parser/parser_test.go
index 2142dca..3bbf610 100644
--- a/cue/parser/parser_test.go
+++ b/cue/parser/parser_test.go
@@ -34,11 +34,31 @@
 	}, {
 		"basic lits", `"a","b", 3,3.4,5,2_3`, `"a", "b", 3, 3.4, 5, 2_3`,
 	}, {
-		"keyword basic lits", `true,false,null,for,in,if,let`, `true, false, null, for, in, if, let`,
+		"keyword basic lits", `true,false,null,for,in,if,let,if`, `true, false, null, for, in, if, let, if`,
+	}, {
+		"keyword basic newline", `
+		true
+		false
+		null
+		for
+		in
+		if
+		let
+		if
+		`, `true, false, null, for, in, if, let, if`,
 	}, {
 		"keywords as labels",
-		`if: 0, for: 1, in: 2, where: 3, div: 4, quo: 5`,
-		`if: 0, for: 1, in: 2, where: 3, div: 4, quo: 5`,
+		`if: 0, for: 1, in: 2, where: 3, div: 4, quo: 5
+		for: if: let: 3
+		`,
+		`if: 0, for: 1, in: 2, where: 3, div: 4, quo: 5, for: {if: {let: 3}}`,
+	}, {
+		"keywords as alias",
+		`if=foo: 0
+		for=bar: 2
+		let=bar: 3
+		`,
+		`if=foo: 0, for=bar: 2, let=bar: 3`,
 	}, {
 		"json",
 		`{
@@ -399,6 +419,10 @@
 		`,
 		`if X <[d2// Comment 2] {<[d0// Comment 1] Field: 2>}>`,
 	}, {
+		"let comments",
+		`let X = foo // Comment 1`,
+		`<[5// Comment 1] let X=foo>`,
+	}, {
 		"emit comments",
 		`// a comment at the beginning of the file
 
diff --git a/cue/scanner/scanner.go b/cue/scanner/scanner.go
index 822d3cb..abd1461 100644
--- a/cue/scanner/scanner.go
+++ b/cue/scanner/scanner.go
@@ -762,13 +762,10 @@
 		if len(lit) > 1 {
 			// keywords are longer than one letter - avoid lookup otherwise
 			tok = token.Lookup(lit)
-			switch tok {
-			case token.IDENT, token.TRUE, token.FALSE, token.NULL, token.BOTTOM:
-				insertEOL = true
-			}
-		} else {
 			insertEOL = true
+		} else {
 			tok = token.IDENT
+			insertEOL = true
 		}
 	case '0' <= ch && ch <= '9':
 		insertEOL = true