fix(parser): do not panic with unbalanced parenthesis (#483)

A set of unbalanced parenthesis caused the new parser to eventually hit
an EOF while it was looking for it. Because it assumed it found the
right parenthesis, it advanced the end position beyond the end of the
slice and caused a runtime panic.

This change does two things:

1. It verifies that the EOF token will have the proper position information.
2. The parser uses the literal length returned by `expect` when finding the end position.

internal/parser/parser.go

@@ -296,10 +296,10 @@ func (p *parser) parseExpressionStatement() *ast.ExpressionStatement {
 func (p *parser) parseBlock() *ast.Block {
 	start, _ := p.expect(token.LBRACE)
 	stmts := p.parseStatementList(token.RBRACE)
-	end, _ := p.expect(token.RBRACE)
+	end, rbrace := p.expect(token.RBRACE)
 	return &ast.Block{
 		Body:     stmts,
-		BaseNode: p.position(start, end+1),
+		BaseNode: p.position(start, end+token.Pos(len(rbrace))),
 	}
 }
 
@@ -644,13 +644,13 @@ func (p *parser) parseDotExpression(expr ast.Expression) ast.Expression {
 func (p *parser) parseCallExpression(callee ast.Expression) ast.Expression {
 	p.expect(token.LPAREN)
 	params := p.parsePropertyList()
-	end, _ := p.expect(token.RPAREN)
+	end, rparen := p.expect(token.RPAREN)
 	expr := &ast.CallExpression{
 		Callee: callee,
 		BaseNode: ast.BaseNode{
 			Loc: p.sourceLocation(
 				locStart(callee),
-				p.s.File().Position(end+1),
+				p.s.File().Position(end+token.Pos(len(rparen))),
 			),
 		},
 	}
@@ -673,15 +673,15 @@ func (p *parser) parseCallExpression(callee ast.Expression) ast.Expression {
 func (p *parser) parseIndexExpression(callee ast.Expression) ast.Expression {
 	p.expect(token.LBRACK)
 	expr := p.parseExpression()
-	end, _ := p.expect(token.RBRACK)
+	end, rbrack := p.expect(token.RBRACK)
 	if lit, ok := expr.(*ast.StringLiteral); ok {
 		return &ast.MemberExpression{
 			Object:   callee,
 			Property: lit,
 			BaseNode: ast.BaseNode{
 				Loc: p.sourceLocation(
 					locStart(callee),
-					p.s.File().Position(end+1),
+					p.s.File().Position(end+token.Pos(len(rbrack))),
 				),
 			},
 		}
@@ -692,7 +692,7 @@ func (p *parser) parseIndexExpression(callee ast.Expression) ast.Expression {
 		BaseNode: ast.BaseNode{
 			Loc: p.sourceLocation(
 				locStart(callee),
-				p.s.File().Position(end+1),
+				p.s.File().Position(end+token.Pos(len(rbrack))),
 			),
 		},
 	}
@@ -803,20 +803,20 @@ func (p *parser) parsePipeLiteral() *ast.PipeLiteral {
 func (p *parser) parseArrayLiteral() ast.Expression {
 	start, _ := p.expect(token.LBRACK)
 	exprs := p.parseExpressionList()
-	end, _ := p.expect(token.RBRACK)
+	end, rbrack := p.expect(token.RBRACK)
 	return &ast.ArrayExpression{
 		Elements: exprs,
-		BaseNode: p.position(start, end+1),
+		BaseNode: p.position(start, end+token.Pos(len(rbrack))),
 	}
 }
 
 func (p *parser) parseObjectLiteral() ast.Expression {
 	start, _ := p.expect(token.LBRACE)
 	properties := p.parsePropertyList()
-	end, _ := p.expect(token.RBRACE)
+	end, rbrace := p.expect(token.RBRACE)
 	return &ast.ObjectExpression{
 		Properties: properties,
-		BaseNode:   p.position(start, end+1),
+		BaseNode:   p.position(start, end+token.Pos(len(rbrace))),
 	}
 }
 

internal/parser/parser_test.go

@@ -46,6 +46,10 @@ func testParser(runFn func(name string, fn func(t testing.TB))) {
 		name string
 		raw  string
 		want *ast.Program
+		// If parseOnly is set to true, then the test will verify
+		// that parsing works and will not verify the contents of
+		// the AST.
+		parseOnly bool
 	}{
 		{
 			name: "package clause",
@@ -3379,8 +3383,26 @@ join(tables:[a,b], on:["t1"], fn: (a,b) => (a["_field"] - b["_field"]) / b["_fie
 				},
 			},
 		},
+		// todo(jsternberg): fix this once we start handling errors. For now, make sure it parses
+		// correctly without panicking.
+		{
+			name:      "function call with unbalanced braces",
+			raw:       `from() |> range() |> map(fn: (r) => { return r._value )`,
+			parseOnly: true,
+		},
 	} {
 		runFn(tt.name, func(tb testing.TB) {
+			defer func() {
+				if err := recover(); err != nil {
+					tb.Fatalf("unexpected panic: %s", err)
+				}
+			}()
+
+			result := parser.NewAST([]byte(tt.raw))
+			if tt.parseOnly {
+				return
+			}
+
 			want := tt.want.Copy()
 			ast.Walk(ast.CreateVisitor(func(node ast.Node) {
 				v := reflect.ValueOf(node)
@@ -3390,10 +3412,10 @@ join(tables:[a,b], on:["t1"], fn: (a,b) => (a["_field"] - b["_field"]) / b["_fie
 				}
 
 				l := loc.Interface().(*ast.SourceLocation)
-				l.Source = source(tt.raw, l)
+				if l != nil {
+					l.Source = source(tt.raw, l)
+				}
 			}), want)
-
-			result := parser.NewAST([]byte(tt.raw))
 			if got, want := result, tt.want; !cmp.Equal(want, got, CompareOptions...) {
 				tb.Fatalf("unexpected statement -want/+got\n%s", cmp.Diff(want, got, CompareOptions...))
 			}

internal/scanner/scanner.go

@@ -86,7 +86,7 @@ func (s *Scanner) scan(cs int) (pos token.Pos, tok token.Token, lit string) {
 		s.p = s.ts + size
 		return s.f.Pos(s.ts), token.ILLEGAL, fmt.Sprintf("%c", ch)
 	} else if s.token == token.ILLEGAL && s.p == s.eof {
-		return s.f.Pos(s.ts), token.EOF, ""
+		return s.f.Pos(len(s.data)), token.EOF, ""
 	}
 	return s.f.Pos(s.ts), s.token, string(s.data[s.ts:s.te])
 }

internal/scanner/scanner_test.go

@@ -480,3 +480,42 @@ line3`,
 		})
 	}
 }
+
+func TestScanner_EOF_Position(t *testing.T) {
+	f := token.NewFile("query.flux", 1)
+	s := scanner.New(f, []byte(`a`))
+
+	pos, tok, lit := s.Scan()
+	if want, got := token.Pos(1), pos; want != got {
+		t.Errorf("unexpected position -want/+got:\n\t- %v\n\t+ %v", want, got)
+	}
+	if want, got := token.IDENT, tok; want != got {
+		t.Errorf("unexpected token -want/+got:\n\t- %v\n\t+ %v", want, got)
+	}
+	if want, got := "a", lit; want != got {
+		t.Errorf("unexpected literal -want/+got:\n\t- %v\n\t+ %v", want, got)
+	}
+
+	pos, tok, lit = s.Scan()
+	if want, got := token.Pos(2), pos; want != got {
+		t.Errorf("unexpected position -want/+got:\n\t- %v\n\t+ %v", want, got)
+	}
+	if want, got := token.EOF, tok; want != got {
+		t.Errorf("unexpected token -want/+got:\n\t- %v\n\t+ %v", want, got)
+	}
+	if want, got := "", lit; want != got {
+		t.Errorf("unexpected literal -want/+got:\n\t- %v\n\t+ %v", want, got)
+	}
+
+	// Multiple scans of the EOF token should continue producing the same value.
+	pos, tok, lit = s.Scan()
+	if want, got := token.Pos(2), pos; want != got {
+		t.Errorf("unexpected position -want/+got:\n\t- %v\n\t+ %v", want, got)
+	}
+	if want, got := token.EOF, tok; want != got {
+		t.Errorf("unexpected token -want/+got:\n\t- %v\n\t+ %v", want, got)
+	}
+	if want, got := "", lit; want != got {
+		t.Errorf("unexpected literal -want/+got:\n\t- %v\n\t+ %v", want, got)
+	}
+}