Skip to content

Commit

Permalink
[ES|QL] Quote automatically for source/policies/fields with special c…
Browse files Browse the repository at this point in the history
…hars (elastic#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
  • Loading branch information
dej611 authored Jan 12, 2024
1 parent d43e7e5 commit 6c36503
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,48 @@ 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`,
type: `list`,
},
];

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}\``,
})),
];

/**
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 =',
Expand Down Expand Up @@ -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',
Expand Down
15 changes: 11 additions & 4 deletions packages/kbn-monaco/src/esql/lib/ast/autocomplete/factories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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`,
Expand All @@ -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`,
Expand All @@ -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`,
Expand Down Expand Up @@ -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}`,
Expand Down

0 comments on commit 6c36503

Please sign in to comment.