From d642570924f3187bcc4b2806fd43512778483abc Mon Sep 17 00:00:00 2001 From: Tobias Guggenmos Date: Wed, 15 Jan 2020 21:01:49 +0100 Subject: [PATCH] PromQL: Use PositionRange in parser errors (#6634) Signed-off-by: Tobias Guggenmos --- promql/generated_parser.y | 20 ++++-- promql/generated_parser.y.go | 126 +++++++++++++++++---------------- promql/lex.go | 17 ----- promql/parse.go | 131 ++++++++++++++++++++++------------- promql/parse_test.go | 14 ++-- promql/test.go | 15 ++-- 6 files changed, 178 insertions(+), 145 deletions(-) diff --git a/promql/generated_parser.y b/promql/generated_parser.y index 923cea19a7..73c4531b62 100644 --- a/promql/generated_parser.y +++ b/promql/generated_parser.y @@ -164,7 +164,7 @@ start : { yylex.(*parser).generatedParserResult = $2 } | START_SERIES_DESCRIPTION series_description | START_EXPRESSION /* empty */ EOF - { yylex.(*parser).errorf("no expression found in input")} + { yylex.(*parser).failf(PositionRange{}, "no expression found in input")} | START_EXPRESSION expr { yylex.(*parser).generatedParserResult = $2 } | START_METRIC_SELECTOR vector_selector @@ -326,7 +326,7 @@ function_call : IDENTIFIER function_call_body { fn, exist := getFunction($1.Val) if !exist{ - yylex.(*parser).errorf("unknown function with name %q", $1.Val) + yylex.(*parser).failf($1.PositionRange(),"unknown function with name %q", $1.Val) } $$ = &Call{ Func: fn, @@ -378,13 +378,19 @@ offset_expr: expr OFFSET duration matrix_selector : expr LEFT_BRACKET duration RIGHT_BRACKET { + var errMsg string vs, ok := $1.(*VectorSelector) if !ok{ - yylex.(*parser).errorf("ranges only allowed for vector selectors") + errMsg = "ranges only allowed for vector selectors" + } else if vs.Offset != 0{ + errMsg = "no offset modifiers allowed before range" } - if vs.Offset != 0{ - yylex.(*parser).errorf("no offset modifiers allowed before range") + + if errMsg != ""{ + errRange := mergeRanges(&$2, &$4) + yylex.(*parser).failf(errRange, errMsg) } + $$ = &MatrixSelector{ VectorSelector: vs, Range: $3, @@ -648,7 +654,7 @@ uint : NUMBER var err error $$, err = strconv.ParseUint($1.Val, 10, 64) if err != nil { - yylex.(*parser).errorf("invalid repetition in series values: %s", err) + yylex.(*parser).failf($1.PositionRange(), "invalid repetition in series values: %s", err) } } ; @@ -658,7 +664,7 @@ duration : DURATION var err error $$, err = parseDuration($1.Val) if err != nil { - yylex.(*parser).error(err) + yylex.(*parser).fail($1.PositionRange(), err) } } ; diff --git a/promql/generated_parser.y.go b/promql/generated_parser.y.go index b568d7d559..99d568b659 100644 --- a/promql/generated_parser.y.go +++ b/promql/generated_parser.y.go @@ -183,7 +183,7 @@ const yyEofCode = 1 const yyErrCode = 2 const yyInitialStackSize = 16 -//line promql/generated_parser.y:689 +//line promql/generated_parser.y:695 //line yacctab:1 var yyExca = [...]int{ @@ -794,7 +794,7 @@ yydefault: yyDollar = yyS[yypt-2 : yypt+1] //line promql/generated_parser.y:167 { - yylex.(*parser).errorf("no expression found in input") + yylex.(*parser).failf(PositionRange{}, "no expression found in input") } case 4: yyDollar = yyS[yypt-2 : yypt+1] @@ -1056,7 +1056,7 @@ yydefault: { fn, exist := getFunction(yyDollar[1].item.Val) if !exist { - yylex.(*parser).errorf("unknown function with name %q", yyDollar[1].item.Val) + yylex.(*parser).failf(yyDollar[1].item.PositionRange(), "unknown function with name %q", yyDollar[1].item.Val) } yyVAL.node = &Call{ Func: fn, @@ -1114,13 +1114,19 @@ yydefault: yyDollar = yyS[yypt-4 : yypt+1] //line promql/generated_parser.y:380 { + var errMsg string vs, ok := yyDollar[1].node.(*VectorSelector) if !ok { - yylex.(*parser).errorf("ranges only allowed for vector selectors") + errMsg = "ranges only allowed for vector selectors" + } else if vs.Offset != 0 { + errMsg = "no offset modifiers allowed before range" } - if vs.Offset != 0 { - yylex.(*parser).errorf("no offset modifiers allowed before range") + + if errMsg != "" { + errRange := mergeRanges(&yyDollar[2].item, &yyDollar[4].item) + yylex.(*parser).failf(errRange, errMsg) } + yyVAL.node = &MatrixSelector{ VectorSelector: vs, Range: yyDollar[3].duration, @@ -1129,7 +1135,7 @@ yydefault: } case 67: yyDollar = yyS[yypt-6 : yypt+1] -//line promql/generated_parser.y:397 +//line promql/generated_parser.y:403 { yyVAL.node = &SubqueryExpr{ Expr: yyDollar[1].node.(Expr), @@ -1141,31 +1147,31 @@ yydefault: } case 68: yyDollar = yyS[yypt-6 : yypt+1] -//line promql/generated_parser.y:407 +//line promql/generated_parser.y:413 { yylex.(*parser).unexpected("subquery selector", "\"]\"") } case 69: yyDollar = yyS[yypt-5 : yypt+1] -//line promql/generated_parser.y:409 +//line promql/generated_parser.y:415 { yylex.(*parser).unexpected("subquery selector", "duration or \"]\"") } case 70: yyDollar = yyS[yypt-4 : yypt+1] -//line promql/generated_parser.y:411 +//line promql/generated_parser.y:417 { yylex.(*parser).unexpected("subquery or range", "\":\" or \"]\"") } case 71: yyDollar = yyS[yypt-3 : yypt+1] -//line promql/generated_parser.y:413 +//line promql/generated_parser.y:419 { yylex.(*parser).unexpected("subquery selector", "duration") } case 72: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:423 +//line promql/generated_parser.y:429 { if nl, ok := yyDollar[2].node.(*NumberLiteral); ok { if yyDollar[1].item.Typ == SUB { @@ -1179,7 +1185,7 @@ yydefault: } case 73: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:441 +//line promql/generated_parser.y:447 { vs := yyDollar[2].node.(*VectorSelector) vs.PosRange = mergeRanges(&yyDollar[1].item, vs) @@ -1189,7 +1195,7 @@ yydefault: } case 74: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:449 +//line promql/generated_parser.y:455 { vs := &VectorSelector{ Name: yyDollar[1].item.Val, @@ -1201,7 +1207,7 @@ yydefault: } case 75: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:459 +//line promql/generated_parser.y:465 { vs := yyDollar[1].node.(*VectorSelector) yylex.(*parser).assembleVectorSelector(vs) @@ -1209,7 +1215,7 @@ yydefault: } case 76: yyDollar = yyS[yypt-3 : yypt+1] -//line promql/generated_parser.y:467 +//line promql/generated_parser.y:473 { yyVAL.node = &VectorSelector{ LabelMatchers: yyDollar[2].matchers, @@ -1218,7 +1224,7 @@ yydefault: } case 77: yyDollar = yyS[yypt-4 : yypt+1] -//line promql/generated_parser.y:474 +//line promql/generated_parser.y:480 { yyVAL.node = &VectorSelector{ LabelMatchers: yyDollar[2].matchers, @@ -1227,7 +1233,7 @@ yydefault: } case 78: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:481 +//line promql/generated_parser.y:487 { yyVAL.node = &VectorSelector{ LabelMatchers: []*labels.Matcher{}, @@ -1236,128 +1242,128 @@ yydefault: } case 79: yyDollar = yyS[yypt-3 : yypt+1] -//line promql/generated_parser.y:490 +//line promql/generated_parser.y:496 { yyVAL.matchers = append(yyDollar[1].matchers, yyDollar[3].matcher) } case 80: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:492 +//line promql/generated_parser.y:498 { yyVAL.matchers = []*labels.Matcher{yyDollar[1].matcher} } case 81: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:494 +//line promql/generated_parser.y:500 { yylex.(*parser).unexpected("label matching", "\",\" or \"}\"") } case 82: yyDollar = yyS[yypt-3 : yypt+1] -//line promql/generated_parser.y:498 +//line promql/generated_parser.y:504 { yyVAL.matcher = yylex.(*parser).newLabelMatcher(yyDollar[1].item, yyDollar[2].item, yyDollar[3].item) } case 83: yyDollar = yyS[yypt-3 : yypt+1] -//line promql/generated_parser.y:500 +//line promql/generated_parser.y:506 { yylex.(*parser).unexpected("label matching", "string") } case 84: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:502 +//line promql/generated_parser.y:508 { yylex.(*parser).unexpected("label matching", "label matching operator") } case 85: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:504 +//line promql/generated_parser.y:510 { yylex.(*parser).unexpected("label matching", "identifier or \"}\"") } case 86: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:512 +//line promql/generated_parser.y:518 { yyVAL.labels = append(yyDollar[2].labels, labels.Label{Name: labels.MetricName, Value: yyDollar[1].item.Val}) sort.Sort(yyVAL.labels) } case 87: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:514 +//line promql/generated_parser.y:520 { yyVAL.labels = yyDollar[1].labels } case 90: yyDollar = yyS[yypt-3 : yypt+1] -//line promql/generated_parser.y:521 +//line promql/generated_parser.y:527 { yyVAL.labels = labels.New(yyDollar[2].labels...) } case 91: yyDollar = yyS[yypt-4 : yypt+1] -//line promql/generated_parser.y:523 +//line promql/generated_parser.y:529 { yyVAL.labels = labels.New(yyDollar[2].labels...) } case 92: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:525 +//line promql/generated_parser.y:531 { yyVAL.labels = labels.New() } case 93: yyDollar = yyS[yypt-0 : yypt+1] -//line promql/generated_parser.y:527 +//line promql/generated_parser.y:533 { yyVAL.labels = labels.New() } case 94: yyDollar = yyS[yypt-3 : yypt+1] -//line promql/generated_parser.y:531 +//line promql/generated_parser.y:537 { yyVAL.labels = append(yyDollar[1].labels, yyDollar[3].label) } case 95: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:533 +//line promql/generated_parser.y:539 { yyVAL.labels = []labels.Label{yyDollar[1].label} } case 96: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:535 +//line promql/generated_parser.y:541 { yylex.(*parser).unexpected("label set", "\",\" or \"}\"") } case 97: yyDollar = yyS[yypt-3 : yypt+1] -//line promql/generated_parser.y:540 +//line promql/generated_parser.y:546 { yyVAL.label = labels.Label{Name: yyDollar[1].item.Val, Value: yylex.(*parser).unquoteString(yyDollar[3].item.Val)} } case 98: yyDollar = yyS[yypt-3 : yypt+1] -//line promql/generated_parser.y:542 +//line promql/generated_parser.y:548 { yylex.(*parser).unexpected("label set", "string") } case 99: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:544 +//line promql/generated_parser.y:550 { yylex.(*parser).unexpected("label set", "\"=\"") } case 100: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:546 +//line promql/generated_parser.y:552 { yylex.(*parser).unexpected("label set", "identifier or \"}\"") } case 101: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:554 +//line promql/generated_parser.y:560 { yylex.(*parser).generatedParserResult = &seriesDescription{ labels: yyDollar[1].labels, @@ -1366,37 +1372,37 @@ yydefault: } case 102: yyDollar = yyS[yypt-0 : yypt+1] -//line promql/generated_parser.y:563 +//line promql/generated_parser.y:569 { yyVAL.series = []sequenceValue{} } case 103: yyDollar = yyS[yypt-3 : yypt+1] -//line promql/generated_parser.y:565 +//line promql/generated_parser.y:571 { yyVAL.series = append(yyDollar[1].series, yyDollar[3].series...) } case 104: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:567 +//line promql/generated_parser.y:573 { yyVAL.series = yyDollar[1].series } case 105: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:569 +//line promql/generated_parser.y:575 { yylex.(*parser).unexpected("series values", "") } case 106: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:573 +//line promql/generated_parser.y:579 { yyVAL.series = []sequenceValue{{omitted: true}} } case 107: yyDollar = yyS[yypt-3 : yypt+1] -//line promql/generated_parser.y:575 +//line promql/generated_parser.y:581 { yyVAL.series = []sequenceValue{} for i := uint64(0); i < yyDollar[3].uint; i++ { @@ -1405,13 +1411,13 @@ yydefault: } case 108: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:582 +//line promql/generated_parser.y:588 { yyVAL.series = []sequenceValue{{value: yyDollar[1].float}} } case 109: yyDollar = yyS[yypt-3 : yypt+1] -//line promql/generated_parser.y:584 +//line promql/generated_parser.y:590 { yyVAL.series = []sequenceValue{} for i := uint64(0); i <= yyDollar[3].uint; i++ { @@ -1420,7 +1426,7 @@ yydefault: } case 110: yyDollar = yyS[yypt-4 : yypt+1] -//line promql/generated_parser.y:591 +//line promql/generated_parser.y:597 { yyVAL.series = []sequenceValue{} for i := uint64(0); i <= yyDollar[4].uint; i++ { @@ -1430,7 +1436,7 @@ yydefault: } case 111: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:601 +//line promql/generated_parser.y:607 { if yyDollar[1].item.Val != "stale" { yylex.(*parser).unexpected("series values", "number or \"stale\"") @@ -1439,7 +1445,7 @@ yydefault: } case 154: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:632 +//line promql/generated_parser.y:638 { yyVAL.node = &NumberLiteral{ Val: yylex.(*parser).number(yyDollar[1].item.Val), @@ -1448,45 +1454,45 @@ yydefault: } case 155: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:640 +//line promql/generated_parser.y:646 { yyVAL.float = yylex.(*parser).number(yyDollar[1].item.Val) } case 156: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:642 +//line promql/generated_parser.y:648 { yyVAL.float = yyDollar[2].float } case 157: yyDollar = yyS[yypt-2 : yypt+1] -//line promql/generated_parser.y:643 +//line promql/generated_parser.y:649 { yyVAL.float = -yyDollar[2].float } case 158: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:647 +//line promql/generated_parser.y:653 { var err error yyVAL.uint, err = strconv.ParseUint(yyDollar[1].item.Val, 10, 64) if err != nil { - yylex.(*parser).errorf("invalid repetition in series values: %s", err) + yylex.(*parser).failf(yyDollar[1].item.PositionRange(), "invalid repetition in series values: %s", err) } } case 159: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:657 +//line promql/generated_parser.y:663 { var err error yyVAL.duration, err = parseDuration(yyDollar[1].item.Val) if err != nil { - yylex.(*parser).error(err) + yylex.(*parser).fail(yyDollar[1].item.PositionRange(), err) } } case 160: yyDollar = yyS[yypt-1 : yypt+1] -//line promql/generated_parser.y:668 +//line promql/generated_parser.y:674 { yyVAL.node = &StringLiteral{ Val: yylex.(*parser).unquoteString(yyDollar[1].item.Val), @@ -1495,13 +1501,13 @@ yydefault: } case 161: yyDollar = yyS[yypt-0 : yypt+1] -//line promql/generated_parser.y:681 +//line promql/generated_parser.y:687 { yyVAL.duration = 0 } case 163: yyDollar = yyS[yypt-0 : yypt+1] -//line promql/generated_parser.y:685 +//line promql/generated_parser.y:691 { yyVAL.strings = nil } diff --git a/promql/lex.go b/promql/lex.go index 27cece6ec8..f8b79a22f6 100644 --- a/promql/lex.go +++ b/promql/lex.go @@ -291,23 +291,6 @@ func (l *Lexer) acceptRun(valid string) { l.backup() } -// lineNumber reports which line we're on, based on the position of -// the previous Item returned by NextItem. Doing it this way -// means we don't have to worry about peek double counting. -func (l *Lexer) lineNumber() int { - return 1 + strings.Count(l.input[:l.lastPos], "\n") -} - -// linePosition reports at which character in the current line -// we are on. -func (l *Lexer) linePosition() int { - lb := strings.LastIndex(l.input[:l.lastPos], "\n") - if lb == -1 { - return 1 + int(l.lastPos) - } - return 1 + int(l.lastPos) - lb -} - // errorf returns an error token and terminates the scan by passing // back a nil pointer that will be the next state, terminating l.NextItem. func (l *Lexer) errorf(format string, args ...interface{}) stateFn { diff --git a/promql/parse.go b/promql/parse.go index 2b5f64510d..2a6cd29d74 100644 --- a/promql/parse.go +++ b/promql/parse.go @@ -54,12 +54,36 @@ type parser struct { // 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 + PositionRange PositionRange + Err error + Query string + + // An additional line offset to be added. Only used inside unit tests. + lineOffset int } func (e *ParseErr) Error() string { - return fmt.Sprintf("%d:%d: parse error: %s", e.Line+1, e.Pos, e.Err) + pos := int(e.PositionRange.Start) + lastLineBreak := -1 + line := e.lineOffset + 1 + + var positionStr string + + if pos < 0 || pos > len(e.Query) { + positionStr = "invalid position:" + } else { + + for i, c := range e.Query[:e.PositionRange.Start] { + if c == '\n' { + lastLineBreak = i + line++ + } + } + + col := pos - lastLineBreak + positionStr = fmt.Sprintf("%d:%d:", line, col) + } + return fmt.Sprintf("%s parse error: %s", positionStr, e.Err) } // ParseExpr returns the expression parsed from the input. @@ -150,20 +174,17 @@ func (p *parser) typecheck(node Node) (err error) { return nil } -// errorf formats the error and terminates processing. -func (p *parser) errorf(format string, args ...interface{}) { - p.error(errors.Errorf(format, args...)) +// failf formats the error and terminates processing. +func (p *parser) failf(positionRange PositionRange, format string, args ...interface{}) { + p.fail(positionRange, errors.Errorf(format, args...)) } -// error terminates processing. -func (p *parser) error(err error) { +// fail terminates processing. +func (p *parser) fail(positionRange PositionRange, err error) { 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 + PositionRange: positionRange, + Err: err, + Query: p.lex.input, } panic(perr) } @@ -187,7 +208,7 @@ func (p *parser) unexpected(context string, expected string) { errMsg.WriteString(expected) } - p.error(errors.New(errMsg.String())) + p.fail(p.yyParser.lval.item.PositionRange(), errors.New(errMsg.String())) } var errUnexpected = errors.New("unexpected error") @@ -236,7 +257,7 @@ func (p *parser) Lex(lval *yySymType) int { switch typ { case ERROR: - p.errorf("%s", lval.item.Val) + p.failf(lval.item.PositionRange(), "%s", lval.item.Val) case EOF: lval.item.Typ = EOF p.InjectItem(0) @@ -281,12 +302,23 @@ func (p *parser) newBinaryExpression(lhs Node, op Item, modifiers Node, rhs Node ret.RHS = rhs.(Expr) ret.Op = op.Typ + // opRange returns the PositionRange of the operator part of the BinaryExpr. + // This is made a function instead of a variable, so it is lazily evaluated on demand. + opRange := func() (r PositionRange) { + // Remove whitespace at the beginning and end of the range. + for r.Start = lhs.PositionRange().End; isSpace(rune(p.lex.input[r.Start])); r.Start++ { + } + for r.End = rhs.PositionRange().Start - 1; isSpace(rune(p.lex.input[r.End])); r.End-- { + } + return + } + if ret.ReturnBool && !op.Typ.isComparisonOperator() { - p.errorf("bool modifier can only be used on comparison operators") + p.failf(opRange(), "bool modifier can only be used on comparison operators") } if op.Typ.isComparisonOperator() && !ret.ReturnBool && ret.RHS.Type() == ValueTypeScalar && ret.LHS.Type() == ValueTypeScalar { - p.errorf("comparisons between scalars must use BOOL modifier") + p.failf(opRange(), "comparisons between scalars must use BOOL modifier") } if op.Typ.isSetOperator() && ret.VectorMatching.Card == CardOneToOne { @@ -296,7 +328,7 @@ func (p *parser) newBinaryExpression(lhs Node, op Item, modifiers Node, rhs Node for _, l1 := range ret.VectorMatching.MatchingLabels { for _, l2 := range ret.VectorMatching.Include { if l1 == l2 && ret.VectorMatching.On { - p.errorf("label %q must not occur in ON and GROUP clause at once", l1) + p.failf(opRange(), "label %q must not occur in ON and GROUP clause at once", l1) } } } @@ -309,7 +341,7 @@ func (p *parser) assembleVectorSelector(vs *VectorSelector) { for _, m := range vs.LabelMatchers { if m.Name == labels.MetricName { - p.errorf("metric name must not be set twice: %q or %q", vs.Name, m.Value) + p.failf(vs.PositionRange(), "metric name must not be set twice: %q or %q", vs.Name, m.Value) } } @@ -330,7 +362,7 @@ func (p *parser) assembleVectorSelector(vs *VectorSelector) { } } if !notEmpty { - p.errorf("vector selector must contain at least one non-empty matcher") + p.failf(vs.PositionRange(), "vector selector must contain at least one non-empty matcher") } } @@ -341,9 +373,9 @@ func (p *parser) newAggregateExpr(op Item, modifier Node, args Node) (ret *Aggre ret.Op = op.Typ if len(arguments) == 0 { - p.errorf("no arguments for aggregate expression provided") + p.failf(ret.PositionRange(), "no arguments for aggregate expression provided") - // Currently p.errorf() panics, so this return is not needed + // Currently p.failf() panics, so this return is not needed // at the moment. // However, this behaviour is likely to be changed in the // future. In case of having non-panicking errors this @@ -359,7 +391,7 @@ func (p *parser) newAggregateExpr(op Item, modifier Node, args Node) (ret *Aggre } if len(arguments) != desiredArgs { - p.errorf("wrong number of arguments for aggregate expression provided, expected %d, got %d", desiredArgs, len(arguments)) + p.failf(ret.PositionRange(), "wrong number of arguments for aggregate expression provided, expected %d, got %d", desiredArgs, len(arguments)) return } @@ -381,7 +413,7 @@ func (p *parser) number(val string) float64 { f, err = strconv.ParseFloat(val, 64) } if err != nil { - p.errorf("error parsing number: %s", err) + p.failf(p.yyParser.lval.item.PositionRange(), "error parsing number: %s", err) } return f } @@ -391,7 +423,7 @@ func (p *parser) number(val string) float64 { func (p *parser) expectType(node Node, want ValueType, context string) { t := p.checkType(node) if t != want { - p.errorf("expected type %s in %s, got %s", documentedType(want), context, documentedType(t)) + p.failf(node.PositionRange(), "expected type %s in %s, got %s", documentedType(want), context, documentedType(t)) } } @@ -409,7 +441,7 @@ func (p *parser) checkType(node Node) (typ ValueType) { case Expr: typ = n.Type() default: - p.errorf("unknown node type: %T", node) + p.failf(node.PositionRange(), "unknown node type: %T", node) } // Recursively check correct typing for child nodes and raise @@ -418,19 +450,19 @@ func (p *parser) checkType(node Node) (typ ValueType) { case *EvalStmt: ty := p.checkType(n.Expr) if ty == ValueTypeNone { - p.errorf("evaluation statement must have a valid expression type but got %s", documentedType(ty)) + p.failf(n.Expr.PositionRange(), "evaluation statement must have a valid expression type but got %s", documentedType(ty)) } case Expressions: for _, e := range n { ty := p.checkType(e) if ty == ValueTypeNone { - p.errorf("expression must have a valid expression type but got %s", documentedType(ty)) + p.failf(e.PositionRange(), "expression must have a valid expression type but got %s", documentedType(ty)) } } case *AggregateExpr: if !n.Op.isAggregator() { - p.errorf("aggregation operator expected in aggregation expression but got %q", n.Op) + p.failf(n.PositionRange(), "aggregation operator expected in aggregation expression but got %q", n.Op) } p.expectType(n.Expr, ValueTypeVector, "aggregation expression") if n.Op == TOPK || n.Op == BOTTOMK || n.Op == QUANTILE { @@ -445,45 +477,48 @@ func (p *parser) checkType(node Node) (typ ValueType) { rt := p.checkType(n.RHS) if !n.Op.isOperator() { - p.errorf("binary expression does not support operator %q", n.Op) + p.failf(n.PositionRange(), "binary expression does not support operator %q", n.Op) } - if (lt != ValueTypeScalar && lt != ValueTypeVector) || (rt != ValueTypeScalar && rt != ValueTypeVector) { - p.errorf("binary expression must contain only scalar and instant vector types") + if lt != ValueTypeScalar && lt != ValueTypeVector { + p.failf(n.LHS.PositionRange(), "binary expression must contain only scalar and instant vector types") + } + if rt != ValueTypeScalar && rt != ValueTypeVector { + p.failf(n.RHS.PositionRange(), "binary expression must contain only scalar and instant vector types") } if (lt != ValueTypeVector || rt != ValueTypeVector) && n.VectorMatching != nil { if len(n.VectorMatching.MatchingLabels) > 0 { - p.errorf("vector matching only allowed between instant vectors") + p.failf(n.PositionRange(), "vector matching only allowed between instant vectors") } n.VectorMatching = nil } else { // Both operands are Vectors. if n.Op.isSetOperator() { if n.VectorMatching.Card == CardOneToMany || n.VectorMatching.Card == CardManyToOne { - p.errorf("no grouping allowed for %q operation", n.Op) + p.failf(n.PositionRange(), "no grouping allowed for %q operation", n.Op) } if n.VectorMatching.Card != CardManyToMany { - p.errorf("set operations must always be many-to-many") + p.failf(n.PositionRange(), "set operations must always be many-to-many") } } } if (lt == ValueTypeScalar || rt == ValueTypeScalar) && n.Op.isSetOperator() { - p.errorf("set operator %q not allowed in binary scalar expression", n.Op) + p.failf(n.PositionRange(), "set operator %q not allowed in binary scalar expression", n.Op) } case *Call: nargs := len(n.Func.ArgTypes) if n.Func.Variadic == 0 { if nargs != len(n.Args) { - p.errorf("expected %d argument(s) in call to %q, got %d", nargs, n.Func.Name, len(n.Args)) + p.failf(n.PositionRange(), "expected %d argument(s) in call to %q, got %d", nargs, n.Func.Name, len(n.Args)) } } else { na := nargs - 1 if na > len(n.Args) { - p.errorf("expected at least %d argument(s) in call to %q, got %d", na, n.Func.Name, len(n.Args)) + p.failf(n.PositionRange(), "expected at least %d argument(s) in call to %q, got %d", na, n.Func.Name, len(n.Args)) } else if nargsmax := na + n.Func.Variadic; n.Func.Variadic > 0 && nargsmax < len(n.Args) { - p.errorf("expected at most %d argument(s) in call to %q, got %d", nargsmax, n.Func.Name, len(n.Args)) + p.failf(n.PositionRange(), "expected at most %d argument(s) in call to %q, got %d", nargsmax, n.Func.Name, len(n.Args)) } } @@ -499,16 +534,16 @@ func (p *parser) checkType(node Node) (typ ValueType) { case *UnaryExpr: if n.Op != ADD && n.Op != SUB { - p.errorf("only + and - operators allowed for unary expressions") + p.failf(n.PositionRange(), "only + and - operators allowed for unary expressions") } if t := p.checkType(n.Expr); t != ValueTypeScalar && t != ValueTypeVector { - p.errorf("unary expression only allowed on expressions of type scalar or instant vector, got %q", documentedType(t)) + p.failf(n.PositionRange(), "unary expression only allowed on expressions of type scalar or instant vector, got %q", documentedType(t)) } case *SubqueryExpr: ty := p.checkType(n.Expr) if ty != ValueTypeVector { - p.errorf("subquery is only allowed on instant vector, got %s in %q instead", ty, n.String()) + p.failf(n.PositionRange(), "subquery is only allowed on instant vector, got %s in %q instead", ty, n.String()) } case *MatrixSelector: p.checkType(n.VectorSelector) @@ -517,7 +552,7 @@ func (p *parser) checkType(node Node) (typ ValueType) { // Nothing to do for terminals. default: - p.errorf("unknown node type: %T", node) + p.failf(n.PositionRange(), "unknown node type: %T", node) } return } @@ -525,7 +560,7 @@ func (p *parser) checkType(node Node) (typ ValueType) { func (p *parser) unquoteString(s string) string { unquoted, err := strutil.Unquote(s) if err != nil { - p.errorf("error unquoting string %q: %s", s, err) + p.failf(p.yyParser.lval.item.PositionRange(), "error unquoting string %q: %s", s, err) } return unquoted } @@ -576,7 +611,7 @@ func (p *parser) newLabelMatcher(label Item, operator Item, value Item) *labels. m, err := labels.NewMatcher(matchType, label.Val, val) if err != nil { - p.error(err) + p.fail(mergeRanges(&label, &value), err) } return m @@ -597,13 +632,13 @@ func (p *parser) addOffset(e Node, offset time.Duration) { offsetp = &s.Offset endPosp = &s.EndPos default: - p.errorf("offset modifier must be preceded by an instant or range selector, but follows a %T instead", e) + p.failf(e.PositionRange(), "offset modifier must be preceded by an instant or range selector, but follows a %T instead", e) return } // it is already ensured by parseDuration func that there never will be a zero offset modifier if *offsetp != 0 { - p.errorf("offset may not be set multiple times") + p.failf(e.PositionRange(), "offset may not be set multiple times") } else { *offsetp = offset } diff --git a/promql/parse_test.go b/promql/parse_test.go index 10cd2258a4..ffeaff210b 100644 --- a/promql/parse_test.go +++ b/promql/parse_test.go @@ -495,7 +495,7 @@ var testExpr = []struct { }, { input: "1 == 1", fail: true, - errMsg: "1:7: parse error: comparisons between scalars must use BOOL modifier", + errMsg: "1:3: parse error: comparisons between scalars must use BOOL modifier", }, { input: "1 or 1", fail: true, @@ -1633,11 +1633,11 @@ var testExpr = []struct { }, { input: `some_metric OFFSET 1m[5m]`, fail: true, - errMsg: "1:25: parse error: no offset modifiers allowed before range", + errMsg: "1:22: parse error: no offset modifiers allowed before range", }, { input: `(foo + bar)[5m]`, fail: true, - errMsg: "1:15: parse error: ranges only allowed for vector selectors", + errMsg: "1:12: parse error: ranges only allowed for vector selectors", }, // Test aggregation. { @@ -1966,11 +1966,11 @@ var testExpr = []struct { }, { input: `topk(some_metric, other_metric)`, fail: true, - errMsg: "1:32: parse error: expected type scalar in aggregation parameter, got instant vector", + errMsg: "1:6: parse error: expected type scalar in aggregation parameter, got instant vector", }, { input: `count_values(5, other_metric)`, fail: true, - errMsg: "1:30: parse error: expected type string in aggregation parameter, got scalar", + errMsg: "1:14: parse error: expected type string in aggregation parameter, got scalar", }, // Test function calls. { @@ -2463,11 +2463,11 @@ var testExpr = []struct { }, { input: "test[5d] OFFSET 10s [10m:5s]", fail: true, - errMsg: "1:29: parse error: subquery is only allowed on instant vector, got matrix in \"test[5d] offset 10s[10m:5s]\"", + errMsg: "1:1: parse error: subquery is only allowed on instant vector, got matrix in \"test[5d] offset 10s[10m:5s]\"", }, { input: `(foo + bar{nm="val"})[5m:][10m:5s]`, fail: true, - errMsg: `1:35: parse error: subquery is only allowed on instant vector, got matrix in "(foo + bar{nm=\"val\"})[5m:][10m:5s]" instead`, + errMsg: `1:1: parse error: subquery is only allowed on instant vector, got matrix in "(foo + bar{nm=\"val\"})[5m:][10m:5s]" instead`, }, } diff --git a/promql/test.go b/promql/test.go index d2c3486149..7c8c688e55 100644 --- a/promql/test.go +++ b/promql/test.go @@ -102,8 +102,8 @@ func (t *Test) Storage() storage.Storage { func raise(line int, format string, v ...interface{}) error { return &ParseErr{ - Line: line + 1, - Err: errors.Errorf(format, v...), + lineOffset: line, + Err: errors.Errorf(format, v...), } } @@ -128,7 +128,7 @@ func parseLoad(lines []string, i int) (int, *loadCmd, error) { metric, vals, err := parseSeriesDesc(defLine) if err != nil { if perr, ok := err.(*ParseErr); ok { - perr.Line = i + 1 + perr.lineOffset = i } return i, nil, err } @@ -150,8 +150,11 @@ func (t *Test) parseEval(lines []string, i int) (int, *evalCmd, error) { _, err := ParseExpr(expr) if err != nil { if perr, ok := err.(*ParseErr); ok { - perr.Line = i + 1 - perr.Pos += strings.Index(lines[i], expr) + perr.lineOffset = i + posOffset := Pos(strings.Index(lines[i], expr)) + perr.PositionRange.Start += posOffset + perr.PositionRange.End += posOffset + perr.Query = lines[i] } return i, nil, err } @@ -184,7 +187,7 @@ func (t *Test) parseEval(lines []string, i int) (int, *evalCmd, error) { metric, vals, err := parseSeriesDesc(defLine) if err != nil { if perr, ok := err.(*ParseErr); ok { - perr.Line = i + 1 + perr.lineOffset = i } return i, nil, err }