From 907eb9031542a0eef31bc301066cb12078bf96d0 Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Thu, 25 Jan 2024 16:41:25 +0100 Subject: [PATCH] [ES|QL] Fix autocomplete for incomplete show command (#175292) ## Summary Fix a regression on `SHOW` command autocomplete within 8.13 (so no public bug). Compared to the previous custom grammar, the ES one does support the `SHOW` command only in complete mode (`SHOW FUNCTIONS` or `SHOW INFO`): any other substring (i.e. `SHOW`) will trigger an error at grammar level and will not be handled by the AST provider (as it won't enter the ParserTree at all). This time I've added unit tests for the autocomplete to avoid regressions like this again. ![show_functions_fix](https://github.com/elastic/kibana/assets/924948/a9eea572-91ea-43d2-bd2e-ede61996866e) ### Checklist - [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: Stratoula Kalafateli --- .../lib/ast/autocomplete/autocomplete.test.ts | 7 +++++++ .../esql/lib/ast/autocomplete/autocomplete.ts | 5 +++++ .../esql/lib/ast/autocomplete/complete_items.ts | 5 +++-- .../src/esql/lib/ast/autocomplete/factories.ts | 5 +++-- .../src/esql/lib/ast/shared/context.ts | 17 +++++++++++++++++ .../esql/lib/ast/validation/validation.test.ts | 4 ++++ 6 files changed, 39 insertions(+), 4 deletions(-) diff --git a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.test.ts b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.test.ts index 24cbcc1a51c86..4f4c78ba95622 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.test.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.test.ts @@ -317,6 +317,13 @@ describe('autocomplete', () => { testSuggestions('from index', suggestedIndexes, 6 /* index index in from */); }); + describe('show', () => { + testSuggestions('show ', ['functions', 'info']); + for (const fn of ['functions', 'info']) { + testSuggestions(`show ${fn} `, ['|']); + } + }); + describe('where', () => { const allEvalFns = getFunctionSignaturesByReturnType('where', 'any', { evalMath: true, diff --git a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.ts b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.ts index 65c2401b3bf6a..f3fc734d122bb 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.ts @@ -603,6 +603,11 @@ async function getExpressionSuggestionsByType( } )) ); + if (command.name === 'show') { + suggestions.push( + ...getBuiltinCompatibleFunctionDefinition(command.name, undefined, 'any') + ); + } } } diff --git a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/complete_items.ts b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/complete_items.ts index a7ce90cda409f..a9233a0c834d6 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/complete_items.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/complete_items.ts @@ -41,9 +41,10 @@ export const getBuiltinCompatibleFunctionDefinition = ( !ignoreAsSuggestion && !/not_/.test(name) && (option ? supportedOptions?.includes(option) : supportedCommands.includes(command)) && - signatures.some(({ params }) => params.some((pArg) => pArg.type === argType)) + signatures.some( + ({ params }) => !params.length || params.some((pArg) => pArg.type === argType) + ) ); - if (!returnTypes) { return compatibleFunctions.map(getAutocompleteBuiltinDefinition); } diff --git a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/factories.ts b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/factories.ts index b0de434647aee..9fc026852f999 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/factories.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/factories.ts @@ -51,10 +51,11 @@ export function getAutocompleteFunctionDefinition(fn: FunctionDefinition) { } export function getAutocompleteBuiltinDefinition(fn: FunctionDefinition) { + const hasArgs = fn.signatures.some(({ params }) => params.length); return { label: fn.name, - insertText: `${fn.name} $0`, - insertTextRules: 4, // monaco.languages.CompletionItemInsertTextRule.InsertAsSnippet, + insertText: hasArgs ? `${fn.name} $0` : fn.name, + ...(hasArgs ? { insertTextRules: 4 } : {}), // monaco.languages.CompletionItemInsertTextRule.InsertAsSnippet, kind: 11, detail: fn.description, documentation: { diff --git a/packages/kbn-monaco/src/esql/lib/ast/shared/context.ts b/packages/kbn-monaco/src/esql/lib/ast/shared/context.ts index 004d4ca5ded6b..5295b50b22525 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/shared/context.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/shared/context.ts @@ -157,6 +157,23 @@ export function getAstContext(innerText: string, ast: ESQLAst, offset: number) { return { type: 'setting' as const, command, node, option, setting }; } } + if (!command && innerText.trim().toLowerCase() === 'show') { + return { + type: 'expression' as const, + // The ES grammar makes the "SHOW" command an invalid type at grammar level + // so we need to create a fake command to make it work the AST in this case + command: { + type: 'command', + name: 'show', + text: innerText.trim(), + location: { min: 0, max: innerText.length }, + incomplete: true, + args: [], + } as ESQLCommand, + node, + option, + }; + } if (!command || (innerText.length <= offset && getLastCharFromTrimmed(innerText) === '|')) { // // ... | diff --git a/packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts b/packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts index cdbffc88efe44..2495111f3a9db 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts @@ -488,6 +488,10 @@ describe('validation logic', () => { testErrorsAndWarnings('show', ['SyntaxError: expected {SHOW} but found ""']); testErrorsAndWarnings('show functions', []); testErrorsAndWarnings('show info', []); + testErrorsAndWarnings('show functions()', [ + "SyntaxError: token recognition error at: '('", + "SyntaxError: token recognition error at: ')'", + ]); testErrorsAndWarnings('show functions blah', [ "SyntaxError: token recognition error at: 'b'", "SyntaxError: token recognition error at: 'l'",