From 25cdff3527ae3bc382c02cbf33b1e4358e2a71f3 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Wed, 29 Apr 2015 11:36:41 +0200 Subject: [PATCH] Remove `name` arg from `Parse*` functions, enhance parsing errors. --- promql/engine.go | 4 ++-- promql/lex.go | 12 +++++------ promql/lex_test.go | 12 +++++------ promql/parse.go | 43 ++++++++++++++++++++++++++++---------- promql/parse_test.go | 6 +++--- rules/rules_test.go | 2 +- tools/rule_checker/main.go | 2 +- web/api/api_test.go | 4 ++-- 8 files changed, 51 insertions(+), 34 deletions(-) diff --git a/promql/engine.go b/promql/engine.go index dc3714ab0..a149de0ad 100644 --- a/promql/engine.go +++ b/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 } diff --git a/promql/lex.go b/promql/lex.go index 4a27bbcc5..d147b39f2 100644 --- a/promql/lex.go +++ b/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 { - lb := Pos(strings.LastIndex(l.input[:l.lastPos], "\n")) +func (l *lexer) linePosition() int { + 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), } diff --git a/promql/lex_test.go b/promql/lex_test.go index c9c45b58c..8d3425659 100644 --- a/promql/lex_test.go +++ b/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(tn, test.input) + l := lex(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) } } } diff --git a/promql/parse.go b/promql/parse.go index ec388c450..f7ac25f9e 100644 --- a/promql/parse.go +++ b/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) { - p := newParser(name, input) +func ParseStmts(input string) (Statements, error) { + 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) { - p := newParser(name, input) +func ParseExpr(input string) (Expr, error) { + 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(name, input), + lex: lex(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) - panic(fmt.Errorf(format, args...)) + p.error(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. diff --git a/promql/parse_test.go b/promql/parse_test.go index 49750859a..02d350b4b 100644 --- a/promql/parse_test.go +++ b/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 { diff --git a/rules/rules_test.go b/rules/rules_test.go index 113d200b9..9511f9682 100644 --- a/rules/rules_test.go +++ b/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) } diff --git a/tools/rule_checker/main.go b/tools/rule_checker/main.go index f53991d5f..c072bd631 100644 --- a/tools/rule_checker/main.go +++ b/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 } diff --git a/web/api/api_test.go b/web/api/api_test.go index 44f52d8f0..5eabf4ed2 100644 --- a/web/api/api_test.go +++ b/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}`, }, }