From 29b62762db73940dfeebe2f346335497667081df Mon Sep 17 00:00:00 2001 From: Augustin Husson Date: Wed, 31 Jul 2024 15:31:42 +0200 Subject: [PATCH] adapt the lezer grammar and codemirror autocompletion with duration and number that are equivalent (#14417) Signed-off-by: Augustin Husson --- .../src/complete/hybrid.test.ts | 2 +- .../codemirror-promql/src/complete/hybrid.ts | 24 ++++--- .../codemirror-promql/src/parser/type.ts | 4 +- web/ui/module/lezer-promql/src/highlight.js | 4 +- web/ui/module/lezer-promql/src/promql.grammar | 20 +++--- .../module/lezer-promql/test/expression.txt | 70 +++++++++---------- 6 files changed, 67 insertions(+), 57 deletions(-) diff --git a/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts b/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts index 7b20bfce3..02d2e99f5 100644 --- a/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts +++ b/web/ui/module/codemirror-promql/src/complete/hybrid.test.ts @@ -775,7 +775,7 @@ describe('computeStartCompletePosition test', () => { it(value.title, () => { const state = createEditorState(value.expr); const node = syntaxTree(state).resolve(value.pos, -1); - const result = computeStartCompletePosition(node, value.pos); + const result = computeStartCompletePosition(state, node, value.pos); expect(value.expectedStart).toEqual(result); }); }); diff --git a/web/ui/module/codemirror-promql/src/complete/hybrid.ts b/web/ui/module/codemirror-promql/src/complete/hybrid.ts index 46748d5dc..28fef816d 100644 --- a/web/ui/module/codemirror-promql/src/complete/hybrid.ts +++ b/web/ui/module/codemirror-promql/src/complete/hybrid.ts @@ -21,7 +21,6 @@ import { BinaryExpr, BoolModifier, Div, - Duration, Eql, EqlRegex, EqlSingle, @@ -40,7 +39,6 @@ import { Mul, Neq, NeqRegex, - NumberLiteral, OffsetExpr, Or, Pow, @@ -54,6 +52,8 @@ import { UnquotedLabelMatcher, QuotedLabelMatcher, QuotedLabelName, + NumberDurationLiteralInDurationContext, + NumberDurationLiteral, } from '@prometheus-io/lezer-promql'; import { Completion, CompletionContext, CompletionResult } from '@codemirror/autocomplete'; import { EditorState } from '@codemirror/state'; @@ -179,7 +179,8 @@ function computeStartCompleteLabelPositionInLabelMatcherOrInGroupingLabel(node: // It is an important step because the start position will be used by CMN to find the string and then to use it to filter the CompletionResult. // A wrong `start` position will lead to have the completion not working. // Note: this method is exported only for testing purpose. -export function computeStartCompletePosition(node: SyntaxNode, pos: number): number { +export function computeStartCompletePosition(state: EditorState, node: SyntaxNode, pos: number): number { + const currentText = state.doc.slice(node.from, pos).toString(); let start = node.from; if (node.type.id === LabelMatchers || node.type.id === GroupingLabels) { start = computeStartCompleteLabelPositionInLabelMatcherOrInGroupingLabel(node, pos); @@ -191,11 +192,16 @@ export function computeStartCompletePosition(node: SyntaxNode, pos: number): num start++; } else if ( node.type.id === OffsetExpr || - (node.type.id === NumberLiteral && node.parent?.type.id === 0 && node.parent.parent?.type.id === SubqueryExpr) || + // Since duration and number are equivalent, writing go[5] or go[5d] is syntactically accurate. + // Before we were able to guess when we had to autocomplete the duration later based on the error node, + // which is not possible anymore. + // So we have to analyze the string about the current node to see if the duration unit is already present or not. + (node.type.id === NumberDurationLiteralInDurationContext && !durationTerms.map((v) => v.label).includes(currentText[currentText.length - 1])) || + (node.type.id === NumberDurationLiteral && node.parent?.type.id === 0 && node.parent.parent?.type.id === SubqueryExpr) || (node.type.id === 0 && (node.parent?.type.id === OffsetExpr || node.parent?.type.id === MatrixSelector || - (node.parent?.type.id === SubqueryExpr && containsAtLeastOneChild(node.parent, Duration)))) + (node.parent?.type.id === SubqueryExpr && containsAtLeastOneChild(node.parent, NumberDurationLiteralInDurationContext)))) ) { start = pos; } @@ -230,7 +236,7 @@ export function analyzeCompletion(state: EditorState, node: SyntaxNode): Context result.push({ kind: ContextKind.Duration }); break; } - if (node.parent?.type.id === SubqueryExpr && containsAtLeastOneChild(node.parent, Duration)) { + if (node.parent?.type.id === SubqueryExpr && containsAtLeastOneChild(node.parent, NumberDurationLiteralInDurationContext)) { // we are likely in the given situation: // `rate(foo[5d:5])` // so we should autocomplete a duration @@ -434,7 +440,7 @@ export function analyzeCompletion(state: EditorState, node: SyntaxNode): Context result.push({ kind: ContextKind.MetricName, metricName: state.sliceDoc(node.from, node.to).slice(1, -1) }); } break; - case NumberLiteral: + case NumberDurationLiteral: if (node.parent?.type.id === 0 && node.parent.parent?.type.id === SubqueryExpr) { // Here we are likely in this situation: // `go[5d:4]` @@ -449,7 +455,7 @@ export function analyzeCompletion(state: EditorState, node: SyntaxNode): Context result.push({ kind: ContextKind.Number }); } break; - case Duration: + case NumberDurationLiteralInDurationContext: case OffsetExpr: result.push({ kind: ContextKind.Duration }); break; @@ -591,7 +597,7 @@ export class HybridComplete implements CompleteStrategy { } } return asyncResult.then((result) => { - return arrayToCompletionResult(result, computeStartCompletePosition(tree, pos), pos, completeSnippet, span); + return arrayToCompletionResult(result, computeStartCompletePosition(state, tree, pos), pos, completeSnippet, span); }); } diff --git a/web/ui/module/codemirror-promql/src/parser/type.ts b/web/ui/module/codemirror-promql/src/parser/type.ts index 4a4276da1..ca712c54c 100644 --- a/web/ui/module/codemirror-promql/src/parser/type.ts +++ b/web/ui/module/codemirror-promql/src/parser/type.ts @@ -17,7 +17,7 @@ import { BinaryExpr, FunctionCall, MatrixSelector, - NumberLiteral, + NumberDurationLiteral, OffsetExpr, ParenExpr, StepInvariantExpr, @@ -42,7 +42,7 @@ export function getType(node: SyntaxNode | null): ValueType { return getType(node.firstChild); case StringLiteral: return ValueType.string; - case NumberLiteral: + case NumberDurationLiteral: return ValueType.scalar; case MatrixSelector: return ValueType.matrix; diff --git a/web/ui/module/lezer-promql/src/highlight.js b/web/ui/module/lezer-promql/src/highlight.js index b8bdab76d..92f27c847 100644 --- a/web/ui/module/lezer-promql/src/highlight.js +++ b/web/ui/module/lezer-promql/src/highlight.js @@ -17,8 +17,8 @@ export const promQLHighLight = styleTags({ LineComment: tags.comment, LabelName: tags.labelName, StringLiteral: tags.string, - NumberLiteral: tags.number, - Duration: tags.number, + NumberDurationLiteral: tags.number, + NumberDurationLiteralInDurationContext: tags.number, Identifier: tags.variableName, 'Abs Absent AbsentOverTime Acos Acosh Asin Asinh Atan Atanh AvgOverTime Ceil Changes Clamp ClampMax ClampMin Cos Cosh CountOverTime DaysInMonth DayOfMonth DayOfWeek DayOfYear Deg Delta Deriv Exp Floor HistogramAvg HistogramCount HistogramFraction HistogramQuantile HistogramSum HoltWinters Hour Idelta Increase Irate LabelReplace LabelJoin LastOverTime Ln Log10 Log2 MaxOverTime MinOverTime Minute Month Pi PredictLinear PresentOverTime QuantileOverTime Rad Rate Resets Round Scalar Sgn Sin Sinh Sort SortDesc SortByLabel SortByLabelDesc Sqrt StddevOverTime StdvarOverTime SumOverTime Tan Tanh Time Timestamp Vector Year': tags.function(tags.variableName), diff --git a/web/ui/module/lezer-promql/src/promql.grammar b/web/ui/module/lezer-promql/src/promql.grammar index 735fede24..95c09d25a 100644 --- a/web/ui/module/lezer-promql/src/promql.grammar +++ b/web/ui/module/lezer-promql/src/promql.grammar @@ -29,7 +29,7 @@ expr[@isGroup=Expr] { BinaryExpr | FunctionCall | MatrixSelector | - NumberLiteral | + NumberDurationLiteral | OffsetExpr | ParenExpr | StringLiteral | @@ -194,16 +194,16 @@ ParenExpr { } OffsetExpr { - expr Offset Sub? Duration + expr Offset NumberDurationLiteralInDurationContext } MatrixSelector { // TODO: Can this not be more specific than "expr"? - expr "[" Duration "]" + expr "[" NumberDurationLiteralInDurationContext "]" } SubqueryExpr { - expr "[" Duration ":" ("" | Duration) "]" + expr "[" NumberDurationLiteralInDurationContext ":" ("" | NumberDurationLiteralInDurationContext) "]" } UnaryExpr { @@ -245,14 +245,18 @@ QuotedLabelName { } StepInvariantExpr { - expr At ( NumberLiteral | AtModifierPreprocessors "(" ")" ) + expr At ( NumberDurationLiteral | AtModifierPreprocessors "(" ")" ) } AtModifierPreprocessors { Start | End } -NumberLiteral { +NumberDurationLiteral { + ("-"|"+")?~signed (number | inf | nan) +} + +NumberDurationLiteralInDurationContext { ("-"|"+")?~signed (number | inf | nan) } @@ -264,7 +268,7 @@ NumberLiteral { number { (std.digit+ (("_")? std.digit)* ("." std.digit+ (("_")? std.digit)*)? | "." std.digit+ (("_")? std.digit)*) (("e" | "E") ("+" | "-")? std.digit+ (("_")? std.digit)*)? | - "0x" (std.digit | $[a-fA-F])+ + "0x" (std.digit | $[a-fA-F])+ | duration } StringLiteral { // TODO: This is for JS, make this work for PromQL. '"' (![\\\n"] | "\\" _)* '"'? | @@ -272,7 +276,7 @@ NumberLiteral { "`" ![`]* "`" } - Duration { + duration { // Each line below is just the same regex repeated over and over, but each time with one of the units made non-optional, // to ensure that at least one + pair is provided and an empty string is not recognized as a valid duration. ( ( std.digit+ "y" ) ( std.digit+ "w" )? ( std.digit+ "d" )? ( std.digit+ "h" )? ( std.digit+ "m" )? ( std.digit+ "s" )? ( std.digit+ "ms" )? ) | diff --git a/web/ui/module/lezer-promql/test/expression.txt b/web/ui/module/lezer-promql/test/expression.txt index daba7d800..fda8a3ba5 100644 --- a/web/ui/module/lezer-promql/test/expression.txt +++ b/web/ui/module/lezer-promql/test/expression.txt @@ -4,7 +4,7 @@ ==> -PromQL(NumberLiteral) +PromQL(NumberDurationLiteral) # Double-quoted string literal @@ -46,7 +46,7 @@ PromQL(StringLiteral) ==> -PromQL(BinaryExpr(NumberLiteral, Add, NumberLiteral)) +PromQL(BinaryExpr(NumberDurationLiteral, Add, NumberDurationLiteral)) # Complex expression @@ -73,7 +73,7 @@ PromQL( VectorSelector( Identifier ), - Duration + NumberDurationLiteralInDurationContext ) ) ) @@ -103,7 +103,7 @@ PromQL( VectorSelector( Identifier ), - Duration + NumberDurationLiteralInDurationContext ) ) ) @@ -240,21 +240,21 @@ PromQL( ) ) -# Duration units +# NumberDurationLiteralInDurationContext units foo[1y2w3d4h5m6s7ms] ==> -PromQL(MatrixSelector(VectorSelector(Identifier),Duration)) +PromQL(MatrixSelector(VectorSelector(Identifier),NumberDurationLiteralInDurationContext)) -# Incorrectly ordered duration units +# Incorrectly ordered NumberDurationLiteralInDurationContext units foo[1m2h] ==> -PromQL(SubqueryExpr(VectorSelector(Identifier),Duration,⚠,Duration)) +PromQL(MatrixSelector(VectorSelector(Identifier),NumberDurationLiteralInDurationContext,⚠)) # Using a function name as a metric name @@ -311,7 +311,7 @@ PromQL( ), Gtr, BoolModifier(Bool), - NumberLiteral + NumberDurationLiteral ) ) @@ -357,7 +357,7 @@ PromQL( VectorSelector( Identifier ), - Duration + NumberDurationLiteralInDurationContext ) ) ) @@ -389,8 +389,8 @@ PromQL( FunctionIdentifier(Clamp), FunctionCallBody( VectorSelector(Identifier), - NumberLiteral, - NumberLiteral + NumberDurationLiteral, + NumberDurationLiteral ) ) ) @@ -450,7 +450,7 @@ PromQL( Identifier ), At, - NumberLiteral + NumberDurationLiteral ) ) @@ -483,7 +483,7 @@ PromQL( FunctionCallBody( MatrixSelector( VectorSelector(Identifier), - Duration + NumberDurationLiteralInDurationContext ) ) ), @@ -491,14 +491,14 @@ PromQL( AggregateExpr( AggregateOp(Topk), FunctionCallBody( - NumberLiteral, + NumberDurationLiteral, FunctionCall( FunctionIdentifier(Rate), FunctionCallBody( StepInvariantExpr( - MatrixSelector(VectorSelector(Identifier), Duration), + MatrixSelector(VectorSelector(Identifier), NumberDurationLiteralInDurationContext), At, - NumberLiteral + NumberDurationLiteral ) ) ) @@ -518,7 +518,7 @@ PromQL( Identifier ), At, - NumberLiteral + NumberDurationLiteral ) ) @@ -533,7 +533,7 @@ PromQL( Identifier ), At, - NumberLiteral + NumberDurationLiteral ) ) @@ -556,98 +556,98 @@ PromQL(VectorSelector(Identifier)) NaN ==> -PromQL(NumberLiteral) +PromQL(NumberDurationLiteral) # Lower-cased NaN. nan ==> -PromQL(NumberLiteral) +PromQL(NumberDurationLiteral) # Inf. Inf ==> -PromQL(NumberLiteral) +PromQL(NumberDurationLiteral) # Negative Inf. -Inf ==> -PromQL(NumberLiteral) +PromQL(NumberDurationLiteral) # Positive Inf. +Inf ==> -PromQL(NumberLiteral) +PromQL(NumberDurationLiteral) # Lower-cased Inf. inf ==> -PromQL(NumberLiteral) +PromQL(NumberDurationLiteral) # Upper-cased Inf. INF ==> -PromQL(NumberLiteral) +PromQL(NumberDurationLiteral) # Negative number literal. -42 ==> -PromQL(NumberLiteral) +PromQL(NumberDurationLiteral) # Explicitly positive number literal. +42 ==> -PromQL(NumberLiteral) +PromQL(NumberDurationLiteral) # Trying to illegally use NaN as a metric name. NaN{foo="bar"} ==> -PromQL(BinaryExpr(NumberLiteral,⚠,VectorSelector(LabelMatchers(UnquotedLabelMatcher(LabelName,MatchOp(EqlSingle),StringLiteral))))) +PromQL(BinaryExpr(NumberDurationLiteral,⚠,VectorSelector(LabelMatchers(UnquotedLabelMatcher(LabelName,MatchOp(EqlSingle),StringLiteral))))) # Trying to illegally use Inf as a metric name. Inf{foo="bar"} ==> -PromQL(BinaryExpr(NumberLiteral,⚠,VectorSelector(LabelMatchers(UnquotedLabelMatcher(LabelName,MatchOp(EqlSingle),StringLiteral))))) +PromQL(BinaryExpr(NumberDurationLiteral,⚠,VectorSelector(LabelMatchers(UnquotedLabelMatcher(LabelName,MatchOp(EqlSingle),StringLiteral))))) # Negative offset foo offset -5d ==> -PromQL(OffsetExpr(VectorSelector(Identifier), Offset, Sub, Duration)) +PromQL(OffsetExpr(VectorSelector(Identifier), Offset, NumberDurationLiteralInDurationContext)) # Negative offset with space foo offset - 5d ==> -PromQL(OffsetExpr(VectorSelector(Identifier), Offset, Sub, Duration)) +PromQL(OffsetExpr(VectorSelector(Identifier), Offset, NumberDurationLiteralInDurationContext)) # Positive offset foo offset 5d ==> -PromQL(OffsetExpr(VectorSelector(Identifier), Offset, Duration)) +PromQL(OffsetExpr(VectorSelector(Identifier), Offset, NumberDurationLiteralInDurationContext)) # Parsing only metric names with alternative @top { "top": "MetricName" } @@ -661,7 +661,7 @@ MetricName(Identifier) 1 + foo atan2 bar ==> -PromQL(BinaryExpr(NumberLiteral,Add,BinaryExpr(VectorSelector(Identifier),Atan2,VectorSelector(Identifier)))) +PromQL(BinaryExpr(NumberDurationLiteral,Add,BinaryExpr(VectorSelector(Identifier),Atan2,VectorSelector(Identifier)))) # Testing quoted metric name @@ -682,4 +682,4 @@ PromQL(VectorSelector(LabelMatchers(QuotedLabelMatcher(QuotedLabelName(StringLit {"metric_name", "foo"="bar"} ==> -PromQL(VectorSelector(LabelMatchers(QuotedLabelName(StringLiteral), QuotedLabelMatcher(QuotedLabelName(StringLiteral), MatchOp(EqlSingle), StringLiteral)))) \ No newline at end of file +PromQL(VectorSelector(LabelMatchers(QuotedLabelName(StringLiteral), QuotedLabelMatcher(QuotedLabelName(StringLiteral), MatchOp(EqlSingle), StringLiteral))))