parser: fix precedence of nested 'if' clauses in list comprehensions (#55)

* parser: fix precedence of nested 'if' clauses in list comprehensions

Before, the parser would consume an arbitrary expression after the
'if' even though it should reduce after x in
  [a for b in c if x if y else z]
Now it consumes only a precedence zero ("or"-level) expression.

The parser similarly reduces after x when parsing
  [a for b in c if lambda: x if y else z]

* fix three comment typos

3 files changed, 53 insertions, 17 deletions

diff --git a/syntax/parse.go b/syntax/parse.go
index dc12da0..e4c5644 100644
--- a/syntax/parse.go
+++ b/syntax/parse.go
@@ -466,21 +466,7 @@ func (p *parser) parseExprs(exprs []Expr, allowTrailingComma bool) []Expr {
 // parseTest parses a 'test', a single-component expression.
 func (p *parser) parseTest() Expr {
 	if p.tok == LAMBDA {
-		lambda := p.nextToken()
-		var params []Expr
-		if p.tok != COLON {
-			params = p.parseParams()
-		}
-		p.consume(COLON)
-		body := p.parseTest()
-		return &LambdaExpr{
-			Lambda: lambda,
-			Function: Function{
-				StartPos: lambda,
-				Params:   params,
-				Body:     []Stmt{&ReturnStmt{Result: body}},
-			},
-		}
+		return p.parseLambda(true)
 	}
 
 	x := p.parseTestPrec(0)
@@ -500,6 +486,42 @@ func (p *parser) parseTest() Expr {
 	return x
 }
 
+// parseTestNoCond parses a a single-component expression without
+// consuming a trailing 'if expr else expr'.
+func (p *parser) parseTestNoCond() Expr {
+	if p.tok == LAMBDA {
+		return p.parseLambda(false)
+	}
+	return p.parseTestPrec(0)
+}
+
+// parseLambda parses a lambda expression.
+// The allowCond flag allows the body to be an 'a if b else c' conditional.
+func (p *parser) parseLambda(allowCond bool) Expr {
+	lambda := p.nextToken()
+	var params []Expr
+	if p.tok != COLON {
+		params = p.parseParams()
+	}
+	p.consume(COLON)
+
+	var body Expr
+	if allowCond {
+		body = p.parseTest()
+	} else {
+		body = p.parseTestNoCond()
+	}
+
+	return &LambdaExpr{
+		Lambda: lambda,
+		Function: Function{
+			StartPos: lambda,
+			Params:   params,
+			Body:     []Stmt{&ReturnStmt{Result: body}},
+		},
+	}
+}
+
 func (p *parser) parseTestPrec(prec int) Expr {
 	if prec >= len(preclevels) {
 		return p.parsePrimaryWithSuffix()
@@ -888,7 +910,7 @@ func (p *parser) parseComprehensionSuffix(lbrace Position, body Expr, endBrace T
 			clauses = append(clauses, &ForClause{For: pos, Vars: vars, In: in, X: x})
 		} else if p.tok == IF {
 			pos := p.nextToken()
-			cond := p.parseTest()
+			cond := p.parseTestNoCond()
 			clauses = append(clauses, &IfClause{If: pos, Cond: cond})
 		} else {
 			p.in.errorf(p.in.pos, "got %#v, want '%s', for, or if", p.tok, endBrace)
diff --git a/syntax/parse_test.go b/syntax/parse_test.go
index 6657df6..39180ef 100644
--- a/syntax/parse_test.go
+++ b/syntax/parse_test.go
@@ -104,6 +104,8 @@ func TestExprParseTrees(t *testing.T) {
 			`(CondExpr Cond=b True=a False=c)`},
 		{`a and not b`,
 			`(BinaryExpr X=a Op=and Y=(UnaryExpr Op=not X=b))`},
+		{`[e for x in y if cond1 if cond2]`,
+			`(Comprehension Body=e Clauses=((ForClause Vars=x X=y) (IfClause Cond=cond1) (IfClause Cond=cond2)))`}, // github.com/google/skylark issue 53
 	} {
 		e, err := syntax.ParseExpr("foo.sky", test.input)
 		if err != nil {
diff --git a/syntax/testdata/errors.sky b/syntax/testdata/errors.sky
index 39ef781..ff1b49c 100644
--- a/syntax/testdata/errors.sky
+++ b/syntax/testdata/errors.sky
@@ -85,8 +85,20 @@ for x in 1, 2, 3:
 _ = [x for x in 1, 2, 3] ### `got ',', want ']', for, or if`
 ---
 # Unparenthesized tuple is not allowed as operand of 'if' in comprehension.
-
 _ = [a for b in c if 1, 2] ### `got ',', want ']', for, or if`
+
+---
+# Lambda is ok though.
+_ = [a for b in c if lambda: d] # ok
+
+# But the body of such a lambda may not be a conditional:
+_ = [a for b in c if (lambda: d if e else f)] # ok
+_ = [a for b in c if lambda: d if e else f]   ### "got else, want ']'"
+
+---
+# A lambda is not allowed as the operand of a 'for' clause.
+_ = [a for b in lambda: c] ### `got lambda, want primary`
+
 ---
 # Comparison operations are not associative.