Merge pull request #660 from prometheus/fabxc/pql/parse-errs

Fix and improve parsing error output.

promql/lex.go

@@ -15,7 +15,6 @@ package promql
 
 import (
 	"fmt"
-	"reflect"
 	"strings"
 	"unicode"
 	"unicode/utf8"
@@ -35,6 +34,8 @@ func (i item) String() string {
 		return "EOF"
 	case i.typ == itemError:
 		return i.val
+	case i.typ == itemIdentifier || i.typ == itemMetricIdentifier:
+		return fmt.Sprintf("%q", i.val)
 	case i.typ.isKeyword():
 		return fmt.Sprintf("<%s>", i.val)
 	case i.typ.isOperator():
@@ -183,6 +184,16 @@ var key = map[string]itemType{
 // These are the default string representations for common items. It does not
 // imply that those are the only character sequences that can be lexed to such an item.
 var itemTypeStr = map[itemType]string{
+	itemLeftParen:    "(",
+	itemRightParen:   ")",
+	itemLeftBrace:    "{",
+	itemRightBrace:   "}",
+	itemLeftBracket:  "[",
+	itemRightBracket: "]",
+	itemComma:        ",",
+	itemAssign:       "=",
+	itemSemicolon:    ";",
+
 	itemSUB:      "-",
 	itemADD:      "+",
 	itemMUL:      "*",
@@ -209,7 +220,39 @@ func (t itemType) String() string {
 	if s, ok := itemTypeStr[t]; ok {
 		return s
 	}
-	return reflect.TypeOf(t).Name()
+	return fmt.Sprintf("<item %d>", t)
+}
+
+func (i item) desc() string {
+	if _, ok := itemTypeStr[i.typ]; ok {
+		return i.String()
+	}
+	if i.typ == itemEOF {
+		return i.typ.desc()
+	}
+	return fmt.Sprintf("%s %s", i.typ.desc(), i)
+}
+
+func (t itemType) desc() string {
+	switch t {
+	case itemError:
+		return "error"
+	case itemEOF:
+		return "end of input"
+	case itemComment:
+		return "comment"
+	case itemIdentifier:
+		return "identifier"
+	case itemMetricIdentifier:
+		return "metric identifier"
+	case itemString:
+		return "string"
+	case itemNumber:
+		return "number"
+	case itemDuration:
+		return "duration"
+	}
+	return fmt.Sprintf("%q", t)
 }
 
 const eof = -1
@@ -377,7 +420,7 @@ func lexStatements(l *lexer) stateFn {
 			l.next()
 			l.emit(itemEQL)
 		} else if t == '~' {
-			return l.errorf("unrecognized character after '=': %#U", t)
+			return l.errorf("unexpected character after '=': %q", t)
 		} else {
 			l.emit(itemAssign)
 		}
@@ -385,7 +428,7 @@ func lexStatements(l *lexer) stateFn {
 		if t := l.next(); t == '=' {
 			l.emit(itemNEQ)
 		} else {
-			return l.errorf("unrecognized character after '!': %#U", t)
+			return l.errorf("unexpected character after '!': %q", t)
 		}
 	case r == '<':
 		if t := l.peek(); t == '=' {
@@ -401,7 +444,7 @@ func lexStatements(l *lexer) stateFn {
 		} else {
 			l.emit(itemGTR)
 		}
-	case '0' <= r && r <= '9' || r == '.':
+	case unicode.IsDigit(r) || (r == '.' && unicode.IsDigit(l.peek())):
 		l.backup()
 		return lexNumberOrDuration
 	case r == '"' || r == '\'':
@@ -422,7 +465,7 @@ func lexStatements(l *lexer) stateFn {
 			}
 		}
 		fallthrough
-	case isAlphaNumeric(r):
+	case isAlphaNumeric(r) || r == ':':
 		l.backup()
 		return lexKeywordOrIdentifier
 	case r == '(':
@@ -433,7 +476,7 @@ func lexStatements(l *lexer) stateFn {
 		l.emit(itemRightParen)
 		l.parenDepth--
 		if l.parenDepth < 0 {
-			return l.errorf("unexpected right parenthesis %#U", r)
+			return l.errorf("unexpected right parenthesis %q", r)
 		}
 		return lexStatements
 	case r == '{':
@@ -442,20 +485,20 @@ func lexStatements(l *lexer) stateFn {
 		return lexInsideBraces(l)
 	case r == '[':
 		if l.bracketOpen {
-			return l.errorf("unexpected left bracket %#U", r)
+			return l.errorf("unexpected left bracket %q", r)
 		}
 		l.emit(itemLeftBracket)
 		l.bracketOpen = true
 		return lexDuration
 	case r == ']':
 		if !l.bracketOpen {
-			return l.errorf("unexpected right bracket %#U", r)
+			return l.errorf("unexpected right bracket %q", r)
 		}
 		l.emit(itemRightBracket)
 		l.bracketOpen = false
 
 	default:
-		return l.errorf("unrecognized character in statement: %#U", r)
+		return l.errorf("unexpected character: %q", r)
 	}
 	return lexStatements
 }
@@ -469,10 +512,10 @@ func lexInsideBraces(l *lexer) stateFn {
 
 	switch r := l.next(); {
 	case r == eof:
-		return l.errorf("unexpected EOF inside braces")
+		return l.errorf("unexpected end of input inside braces")
 	case isSpace(r):
 		return lexSpace
-	case isAlphaNumeric(r):
+	case unicode.IsLetter(r) || r == '_':
 		l.backup()
 		return lexIdentifier
 	case r == ',':
@@ -494,16 +537,16 @@ func lexInsideBraces(l *lexer) stateFn {
 		case nr == '=':
 			l.emit(itemNEQ)
 		default:
-			return l.errorf("unrecognized character after '!' inside braces: %#U", nr)
+			return l.errorf("unexpected character after '!' inside braces: %q", nr)
 		}
 	case r == '{':
-		return l.errorf("unexpected left brace %#U", r)
+		return l.errorf("unexpected left brace %q", r)
 	case r == '}':
 		l.emit(itemRightBrace)
 		l.braceOpen = false
 		return lexStatements
 	default:
-		return l.errorf("unrecognized character inside braces: %#U", r)
+		return l.errorf("unexpected character inside braces: %q", r)
 	}
 	return lexInsideBraces
 }
@@ -553,7 +596,11 @@ func lexDuration(l *lexer) stateFn {
 		return l.errorf("missing unit character in duration")
 	}
 	// Next two chars must be a valid unit and a non-alphanumeric.
-	if l.accept("smhdwy") && !isAlphaNumeric(l.peek()) {
+	if l.accept("smhdwy") {
+		if isAlphaNumeric(l.next()) {
+			return l.errorf("bad duration syntax: %q", l.input[l.start:l.pos])
+		}
+		l.backup()
 		l.emit(itemDuration)
 		return lexStatements
 	}
@@ -576,7 +623,11 @@ func lexNumberOrDuration(l *lexer) stateFn {
 		return lexStatements
 	}
 	// Next two chars must be a valid unit and a non-alphanumeric.
-	if l.accept("smhdwy") && !isAlphaNumeric(l.peek()) {
+	if l.accept("smhdwy") {
+		if isAlphaNumeric(l.next()) {
+			return l.errorf("bad number or duration syntax: %q", l.input[l.start:l.pos])
+		}
+		l.backup()
 		l.emit(itemDuration)
 		return lexStatements
 	}
@@ -605,7 +656,8 @@ func (l *lexer) scanNumber() bool {
 	return true
 }
 
-// lexIdentifier scans an alphanumeric identifier.
+// lexIdentifier scans an alphanumeric identifier. The next character
+// is known to be a letter.
 func lexIdentifier(l *lexer) stateFn {
 	for isAlphaNumeric(l.next()) {
 		// absorb
@@ -651,5 +703,5 @@ func isEndOfLine(r rune) bool {
 
 // isAlphaNumeric reports whether r is an alphabetic, digit, or underscore.
 func isAlphaNumeric(r rune) bool {
-	return r == '_' || unicode.IsLetter(r) || unicode.IsDigit(r)
+	return r == '_' || ('a' <= r && r <= 'z') || ('A' <= r && r <= 'Z') || unicode.IsDigit(r)
 }

promql/lex_test.go

@@ -245,6 +245,9 @@ var tests = []struct {
 	},
 	// Test Selector.
 	{
+		input: `台北`,
+		fail:  true,
+	}, {
 		input: `{foo="bar"}`,
 		expected: []item{
 			{itemLeftBrace, 0, `{`},

promql/parse.go

@@ -101,7 +101,7 @@ func (p *parser) parseExpr() (expr Expr, err error) {
 			continue
 		}
 		if expr != nil {
-			p.errorf("expression read but input remaining")
+			p.errorf("could not parse remaining input %.15q...", p.lex.input[p.lex.lastPos:])
 		}
 		expr = p.expr()
 	}
@@ -132,6 +132,9 @@ func (p *parser) next() item {
 		}
 		p.token[0] = t
 	}
+	if p.token[p.peekCount].typ == itemError {
+		p.errorf("%s", p.token[p.peekCount].val)
+	}
 	return p.token[p.peekCount]
 }
 
@@ -175,28 +178,23 @@ func (p *parser) error(err error) {
 }
 
 // expect consumes the next token and guarantees it has the required type.
-func (p *parser) expect(expected itemType, context string) item {
+func (p *parser) expect(exp itemType, context string) item {
 	token := p.next()
-	if token.typ != expected {
+	if token.typ != exp {
-		p.unexpected(token, context)
+		p.errorf("unexpected %s in %s, expected %s", token.desc(), context, exp.desc())
 	}
 	return token
 }
 
 // expectOneOf consumes the next token and guarantees it has one of the required types.
-func (p *parser) expectOneOf(expected1, expected2 itemType, context string) item {
+func (p *parser) expectOneOf(exp1, exp2 itemType, context string) item {
 	token := p.next()
-	if token.typ != expected1 && token.typ != expected2 {
+	if token.typ != exp1 && token.typ != exp2 {
-		p.unexpected(token, context)
+		p.errorf("unexpected %s in %s, expected %s or %s", token.desc(), context, exp1.desc(), exp2.desc())
 	}
 	return token
 }
 
-// unexpected complains about the token and terminates processing.
-func (p *parser) unexpected(token item, context string) {
-	p.errorf("unexpected %s in %s", token, context)
-}
-
 // recover is the handler that turns panics into returns from the top level of Parse.
 func (p *parser) recover(errp *error) {
 	e := recover()
@@ -303,10 +301,11 @@ func (p *parser) recordStmt() *RecordStmt {
 
 // expr parses any expression.
 func (p *parser) expr() Expr {
-	const ctx = "binary expression"
-
 	// Parse the starting expression.
 	expr := p.unaryExpr()
+	if expr == nil {
+		p.errorf("no valid expression found")
+	}
 
 	// Loop through the operations and construct a binary operation tree based
 	// on the operators' precedence.
@@ -354,6 +353,9 @@ func (p *parser) expr() Expr {
 
 		// Parse the next operand.
 		rhs := p.unaryExpr()
+		if rhs == nil {
+			p.errorf("missing right-hand side in binary expression")
+		}
 
 		// Assign the new root based on the precendence of the LHS and RHS operators.
 		if lhs, ok := expr.(*BinaryExpr); ok && lhs.Op.precedence() < op.precedence() {
@@ -497,7 +499,6 @@ func (p *parser) primaryExpr() Expr {
 		p.backup()
 		return p.aggrExpr()
 	}
-	p.errorf("invalid primary expression")
 	return nil
 }
 
@@ -535,7 +536,7 @@ func (p *parser) aggrExpr() *AggregateExpr {
 
 	agop := p.next()
 	if !agop.typ.isAggregator() {
-		p.errorf("%s is not an aggregation operator", agop)
+		p.errorf("expected aggregation operator but got %s", agop)
 	}
 	var grouping clientmodel.LabelNames
 	var keepExtra bool
@@ -650,7 +651,7 @@ func (p *parser) labelMatchers(operators ...itemType) metric.LabelMatchers {
 
 		op := p.next().typ
 		if !op.isOperator() {
-			p.errorf("item %s is not a valid operator for label matching", op)
+			p.errorf("expected label matching operator but got %s", op)
 		}
 		var validOp = false
 		for _, allowedOp := range operators {
@@ -849,10 +850,10 @@ func (p *parser) checkType(node Node) (typ ExprType) {
 	case *Call:
 		nargs := len(n.Func.ArgTypes)
 		if na := nargs - n.Func.OptionalArgs; na > len(n.Args) {
-			p.errorf("expected at least %d arguments in call to %q, got %d", na, n.Func.Name, len(n.Args))
+			p.errorf("expected at least %d argument(s) in call to %q, got %d", na, n.Func.Name, len(n.Args))
 		}
 		if nargs < len(n.Args) {
-			p.errorf("expected at most %d arguments in call to %q, got %d", nargs, n.Func.Name, len(n.Args))
+			p.errorf("expected at most %d argument(s) in call to %q, got %d", nargs, n.Func.Name, len(n.Args))
 		}
 		for i, arg := range n.Args {
 			p.expectType(arg, n.Func.ArgTypes[i], fmt.Sprintf("call to function %q", n.Func.Name))

promql/parse_test.go

@@ -17,6 +17,7 @@ import (
 	"fmt"
 	"math"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
@@ -25,9 +26,10 @@ import (
 )
 
 var testExpr = []struct {
-	input    string
+	input    string // The input to be parsed.
-	expected Expr
+	expected Expr   // The expected expression AST.
-	fail     bool
+	fail     bool   // Whether parsing is supposed to fail.
+	errMsg   string // If not empty the parsing error has to contain this string.
 }{
 	// Scalars and scalar-to-scalar operations.
 	{
@@ -122,39 +124,77 @@ var testExpr = []struct {
 			},
 		},
 	}, {
-		input: "", fail: true,
+		input:  "",
+		fail:   true,
+		errMsg: "no expression found in input",
 	}, {
-		input: "# just a comment\n\n", fail: true,
+		input:  "# just a comment\n\n",
+		fail:   true,
+		errMsg: "no expression found in input",
 	}, {
-		input: "1+", fail: true,
+		input:  "1+",
+		fail:   true,
+		errMsg: "missing right-hand side in binary expression",
 	}, {
-		input: "2.5.", fail: true,
+		input:  ".",
+		fail:   true,
+		errMsg: "unexpected character: '.'",
 	}, {
-		input: "100..4", fail: true,
+		input:  "2.5.",
+		fail:   true,
+		errMsg: "could not parse remaining input \".\"...",
 	}, {
-		input: "0deadbeef", fail: true,
+		input:  "100..4",
+		fail:   true,
+		errMsg: "could not parse remaining input \".4\"...",
 	}, {
-		input: "1 /", fail: true,
+		input:  "0deadbeef",
+		fail:   true,
+		errMsg: "bad number or duration syntax: \"0de\"",
 	}, {
-		input: "*1", fail: true,
+		input:  "1 /",
+		fail:   true,
+		errMsg: "missing right-hand side in binary expression",
 	}, {
-		input: "(1))", fail: true,
+		input:  "*1",
+		fail:   true,
+		errMsg: "no valid expression found",
 	}, {
-		input: "((1)", fail: true,
+		input:  "(1))",
+		fail:   true,
+		errMsg: "could not parse remaining input \")\"...",
 	}, {
-		input: "(", fail: true,
+		input:  "((1)",
+		fail:   true,
+		errMsg: "unclosed left parenthesis",
 	}, {
-		input: "1 and 1", fail: true,
+		input:  "(",
+		fail:   true,
+		errMsg: "unclosed left parenthesis",
 	}, {
-		input: "1 or 1", fail: true,
+		input:  "1 and 1",
+		fail:   true,
+		errMsg: "AND and OR not allowed in binary scalar expression",
 	}, {
-		input: "1 !~ 1", fail: true,
+		input:  "1 or 1",
+		fail:   true,
+		errMsg: "AND and OR not allowed in binary scalar expression",
 	}, {
-		input: "1 =~ 1", fail: true,
+		input:  "1 !~ 1",
+		fail:   true,
+		errMsg: "could not parse remaining input \"!~ 1\"...",
 	}, {
-		input: "-some_metric", fail: true,
+		input:  "1 =~ 1",
+		fail:   true,
+		errMsg: "could not parse remaining input \"=~ 1\"...",
 	}, {
-		input: `-"string"`, fail: true,
+		input:  "-some_metric",
+		fail:   true,
+		errMsg: "expected type scalar in unary expression, got vector",
+	}, {
+		input:  `-"string"`,
+		fail:   true,
+		errMsg: "expected type scalar in unary expression, got string",
 	},
 	// Vector binary operations.
 	{
@@ -397,25 +437,45 @@ var testExpr = []struct {
 			},
 		},
 	}, {
-		input: "foo and 1", fail: true,
+		input:  "foo and 1",
+		fail:   true,
+		errMsg: "AND and OR not allowed in binary scalar expression",
 	}, {
-		input: "1 and foo", fail: true,
+		input:  "1 and foo",
+		fail:   true,
+		errMsg: "AND and OR not allowed in binary scalar expression",
 	}, {
-		input: "foo or 1", fail: true,
+		input:  "foo or 1",
+		fail:   true,
+		errMsg: "AND and OR not allowed in binary scalar expression",
 	}, {
-		input: "1 or foo", fail: true,
+		input:  "1 or foo",
+		fail:   true,
+		errMsg: "AND and OR not allowed in binary scalar expression",
 	}, {
-		input: "1 or on(bar) foo", fail: true,
+		input:  "1 or on(bar) foo",
+		fail:   true,
+		errMsg: "vector matching only allowed between vectors",
 	}, {
-		input: "foo == on(bar) 10", fail: true,
+		input:  "foo == on(bar) 10",
+		fail:   true,
+		errMsg: "vector matching only allowed between vectors",
 	}, {
-		input: "foo and on(bar) group_left(baz) bar", fail: true,
+		input:  "foo and on(bar) group_left(baz) bar",
+		fail:   true,
+		errMsg: "no grouping allowed for AND and OR operations",
 	}, {
-		input: "foo and on(bar) group_right(baz) bar", fail: true,
+		input:  "foo and on(bar) group_right(baz) bar",
+		fail:   true,
+		errMsg: "no grouping allowed for AND and OR operations",
 	}, {
-		input: "foo or on(bar) group_left(baz) bar", fail: true,
+		input:  "foo or on(bar) group_left(baz) bar",
+		fail:   true,
+		errMsg: "no grouping allowed for AND and OR operations",
 	}, {
-		input: "foo or on(bar) group_right(baz) bar", fail: true,
+		input:  "foo or on(bar) group_right(baz) bar",
+		fail:   true,
+		errMsg: "no grouping allowed for AND and OR operations",
 	},
 	// Test vector selector.
 	{
@@ -470,31 +530,59 @@ var testExpr = []struct {
 			},
 		},
 	}, {
-		input: `{`, fail: true,
+		input:  `{`,
+		fail:   true,
+		errMsg: "unexpected end of input inside braces",
 	}, {
-		input: `}`, fail: true,
+		input:  `}`,
+		fail:   true,
+		errMsg: "unexpected character: '}'",
 	}, {
-		input: `some{`, fail: true,
+		input:  `some{`,
+		fail:   true,
+		errMsg: "unexpected end of input inside braces",
 	}, {
-		input: `some}`, fail: true,
+		input:  `some}`,
+		fail:   true,
+		errMsg: "could not parse remaining input \"}\"...",
 	}, {
-		input: `some_metric{a=b}`, fail: true,
+		input:  `some_metric{a=b}`,
+		fail:   true,
+		errMsg: "unexpected identifier \"b\" in label matching, expected string",
 	}, {
-		input: `some_metric{a:b="b"}`, fail: true,
+		input:  `some_metric{a:b="b"}`,
+		fail:   true,
+		errMsg: "unexpected character inside braces: ':'",
 	}, {
-		input: `foo{a*"b"}`, fail: true,
+		input:  `foo{a*"b"}`,
+		fail:   true,
+		errMsg: "unexpected character inside braces: '*'",
 	}, {
-		input: `foo{a>="b"}`, fail: true,
+		input: `foo{a>="b"}`,
+		fail:  true,
+		// TODO(fabxc): willingly lexing wrong tokens allows for more precrise error
+		// messages from the parser - consider if this is an option.
+		errMsg: "unexpected character inside braces: '>'",
 	}, {
-		input: `foo{gibberish}`, fail: true,
+		input:  `foo{gibberish}`,
+		fail:   true,
+		errMsg: "expected label matching operator but got }",
 	}, {
-		input: `foo{1}`, fail: true,
+		input:  `foo{1}`,
+		fail:   true,
+		errMsg: "unexpected character inside braces: '1'",
 	}, {
-		input: `{}`, fail: true,
+		input:  `{}`,
+		fail:   true,
+		errMsg: "vector selector must contain label matchers or metric name",
 	}, {
-		input: `foo{__name__="bar"}`, fail: true,
+		input:  `foo{__name__="bar"}`,
-	}, {
+		fail:   true,
-		input: `:foo`, fail: true,
+		errMsg: "metric name must not be set twice: \"foo\" or \"bar\"",
+		// }, {
+		// 	input:  `:foo`,
+		// 	fail:   true,
+		// 	errMsg: "bla",
 	},
 	// Test matrix selector.
 	{
@@ -559,25 +647,45 @@ var testExpr = []struct {
 			},
 		},
 	}, {
-		input: `foo[5mm]`, fail: true,
+		input:  `foo[5mm]`,
+		fail:   true,
+		errMsg: "bad duration syntax: \"5mm\"",
 	}, {
-		input: `foo[0m]`, fail: true,
+		input:  `foo[0m]`,
+		fail:   true,
+		errMsg: "duration must be greater than 0",
 	}, {
-		input: `foo[5m30s]`, fail: true,
+		input:  `foo[5m30s]`,
+		fail:   true,
+		errMsg: "bad duration syntax: \"5m3\"",
 	}, {
-		input: `foo[5m] OFFSET 1h30m`, fail: true,
+		input:  `foo[5m] OFFSET 1h30m`,
+		fail:   true,
+		errMsg: "bad number or duration syntax: \"1h3\"",
 	}, {
-		input: `foo[]`, fail: true,
+		input:  `foo[]`,
+		fail:   true,
+		errMsg: "missing unit character in duration",
 	}, {
-		input: `foo[1]`, fail: true,
+		input:  `foo[1]`,
+		fail:   true,
+		errMsg: "missing unit character in duration",
 	}, {
-		input: `some_metric[5m] OFFSET 1`, fail: true,
+		input:  `some_metric[5m] OFFSET 1`,
+		fail:   true,
+		errMsg: "unexpected number \"1\" in matrix selector, expected duration",
 	}, {
-		input: `some_metric[5m] OFFSET 1mm`, fail: true,
+		input:  `some_metric[5m] OFFSET 1mm`,
+		fail:   true,
+		errMsg: "bad number or duration syntax: \"1mm\"",
 	}, {
-		input: `some_metric[5m] OFFSET`, fail: true,
+		input:  `some_metric[5m] OFFSET`,
+		fail:   true,
+		errMsg: "unexpected end of input in matrix selector, expected duration",
 	}, {
-		input: `(foo + bar)[5m]`, fail: true,
+		input:  `(foo + bar)[5m]`,
+		fail:   true,
+		errMsg: "could not parse remaining input \"[5m]\"...",
 	},
 	// Test aggregation.
 	{
@@ -692,21 +800,37 @@ var testExpr = []struct {
 			Grouping: clientmodel.LabelNames{"foo"},
 		},
 	}, {
-		input: `sum some_metric by (test)`, fail: true,
+		input:  `sum some_metric by (test)`,
+		fail:   true,
+		errMsg: "unexpected identifier \"some_metric\" in aggregation, expected \"(\"",
 	}, {
-		input: `sum (some_metric) by test`, fail: true,
+		input:  `sum (some_metric) by test`,
+		fail:   true,
+		errMsg: "unexpected identifier \"test\" in grouping opts, expected \"(\"",
 	}, {
-		input: `sum (some_metric) by ()`, fail: true,
+		input:  `sum (some_metric) by ()`,
+		fail:   true,
+		errMsg: "unexpected \")\" in grouping opts, expected identifier",
 	}, {
-		input: `sum (some_metric) by test`, fail: true,
+		input:  `sum (some_metric) by test`,
+		fail:   true,
+		errMsg: "unexpected identifier \"test\" in grouping opts, expected \"(\"",
 	}, {
-		input: `some_metric[5m] OFFSET`, fail: true,
+		input:  `some_metric[5m] OFFSET`,
+		fail:   true,
+		errMsg: "unexpected end of input in matrix selector, expected duration",
 	}, {
-		input: `sum () by (test)`, fail: true,
+		input:  `sum () by (test)`,
+		fail:   true,
+		errMsg: "no valid expression found",
 	}, {
-		input: "MIN keeping_extra (some_metric) by (foo)", fail: true,
+		input:  "MIN keeping_extra (some_metric) by (foo)",
+		fail:   true,
+		errMsg: "could not parse remaining input \"by (foo)\"...",
 	}, {
-		input: "MIN by(test) (some_metric) keeping_extra", fail: true,
+		input:  "MIN by(test) (some_metric) keeping_extra",
+		fail:   true,
+		errMsg: "could not parse remaining input \"keeping_extra\"...",
 	},
 	// Test function calls.
 	{
@@ -770,21 +894,30 @@ var testExpr = []struct {
 			},
 		},
 	}, {
-		input: "floor()", fail: true,
+		input:  "floor()",
+		fail:   true,
+		errMsg: "expected at least 1 argument(s) in call to \"floor\", got 0",
 	}, {
-		input: "floor(some_metric, other_metric)", fail: true,
+		input:  "floor(some_metric, other_metric)",
+		fail:   true,
+		errMsg: "expected at most 1 argument(s) in call to \"floor\", got 2",
 	}, {
-		input: "floor(1)", fail: true,
+		input:  "floor(1)",
+		fail:   true,
+		errMsg: "expected type vector in call to function \"floor\", got scalar",
 	}, {
-		input: "non_existant_function_far_bar()", fail: true,
+		input:  "non_existant_function_far_bar()",
+		fail:   true,
+		errMsg: "unknown function with name \"non_existant_function_far_bar\"",
 	}, {
-		input: "rate(some_metric)", fail: true,
+		input:  "rate(some_metric)",
+		fail:   true,
+		errMsg: "expected type matrix in call to function \"rate\", got vector",
 	},
 }
 
 func TestParseExpressions(t *testing.T) {
 	for _, test := range testExpr {
-
 		parser := newParser(test.input)
 
 		expr, err := parser.parseExpr()
@@ -793,6 +926,10 @@ func TestParseExpressions(t *testing.T) {
 			t.Fatalf("could not parse: %s", err)
 		}
 		if test.fail && err != nil {
+			if !strings.Contains(err.Error(), test.errMsg) {
+				t.Errorf("unexpected error on input '%s'", test.input)
+				t.Fatalf("expected error to contain %q but got %q", test.errMsg, err)
+			}
 			continue
 		}
 
@@ -804,6 +941,10 @@ func TestParseExpressions(t *testing.T) {
 
 		if test.fail {
 			if err != nil {
+				if !strings.Contains(err.Error(), test.errMsg) {
+					t.Errorf("unexpected error on input '%s'", test.input)
+					t.Fatalf("expected error to contain %q but got %q", test.errMsg, err)
+				}
 				continue
 			}
 			t.Errorf("error on input '%s'", test.input)

promql/printer.go

@@ -72,6 +72,9 @@ func Tree(node Node) string {
 }
 
 func tree(node Node, level string) string {
+	if node == nil {
+		return fmt.Sprintf("%s |---- %T\n", level, node)
+	}
 	typs := strings.Split(fmt.Sprintf("%T", node), ".")[1]
 
 	var t string

web/api/api_test.go

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