apply same changes in core prometheus frontend

public/app/plugins/datasource/prometheus/components/monaco-query-field/monaco-completion-provider/situation.ts

@@ -2,6 +2,7 @@ import type { SyntaxNode, Tree } from '@lezer/common';
 import {
   AggregateExpr,
   AggregateModifier,
+  BinaryExpr,
   EqlRegex,
   EqlSingle,
   FunctionCallBody,
@@ -9,11 +10,9 @@ import {
   Identifier,
   LabelMatcher,
   LabelMatchers,
-  LabelMatchList,
   LabelName,
   MatchOp,
   MatrixSelector,
-  MetricIdentifier,
   Neq,
   NeqRegex,
   parser,
@@ -35,9 +34,7 @@ type NodeTypeId =
   | typeof Identifier
   | typeof LabelMatcher
   | typeof LabelMatchers
-  | typeof LabelMatchList
   | typeof LabelName
-  | typeof MetricIdentifier
   | typeof PromQL
   | typeof StringLiteral
   | typeof VectorSelector
@@ -183,6 +180,10 @@ const RESOLVERS: Resolver[] = [
     path: [StringLiteral, LabelMatcher],
     fun: resolveLabelMatcher,
   },
+  {
+    path: [ERROR_NODE_NAME, BinaryExpr, PromQL],
+    fun: resolveTopLevel,
+  },
   {
     path: [ERROR_NODE_NAME, LabelMatcher],
     fun: resolveLabelMatcher,
@@ -251,30 +252,8 @@ function getLabels(labelMatchersNode: SyntaxNode, text: string): Label[] {
     return [];
   }
 
-  let listNode: SyntaxNode | null = walk(labelMatchersNode, [['firstChild', LabelMatchList]]);
+  const labelNodes = labelMatchersNode.getChildren(LabelMatcher);
-
+  return labelNodes.map((ln) => getLabel(ln, text)).filter(notEmpty);
-  const labels: Label[] = [];
-
-  while (listNode !== null) {
-    const matcherNode = walk(listNode, [['lastChild', LabelMatcher]]);
-    if (matcherNode === null) {
-      // unexpected, we stop
-      return [];
-    }
-
-    const label = getLabel(matcherNode, text);
-    if (label !== null) {
-      labels.push(label);
-    }
-
-    // there might be more labels
-    listNode = walk(listNode, [['firstChild', LabelMatchList]]);
-  }
-
-  // our labels-list is last-first, so we reverse it
-  labels.reverse();
-
-  return labels;
 }
 
 function getNodeChildren(node: SyntaxNode): SyntaxNode[] {
@@ -318,17 +297,12 @@ function resolveLabelsForGrouping(node: SyntaxNode, text: string, pos: number):
     return null;
   }
 
-  const metricIdNode = getNodeInSubtree(bodyNode, MetricIdentifier);
+  const metricIdNode = getNodeInSubtree(bodyNode, Identifier);
   if (metricIdNode === null) {
     return null;
   }
 
-  const idNode = walk(metricIdNode, [['firstChild', Identifier]]);
+  const metricName = getNodeText(metricIdNode, text);
-  if (idNode === null) {
-    return null;
-  }
-
-  const metricName = getNodeText(idNode, text);
   return {
     type: 'IN_GROUPING',
     metricName,
@@ -354,44 +328,11 @@ function resolveLabelMatcher(node: SyntaxNode, text: string, pos: number): Situa
 
   const labelName = getNodeText(labelNameNode, text);
 
-  // now we need to go up, to the parent of LabelMatcher,
+  const labelMatchersNode = walk(parent, [['parent', LabelMatchers]]);
-  // there can be one or many `LabelMatchList` parents, we have
+  if (labelMatchersNode === null) {
-  // to go through all of them
-
-  const firstListNode = walk(parent, [['parent', LabelMatchList]]);
-  if (firstListNode === null) {
     return null;
   }
 
-  let listNode = firstListNode;
-
-  // we keep going through the parent-nodes
-  // as long as they are LabelMatchList.
-  // as soon as we reawch LabelMatchers, we stop
-  let labelMatchersNode: SyntaxNode | null = null;
-  while (labelMatchersNode === null) {
-    const p = listNode.parent;
-    if (p === null) {
-      return null;
-    }
-
-    const { id } = p.type;
-
-    switch (id) {
-      case LabelMatchList:
-        //we keep looping
-        listNode = p;
-        continue;
-      case LabelMatchers:
-        // we reached the end, we can stop the loop
-        labelMatchersNode = p;
-        continue;
-      default:
-        // we reached some other node, we stop
-        return null;
-    }
-  }
-
   // now we need to find the other names
   const allLabels = getLabels(labelMatchersNode, text);
 
@@ -400,7 +341,6 @@ function resolveLabelMatcher(node: SyntaxNode, text: string, pos: number): Situa
 
   const metricNameNode = walk(labelMatchersNode, [
     ['parent', VectorSelector],
-    ['firstChild', MetricIdentifier],
     ['firstChild', Identifier],
   ]);
 
@@ -459,7 +399,7 @@ function resolveLabelKeysWithEquals(node: SyntaxNode, text: string, pos: number)
 
   // next false positive:
   // `something{a="1"^}`
-  const child = walk(node, [['firstChild', LabelMatchList]]);
+  const child = walk(node, [['firstChild', LabelMatcher]]);
   if (child !== null) {
     // means the label-matching part contains at least one label already.
     //
@@ -476,7 +416,6 @@ function resolveLabelKeysWithEquals(node: SyntaxNode, text: string, pos: number)
 
   const metricNameNode = walk(node, [
     ['parent', VectorSelector],
-    ['firstChild', MetricIdentifier],
     ['firstChild', Identifier],
   ]);
 
@@ -532,12 +471,12 @@ export function getSituation(text: string, pos: number): Situation | null {
     };
   }
 
-  /*
+  /**
-	PromQL
+   PromQL
-  Expr
+   Expr
-  VectorSelector
+   VectorSelector
-  LabelMatchers
+   LabelMatchers
-  */
+   */
   const tree = parser.parse(text);
 
   // if the tree contains error, it is very probable that
@@ -565,3 +504,7 @@ export function getSituation(text: string, pos: number): Situation | null {
 
   return null;
 }
+
+function notEmpty<TValue>(value: TValue | null | undefined): value is TValue {
+  return value !== null && value !== undefined;
+}

public/app/plugins/datasource/prometheus/querybuilder/parsing.test.ts

@@ -434,6 +434,12 @@ describe('buildVisualQueryFromString', () => {
           to: 27,
           parentType: 'VectorSelector',
         },
+        {
+          text: ')',
+          from: 38,
+          to: 39,
+          parentType: 'PromQL',
+        },
       ],
       query: {
         metric: '${func_var}',
@@ -687,7 +693,7 @@ describe('buildVisualQueryFromString', () => {
       errors: [
         {
           from: 6,
-          parentType: 'Expr',
+          parentType: 'BinaryExpr',
           text: '(bar + baz)',
           to: 17,
         },

public/app/plugins/datasource/prometheus/querybuilder/parsing.ts

@@ -4,22 +4,18 @@ import {
   AggregateModifier,
   AggregateOp,
   BinaryExpr,
-  BinModifiers,
+  BoolModifier,
-  Expr,
   FunctionCall,
-  FunctionCallArgs,
   FunctionCallBody,
   FunctionIdentifier,
-  GroupingLabel,
-  GroupingLabelList,
   GroupingLabels,
+  Identifier,
   LabelMatcher,
   LabelName,
+  MatchingModifierClause,
   MatchOp,
-  MetricIdentifier,
   NumberLiteral,
   On,
-  OnOrIgnoring,
   ParenExpr,
   parser,
   StringLiteral,
@@ -101,6 +97,7 @@ interface Context {
   errors: ParsingError[];
 }
 
+// TODO find a better approach for grafana global variables
 function isValidPromQLMinusGrafanaGlobalVariables(expr: string) {
   const context: Context = {
     query: {
@@ -141,7 +138,7 @@ export function handleExpression(expr: string, node: SyntaxNode, context: Contex
   const visQuery = context.query;
 
   switch (node.type.id) {
-    case MetricIdentifier: {
+    case Identifier: {
       // Expectation is that there is only one of those per query.
       visQuery.metric = getString(expr, node);
       break;
@@ -182,8 +179,8 @@ export function handleExpression(expr: string, node: SyntaxNode, context: Contex
 
     default: {
       if (node.type.id === ParenExpr) {
-        // We don't support parenthesis in the query to group expressions. We just report error but go on with the
+        // We don't support parenthesis in the query to group expressions.
-        // parsing.
+        // We just report error but go on with the parsing.
         context.errors.push(makeError(expr, node));
       }
       // Any other nodes we just ignore and go to its children. This should be fine as there are lots of wrapper
@@ -199,8 +196,9 @@ export function handleExpression(expr: string, node: SyntaxNode, context: Contex
   }
 }
 
+// TODO check if we still need this
 function isIntervalVariableError(node: SyntaxNode) {
-  return node.prevSibling?.type.id === Expr && node.prevSibling?.firstChild?.type.id === VectorSelector;
+  return node.prevSibling?.firstChild?.type.id === VectorSelector;
 }
 
 function getLabel(expr: string, node: SyntaxNode): QueryBuilderLabelFilter {
@@ -228,7 +226,6 @@ function handleFunction(expr: string, node: SyntaxNode, context: Context) {
   const funcName = getString(expr, nameNode);
 
   const body = node.getChild(FunctionCallBody);
-  const callArgs = body!.getChild(FunctionCallArgs);
   const params = [];
   let interval = '';
 
@@ -248,13 +245,13 @@ function handleFunction(expr: string, node: SyntaxNode, context: Context) {
   // We unshift operations to keep the more natural order that we want to have in the visual query editor.
   visQuery.operations.unshift(op);
 
-  if (callArgs) {
+  if (body) {
-    if (getString(expr, callArgs) === interval + ']') {
+    if (getString(expr, body) === '([' + interval + '])') {
       // This is a special case where we have a function with a single argument and it is the interval.
       // This happens when you start adding operations in query builder and did not set a metric yet.
       return;
     }
-    updateFunctionArgs(expr, callArgs, context, op);
+    updateFunctionArgs(expr, body, context, op);
   }
 }
 
@@ -283,25 +280,14 @@ function handleAggregation(expr: string, node: SyntaxNode, context: Context) {
       funcName = `__${funcName}_without`;
     }
 
-    labels.push(...getAllByType(expr, modifier, GroupingLabel));
+    labels.push(...getAllByType(expr, modifier, LabelName));
   }
 
   const body = node.getChild(FunctionCallBody);
-  const callArgs = body!.getChild(FunctionCallArgs);
-  const callArgsExprChild = callArgs?.getChild(Expr);
-  const binaryExpressionWithinAggregationArgs = callArgsExprChild?.getChild(BinaryExpr);
-
-  if (binaryExpressionWithinAggregationArgs) {
-    context.errors.push({
-      text: 'Query parsing is ambiguous.',
-      from: binaryExpressionWithinAggregationArgs.from,
-      to: binaryExpressionWithinAggregationArgs.to,
-    });
-  }
 
   const op: QueryBuilderOperation = { id: funcName, params: [] };
   visQuery.operations.unshift(op);
-  updateFunctionArgs(expr, callArgs, context, op);
+  updateFunctionArgs(expr, body, context, op);
   // We add labels after params in the visual query editor.
   op.params.push(...labels);
 }
@@ -309,8 +295,7 @@ function handleAggregation(expr: string, node: SyntaxNode, context: Context) {
 /**
  * Handle (probably) all types of arguments that function or aggregation can have.
  *
- *  FunctionCallArgs are nested bit weirdly basically its [firstArg, ...rest] where rest is again FunctionCallArgs so
+ * We cannot just get all the children and iterate them as arguments we have to again recursively traverse through
- *  we cannot just get all the children and iterate them as arguments we have to again recursively traverse through
  *  them.
  *
  * @param expr
@@ -323,15 +308,16 @@ function updateFunctionArgs(expr: string, node: SyntaxNode | null, context: Cont
     return;
   }
   switch (node.type.id) {
-    // In case we have an expression we don't know what kind so we have to look at the child as it can be anything.
+    case FunctionCallBody: {
-    case Expr:
-    // FunctionCallArgs are nested bit weirdly as mentioned so we have to go one deeper in this case.
-    case FunctionCallArgs: {
       let child = node.firstChild;
 
       while (child) {
-        const callArgsExprChild = child.getChild(Expr);
+        let binaryExpressionWithinFunctionArgs: SyntaxNode | null;
-        const binaryExpressionWithinFunctionArgs = callArgsExprChild?.getChild(BinaryExpr);
+        if (child?.type.id === BinaryExpr) {
+          binaryExpressionWithinFunctionArgs = child;
+        } else {
+          binaryExpressionWithinFunctionArgs = child?.getChild(BinaryExpr);
+        }
 
         if (binaryExpressionWithinFunctionArgs) {
           context.errors.push({
@@ -344,7 +330,6 @@ function updateFunctionArgs(expr: string, node: SyntaxNode | null, context: Cont
         updateFunctionArgs(expr, child, context, op);
         child = child.nextSibling;
       }
-
       break;
     }
 
@@ -377,16 +362,16 @@ function handleBinary(expr: string, node: SyntaxNode, context: Context) {
   const visQuery = context.query;
   const left = node.firstChild!;
   const op = getString(expr, left.nextSibling);
-  const binModifier = getBinaryModifier(expr, node.getChild(BinModifiers));
+  const binModifier = getBinaryModifier(expr, node.getChild(BoolModifier) ?? node.getChild(MatchingModifierClause));
 
   const right = node.lastChild!;
 
   const opDef = binaryScalarOperatorToOperatorName[op];
 
-  const leftNumber = left.getChild(NumberLiteral);
+  const leftNumber = left.type.id === NumberLiteral;
-  const rightNumber = right.getChild(NumberLiteral);
+  const rightNumber = right.type.id === NumberLiteral;
 
-  const rightBinary = right.getChild(BinaryExpr);
+  const rightBinary = right.type.id === BinaryExpr;
 
   if (leftNumber) {
     // TODO: this should be already handled in case parent is binary expression as it has to be added to parent
@@ -432,6 +417,7 @@ function handleBinary(expr: string, node: SyntaxNode, context: Context) {
   }
 }
 
+// TODO revisit this function.
 function getBinaryModifier(
   expr: string,
   node: SyntaxNode | null
@@ -439,26 +425,26 @@ function getBinaryModifier(
   | { isBool: true; isMatcher: false }
   | { isBool: false; isMatcher: true; matches: string; matchType: 'ignoring' | 'on' }
   | undefined {
-  if (!node) {
+    if (!node) {
-    return undefined;
-  }
-  if (node.getChild('Bool')) {
-    return { isBool: true, isMatcher: false };
-  } else {
-    const matcher = node.getChild(OnOrIgnoring);
-    if (!matcher) {
-      // Not sure what this could be, maybe should be an error.
       return undefined;
     }
-    const labels = getString(expr, matcher.getChild(GroupingLabels)?.getChild(GroupingLabelList));
+    if (node.getChild('Bool')) {
-    return {
+      return { isBool: true, isMatcher: false };
-      isMatcher: true,
+    } else {
-      isBool: false,
+      let labels = '';
-      matches: labels,
+      const groupingLabels = node.getChild(GroupingLabels);
-      matchType: matcher.getChild(On) ? 'on' : 'ignoring',
+      if (groupingLabels) {
-    };
+        labels = getAllByType(expr, groupingLabels, LabelName).join(', ');
+      }
+
+      return {
+        isMatcher: true,
+        isBool: false,
+        matches: labels,
+        matchType: node.getChild(On) ? 'on' : 'ignoring',
+      };
+    }
   }
-}
 
 function isEmptyQuery(query: PromVisualQuery) {
   if (query.labels.length === 0 && query.operations.length === 0 && !query.metric) {

public/app/plugins/datasource/prometheus/querybuilder/parsingUtils.ts

@@ -113,11 +113,10 @@ export function makeBinOp(
  * not be safe is it would also find arguments of nested functions.
  * @param expr
  * @param cur
- * @param type - can be string or number, some data-sources (loki) haven't migrated over to using numeric constants defined in the lezer parsing library (e.g. lezer-promql).
+ * @param type
- * @todo Remove string type definition when all data-sources have migrated to numeric constants
  */
-export function getAllByType(expr: string, cur: SyntaxNode, type: number | string): string[] {
+export function getAllByType(expr: string, cur: SyntaxNode, type: number): string[] {
-  if (cur.type.id === type || cur.name === type) {
+  if (cur.type.id === type) {
     return [getString(expr, cur)];
   }
   const values: string[] = [];