Merge pull request #659 from prometheus/fabxc/pql/parse-err

Remove `name` arg from `Parse*` functions, enhance parsing errors.

promql/engine.go

@@ -294,7 +294,7 @@ func (ng *Engine) Stop() {
 
 // NewQuery returns a new query of the given query string.
 func (ng *Engine) NewQuery(qs string) (Query, error) {
-	stmts, err := ParseStmts("query", qs)
+	stmts, err := ParseStmts(qs)
 	if err != nil {
 		return nil, err
 	}
@@ -325,7 +325,7 @@ func (ng *Engine) NewInstantQuery(es string, ts clientmodel.Timestamp) (Query, e
 // NewRangeQuery returns an evaluation query for the given time range and with
 // the resolution set by the interval.
 func (ng *Engine) NewRangeQuery(qs string, start, end clientmodel.Timestamp, interval time.Duration) (Query, error) {
-	expr, err := ParseExpr("query", qs)
+	expr, err := ParseExpr(qs)
 	if err != nil {
 		return nil, err
 	}

promql/lex.go

@@ -222,7 +222,6 @@ type Pos int
 
 // lexer holds the state of the scanner.
 type lexer struct {
-	name    string    // The name of the input; used only for error reports.
 	input   string    // The string being scanned.
 	state   stateFn   // The next lexing function to enter.
 	pos     Pos       // Current position in the input.
@@ -298,12 +297,12 @@ func (l *lexer) lineNumber() int {
 
 // linePosition reports at which character in the current line
 // we are on.
-func (l *lexer) linePosition() Pos {
+func (l *lexer) linePosition() int {
-	lb := Pos(strings.LastIndex(l.input[:l.lastPos], "\n"))
+	lb := strings.LastIndex(l.input[:l.lastPos], "\n")
 	if lb == -1 {
-		return 1 + l.lastPos
+		return 1 + int(l.lastPos)
 	}
-	return 1 + l.lastPos - lb
+	return 1 + int(l.lastPos) - lb
 }
 
 // errorf returns an error token and terminates the scan by passing
@@ -321,9 +320,8 @@ func (l *lexer) nextItem() item {
 }
 
 // lex creates a new scanner for the input string.
-func lex(name, input string) *lexer {
+func lex(input string) *lexer {
 	l := &lexer{
-		name:  name,
 		input: input,
 		items: make(chan item),
 	}

promql/lex_test.go

@@ -14,7 +14,6 @@
 package promql
 
 import (
-	"fmt"
 	"reflect"
 	"testing"
 )
@@ -328,8 +327,7 @@ var tests = []struct {
 // for the parser to avoid duplicated effort.
 func TestLexer(t *testing.T) {
 	for i, test := range tests {
-		tn := fmt.Sprintf("test.%d \"%s\"", i, test.input)
+		l := lex(test.input)
-		l := lex(tn, test.input)
 
 		out := []item{}
 		for it := range l.items {
@@ -339,20 +337,20 @@ func TestLexer(t *testing.T) {
 		lastItem := out[len(out)-1]
 		if test.fail {
 			if lastItem.typ != itemError {
-				t.Fatalf("%s: expected lexing error but did not fail", tn)
+				t.Fatalf("%d: expected lexing error but did not fail", i)
 			}
 			continue
 		}
 		if lastItem.typ == itemError {
-			t.Fatalf("%s: unexpected lexing error: %s", tn, lastItem)
+			t.Fatalf("%d: unexpected lexing error: %s", i, lastItem)
 		}
 
 		if !reflect.DeepEqual(lastItem, item{itemEOF, Pos(len(test.input)), ""}) {
-			t.Fatalf("%s: lexing error: expected output to end with EOF item", tn)
+			t.Fatalf("%d: lexing error: expected output to end with EOF item", i)
 		}
 		out = out[:len(out)-1]
 		if !reflect.DeepEqual(out, test.expected) {
-			t.Errorf("%s: lexing mismatch:\nexpected: %#v\n-----\ngot: %#v", tn, test.expected, out)
+			t.Errorf("%d: lexing mismatch:\nexpected: %#v\n-----\ngot: %#v", i, test.expected, out)
 		}
 	}
 }

promql/parse.go

@@ -17,6 +17,7 @@ import (
 	"fmt"
 	"runtime"
 	"strconv"
+	"strings"
 	"time"
 
 	clientmodel "github.com/prometheus/client_golang/model"
@@ -25,15 +26,29 @@ import (
 )
 
 type parser struct {
-	name      string
 	lex       *lexer
 	token     [3]item
 	peekCount int
 }
 
+// ParseErr wraps a parsing error with line and position context.
+// If the parsing input was a single line, line will be 0 and omitted
+// from the error string.
+type ParseErr struct {
+	Line, Pos int
+	Err       error
+}
+
+func (e *ParseErr) Error() string {
+	if e.Line == 0 {
+		return fmt.Sprintf("Parse error at char %d: %s", e.Pos, e.Err)
+	}
+	return fmt.Sprintf("Parse error at line %d, char %d: %s", e.Line, e.Pos, e.Err)
+}
+
 // ParseStmts parses the input and returns the resulting statements or any ocurring error.
-func ParseStmts(name, input string) (Statements, error) {
+func ParseStmts(input string) (Statements, error) {
-	p := newParser(name, input)
+	p := newParser(input)
 
 	stmts, err := p.parseStmts()
 	if err != nil {
@@ -44,8 +59,8 @@ func ParseStmts(name, input string) (Statements, error) {
 }
 
 // ParseExpr returns the expression parsed from the input.
-func ParseExpr(name, input string) (Expr, error) {
+func ParseExpr(input string) (Expr, error) {
-	p := newParser(name, input)
+	p := newParser(input)
 
 	expr, err := p.parseExpr()
 	if err != nil {
@@ -56,10 +71,9 @@ func ParseExpr(name, input string) (Expr, error) {
 }
 
 // newParser returns a new parser.
-func newParser(name, input string) *parser {
+func newParser(input string) *parser {
 	p := &parser{
-		name: name,
+		lex: lex(input),
-		lex:  lex(name, input),
 	}
 	return p
 }
@@ -144,13 +158,20 @@ func (p *parser) backup() {
 
 // errorf formats the error and terminates processing.
 func (p *parser) errorf(format string, args ...interface{}) {
-	format = fmt.Sprintf("%s:%d,%d %s", p.name, p.lex.lineNumber(), p.lex.linePosition(), format)
+	p.error(fmt.Errorf(format, args...))
-	panic(fmt.Errorf(format, args...))
 }
 
 // error terminates processing.
 func (p *parser) error(err error) {
-	p.errorf("%s", err)
+	perr := &ParseErr{
+		Line: p.lex.lineNumber(),
+		Pos:  p.lex.linePosition(),
+		Err:  err,
+	}
+	if strings.Count(strings.TrimSpace(p.lex.input), "\n") == 0 {
+		perr.Line = 0
+	}
+	panic(perr)
 }
 
 // expect consumes the next token and guarantees it has the required type.

promql/parse_test.go

@@ -785,7 +785,7 @@ var testExpr = []struct {
 func TestParseExpressions(t *testing.T) {
 	for _, test := range testExpr {
 
-		parser := newParser("test", test.input)
+		parser := newParser(test.input)
 
 		expr, err := parser.parseExpr()
 		if !test.fail && err != nil {
@@ -819,7 +819,7 @@ func TestParseExpressions(t *testing.T) {
 
 // NaN has no equality. Thus, we need a separate test for it.
 func TestNaNExpression(t *testing.T) {
-	parser := newParser("test", "NaN")
+	parser := newParser("NaN")
 
 	expr, err := parser.parseExpr()
 	if err != nil {
@@ -1028,7 +1028,7 @@ var testStatement = []struct {
 
 func TestParseStatements(t *testing.T) {
 	for _, test := range testStatement {
-		parser := newParser("test", test.input)
+		parser := newParser(test.input)
 
 		stmts, err := parser.parseStmts()
 		if !test.fail && err != nil {

rules/rules_test.go

@@ -173,7 +173,7 @@ func TestAlertingRule(t *testing.T) {
 	engine := promql.NewEngine(storage)
 	defer engine.Stop()
 
-	expr, err := promql.ParseExpr("test", `http_requests{group="canary", job="app-server"} < 100`)
+	expr, err := promql.ParseExpr(`http_requests{group="canary", job="app-server"} < 100`)
 	if err != nil {
 		t.Fatalf("Unable to parse alert expression: %s", err)
 	}

tools/rule_checker/main.go

@@ -39,7 +39,7 @@ func checkRules(filename string, in io.Reader, out io.Writer) error {
 		return err
 	}
 
-	rules, err := promql.ParseStmts(filename, string(content))
+	rules, err := promql.ParseStmts(string(content))
 	if err != nil {
 		return err
 	}

web/api/api_test.go

@@ -52,7 +52,7 @@ func TestQuery(t *testing.T) {
 		{
 			queryStr: "",
 			status:   http.StatusOK,
-			bodyRe:   `{"type":"error","value":"query:1,1 no expression found in input","version":1}`,
+			bodyRe:   `{"type":"error","value":"Parse error at char 1: no expression found in input","version":1}`,
 		},
 		{
 			queryStr: "expr=testmetric",
@@ -77,7 +77,7 @@ func TestQuery(t *testing.T) {
 		{
 			queryStr: "expr=(badexpression",
 			status:   http.StatusOK,
-			bodyRe:   `{"type":"error","value":"query:1,15 unexpected unclosed left parenthesis in paren expression","version":1}`,
+			bodyRe:   `{"type":"error","value":"Parse error at char 15: unexpected unclosed left parenthesis in paren expression","version":1}`,
 		},
 	}