Merge pull request #18894 from MichaHoffmann/mhoffmann/codemirror-fix-autocomplete-for-functions

codemirror: fix autocomplete edgecase for functions

web/ui/module/codemirror-promql/src/complete/hybrid.test.ts

@@ -74,6 +74,31 @@ describe('analyzeCompletion test', () => {
         { kind: ContextKind.Aggregation },
       ],
     },
+    {
+      title: 'autocomplete AggregateOpModifier or BinOp after closing aggregation',
+      expr: 'sum()',
+      pos: 5, // cursor is after the closing bracket
+      expectedContext: [{ kind: ContextKind.AggregateOpModifier }, { kind: ContextKind.BinOp }],
+    },
+    {
+      title: 'metric/function/aggregation autocompletion in incomplete function',
+      expr: 'sum(',
+      pos: 4,
+      expectedContext: [
+        {
+          kind: ContextKind.MetricName,
+          metricName: '',
+        },
+        { kind: ContextKind.Function },
+        { kind: ContextKind.Aggregation },
+      ],
+    },
+    {
+      title: 'autocomplete binOp after closing function',
+      expr: 'rate(foo[5m])',
+      pos: 13, // cursor is after the closing bracket
+      expectedContext: [{ kind: ContextKind.BinOp }],
+    },
     {
       title: 'metric/function/aggregation autocompletion 2',
       expr: 'sum(rat)',
@@ -100,6 +125,18 @@ describe('analyzeCompletion test', () => {
         { kind: ContextKind.Aggregation },
       ],
     },
+    {
+      title: 'autocomplete AggregateOpModifier or BinOp after closing nested aggregation',
+      expr: 'sum(rate(foo[5m]))',
+      pos: 18, // cursor is after the closing bracket
+      expectedContext: [{ kind: ContextKind.AggregateOpModifier }, { kind: ContextKind.BinOp }],
+    },
+    {
+      title: 'autocomplete binOp after closing aggregation with existing modifier',
+      expr: 'sum by(job)(rate(foo[5m]))',
+      pos: 26, // cursor is after the closing bracket
+      expectedContext: [{ kind: ContextKind.BinOp }],
+    },
     {
       title: 'metric/function/aggregation autocompletion 4',
       expr: 'sum(rate(my_))',
@@ -717,6 +754,18 @@ describe('computeStartCompletePosition test', () => {
       pos: 9, // cursor is between the bracket
       expectedStart: 9,
     },
+    {
+      title: 'start should be equal to the pos after closing function',
+      expr: 'rate(foo[5m])',
+      pos: 13, // cursor is after the closing bracket
+      expectedStart: 13,
+    },
+    {
+      title: 'start should be equal to the pos after closing aggregation',
+      expr: 'sum(rate(foo[5m]))',
+      pos: 18, // cursor is after the closing bracket
+      expectedStart: 18,
+    },
     {
       title: 'bracket containing a substring',
       expr: '{myL}',
@@ -994,12 +1043,24 @@ describe('computeEndCompletePosition test', () => {
       pos: 13, // cursor at '!' (error node)
       expectedEnd: 13, // error node returns pos
     },
+    {
+      title: 'end should be equal to the pos after closing function',
+      expr: 'rate(foo[5m])',
+      pos: 13, // cursor is after the closing bracket
+      expectedEnd: 13,
+    },
+    {
+      title: 'end should be equal to the pos after closing aggregation',
+      expr: 'sum(rate(foo[5m]))',
+      pos: 18, // cursor is after the closing bracket
+      expectedEnd: 18,
+    },
   ];
   testCases.forEach((value) => {
     it(value.title, () => {
       const state = createEditorState(value.expr);
       const node = syntaxTree(state).resolve(value.pos, -1);
-      const result = computeEndCompletePosition(node, value.pos);
+      const result = computeEndCompletePosition(state, node, value.pos);
       expect(result).toEqual(value.expectedEnd);
     });
   });
@@ -1043,6 +1104,28 @@ describe('autocomplete promQL test', () => {
         validFor: /^[a-zA-Z0-9_:]+$/,
       },
     },
+    {
+      title: 'offline autocomplete aggregate operation modifier or binary operator after closing aggregation',
+      expr: 'sum()',
+      pos: 5, // cursor is after the closing bracket
+      expectedResult: {
+        options: ([] as Completion[]).concat(aggregateOpModifierTerms, binOpTerms),
+        from: 5,
+        to: 5,
+        validFor: /^[a-zA-Z0-9_:]+$/,
+      },
+    },
+    {
+      title: 'offline autocomplete binary operator after closing function',
+      expr: 'rate(foo[5m])',
+      pos: 13, // cursor is after the closing bracket
+      expectedResult: {
+        options: binOpTerms,
+        from: 13,
+        to: 13,
+        validFor: /^[a-zA-Z0-9_:]+$/,
+      },
+    },
     {
       title: 'offline function/aggregation autocompletion in aggregation 2',
       expr: 'sum(ra)',
@@ -1087,6 +1170,17 @@ describe('autocomplete promQL test', () => {
         validFor: /^[a-zA-Z0-9_:]+$/,
       },
     },
+    {
+      title: 'offline autocomplete aggregate operation modifier or binary operator after closing nested aggregation',
+      expr: 'sum(rate(foo[5m]))',
+      pos: 18, // cursor is after the closing bracket
+      expectedResult: {
+        options: ([] as Completion[]).concat(aggregateOpModifierTerms, binOpTerms),
+        from: 18,
+        to: 18,
+        validFor: /^[a-zA-Z0-9_:]+$/,
+      },
+    },
     {
       title: 'offline function/aggregation autocompletion in aggregation 4',
       expr: 'sum by (instance, job) ( sum_over(scrape_series_added[1h])) / sum by (instance, job) (sum_over_time(scrape_samples_scraped[1h])) > 0.1 and sum by(instance, job) (scrape_samples_scraped{) > 100',

web/ui/module/codemirror-promql/src/complete/hybrid.ts

@@ -17,6 +17,7 @@ import { PrometheusClient } from '../client';
 import {
   Add,
   AggregateExpr,
+  AggregateModifier,
   And,
   BinaryExpr,
   BoolModifier,
@@ -175,16 +176,24 @@ function escapePromQLString(str: string): string {
   return str.replace(/([\\"])/g, '\\$1');
 }
 
+function isAfterClosedFunctionCallBody(state: EditorState, node: SyntaxNode, pos: number): boolean {
+  return node.type.id === FunctionCallBody && pos >= node.to && node.from < node.to && state.sliceDoc(node.to - 1, node.to) === ')';
+}
+
 // computeEndCompletePosition calculates the end position for autocompletion replacement.
 // When the cursor is in the middle of a token, this ensures the entire token is replaced,
 // not just the portion before the cursor. This fixes issue #15839.
 // Note: this method is exported only for testing purpose.
-export function computeEndCompletePosition(node: SyntaxNode, pos: number): number {
+export function computeEndCompletePosition(state: EditorState, node: SyntaxNode, pos: number): number {
   // For error nodes, use the cursor position as the end position
   if (node.type.id === 0) {
     return pos;
   }
 
+  if (isAfterClosedFunctionCallBody(state, node, pos)) {
+    return pos;
+  }
+
   if (
     node.type.id === LabelMatchers ||
     node.type.id === GroupingLabels ||
@@ -240,7 +249,9 @@ function computeStartCompleteLabelPositionInLabelMatcherOrInGroupingLabel(node:
 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) {
+  if (isAfterClosedFunctionCallBody(state, node, pos)) {
+    start = pos;
+  } else if (node.type.id === LabelMatchers || node.type.id === GroupingLabels) {
     start = computeStartCompleteLabelPositionInLabelMatcherOrInGroupingLabel(node, pos);
   } else if (
     (node.type.id === FunctionCallBody && node.firstChild === null) ||
@@ -545,6 +556,13 @@ export function analyzeCompletion(state: EditorState, node: SyntaxNode, pos: num
       result.push({ kind: ContextKind.Duration });
       break;
     case FunctionCallBody:
+      if (isAfterClosedFunctionCallBody(state, node, pos)) {
+        if (node.parent?.type.id === AggregateExpr && !containsAtLeastOneChild(node.parent, AggregateModifier)) {
+          result.push({ kind: ContextKind.AggregateOpModifier });
+        }
+        result.push({ kind: ContextKind.BinOp });
+        break;
+      }
       // For aggregation function such as Topk, the first parameter is a number.
       // The second one is an expression.
       // When moving to the second parameter, the node is an error node.
@@ -711,7 +729,7 @@ export class HybridComplete implements CompleteStrategy {
       return arrayToCompletionResult(
         result,
         computeStartCompletePosition(state, tree, pos),
-        computeEndCompletePosition(tree, pos),
+        computeEndCompletePosition(state, tree, pos),
         completeSnippet,
         span
       );