adapt the lezer grammar and codemirror autocompletion with duration and number that are equivalent (#14417)

Signed-off-by: Augustin Husson <husson.augustin@gmail.com>

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);
     });
   });

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);
     });
   }
 

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;

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),

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 <number>+<unit> 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" )? ) |

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))))
+PromQL(VectorSelector(LabelMatchers(QuotedLabelName(StringLiteral), QuotedLabelMatcher(QuotedLabelName(StringLiteral), MatchOp(EqlSingle), StringLiteral))))