[ES|QL] don't suggest in comments (elastic#197341)

## Summary

Close elastic#191866

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Elastic Machine <[email protected]>

packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts

@@ -8,7 +8,7 @@
  */
 
 import { camelCase } from 'lodash';
-import { getAstAndSyntaxErrors } from '@kbn/esql-ast';
+import { parse } from '@kbn/esql-ast';
 import { scalarFunctionDefinitions } from '../../definitions/generated/scalar_functions';
 import { builtinFunctions } from '../../definitions/builtin';
 import { aggregationFunctionDefinitions } from '../../definitions/generated/aggregation_functions';
@@ -312,7 +312,7 @@ export const setup = async (caret = '/') => {
       querySansCaret,
       pos,
       ctx,
-      getAstAndSyntaxErrors,
+      (_query: string | undefined) => parse(_query, { withFormatting: true }),
       opts.callbacks ?? callbacks
     );
   };

packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/suggestions_in_comments.test.ts

@@ -0,0 +1,43 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+import { setup } from './helpers';
+
+describe('suggestions in comments', () => {
+  it('does not suggest in single-line comments', async () => {
+    const { assertSuggestions } = await setup('^');
+    await assertSuggestions('FROM index | EVAL // hey there ^', []);
+  });
+
+  it('does not suggest in multi-line comments', async () => {
+    const { assertSuggestions } = await setup('^');
+    await assertSuggestions('FROM index | EVAL /* ^ */', []);
+    await assertSuggestions('FROM index | EVAL /* (^) */', []);
+  });
+
+  it('does not suggest in incomplete multi-line comments', async () => {
+    const { assertSuggestions } = await setup('^');
+    assertSuggestions('FROM index | EVAL /* ^', []);
+  });
+
+  test('suggests next to comments', async () => {
+    const { suggest } = await setup('^');
+    expect((await suggest('FROM index | EVAL ^/* */')).length).toBeGreaterThan(0);
+    expect((await suggest('FROM index | EVAL /* */^')).length).toBeGreaterThan(0);
+    expect((await suggest('FROM index | EVAL ^// a comment')).length).toBeGreaterThan(0);
+    expect((await suggest('FROM index | EVAL // a comment\n^')).length).toBeGreaterThan(0);
+  });
+
+  test('handles multiple comments', async () => {
+    const { assertSuggestions } = await setup('^');
+    assertSuggestions('FROM index | EVAL /* comment1 */ x + /* comment2 ^ */ 1', []);
+    assertSuggestions('FROM index | EVAL /* ^ comment1 */ x + /* comment2 ^ */ 1', []);
+    assertSuggestions('FROM index | EVAL /* comment1 */ x + /* comment2 */ 1 // comment3 ^', []);
+  });
+});

packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts

@@ -159,6 +159,11 @@ export async function suggest(
   const { ast } = await astProvider(correctedQuery);
 
   const astContext = getAstContext(innerText, ast, offset);
+
+  if (astContext.type === 'comment') {
+    return [];
+  }
+
   // build the correct query to fetch the list of fields
   const queryForFields = getQueryForFields(
     buildQueryUntilPreviousCommand(ast, correctedQuery),

packages/kbn-esql-validation-autocomplete/src/shared/context.ts

@@ -7,14 +7,15 @@
  * License v3.0 only", or the "Server Side Public License, v 1".
  */
 
-import type {
-  ESQLAstItem,
-  ESQLSingleAstItem,
-  ESQLAst,
-  ESQLFunction,
-  ESQLCommand,
-  ESQLCommandOption,
-  ESQLCommandMode,
+import {
+  type ESQLAstItem,
+  type ESQLSingleAstItem,
+  type ESQLAst,
+  type ESQLFunction,
+  type ESQLCommand,
+  type ESQLCommandOption,
+  type ESQLCommandMode,
+  Walker,
 } from '@kbn/esql-ast';
 import { ENRICH_MODES } from '../definitions/settings';
 import { EDITOR_MARKER } from './constants';
@@ -152,6 +153,20 @@ function isBuiltinFunction(node: ESQLFunction) {
  * * "newCommand": the cursor is at the beginning of a new command (i.e. `command1 | command2 | <here>`)
  */
 export function getAstContext(queryString: string, ast: ESQLAst, offset: number) {
+  let inComment = false;
+
+  Walker.visitComments(ast, (node) => {
+    if (node.location && node.location.min <= offset && node.location.max > offset) {
+      inComment = true;
+    }
+  });
+
+  if (inComment) {
+    return {
+      type: 'comment' as const,
+    };
+  }
+
   const { command, option, setting, node } = findAstPosition(ast, offset);
   if (node) {
     if (node.type === 'literal' && node.literalType === 'keyword') {

packages/kbn-esql-validation-autocomplete/src/shared/helpers.test.ts

@@ -8,8 +8,8 @@
  */
 
 import { parse } from '@kbn/esql-ast';
-import { getExpressionType, shouldBeQuotedSource } from './helpers';
-import type { SupportedDataType } from '../definitions/types';
+import { getBracketsToClose, getExpressionType, shouldBeQuotedSource } from './helpers';
+import { SupportedDataType } from '../definitions/types';
 import { setTestFunctions } from './test_functions';
 
 describe('shouldBeQuotedSource', () => {
@@ -324,3 +324,16 @@ describe('getExpressionType', () => {
     );
   });
 });
+
+describe('getBracketsToClose', () => {
+  it('returns the number of brackets to close', () => {
+    expect(getBracketsToClose('foo(bar(baz')).toEqual([')', ')']);
+    expect(getBracketsToClose('foo(bar[baz')).toEqual([']', ')']);
+    expect(getBracketsToClose('foo(bar[baz"bap')).toEqual(['"', ']', ')']);
+    expect(
+      getBracketsToClose(
+        'from a | eval case(integerField < 0, "negative", integerField > 0, "positive", '
+      )
+    ).toEqual([')']);
+  });
+});

packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts

@@ -651,26 +651,59 @@ export const isParam = (x: unknown): x is ESQLParamLiteral =>
 export const noCaseCompare = (a: string, b: string) => a.toLowerCase() === b.toLowerCase();
 
 /**
- * This function count the number of unclosed brackets in order to
- * locally fix the queryString to generate a valid AST
+ * This function returns a list of closing brackets that can be appended to 
+ * a partial query to make it valid.
+
+* locally fix the queryString to generate a valid AST
  * A known limitation of this is that is not aware of commas "," or pipes "|"
  * so it is not yet helpful on a multiple commands errors (a workaround it to pass each command here...)
- * @param bracketType
  * @param text
  * @returns
  */
-export function countBracketsUnclosed(bracketType: '(' | '[' | '"' | '"""', text: string) {
+export function getBracketsToClose(text: string) {
   const stack = [];
-  const closingBrackets = { '(': ')', '[': ']', '"': '"', '"""': '"""' };
+  const pairs: Record<string, string> = { '"""': '"""', '/*': '*/', '(': ')', '[': ']', '"': '"' };
+  const pairsReversed: Record<string, string> = {
+    '"""': '"""',
+    '*/': '/*',
+    ')': '(',
+    ']': '[',
+    '"': '"',
+  };
+
   for (let i = 0; i < text.length; i++) {
-    const substr = text.substring(i, i + bracketType.length);
-    if (substr === closingBrackets[bracketType] && stack.length) {
-      stack.pop();
-    } else if (substr === bracketType) {
-      stack.push(bracketType);
+    for (const openBracket in pairs) {
+      if (!Object.hasOwn(pairs, openBracket)) {
+        continue;
+      }
+
+      const substr = text.slice(i, i + openBracket.length);
+      if (substr === openBracket) {
+        stack.push(substr);
+        break;
+      } else if (pairsReversed[substr] && pairsReversed[substr] === stack[stack.length - 1]) {
+        stack.pop();
+        break;
+      }
     }
   }
-  return stack.length;
+  return stack.reverse().map((bracket) => pairs[bracket]);
+}
+
+/**
+ * This function counts the number of unclosed parentheses
+ * @param text
+ */
+export function countUnclosedParens(text: string) {
+  let unclosedCount = 0;
+  for (let i = 0; i < text.length; i++) {
+    if (text[i] === ')' && unclosedCount > 0) {
+      unclosedCount--;
+    } else if (text[i] === '(') {
+      unclosedCount++;
+    }
+  }
+  return unclosedCount;
 }
 
 /**
@@ -685,37 +718,22 @@ export function countBracketsUnclosed(bracketType: '(' | '[' | '"' | '"""', text
 export function correctQuerySyntax(_query: string, context: EditorContext) {
   let query = _query;
   // check if all brackets are closed, otherwise close them
-  const unclosedRoundBrackets = countBracketsUnclosed('(', query);
-  const unclosedSquaredBrackets = countBracketsUnclosed('[', query);
-  const unclosedQuotes = countBracketsUnclosed('"', query);
-  const unclosedTripleQuotes = countBracketsUnclosed('"""', query);
+  const bracketsToAppend = getBracketsToClose(query);
+  const unclosedRoundBracketCount = bracketsToAppend.filter((bracket) => bracket === ')').length;
   // if it's a comma by the user or a forced trigger by a function argument suggestion
   // add a marker to make the expression still valid
   const charThatNeedMarkers = [',', ':'];
   if (
     (context.triggerCharacter && charThatNeedMarkers.includes(context.triggerCharacter)) ||
     // monaco.editor.CompletionTriggerKind['Invoke'] === 0
-    (context.triggerKind === 0 && unclosedRoundBrackets === 0) ||
+    (context.triggerKind === 0 && unclosedRoundBracketCount === 0) ||
     (context.triggerCharacter === ' ' && isMathFunction(query, query.length)) ||
     isComma(query.trimEnd()[query.trimEnd().length - 1])
   ) {
     query += EDITOR_MARKER;
   }
 
-  // if there are unclosed brackets, close them
-  if (unclosedRoundBrackets || unclosedSquaredBrackets || unclosedQuotes) {
-    for (const [char, count] of [
-      ['"""', unclosedTripleQuotes],
-      ['"', unclosedQuotes],
-      [')', unclosedRoundBrackets],
-      [']', unclosedSquaredBrackets],
-    ]) {
-      if (count) {
-        // inject the closing brackets
-        query += Array(count).fill(char).join('');
-      }
-    }
-  }
+  query += bracketsToAppend.join('');
 
   return query;
 }

packages/kbn-monaco/src/esql/worker/esql_worker.ts

@@ -43,7 +43,7 @@ export class ESQLWorker implements BaseWorkerDefinition {
   }
 
   getAst(text: string | undefined) {
-    const rawAst = parse(text);
+    const rawAst = parse(text, { withFormatting: true });
     return {
       ast: rawAst.root.commands,
       errors: rawAst.errors.map(inlineToMonacoErrors),