From db1258f2a53a1f7a9cda35b16041e878deffcd77 Mon Sep 17 00:00:00 2001 From: Tobias Guggenmos Date: Wed, 18 Dec 2019 18:36:43 +0100 Subject: [PATCH] PromQL: Refactor error message generation (#6481) * Add parser method to produce errors messages about unexpected items * PromQL: use parser.unexpected in generated parser Signed-off-by: Tobias Guggenmos --- promql/generated_parser.y | 26 +++++++++++++------------- promql/generated_parser.y.go | 26 +++++++++++++------------- promql/parse.go | 27 +++++++++++++++++++++++++-- promql/parse_test.go | 2 +- 4 files changed, 52 insertions(+), 29 deletions(-) diff --git a/promql/generated_parser.y b/promql/generated_parser.y index 87c88b9207..8a18ac4885 100644 --- a/promql/generated_parser.y +++ b/promql/generated_parser.y @@ -134,7 +134,7 @@ start : START_LABELS label_matchers | START_GROUPING_LABELS grouping_labels { yylex.(*parser).generatedParserResult = $2 } | error /* If none of the more detailed error messages are triggered, we fall back to this. */ - { yylex.(*parser).errorf("unexpected %v", yylex.(*parser).token.desc()) } + { yylex.(*parser).unexpected("","") } ; @@ -154,18 +154,18 @@ label_match_list: | label_matcher { $$ = []*labels.Matcher{$1}} | label_match_list error - { yylex.(*parser).errorf("unexpected %v in label matching, expected \",\" or \"}\"", yylex.(*parser).token.desc()) } + { yylex.(*parser).unexpected("label matching", "\",\" or \"}\"") } ; label_matcher : IDENTIFIER match_op STRING { $$ = yylex.(*parser).newLabelMatcher($1, $2, $3) } | IDENTIFIER match_op error - { yylex.(*parser).errorf("unexpected %v in label matching, expected string", yylex.(*parser).token.desc())} + { yylex.(*parser).unexpected("label matching", "string")} | IDENTIFIER error - { yylex.(*parser).errorf("unexpected %v in label matching, expected label matching operator", yylex.(*parser).token.Val) } + { yylex.(*parser).unexpected("label matching", "label matching operator") } | error - { yylex.(*parser).errorf("unexpected %v in label matching, expected identifier or \"}\"", yylex.(*parser).token.desc()) } + { yylex.(*parser).unexpected("label matching", "identifier or \"}\"")} ; match_op : @@ -207,7 +207,7 @@ label_set_list : | label_set_item { $$ = []labels.Label{$1} } | label_set_list error - { yylex.(*parser).errorf("unexpected %v in label set, expected \",\" or \"}\"", yylex.(*parser).token.desc()) } + { yylex.(*parser).unexpected("label set", "\",\" or \"}\"", ) } ; @@ -215,11 +215,11 @@ label_set_item : IDENTIFIER EQL STRING { $$ = labels.Label{Name: $1.Val, Value: yylex.(*parser).unquoteString($3.Val) } } | IDENTIFIER EQL error - { yylex.(*parser).errorf("unexpected %v in label set, expected string", yylex.(*parser).token.desc())} + { yylex.(*parser).unexpected("label set", "string")} | IDENTIFIER error - { yylex.(*parser).errorf("unexpected %v in label set, expected \"=\"", yylex.(*parser).token.desc())} + { yylex.(*parser).unexpected("label set", "\"=\"")} | error - { yylex.(*parser).errorf("unexpected %v in label set, expected identifier or \"}\"", yylex.(*parser).token.desc()) } + { yylex.(*parser).unexpected("label set", "identifier or \"}\"") } ; grouping_labels : @@ -228,7 +228,7 @@ grouping_labels : | LEFT_PAREN RIGHT_PAREN { $$ = []string{} } | error - { yylex.(*parser).errorf("unexpected %v in grouping opts, expected \"(\"", yylex.(*parser).token.desc()) } + { yylex.(*parser).unexpected("grouping opts", "\"(\"") } ; @@ -238,19 +238,19 @@ grouping_label_list: | grouping_label { $$ = []string{$1.Val} } | grouping_label_list error - { yylex.(*parser).errorf("unexpected %v in grouping opts, expected \",\" or \"}\"", yylex.(*parser).token.desc()) } + { yylex.(*parser).unexpected("grouping opts", "\",\" or \"}\"") } ; grouping_label : maybe_label { if !isLabel($1.Val) { - yylex.(*parser).errorf("unexpected %s in grouping opts, expected label", $1.desc()) + yylex.(*parser).unexpected("grouping opts", "label") } $$ = $1 } | error - { yylex.(*parser).errorf("unexpected %s in grouping opts, expected label", yylex.(*parser).token.desc()) } + { yylex.(*parser).unexpected("grouping opts", "label") } ; diff --git a/promql/generated_parser.y.go b/promql/generated_parser.y.go index 7aecb2f554..02c36a7ee3 100644 --- a/promql/generated_parser.y.go +++ b/promql/generated_parser.y.go @@ -651,7 +651,7 @@ yydefault: yyDollar = yyS[yypt-1 : yypt+1] //line promql/generated_parser.y:137 { - yylex.(*parser).errorf("unexpected %v", yylex.(*parser).token.desc()) + yylex.(*parser).unexpected("", "") } case 6: yyDollar = yyS[yypt-3 : yypt+1] @@ -687,7 +687,7 @@ yydefault: yyDollar = yyS[yypt-2 : yypt+1] //line promql/generated_parser.y:157 { - yylex.(*parser).errorf("unexpected %v in label matching, expected \",\" or \"}\"", yylex.(*parser).token.desc()) + yylex.(*parser).unexpected("label matching", "\",\" or \"}\"") } case 12: yyDollar = yyS[yypt-3 : yypt+1] @@ -699,19 +699,19 @@ yydefault: yyDollar = yyS[yypt-3 : yypt+1] //line promql/generated_parser.y:164 { - yylex.(*parser).errorf("unexpected %v in label matching, expected string", yylex.(*parser).token.desc()) + yylex.(*parser).unexpected("label matching", "string") } case 14: yyDollar = yyS[yypt-2 : yypt+1] //line promql/generated_parser.y:166 { - yylex.(*parser).errorf("unexpected %v in label matching, expected label matching operator", yylex.(*parser).token.Val) + yylex.(*parser).unexpected("label matching", "label matching operator") } case 15: yyDollar = yyS[yypt-1 : yypt+1] //line promql/generated_parser.y:168 { - yylex.(*parser).errorf("unexpected %v in label matching, expected identifier or \"}\"", yylex.(*parser).token.desc()) + yylex.(*parser).unexpected("label matching", "identifier or \"}\"") } case 16: yyDollar = yyS[yypt-1 : yypt+1] @@ -808,7 +808,7 @@ yydefault: yyDollar = yyS[yypt-2 : yypt+1] //line promql/generated_parser.y:210 { - yylex.(*parser).errorf("unexpected %v in label set, expected \",\" or \"}\"", yylex.(*parser).token.desc()) + yylex.(*parser).unexpected("label set", "\",\" or \"}\"") } case 32: yyDollar = yyS[yypt-3 : yypt+1] @@ -820,19 +820,19 @@ yydefault: yyDollar = yyS[yypt-3 : yypt+1] //line promql/generated_parser.y:218 { - yylex.(*parser).errorf("unexpected %v in label set, expected string", yylex.(*parser).token.desc()) + yylex.(*parser).unexpected("label set", "string") } case 34: yyDollar = yyS[yypt-2 : yypt+1] //line promql/generated_parser.y:220 { - yylex.(*parser).errorf("unexpected %v in label set, expected \"=\"", yylex.(*parser).token.desc()) + yylex.(*parser).unexpected("label set", "\"=\"") } case 35: yyDollar = yyS[yypt-1 : yypt+1] //line promql/generated_parser.y:222 { - yylex.(*parser).errorf("unexpected %v in label set, expected identifier or \"}\"", yylex.(*parser).token.desc()) + yylex.(*parser).unexpected("label set", "identifier or \"}\"") } case 36: yyDollar = yyS[yypt-3 : yypt+1] @@ -850,7 +850,7 @@ yydefault: yyDollar = yyS[yypt-1 : yypt+1] //line promql/generated_parser.y:231 { - yylex.(*parser).errorf("unexpected %v in grouping opts, expected \"(\"", yylex.(*parser).token.desc()) + yylex.(*parser).unexpected("grouping opts", "\"(\"") } case 39: yyDollar = yyS[yypt-3 : yypt+1] @@ -868,14 +868,14 @@ yydefault: yyDollar = yyS[yypt-2 : yypt+1] //line promql/generated_parser.y:241 { - yylex.(*parser).errorf("unexpected %v in grouping opts, expected \",\" or \"}\"", yylex.(*parser).token.desc()) + yylex.(*parser).unexpected("grouping opts", "\",\" or \"}\"") } case 42: yyDollar = yyS[yypt-1 : yypt+1] //line promql/generated_parser.y:246 { if !isLabel(yyDollar[1].item.Val) { - yylex.(*parser).errorf("unexpected %s in grouping opts, expected label", yyDollar[1].item.desc()) + yylex.(*parser).unexpected("grouping opts", "label") } yyVAL.item = yyDollar[1].item } @@ -883,7 +883,7 @@ yydefault: yyDollar = yyS[yypt-1 : yypt+1] //line promql/generated_parser.y:253 { - yylex.(*parser).errorf("unexpected %s in grouping opts, expected label", yylex.(*parser).token.desc()) + yylex.(*parser).unexpected("grouping opts", "label") } } goto yystack /* stack new state and value */ diff --git a/promql/parse.go b/promql/parse.go index 8c71f9d226..d74dba9d8c 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -301,11 +301,33 @@ func (p *parser) error(err error) { panic(perr) } +// unexpected creates a parser error complaining about an unexpected lexer item. +// The item that is presented as unexpected is always the last item produced +// by the lexer. +func (p *parser) unexpected(context string, expected string) { + var errMsg strings.Builder + + errMsg.WriteString("unexpected ") + errMsg.WriteString(p.token.desc()) + + if context != "" { + errMsg.WriteString(" in ") + errMsg.WriteString(context) + } + + if expected != "" { + errMsg.WriteString(", expected ") + errMsg.WriteString(expected) + } + + p.error(errors.New(errMsg.String())) +} + // expect consumes the next token and guarantees it has the required type. func (p *parser) expect(exp ItemType, context string) Item { token := p.next() if token.Typ != exp { - p.errorf("unexpected %s in %s, expected %s", token.desc(), context, exp.desc()) + p.unexpected(context, exp.desc()) } return token } @@ -314,7 +336,8 @@ func (p *parser) expect(exp ItemType, context string) Item { func (p *parser) expectOneOf(exp1, exp2 ItemType, context string) Item { token := p.next() if token.Typ != exp1 && token.Typ != exp2 { - p.errorf("unexpected %s in %s, expected %s or %s", token.desc(), context, exp1.desc(), exp2.desc()) + expected := exp1.desc() + " or " + exp2.desc() + p.unexpected(context, expected) } return token } diff --git a/promql/parse_test.go b/promql/parse_test.go index ff2a356da3..5d021bc00e 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -936,7 +936,7 @@ var testExpr = []struct { }, { input: `foo{gibberish}`, fail: true, - errMsg: "unexpected } in label matching, expected label matching operator", + errMsg: `unexpected "}" in label matching, expected label matching operator`, }, { input: `foo{1}`, fail: true,