Skip to content

Commit

Permalink
[ES|QL] Fix autocomplete for incomplete show command (elastic#175292)
Browse files Browse the repository at this point in the history
## 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 <[email protected]>
  • Loading branch information
dej611 and stratoula authored Jan 25, 2024
1 parent 6faccba commit 907eb90
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,11 @@ async function getExpressionSuggestionsByType(
}
))
);
if (command.name === 'show') {
suggestions.push(
...getBuiltinCompatibleFunctionDefinition(command.name, undefined, 'any')
);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
17 changes: 17 additions & 0 deletions packages/kbn-monaco/src/esql/lib/ast/shared/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) === '|')) {
// // ... | <here>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,10 @@ describe('validation logic', () => {
testErrorsAndWarnings('show', ['SyntaxError: expected {SHOW} but found "<EOF>"']);
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'",
Expand Down

0 comments on commit 907eb90

Please sign in to comment.