From 6c36503a6362435f17e7f2f3bdc0f69974d1e4c5 Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Fri, 12 Jan 2024 09:51:07 +0100 Subject: [PATCH] [ES|QL] Quote automatically for source/policies/fields with special chars (#174676) ## Summary Add support for automatic quoting on autocomplete. Added tests. ![esql_quote_special_fields](https://github.com/elastic/kibana/assets/924948/c74d2535-5ff5-42fe-9ec8-285fc4818a3c) ### 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 --- .../lib/ast/autocomplete/autocomplete.test.ts | 59 ++++++++++++------- .../esql/lib/ast/autocomplete/factories.ts | 15 +++-- 2 files changed, 50 insertions(+), 24 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 c6bf6983192280..3aaeeb9344b83b 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 @@ -20,12 +20,12 @@ import { commandDefinitions } from '../definitions/commands'; const triggerCharacters = [',', '(', '=', ' ']; -const fields = [ +const fields: Array<{ name: string; type: string; suggestedAs?: string }> = [ ...['string', 'number', 'date', 'boolean', 'ip'].map((type) => ({ name: `${type}Field`, type, })), - { name: 'any#Char$ field', type: 'number' }, + { name: 'any#Char$ field', type: 'number', suggestedAs: '`any#Char$ field`' }, { name: 'kubernetes.something.something', type: 'number' }, { name: `listField`, @@ -33,17 +33,35 @@ const fields = [ }, ]; -const indexes = ['a', 'index', 'otherIndex', '.secretIndex'].map((name) => ({ - name, - hidden: name.startsWith('.'), -})); +const indexes = ( + [] as Array<{ name: string; hidden: boolean; suggestedAs: string | undefined }> +).concat( + ['a', 'index', 'otherIndex', '.secretIndex', 'my-index'].map((name) => ({ + name, + hidden: name.startsWith('.'), + suggestedAs: undefined, + })), + ['my-index[quoted]', 'my-index$', 'my_index{}'].map((name) => ({ + name, + hidden: false, + suggestedAs: `\`${name}\``, + })) +); const policies = [ { name: 'policy', sourceIndices: ['enrichIndex1'], matchField: 'otherStringField', - enrichFields: ['otherField', 'yetAnotherField'], + enrichFields: ['otherField', 'yetAnotherField', 'yet-special-field'], + suggestedAs: undefined, }, + ...['my-policy[quoted]', 'my-policy$', 'my_policy{}'].map((name) => ({ + name, + sourceIndices: ['enrichIndex1'], + matchField: 'otherStringField', + enrichFields: ['otherField', 'yetAnotherField', 'yet-special-field'], + suggestedAs: `\`${name}\``, + })), ]; /** @@ -111,7 +129,7 @@ function getFunctionSignaturesByReturnType( function getFieldNamesByType(requestedType: string) { return fields .filter(({ type }) => requestedType === 'any' || type === requestedType) - .map(({ name }) => name); + .map(({ name, suggestedAs }) => suggestedAs || name); } function getLiteralsByType(type: string) { @@ -169,7 +187,10 @@ function createSuggestContext(text: string, triggerCharacter?: string) { function getPolicyFields(policyName: string) { return policies .filter(({ name }) => name === policyName) - .flatMap(({ enrichFields }) => enrichFields); + .flatMap(({ enrichFields }) => + // ok, this is a bit of cheating as it's using the same logic as in the helper + enrichFields.map((field) => (/[^a-zA-Z\d_\.@]/.test(field) ? `\`${field}\`` : field)) + ); } describe('autocomplete', () => { @@ -284,7 +305,9 @@ describe('autocomplete', () => { }); describe('from', () => { - const suggestedIndexes = indexes.filter(({ hidden }) => !hidden).map(({ name }) => name); + const suggestedIndexes = indexes + .filter(({ hidden }) => !hidden) + .map(({ name, suggestedAs }) => suggestedAs || name); // Monaco will filter further down here testSuggestions('f', sourceCommands); testSuggestions('from ', suggestedIndexes); @@ -429,19 +452,12 @@ describe('autocomplete', () => { testSuggestions('from a | stats a=c by d ', ['|', ',']); testSuggestions('from a | stats a=c by d, ', getFieldNamesByType('any')); testSuggestions('from a | stats a=max(b), ', ['var0 =', ...allAggFunctions]); - testSuggestions( - 'from a | stats a=min()', - fields.filter(({ type }) => type === 'number').map(({ name }) => name), - '(' - ); + testSuggestions('from a | stats a=min()', getFieldNamesByType('number'), '('); testSuggestions('from a | stats a=min(b) ', ['by', '|', ',']); testSuggestions('from a | stats a=min(b) by ', getFieldNamesByType('any')); testSuggestions('from a | stats a=min(b),', ['var0 =', ...allAggFunctions]); testSuggestions('from a | stats var0=min(b),var1=c,', ['var2 =', ...allAggFunctions]); - testSuggestions( - 'from a | stats a=min(b), b=max()', - fields.filter(({ type }) => type === 'number').map(({ name }) => name) - ); + testSuggestions('from a | stats a=min(b), b=max()', getFieldNamesByType('number')); // @TODO: remove last 2 suggestions if possible testSuggestions('from a | eval var0=round(b), var1=round(c) | stats ', [ 'var2 =', @@ -470,7 +486,10 @@ describe('autocomplete', () => { '| enrich other-policy on b ', '| enrich other-policy with c ', ]) { - testSuggestions(`from a ${prevCommand}| enrich `, ['policy']); + testSuggestions( + `from a ${prevCommand}| enrich `, + policies.map(({ name, suggestedAs }) => suggestedAs || name) + ); testSuggestions(`from a ${prevCommand}| enrich policy `, ['on', 'with', '|']); testSuggestions(`from a ${prevCommand}| enrich policy on `, [ 'stringField', 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 fc136a87b23da7..d3da3be7a5ba9e 100644 --- a/packages/kbn-monaco/src/esql/lib/ast/autocomplete/factories.ts +++ b/packages/kbn-monaco/src/esql/lib/ast/autocomplete/factories.ts @@ -27,6 +27,13 @@ export const TRIGGER_SUGGESTION_COMMAND = { id: 'editor.action.triggerSuggest', }; +function getSafeInsertText(text: string, { dashSupported }: { dashSupported?: boolean } = {}) { + if (dashSupported) { + return /[^a-zA-Z\d_\.@-]/.test(text) ? `\`${text}\`` : text; + } + return /[^a-zA-Z\d_\.@]/.test(text) ? `\`${text}\`` : text; +} + export function getAutocompleteFunctionDefinition(fn: FunctionDefinition) { const fullSignatures = getFunctionSignatures(fn); return { @@ -104,7 +111,7 @@ export function getAutocompleteCommandDefinition( export const buildFieldsDefinitions = (fields: string[]): AutocompleteCommandDefinition[] => fields.map((label) => ({ label, - insertText: label, + insertText: getSafeInsertText(label), kind: 4, detail: i18n.translate('monaco.esql.autocomplete.fieldDefinition', { defaultMessage: `Field specified by the input table`, @@ -115,7 +122,7 @@ export const buildFieldsDefinitions = (fields: string[]): AutocompleteCommandDef export const buildVariablesDefinitions = (variables: string[]): AutocompleteCommandDefinition[] => variables.map((label) => ({ label, - insertText: /[^a-zA-Z\d]/.test(label) ? `\`${label}\`` : label, + insertText: getSafeInsertText(label), kind: 4, detail: i18n.translate('monaco.esql.autocomplete.variableDefinition', { defaultMessage: `Variable specified by the user within the ES|QL query`, @@ -126,7 +133,7 @@ export const buildVariablesDefinitions = (variables: string[]): AutocompleteComm export const buildSourcesDefinitions = (sources: string[]): AutocompleteCommandDefinition[] => sources.map((label) => ({ label, - insertText: label, + insertText: getSafeInsertText(label, { dashSupported: true }), kind: 21, detail: i18n.translate('monaco.esql.autocomplete.sourceDefinition', { defaultMessage: `Input table`, @@ -167,7 +174,7 @@ export const buildPoliciesDefinitions = ( ): AutocompleteCommandDefinition[] => policies.map(({ name: label, sourceIndices }) => ({ label, - insertText: label, + insertText: getSafeInsertText(label, { dashSupported: true }), kind: 5, detail: i18n.translate('monaco.esql.autocomplete.policyDefinition', { defaultMessage: `Policy defined on {count, plural, one {index} other {indices}}: {indices}`,