From d95562adc2d03dd6020d0caa9505d60c9ac72e2d Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Fri, 9 Aug 2024 12:48:26 -0500 Subject: [PATCH 01/19] Improve function arg suggestions --- .../src/cookie_session_storage.ts | 2 +- .../src/autocomplete/autocomplete.test.ts | 124 ++++++++++++++---- .../src/autocomplete/autocomplete.ts | 53 +++++++- .../src/autocomplete/helper.ts | 58 ++++++++ 4 files changed, 209 insertions(+), 28 deletions(-) diff --git a/packages/core/http/core-http-server-internal/src/cookie_session_storage.ts b/packages/core/http/core-http-server-internal/src/cookie_session_storage.ts index 7962e0b40777a..f006f658d5bbc 100644 --- a/packages/core/http/core-http-server-internal/src/cookie_session_storage.ts +++ b/packages/core/http/core-http-server-internal/src/cookie_session_storage.ts @@ -56,7 +56,7 @@ class ScopedCookieSessionStorage implements SessionStorage } public clear() { - return this.request.cookieAuth.clear(); + return this.request.cookieAuth?.clear(); } } diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts index 132ff6e572dd7..4d446a676fed8 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -17,14 +17,14 @@ import { TRIGGER_SUGGESTION_COMMAND, } from './factories'; import { camelCase, partition } from 'lodash'; -import { getAstAndSyntaxErrors } from '@kbn/esql-ast'; +import { ESQLAstItem, getAstAndSyntaxErrors } from '@kbn/esql-ast'; import { FunctionParameter, isFieldType, isSupportedDataType, SupportedDataType, } from '../definitions/types'; -import { getParamAtPosition } from './helper'; +import { getParamAtPosition, narrowDownCompatibleTypesToSuggestNext } from './helper'; import { nonNullable } from '../shared/helpers'; import { policies, @@ -40,6 +40,7 @@ import { } from './__tests__/helpers'; import { METADATA_FIELDS } from '../shared/constants'; import { ESQL_COMMON_NUMERIC_TYPES, ESQL_STRING_TYPES } from '../shared/esql_types'; +import { fieldNameFromType } from '../validation/validation.test'; const roundParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; const powParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; @@ -465,7 +466,8 @@ describe('autocomplete', () => { } }); - describe('eval', () => { + // @todo: + describe.only('eval', () => { testSuggestions('from a | eval ', [ 'var0 = ', ...getFieldNamesByType('any'), @@ -786,17 +788,83 @@ describe('autocomplete', () => { // Test suggestions for each possible param, within each signature variation, for each function for (const fn of evalFunctionDefinitions) { - // skip this fn for the moment as it's quite hard to test + const testedCases = new Set(); + // skip bucket and case fn for the moment as it's quite hard to test + // if (!['bucket', 'date_extract', 'date_diff', 'case'].includes(fn.name)) { + if (!['bucket', 'date_extract', 'date_diff', 'case'].includes(fn.name)) { - for (const signature of fn.signatures) { + // 1. Empty expression + // i.e. eval pow( ), round ( ) + // const allParamDefs = fn.signatures.map((s) => getParamAtPosition(s, 0)).filter(nonNullable); + // // get all possible types for this param + // const [constantOnlyParamDefs, acceptsFieldParamDefs] = partition( + // allParamDefs, + // (p) => p.constantOnly || /_literal/.test(p.type as string) + // ); + // const getTypesFromParamDefs = (paramDefs: FunctionParameter[]): SupportedDataType[] => + // Array.from(new Set(paramDefs.map((p) => p.type))).filter(isSupportedDataType); + // const addCommaIfRequired = (s: string | PartialSuggestionWithText) => { + // // don't add commas to the empty string or if there are no more required args + // if (s === '' || (typeof s === 'object' && s.text === '')) { + // return s; + // } + // return typeof s === 'string' ? `${s}, ` : { ...s, text: `${s.text},` }; + // }; + + // testSuggestions( + // `from a | eval ${fn.name}( `, + // [ + // ...getDateLiteralsByFieldType( + // getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) + // ), + // ...getFieldNamesByType( + // getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) + // ), + // ...getFunctionSignaturesByReturnType( + // 'eval', + // getTypesFromParamDefs(acceptsFieldParamDefs), + // { scalar: true }, + // undefined, + // [fn.name] + // ), + // ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), + // ].map(addCommaIfRequired) + // ); + + for (let signatureIndex = 0; signatureIndex < fn.signatures.length; signatureIndex++) { + const signature = fn.signatures[signatureIndex]; + // @ts-ignore Partial + const enrichedArgs: Array< + ESQLAstItem & { + esType: string; + } + > = signature.params.map(({ type }) => ({ + type: 'column', + esType: type, + })); + + // @todo: add for empty case signature.params.forEach((param, i) => { + // Generate all possible cases from the signature param + // E.g. eval st_distance( ) | st_distance(cartesianPointField, ) | eval st_distance(geoPointField, ) | st_distance(geoPointField, ) + const testCase = `${fn.name}(${signature.params + .slice(0, i + 1) + .map((p) => fieldNameFromType(p.type)) + .join(',')}${i === 0 ? ',' : ''} )`; + + // Avoid repeated testing of expressions we have previously seen + if (testedCases.has(testCase)) { + return; + } else { + testedCases.add(testCase); + } if (i < signature.params.length) { - // This ref signature thing is probably wrong in a few cases, but it matches - // the logic in getFunctionArgsSuggestions. They should both be updated - const refSignature = fn.signatures[0]; - const requiresMoreArgs = - i + 1 < (refSignature.minParams ?? 0) || - refSignature.params.filter(({ optional }, j) => !optional && j > i).length > 0; + const typesToSuggestNext = narrowDownCompatibleTypesToSuggestNext( + fn, + enrichedArgs, + i + 1 + ); + const requiresMoreArgs = typesToSuggestNext.length - 1 > 0; const allParamDefs = fn.signatures .map((s) => getParamAtPosition(s, i)) @@ -814,27 +882,41 @@ describe('autocomplete', () => { const suggestedConstants = param.literalSuggestions || param.literalOptions; const addCommaIfRequired = (s: string | PartialSuggestionWithText) => { - // don't add commas to the empty string or if there are no more required args if (!requiresMoreArgs || s === '' || (typeof s === 'object' && s.text === '')) { return s; } return typeof s === 'string' ? `${s}, ` : { ...s, text: `${s.text},` }; }; + // @TODO: remove + console.log(`\n--@@from a | eval ${testCase}`); + // @TODO: remove + console.log( + `--@@typesToSuggestNext`, + JSON.stringify( + typesToSuggestNext.map((d) => `${d.type}-${d.optional ? 'optional' : 'required'}`) + ) + ); + + // @TODO: remove + console.log(`--@@requiresMoreArgs`, requiresMoreArgs); + + // Compose eval expression that matches what params suggest + // E.g. from a | eval round() testSuggestions( - `from a | eval ${fn.name}(${Array(i).fill('field').join(', ')}${i ? ',' : ''} )`, + `from a | eval ${testCase}`, suggestedConstants?.length ? suggestedConstants.map((option) => `"${option}"${requiresMoreArgs ? ', ' : ''}`) : [ ...getDateLiteralsByFieldType( - getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) + getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) ), ...getFieldNamesByType( - getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) + getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) ), ...getFunctionSignaturesByReturnType( 'eval', - getTypesFromParamDefs(acceptsFieldParamDefs), + getTypesFromParamDefs(typesToSuggestNext), { scalar: true }, undefined, [fn.name] @@ -844,21 +926,19 @@ describe('autocomplete', () => { ' ' ); testSuggestions( - `from a | eval var0 = ${fn.name}(${Array(i).fill('field').join(', ')}${ - i ? ',' : '' - } )`, + `from a | eval var0 = ${testCase}`, suggestedConstants?.length ? suggestedConstants.map((option) => `"${option}"${requiresMoreArgs ? ', ' : ''}`) : [ ...getDateLiteralsByFieldType( - getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) + getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) ), ...getFieldNamesByType( - getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) + getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) ), ...getFunctionSignaturesByReturnType( 'eval', - getTypesFromParamDefs(acceptsFieldParamDefs), + getTypesFromParamDefs(typesToSuggestNext), { scalar: true }, undefined, [fn.name] diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index e775c3f05fe5f..edee60184dd40 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import uniqBy from 'lodash/uniqBy'; +import { uniqBy } from 'lodash'; import type { AstProviderFn, ESQLAstItem, @@ -93,6 +93,8 @@ import { getSourcesFromCommands, getSupportedTypesForBinaryOperators, isAggFunctionUsedAlready, + narrowDownCompatibleTypesToSuggestNext, + narrowDownRelevantFunctionSignatures, removeQuoteForSuggestedSources, } from './helper'; import { FunctionParameter, FunctionReturnType, SupportedDataType } from '../definitions/types'; @@ -1186,6 +1188,18 @@ async function getFunctionArgsSuggestions( return []; } const fieldsMap: Map = await getFieldsMap(); + const anyVariables = collectVariables(commands, fieldsMap, innerText); + + const references = { + fields: fieldsMap, + variables: anyVariables, + }; + + const enrichedArgs = node.args.map((nodeArg) => { + const esType = extractFinalTypeFromArg(nodeArg, references); + return { ...nodeArg, esType } as ESQLAstItem & { esType: string }; + }); + const variablesExcludingCurrentCommandOnes = excludeVariablesFromCurrentCommand( commands, command, @@ -1199,13 +1213,28 @@ async function getFunctionArgsSuggestions( argIndex -= 1; } - const arg = node.args[argIndex]; + const arg: ESQLAstItem = enrichedArgs[argIndex]; + + // Retrieve unique of types that are compatiable for the current arg + const relevantFuncSignatures = narrowDownRelevantFunctionSignatures( + fnDefinition, + enrichedArgs, + argIndex + ); + const compatibleTypesToSuggestForArg = uniqBy( + relevantFuncSignatures.map((f) => f.params[argIndex]).filter((d) => d), + (o) => `${o.type}-${o.constantOnly}` + ); + // @TODO: remove + console.log(`--@@compatibleTypesToSuggestForArg`, compatibleTypesToSuggestForArg); + // @TODO: remove + // console.log(`--@@compatibleTypesToSuggestForArg`, compatibleTypesToSuggestForArg); // the first signature is used as reference // TODO - take into consideration all signatures that match the current args const refSignature = fnDefinition.signatures[0]; - const hasMoreMandatoryArgs = + const old_hasMoreMandatoryArgs = (refSignature.params.length >= argIndex && refSignature.params.filter(({ optional }, index) => !optional && index > argIndex).length > 0) || @@ -1213,6 +1242,18 @@ async function getFunctionArgsSuggestions( ? refSignature.minParams - 1 > argIndex : false); + const needMoreArgsChecks = relevantFuncSignatures.map((signature) => { + return ( + (signature.params.length >= argIndex && + signature.params.filter(({ optional }, index) => !optional && index > argIndex).length > + 0) || + ('minParams' in signature && signature.minParams ? signature.minParams - 1 > argIndex : false) + ); + }); + const hasMoreMandatoryArgs = + needMoreArgsChecks.length === 0 ? false : needMoreArgsChecks.some((d) => d === false) === false; + + const canHaveMoreArgs = compatibleTypesToSuggestForArg.length > 0; const shouldAddComma = hasMoreMandatoryArgs && fnDefinition.type !== 'builtin'; const suggestedConstants = Array.from( @@ -1296,7 +1337,9 @@ async function getFunctionArgsSuggestions( .filter((signature) => { if (existingTypes.length) { return existingTypes.every((type, index) => - compareTypesWithLiterals(signature.params[index].type, type) + signature.params[index] + ? compareTypesWithLiterals(signature.params[index].type, type) + : false ); } return true; @@ -1390,7 +1433,7 @@ async function getFunctionArgsSuggestions( } } - if (hasMoreMandatoryArgs) { + if (hasMoreMandatoryArgs || canHaveMoreArgs) { // suggest a comma if there's another argument for the function suggestions.push(commaCompleteItem); } diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts index 65f6601c51c13..6f80396c933af 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts @@ -7,9 +7,11 @@ */ import type { ESQLAstItem, ESQLCommand, ESQLFunction, ESQLSource } from '@kbn/esql-ast'; +import { uniqBy } from 'lodash'; import type { FunctionDefinition } from '../definitions/types'; import { getFunctionDefinition, isAssignment, isFunctionItem } from '../shared/helpers'; import type { SuggestionRawDefinition } from './types'; +import { compareTypesWithLiterals } from '../shared/esql_types'; function extractFunctionArgs(args: ESQLAstItem[]): ESQLFunction[] { return args.flatMap((arg) => (isAssignment(arg) ? arg.args[1] : arg)).filter(isFunctionItem); @@ -92,3 +94,59 @@ export function getSupportedTypesForBinaryOperators( .map(({ params }) => params[1].type) : [previousType]; } + +// @TODO: +// Filter down to signatures that match every params up to the current argIndex +// e.g. BUCKET(longField, /) => all signatures with first param as long column type +// or BUCKET(longField, 2, /) => all signatures with (longField, integer, ...) + +export function narrowDownRelevantFunctionSignatures( + fnDefinition: FunctionDefinition, + enrichedArgs: Array< + ESQLAstItem & { + esType: string; + } + >, + argIndex: number +) { + const relevantFuncSignatures = fnDefinition.signatures.filter( + (s) => + s.params?.length >= argIndex && + s.params + .slice(0, argIndex) + .every( + ({ type: esType }, idx) => + esType === enrichedArgs[idx].esType || + compareTypesWithLiterals(esType, enrichedArgs[idx].esType) + ) + ); + return relevantFuncSignatures; +} + +/** + * + * @param fnDefinition + * @param enrichedArgs + * @param argIndex + * @returns + */ +export function narrowDownCompatibleTypesToSuggestNext( + fnDefinition: FunctionDefinition, + enrichedArgs: Array< + ESQLAstItem & { + esType: string; + } + >, + argIndex: number +) { + const relevantFuncSignatures = narrowDownRelevantFunctionSignatures( + fnDefinition, + enrichedArgs, + argIndex + ); + const compatibleTypesToSuggestForArg = uniqBy( + relevantFuncSignatures.map((f) => f.params[argIndex]).filter((d) => d), + (o) => `${o.type}-${o.constantOnly}` + ); + return compatibleTypesToSuggestForArg; +} From 767e253c2274566c38f7ebe2b189fb82bd9ffbe4 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Wed, 14 Aug 2024 12:19:24 -0500 Subject: [PATCH 02/19] TODo: Improve suggestions --- .../src/autocomplete/autocomplete.test.ts | 70 ++++++------------- .../src/autocomplete/autocomplete.ts | 25 ++++++- 2 files changed, 47 insertions(+), 48 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts index 4d446a676fed8..54f296fded6c1 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -792,7 +792,7 @@ describe('autocomplete', () => { // skip bucket and case fn for the moment as it's quite hard to test // if (!['bucket', 'date_extract', 'date_diff', 'case'].includes(fn.name)) { - if (!['bucket', 'date_extract', 'date_diff', 'case'].includes(fn.name)) { + if (['substring'].includes(fn.name)) { // 1. Empty expression // i.e. eval pow( ), round ( ) // const allParamDefs = fn.signatures.map((s) => getParamAtPosition(s, 0)).filter(nonNullable); @@ -849,8 +849,8 @@ describe('autocomplete', () => { // E.g. eval st_distance( ) | st_distance(cartesianPointField, ) | eval st_distance(geoPointField, ) | st_distance(geoPointField, ) const testCase = `${fn.name}(${signature.params .slice(0, i + 1) - .map((p) => fieldNameFromType(p.type)) - .join(',')}${i === 0 ? ',' : ''} )`; + .map((p, i) => `${fieldNameFromType(p.type)},`) + .join('')})`; // Avoid repeated testing of expressions we have previously seen if (testedCases.has(testCase)) { @@ -901,52 +901,28 @@ describe('autocomplete', () => { // @TODO: remove console.log(`--@@requiresMoreArgs`, requiresMoreArgs); + const expected = suggestedConstants?.length + ? suggestedConstants.map((option) => `"${option}"${requiresMoreArgs ? ', ' : ''}`) + : [ + ...getDateLiteralsByFieldType( + getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) + ), + ...getFieldNamesByType( + getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) + ), + ...getFunctionSignaturesByReturnType( + 'eval', + getTypesFromParamDefs(typesToSuggestNext), + { scalar: true }, + undefined, + [fn.name] + ), + ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), + ].map(addCommaIfRequired); // Compose eval expression that matches what params suggest // E.g. from a | eval round() - testSuggestions( - `from a | eval ${testCase}`, - suggestedConstants?.length - ? suggestedConstants.map((option) => `"${option}"${requiresMoreArgs ? ', ' : ''}`) - : [ - ...getDateLiteralsByFieldType( - getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) - ), - ...getFieldNamesByType( - getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) - ), - ...getFunctionSignaturesByReturnType( - 'eval', - getTypesFromParamDefs(typesToSuggestNext), - { scalar: true }, - undefined, - [fn.name] - ), - ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), - ].map(addCommaIfRequired), - ' ' - ); - testSuggestions( - `from a | eval var0 = ${testCase}`, - suggestedConstants?.length - ? suggestedConstants.map((option) => `"${option}"${requiresMoreArgs ? ', ' : ''}`) - : [ - ...getDateLiteralsByFieldType( - getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) - ), - ...getFieldNamesByType( - getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) - ), - ...getFunctionSignaturesByReturnType( - 'eval', - getTypesFromParamDefs(typesToSuggestNext), - { scalar: true }, - undefined, - [fn.name] - ), - ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), - ].map(addCommaIfRequired), - ' ' - ); + testSuggestions(`from a | eval ${testCase}`, expected, ' '); + testSuggestions(`from a | eval var0 = ${testCase}`, expected, ' '); } }); } diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index edee60184dd40..34041c782ff4a 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -1382,6 +1382,14 @@ async function getFunctionArgsSuggestions( ) ); + // @TODO: remove + // @TODO: remove + console.log(`--@@paramDefsWhichSupportFields`, paramDefsWhichSupportFields); + console.log( + `--@@getTypesFromParamDefs(paramDefsWhichSupportFields)`, + getTypesFromParamDefs(paramDefsWhichSupportFields) + ); + // Fields suggestions.push( ...pushItUpInTheList( @@ -1392,6 +1400,21 @@ async function getFunctionArgsSuggestions( true ) ); + // @TODO: remove + console.log( + `--@@getCompatibleFunctionDefinition( + command.name, + option?.name, + getTypesFromParamDefs(paramDefsWhichSupportFields) as string[], + fnToIgnore + )`, + getCompatibleFunctionDefinition( + command.name, + option?.name, + getTypesFromParamDefs(paramDefsWhichSupportFields) as string[], + fnToIgnore + ) + ); // Functions suggestions.push( @@ -1433,7 +1456,7 @@ async function getFunctionArgsSuggestions( } } - if (hasMoreMandatoryArgs || canHaveMoreArgs) { + if (hasMoreMandatoryArgs) { // suggest a comma if there's another argument for the function suggestions.push(commaCompleteItem); } From de8435f385e1bb6d030863beba7f2d7a4d5fab9c Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Fri, 16 Aug 2024 10:06:17 -0500 Subject: [PATCH 03/19] Add new test file --- .../autocomplete.suggest.eval.test.ts | 536 ++++++++++++++++++ .../src/autocomplete/__tests__/helpers.ts | 1 + .../src/autocomplete/autocomplete.test.ts | 531 +---------------- .../src/autocomplete/autocomplete.ts | 31 +- 4 files changed, 542 insertions(+), 557 deletions(-) create mode 100644 packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts new file mode 100644 index 0000000000000..2a0430f953102 --- /dev/null +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts @@ -0,0 +1,536 @@ +/* + * 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 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 or the Server + * Side Public License, v 1. + */ + +import { + setup, + getFunctionSignaturesByReturnType, + getFieldNamesByType, + createCustomCallbackMocks, + roundParameterTypes, + getLiteralsByType, + PartialSuggestionWithText, + getDateLiteralsByFieldType, +} from './helpers'; +import { ESQL_COMMON_NUMERIC_TYPES } from '../../shared/esql_types'; +import { evalFunctionDefinitions } from '../../definitions/functions'; +import { timeUnitsToSuggest } from '../../definitions/literals'; +import { getUnitDuration } from '../factories'; +import { getParamAtPosition } from '../helper'; +import { nonNullable } from '../../shared/helpers'; +import { partition } from 'lodash'; +import { + FunctionParameter, + SupportedDataType, + isFieldType, + isSupportedDataType, +} from '../../definitions/types'; + +describe('autocomplete.suggest', () => { + describe('eval', () => { + test('suggestions', async () => { + const { assertSuggestions } = await setup(); + await assertSuggestions('from a | eval /', [ + 'var0 = ', + ...getFieldNamesByType('any'), + ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), + ]); + await assertSuggestions('from a | eval doubleField /', [ + ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ + 'double', + ]), + ',', + '| ', + ]); + await assertSuggestions('from index | EVAL keywordField not /', [ + 'LIKE $0', + 'RLIKE $0', + 'IN $0', + ]); + await assertSuggestions('from index | EVAL keywordField NOT /', [ + 'LIKE $0', + 'RLIKE $0', + 'IN $0', + ]); + await assertSuggestions('from index | EVAL doubleField in /', ['( $0 )']); + await assertSuggestions( + 'from index | EVAL doubleField in (/)', + [ + ...getFieldNamesByType('double').filter((name) => name !== 'doubleField'), + ...getFunctionSignaturesByReturnType('eval', 'double', { scalar: true }), + ], + { triggerCharacter: '(' } + ); + await assertSuggestions('from index | EVAL doubleField not in /', ['( $0 )']); + await assertSuggestions('from index | EVAL not /', [ + ...getFieldNamesByType('boolean'), + ...getFunctionSignaturesByReturnType('eval', 'boolean', { scalar: true }), + ]); + await assertSuggestions( + 'from a | eval a=/', + [...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true })], + { triggerCharacter: '=' } + ); + await assertSuggestions( + 'from a | eval a=abs(doubleField), b= /', + [...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true })], + { triggerCharacter: '=' } + ); + await assertSuggestions('from a | eval a=doubleField, /', [ + 'var0 = ', + ...getFieldNamesByType('any'), + 'a', + ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), + ]); + await assertSuggestions( + 'from a | eval a=round(/)', + [ + ...getFieldNamesByType(roundParameterTypes), + ...getFunctionSignaturesByReturnType( + 'eval', + roundParameterTypes, + { scalar: true }, + undefined, + ['round'] + ), + ], + { triggerCharacter: '(' } + ); + await assertSuggestions( + 'from a | eval a=raund(/)', // note the typo in round + [], + { triggerCharacter: '(' } + ); + await assertSuggestions( + 'from a | eval a=raund(/', // note the typo in round + [] + ); + await assertSuggestions( + 'from a | eval raund(/', // note the typo in round + [] + ); + await assertSuggestions( + 'from a | eval raund(5, /', // note the typo in round + [], + { triggerCharacter: '(' } + ); + await assertSuggestions( + 'from a | eval var0 = raund(5, /', // note the typo in round + [], + { triggerCharacter: '(' } + ); + await assertSuggestions('from a | eval a=round(doubleField) /', [ + ',', + '| ', + ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ + 'double', + ]), + ]); + await assertSuggestions( + 'from a | eval a=round(doubleField, /', + [ + ...getFieldNamesByType('integer'), + ...getFunctionSignaturesByReturnType('eval', 'integer', { scalar: true }, undefined, [ + 'round', + ]), + ], + { triggerCharacter: '(' } + ); + await assertSuggestions( + 'from a | eval round(doubleField, /', + [ + ...getFieldNamesByType('integer'), + ...getFunctionSignaturesByReturnType('eval', 'integer', { scalar: true }, undefined, [ + 'round', + ]), + ], + { triggerCharacter: '(' } + ); + await assertSuggestions('from a | eval a=round(doubleField),/', [ + 'var0 = ', + ...getFieldNamesByType('any'), + 'a', + ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), + ]); + await assertSuggestions('from a | eval a=round(doubleField) + /', [ + ...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES), + ...getFunctionSignaturesByReturnType('eval', ESQL_COMMON_NUMERIC_TYPES, { + scalar: true, + }), + ]); + await assertSuggestions('from a | eval a=round(doubleField)+ /', [ + ...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES), + ...getFunctionSignaturesByReturnType('eval', ESQL_COMMON_NUMERIC_TYPES, { + scalar: true, + }), + ]); + await assertSuggestions('from a | eval a=doubleField+ /', [ + ...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES), + ...getFunctionSignaturesByReturnType('eval', ESQL_COMMON_NUMERIC_TYPES, { + scalar: true, + }), + ]); + await assertSuggestions('from a | eval a=`any#Char$Field`+ /', [ + ...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES), + ...getFunctionSignaturesByReturnType('eval', ESQL_COMMON_NUMERIC_TYPES, { + scalar: true, + }), + ]); + await assertSuggestions( + 'from a | stats avg(doubleField) by keywordField | eval /', + [ + 'var0 = ', + '`avg(doubleField)`', + ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), + ], + { + triggerCharacter: ' ', + // make aware EVAL of the previous STATS command + callbacks: createCustomCallbackMocks([], undefined, undefined), + } + ); + await assertSuggestions( + 'from a | eval abs(doubleField) + 1 | eval /', + [ + 'var0 = ', + ...getFieldNamesByType('any'), + '`abs(doubleField) + 1`', + ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), + ], + { triggerCharacter: ' ' } + ); + await assertSuggestions( + 'from a | stats avg(doubleField) by keywordField | eval /', + [ + 'var0 = ', + '`avg(doubleField)`', + ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), + ], + { + triggerCharacter: ' ', + callbacks: createCustomCallbackMocks( + [{ name: 'avg_doubleField_', type: 'double' }], + undefined, + undefined + ), + } + // make aware EVAL of the previous STATS command with the buggy field name from expression + ); + await assertSuggestions( + 'from a | stats avg(doubleField), avg(kubernetes.something.something) by keywordField | eval /', + [ + 'var0 = ', + '`avg(doubleField)`', + '`avg(kubernetes.something.something)`', + ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), + ], + { + triggerCharacter: ' ', + // make aware EVAL of the previous STATS command with the buggy field name from expression + callbacks: createCustomCallbackMocks( + [{ name: 'avg_doubleField_', type: 'double' }], + undefined, + undefined + ), + } + ); + + await assertSuggestions( + 'from a | eval a=round(doubleField), b=round(/)', + [ + ...getFieldNamesByType(roundParameterTypes), + ...getFunctionSignaturesByReturnType( + 'eval', + roundParameterTypes, + { scalar: true }, + undefined, + ['round'] + ), + ], + { triggerCharacter: '(' } + ); + // test that comma is correctly added to the suggestions if minParams is not reached yet + await assertSuggestions('from a | eval a=concat( /', [ + ...getFieldNamesByType(['text', 'keyword']).map((v) => `${v}, `), + ...getFunctionSignaturesByReturnType( + 'eval', + ['text', 'keyword'], + { scalar: true }, + undefined, + ['concat'] + ).map((v) => ({ ...v, text: `${v.text},` })), + ]); + await assertSuggestions( + 'from a | eval a=concat(textField, /', + [ + ...getFieldNamesByType(['text', 'keyword']), + ...getFunctionSignaturesByReturnType( + 'eval', + ['text', 'keyword'], + { scalar: true }, + undefined, + ['concat'] + ), + ], + { triggerCharacter: ' ' } + ); + // test that the arg type is correct after minParams + await assertSuggestions( + 'from a | eval a=cidr_match(ipField, textField, /', + [ + ...getFieldNamesByType('text'), + ...getFunctionSignaturesByReturnType('eval', 'text', { scalar: true }, undefined, [ + 'cidr_match', + ]), + ], + { triggerCharacter: ' ' } + ); + // test that comma is correctly added to the suggestions if minParams is not reached yet + await assertSuggestions('from a | eval a=cidr_match(/', [ + ...getFieldNamesByType('ip').map((v) => `${v}, `), + ...getFunctionSignaturesByReturnType('eval', 'ip', { scalar: true }, undefined, [ + 'cidr_match', + ]).map((v) => ({ ...v, text: `${v.text},` })), + ]); + await assertSuggestions( + 'from a | eval a=cidr_match(ipField, /', + [ + ...getFieldNamesByType(['text', 'keyword']), + ...getFunctionSignaturesByReturnType( + 'eval', + ['text', 'keyword'], + { scalar: true }, + undefined, + ['cidr_match'] + ), + ], + { triggerCharacter: ' ' } + ); + // test deep function nesting suggestions (and check that the same function is not suggested) + // round(round( + // round(round(round( + // etc... + + for (const nesting of [1, 2, 3, 4]) { + await assertSuggestions( + `from a | eval a=${Array(nesting).fill('round(/').join('')}`, + [ + ...getFieldNamesByType(roundParameterTypes), + ...getFunctionSignaturesByReturnType( + 'eval', + roundParameterTypes, + { scalar: true }, + undefined, + ['round'] + ), + ], + { triggerCharacter: '(' } + ); + } + + const absParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; + + // Smoke testing for suggestions in previous position than the end of the statement + await assertSuggestions('from a | eval var0 = abs(doubleField) / | eval abs(var0)', [ + ',', + '| ', + ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ + 'double', + ]), + ]); + await assertSuggestions('from a | eval var0 = abs(b/) | eval abs(var0)', [ + ...getFieldNamesByType(absParameterTypes), + ...getFunctionSignaturesByReturnType( + 'eval', + absParameterTypes, + { scalar: true }, + undefined, + ['abs'] + ), + ]); + }); + + describe('eval functions', () => { + // // Test suggestions for each possible param, within each signature variation, for each function + for (const fn of evalFunctionDefinitions) { + // skip this fn for the moment as it's quite hard to test + if (!['bucket', 'date_extract', 'date_diff', 'case'].includes(fn.name)) { + test(`${fn.name}`, async () => { + const { assertSuggestions } = await setup(); + + for (const signature of fn.signatures) { + for (const [i, param] of signature.params.entries()) { + if (i < signature.params.length) { + // This ref signature thing is probably wrong in a few cases, but it matches + // the logic in getFunctionArgsSuggestions. They should both be updated + const refSignature = fn.signatures[0]; + const requiresMoreArgs = + i + 1 < (refSignature.minParams ?? 0) || + refSignature.params.filter(({ optional }, j) => !optional && j > i).length > 0; + + const allParamDefs = fn.signatures + .map((s) => getParamAtPosition(s, i)) + .filter(nonNullable); + + // get all possible types for this param + const [constantOnlyParamDefs, acceptsFieldParamDefs] = partition( + allParamDefs, + (p) => p.constantOnly || /_literal/.test(p.type as string) + ); + + const getTypesFromParamDefs = ( + paramDefs: FunctionParameter[] + ): SupportedDataType[] => + Array.from(new Set(paramDefs.map((p) => p.type))).filter(isSupportedDataType); + + const suggestedConstants = param.literalSuggestions || param.literalOptions; + + const addCommaIfRequired = (s: string | PartialSuggestionWithText) => { + // don't add commas to the empty string or if there are no more required args + if (!requiresMoreArgs || s === '' || (typeof s === 'object' && s.text === '')) { + return s; + } + return typeof s === 'string' ? `${s}, ` : { ...s, text: `${s.text},` }; + }; + + await assertSuggestions( + `from a | eval ${fn.name}(${Array(i).fill('field').join(', ')}${ + i ? ',' : '' + } /)`, + suggestedConstants?.length + ? suggestedConstants.map( + (option) => `"${option}"${requiresMoreArgs ? ', ' : ''}` + ) + : [ + ...getDateLiteralsByFieldType( + getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) + ), + ...getFieldNamesByType( + getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) + ), + ...getFunctionSignaturesByReturnType( + 'eval', + getTypesFromParamDefs(acceptsFieldParamDefs), + { scalar: true }, + undefined, + [fn.name] + ), + ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), + ].map(addCommaIfRequired), + { triggerCharacter: ' ' } + ); + await assertSuggestions( + `from a | eval var0 = ${fn.name}(${Array(i).fill('field').join(', ')}${ + i ? ',' : '' + } /)`, + suggestedConstants?.length + ? suggestedConstants.map( + (option) => `"${option}"${requiresMoreArgs ? ', ' : ''}` + ) + : [ + ...getDateLiteralsByFieldType( + getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) + ), + ...getFieldNamesByType( + getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) + ), + ...getFunctionSignaturesByReturnType( + 'eval', + getTypesFromParamDefs(acceptsFieldParamDefs), + { scalar: true }, + undefined, + [fn.name] + ), + ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), + ].map(addCommaIfRequired), + { triggerCharacter: ' ' } + ); + } + } + } + }); + } + + // @TODO + // The above test fails cause it expects nested functions like + // DATE_EXTRACT(concat("aligned_day_","of_week_in_month"), date) to also be suggested + // which is actually valid according to func signature + // but currently, our autocomplete only suggests the literal suggestions + if (['date_extract', 'date_diff'].includes(fn.name)) { + test(`${fn.name}`, async () => { + const { assertSuggestions } = await setup(); + const firstParam = fn.signatures[0].params[0]; + const suggestedConstants = firstParam?.literalSuggestions || firstParam?.literalOptions; + const requiresMoreArgs = true; + + await assertSuggestions( + `from a | eval ${fn.name}(/`, + suggestedConstants?.length + ? [ + ...suggestedConstants.map( + (option) => `"${option}"${requiresMoreArgs ? ', ' : ''}` + ), + ] + : [] + ); + }); + } + } + }); + + test('date math', async () => { + const { assertSuggestions } = await setup(); + const dateSuggestions = timeUnitsToSuggest.map(({ name }) => name); + + await assertSuggestions('from a | eval var0 = bucket(@timestamp, /', getUnitDuration(1), { + triggerCharacter: ' ', + }); + + // If a literal number is detected then suggest also date period keywords + await assertSuggestions( + 'from a | eval a = 1 /', + [ + ...dateSuggestions, + ',', + '| ', + ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ + 'integer', + ]), + ], + { triggerCharacter: ' ' } + ); + await assertSuggestions('from a | eval a = 1 year /', [ + ',', + '| ', + ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ + 'time_interval', + ]), + ]); + await assertSuggestions('from a | eval a = 1 day + 2 /', [',', '| ']); + await assertSuggestions( + 'from a | eval 1 day + 2 /', + [ + ...dateSuggestions, + ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ + 'integer', + ]), + ], + { triggerCharacter: ' ' } + ); + await assertSuggestions( + 'from a | eval var0=date_trunc(/)', + getLiteralsByType('time_literal').map((t) => `${t}, `), + { triggerCharacter: '(' } + ); + await assertSuggestions( + 'from a | eval var0=date_trunc(2 /)', + [...dateSuggestions.map((t) => `${t}, `), ','], + { triggerCharacter: ' ' } + ); + }); + }); +}); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts index a05e8059d1308..0774dcd29ddab 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts @@ -344,3 +344,4 @@ export const setup = async (caret = '/') => { assertSuggestions, }; }; +export const roundParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts index 78f2cd5fb5688..7e55c2722bc81 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -10,28 +10,13 @@ import { suggest } from './autocomplete'; import { evalFunctionDefinitions } from '../definitions/functions'; import { timeUnitsToSuggest } from '../definitions/literals'; import { commandDefinitions as unmodifiedCommandDefinitions } from '../definitions/commands'; -import { - getSafeInsertText, - getUnitDuration, - TIME_SYSTEM_PARAMS, - TRIGGER_SUGGESTION_COMMAND, -} from './factories'; -import { camelCase, partition } from 'lodash'; -import { ESQLAstItem, getAstAndSyntaxErrors } from '@kbn/esql-ast'; -import { - FunctionParameter, - isFieldType, - isSupportedDataType, - SupportedDataType, -} from '../definitions/types'; -import { getParamAtPosition, narrowDownCompatibleTypesToSuggestNext } from './helper'; -import { nonNullable } from '../shared/helpers'; +import { getSafeInsertText, TIME_SYSTEM_PARAMS, TRIGGER_SUGGESTION_COMMAND } from './factories'; +import { camelCase } from 'lodash'; +import { getAstAndSyntaxErrors } from '@kbn/esql-ast'; import { policies, getFunctionSignaturesByReturnType, getFieldNamesByType, - getLiteralsByType, - getDateLiteralsByFieldType, createCustomCallbackMocks, createCompletionContext, getPolicyFields, @@ -41,7 +26,6 @@ import { } from './__tests__/helpers'; import { METADATA_FIELDS } from '../shared/constants'; import { ESQL_COMMON_NUMERIC_TYPES, ESQL_STRING_TYPES } from '../shared/esql_types'; -import { fieldNameFromType } from '../validation/validation.test'; const roundParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; const powParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; @@ -428,515 +412,6 @@ describe('autocomplete', () => { } }); - describe('eval', () => { - testSuggestions('from a | eval /', [ - 'var0 = ', - ...getFieldNamesByType('any'), - ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), - ]); - testSuggestions('from a | eval doubleField /', [ - ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ - 'double', - ]), - ',', - '| ', - ]); - testSuggestions('from index | EVAL keywordField not /', ['LIKE $0', 'RLIKE $0', 'IN $0']); - testSuggestions('from index | EVAL keywordField NOT /', ['LIKE $0', 'RLIKE $0', 'IN $0']); - testSuggestions('from index | EVAL doubleField in /', ['( $0 )']); - testSuggestions( - 'from index | EVAL doubleField in (/)', - [ - ...getFieldNamesByType('double').filter((name) => name !== 'doubleField'), - ...getFunctionSignaturesByReturnType('eval', 'double', { scalar: true }), - ], - '(' - ); - testSuggestions('from index | EVAL doubleField not in /', ['( $0 )']); - testSuggestions('from index | EVAL not /', [ - ...getFieldNamesByType('boolean'), - ...getFunctionSignaturesByReturnType('eval', 'boolean', { scalar: true }), - ]); - testSuggestions( - 'from a | eval a=/', - [...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true })], - '=' - ); - testSuggestions( - 'from a | eval a=abs(doubleField), b= /', - [...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true })], - '=' - ); - testSuggestions('from a | eval a=doubleField, /', [ - 'var0 = ', - ...getFieldNamesByType('any'), - 'a', - ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), - ]); - testSuggestions( - 'from a | eval a=round(/)', - [ - ...getFieldNamesByType(roundParameterTypes), - ...getFunctionSignaturesByReturnType( - 'eval', - roundParameterTypes, - { scalar: true }, - undefined, - ['round'] - ), - ], - '(' - ); - testSuggestions( - 'from a | eval a=raund(/)', // note the typo in round - [], - '(' - ); - testSuggestions( - 'from a | eval a=raund(/', // note the typo in round - [] - ); - testSuggestions( - 'from a | eval raund(/', // note the typo in round - [] - ); - testSuggestions( - 'from a | eval raund(5, /', // note the typo in round - [], - ' ' - ); - testSuggestions( - 'from a | eval var0 = raund(5, /', // note the typo in round - [], - ' ' - ); - testSuggestions('from a | eval a=round(doubleField) /', [ - ',', - '| ', - ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ - 'double', - ]), - ]); - testSuggestions( - 'from a | eval a=round(doubleField, /', - [ - ...getFieldNamesByType('integer'), - ...getFunctionSignaturesByReturnType('eval', 'integer', { scalar: true }, undefined, [ - 'round', - ]), - ], - ' ' - ); - testSuggestions( - 'from a | eval round(doubleField, /', - [ - ...getFieldNamesByType('integer'), - ...getFunctionSignaturesByReturnType('eval', 'integer', { scalar: true }, undefined, [ - 'round', - ]), - ], - ' ' - ); - testSuggestions('from a | eval a=round(doubleField),/', [ - 'var0 = ', - ...getFieldNamesByType('any'), - 'a', - ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), - ]); - testSuggestions('from a | eval a=round(doubleField) + /', [ - ...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES), - ...getFunctionSignaturesByReturnType('eval', ESQL_COMMON_NUMERIC_TYPES, { - scalar: true, - }), - ]); - testSuggestions('from a | eval a=round(doubleField)+ /', [ - ...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES), - ...getFunctionSignaturesByReturnType('eval', ESQL_COMMON_NUMERIC_TYPES, { - scalar: true, - }), - ]); - testSuggestions('from a | eval a=doubleField+ /', [ - ...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES), - ...getFunctionSignaturesByReturnType('eval', ESQL_COMMON_NUMERIC_TYPES, { - scalar: true, - }), - ]); - testSuggestions('from a | eval a=`any#Char$Field`+ /', [ - ...getFieldNamesByType(ESQL_COMMON_NUMERIC_TYPES), - ...getFunctionSignaturesByReturnType('eval', ESQL_COMMON_NUMERIC_TYPES, { - scalar: true, - }), - ]); - testSuggestions( - 'from a | stats avg(doubleField) by keywordField | eval /', - [ - 'var0 = ', - '`avg(doubleField)`', - ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), - ], - ' ', - // make aware EVAL of the previous STATS command - [[], undefined, undefined] - ); - testSuggestions( - 'from a | eval abs(doubleField) + 1 | eval /', - [ - 'var0 = ', - ...getFieldNamesByType('any'), - '`abs(doubleField) + 1`', - ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), - ], - ' ' - ); - testSuggestions( - 'from a | stats avg(doubleField) by keywordField | eval /', - [ - 'var0 = ', - '`avg(doubleField)`', - ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), - ], - ' ', - // make aware EVAL of the previous STATS command with the buggy field name from expression - [[{ name: 'avg_doubleField_', type: 'double' }], undefined, undefined] - ); - testSuggestions( - 'from a | stats avg(doubleField), avg(kubernetes.something.something) by keywordField | eval /', - [ - 'var0 = ', - '`avg(doubleField)`', - '`avg(kubernetes.something.something)`', - ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), - ], - ' ', - // make aware EVAL of the previous STATS command with the buggy field name from expression - [ - [ - { name: 'avg_doubleField_', type: 'double' }, - { name: 'avg_kubernetes.something.something_', type: 'double' }, - ], - undefined, - undefined, - ] - ); - - testSuggestions( - 'from a | eval a=round(doubleField), b=round(/)', - [ - ...getFieldNamesByType(roundParameterTypes), - ...getFunctionSignaturesByReturnType( - 'eval', - roundParameterTypes, - { scalar: true }, - undefined, - ['round'] - ), - ], - '(' - ); - // test that comma is correctly added to the suggestions if minParams is not reached yet - testSuggestions('from a | eval a=concat( /', [ - ...getFieldNamesByType(['text', 'keyword']).map((v) => `${v}, `), - ...getFunctionSignaturesByReturnType( - 'eval', - ['text', 'keyword'], - { scalar: true }, - undefined, - ['concat'] - ).map((v) => ({ ...v, text: `${v.text},` })), - ]); - testSuggestions( - 'from a | eval a=concat(textField, /', - [ - ...getFieldNamesByType(['text', 'keyword']), - ...getFunctionSignaturesByReturnType( - 'eval', - ['text', 'keyword'], - { scalar: true }, - undefined, - ['concat'] - ), - ], - ' ' - ); - // test that the arg type is correct after minParams - testSuggestions( - 'from a | eval a=cidr_match(ipField, textField, /', - [ - ...getFieldNamesByType('text'), - ...getFunctionSignaturesByReturnType('eval', 'text', { scalar: true }, undefined, [ - 'cidr_match', - ]), - ], - ' ' - ); - // test that comma is correctly added to the suggestions if minParams is not reached yet - testSuggestions('from a | eval a=cidr_match(/', [ - ...getFieldNamesByType('ip').map((v) => `${v}, `), - ...getFunctionSignaturesByReturnType('eval', 'ip', { scalar: true }, undefined, [ - 'cidr_match', - ]).map((v) => ({ ...v, text: `${v.text},` })), - ]); - testSuggestions( - 'from a | eval a=cidr_match(ipField, /', - [ - ...getFieldNamesByType(['text', 'keyword']), - ...getFunctionSignaturesByReturnType( - 'eval', - ['text', 'keyword'], - { scalar: true }, - undefined, - ['cidr_match'] - ), - ], - ' ' - ); - // test deep function nesting suggestions (and check that the same function is not suggested) - // round(round( - // round(round(round( - // etc... - - for (const nesting of [1, 2, 3, 4]) { - testSuggestions( - `from a | eval a=${Array(nesting).fill('round(/').join('')}`, - [ - ...getFieldNamesByType(roundParameterTypes), - ...getFunctionSignaturesByReturnType( - 'eval', - roundParameterTypes, - { scalar: true }, - undefined, - ['round'] - ), - ], - '(' - ); - } - - const absParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; - - // Smoke testing for suggestions in previous position than the end of the statement - testSuggestions('from a | eval var0 = abs(doubleField) / | eval abs(var0)', [ - ',', - '| ', - ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ - 'double', - ]), - ]); - testSuggestions('from a | eval var0 = abs(b/) | eval abs(var0)', [ - ...getFieldNamesByType(absParameterTypes), - ...getFunctionSignaturesByReturnType('eval', absParameterTypes, { scalar: true }, undefined, [ - 'abs', - ]), - ]); - - // Test suggestions for each possible param, within each signature variation, for each function - for (const fn of evalFunctionDefinitions) { - const testedCases = new Set(); - // skip bucket and case fn for the moment as it's quite hard to test - // if (!['bucket', 'date_extract', 'date_diff', 'case'].includes(fn.name)) { - - if (['substring'].includes(fn.name)) { - // 1. Empty expression - // i.e. eval pow( ), round ( ) - // const allParamDefs = fn.signatures.map((s) => getParamAtPosition(s, 0)).filter(nonNullable); - // // get all possible types for this param - // const [constantOnlyParamDefs, acceptsFieldParamDefs] = partition( - // allParamDefs, - // (p) => p.constantOnly || /_literal/.test(p.type as string) - // ); - // const getTypesFromParamDefs = (paramDefs: FunctionParameter[]): SupportedDataType[] => - // Array.from(new Set(paramDefs.map((p) => p.type))).filter(isSupportedDataType); - // const addCommaIfRequired = (s: string | PartialSuggestionWithText) => { - // // don't add commas to the empty string or if there are no more required args - // if (s === '' || (typeof s === 'object' && s.text === '')) { - // return s; - // } - // return typeof s === 'string' ? `${s}, ` : { ...s, text: `${s.text},` }; - // }; - - // testSuggestions( - // `from a | eval ${fn.name}( `, - // [ - // ...getDateLiteralsByFieldType( - // getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) - // ), - // ...getFieldNamesByType( - // getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) - // ), - // ...getFunctionSignaturesByReturnType( - // 'eval', - // getTypesFromParamDefs(acceptsFieldParamDefs), - // { scalar: true }, - // undefined, - // [fn.name] - // ), - // ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), - // ].map(addCommaIfRequired) - // ); - - for (let signatureIndex = 0; signatureIndex < fn.signatures.length; signatureIndex++) { - const signature = fn.signatures[signatureIndex]; - // @ts-ignore Partial - const enrichedArgs: Array< - ESQLAstItem & { - esType: string; - } - > = signature.params.map(({ type }) => ({ - type: 'column', - esType: type, - })); - - // @todo: add for empty case - signature.params.forEach((param, i) => { - // Generate all possible cases from the signature param - // E.g. eval st_distance( ) | st_distance(cartesianPointField, ) | eval st_distance(geoPointField, ) | st_distance(geoPointField, ) - const testCase = `${fn.name}(${signature.params - .slice(0, i + 1) - .map((p, i) => `${fieldNameFromType(p.type)},`) - .join('')})`; - - // Avoid repeated testing of expressions we have previously seen - if (testedCases.has(testCase)) { - return; - } else { - testedCases.add(testCase); - } - if (i < signature.params.length) { - const typesToSuggestNext = narrowDownCompatibleTypesToSuggestNext( - fn, - enrichedArgs, - i + 1 - ); - const requiresMoreArgs = typesToSuggestNext.length - 1 > 0; - - const allParamDefs = fn.signatures - .map((s) => getParamAtPosition(s, i)) - .filter(nonNullable); - - // get all possible types for this param - const [constantOnlyParamDefs, acceptsFieldParamDefs] = partition( - allParamDefs, - (p) => p.constantOnly || /_literal/.test(p.type as string) - ); - - const getTypesFromParamDefs = (paramDefs: FunctionParameter[]): SupportedDataType[] => - Array.from(new Set(paramDefs.map((p) => p.type))).filter(isSupportedDataType); - - const suggestedConstants = param.literalSuggestions || param.literalOptions; - - const addCommaIfRequired = (s: string | PartialSuggestionWithText) => { - if (!requiresMoreArgs || s === '' || (typeof s === 'object' && s.text === '')) { - return s; - } - return typeof s === 'string' ? `${s}, ` : { ...s, text: `${s.text},` }; - }; - - // @TODO: remove - console.log(`\n--@@from a | eval ${testCase}`); - // @TODO: remove - console.log( - `--@@typesToSuggestNext`, - JSON.stringify( - typesToSuggestNext.map((d) => `${d.type}-${d.optional ? 'optional' : 'required'}`) - ) - ); - - // @TODO: remove - console.log(`--@@requiresMoreArgs`, requiresMoreArgs); - - const expected = suggestedConstants?.length - ? suggestedConstants.map((option) => `"${option}"${requiresMoreArgs ? ', ' : ''}`) - : [ - ...getDateLiteralsByFieldType( - getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) - ), - ...getFieldNamesByType( - getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) - ), - ...getFunctionSignaturesByReturnType( - 'eval', - getTypesFromParamDefs(typesToSuggestNext), - { scalar: true }, - undefined, - [fn.name] - ), - ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), - ].map(addCommaIfRequired); - // Compose eval expression that matches what params suggest - // E.g. from a | eval round() - testSuggestions(`from a | eval ${testCase}`, expected, ' '); - testSuggestions(`from a | eval var0 = ${testCase}`, expected, ' '); - } - }); - } - } - - // The above test fails cause it expects nested functions like - // DATE_EXTRACT(concat("aligned_day_","of_week_in_month"), date) to also be suggested - // which is actually valid according to func signature - // but currently, our autocomplete only suggests the literal suggestions - if (['date_extract', 'date_diff'].includes(fn.name)) { - const firstParam = fn.signatures[0].params[0]; - const suggestedConstants = firstParam?.literalSuggestions || firstParam?.literalOptions; - const requiresMoreArgs = true; - - testSuggestions( - `from a | eval ${fn.name}(/`, - suggestedConstants?.length - ? [...suggestedConstants.map((option) => `"${option}"${requiresMoreArgs ? ', ' : ''}`)] - : [] - ); - } - } - - testSuggestions('from a | eval var0 = bucket(@timestamp, /', getUnitDuration(1), ' '); - - describe('date math', () => { - const dateSuggestions = timeUnitsToSuggest.map(({ name }) => name); - // If a literal number is detected then suggest also date period keywords - testSuggestions( - 'from a | eval a = 1 /', - [ - ...dateSuggestions, - ',', - '| ', - ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ - 'integer', - ]), - ], - ' ' - ); - testSuggestions('from a | eval a = 1 year /', [ - ',', - '| ', - ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ - 'time_interval', - ]), - ]); - testSuggestions('from a | eval a = 1 day + 2 /', [',', '| ']); - testSuggestions( - 'from a | eval 1 day + 2 /', - [ - ...dateSuggestions, - ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ - 'integer', - ]), - ], - ' ' - ); - testSuggestions( - 'from a | eval var0=date_trunc(/)', - getLiteralsByType('time_literal').map((t) => `${t}, `), - '(' - ); - testSuggestions( - 'from a | eval var0=date_trunc(2 /)', - [...dateSuggestions.map((t) => `${t}, `), ','], - ' ' - ); - }); - }); - describe('values suggestions', () => { testSuggestions('FROM "a/"', ['a ', 'b '], undefined, [ , diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index e106295ab6ae8..5eba5c34080e2 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -1224,16 +1224,13 @@ async function getFunctionArgsSuggestions( relevantFuncSignatures.map((f) => f.params[argIndex]).filter((d) => d), (o) => `${o.type}-${o.constantOnly}` ); - // @TODO: remove - console.log(`--@@compatibleTypesToSuggestForArg`, compatibleTypesToSuggestForArg); - // @TODO: remove - // console.log(`--@@compatibleTypesToSuggestForArg`, compatibleTypesToSuggestForArg); // the first signature is used as reference // TODO - take into consideration all signatures that match the current args const refSignature = fnDefinition.signatures[0]; - const old_hasMoreMandatoryArgs = + // @TODO: remove + const oldHasMoreMandatoryArgs = (refSignature.params.length >= argIndex && refSignature.params.filter(({ optional }, index) => !optional && index > argIndex).length > 0) || @@ -1381,14 +1378,6 @@ async function getFunctionArgsSuggestions( ) ); - // @TODO: remove - // @TODO: remove - console.log(`--@@paramDefsWhichSupportFields`, paramDefsWhichSupportFields); - console.log( - `--@@getTypesFromParamDefs(paramDefsWhichSupportFields)`, - getTypesFromParamDefs(paramDefsWhichSupportFields) - ); - // Fields suggestions.push( ...pushItUpInTheList( @@ -1399,22 +1388,6 @@ async function getFunctionArgsSuggestions( true ) ); - // @TODO: remove - console.log( - `--@@getCompatibleFunctionDefinition( - command.name, - option?.name, - getTypesFromParamDefs(paramDefsWhichSupportFields) as string[], - fnToIgnore - )`, - getCompatibleFunctionDefinition( - command.name, - option?.name, - getTypesFromParamDefs(paramDefsWhichSupportFields) as string[], - fnToIgnore - ) - ); - // Functions suggestions.push( ...getCompatibleFunctionDefinition( From 47e71608dcb23907e055cb4c603067693cbdb9a2 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Sun, 18 Aug 2024 23:25:09 -0500 Subject: [PATCH 04/19] Fix tests --- .../autocomplete.suggest.eval.test.ts | 258 +++++++++++------- .../src/autocomplete/autocomplete.test.ts | 1 - .../src/autocomplete/autocomplete.ts | 67 +---- .../src/shared/esql_types.ts | 5 + 4 files changed, 176 insertions(+), 155 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts index 2a0430f953102..3d096976a106a 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts @@ -20,15 +20,17 @@ import { ESQL_COMMON_NUMERIC_TYPES } from '../../shared/esql_types'; import { evalFunctionDefinitions } from '../../definitions/functions'; import { timeUnitsToSuggest } from '../../definitions/literals'; import { getUnitDuration } from '../factories'; -import { getParamAtPosition } from '../helper'; +import { getParamAtPosition, narrowDownCompatibleTypesToSuggestNext } from '../helper'; import { nonNullable } from '../../shared/helpers'; -import { partition } from 'lodash'; +import { partition, uniq } from 'lodash'; import { FunctionParameter, SupportedDataType, isFieldType, isSupportedDataType, } from '../../definitions/types'; +import { fieldNameFromType } from '../../validation/validation.test'; +import { ESQLAstItem } from '@kbn/esql-ast'; describe('autocomplete.suggest', () => { describe('eval', () => { @@ -354,102 +356,154 @@ describe('autocomplete.suggest', () => { ]); }); - describe('eval functions', () => { + describe.only('eval functions', () => { + const getTypesFromParamDefs = (paramDefs: FunctionParameter[]): SupportedDataType[] => + Array.from(new Set(paramDefs.map((p) => p.type))).filter(isSupportedDataType); + // // Test suggestions for each possible param, within each signature variation, for each function for (const fn of evalFunctionDefinitions) { + // @TODO: add test case eval(fn.name /) + const testedCases = new Set(); // skip this fn for the moment as it's quite hard to test - if (!['bucket', 'date_extract', 'date_diff', 'case'].includes(fn.name)) { + // if (!['bucket', 'date_extract', 'date_diff', 'case'].includes(fn.name)) { + if (['date_trunc'].includes(fn.name)) { test(`${fn.name}`, async () => { const { assertSuggestions } = await setup(); for (const signature of fn.signatures) { + // @ts-ignore Partial + const enrichedArgs: Array< + ESQLAstItem & { + esType: string; + } + > = signature.params.map(({ type }) => ({ + type: 'column', + esType: type, + })); + for (const [i, param] of signature.params.entries()) { - if (i < signature.params.length) { - // This ref signature thing is probably wrong in a few cases, but it matches - // the logic in getFunctionArgsSuggestions. They should both be updated - const refSignature = fn.signatures[0]; - const requiresMoreArgs = - i + 1 < (refSignature.minParams ?? 0) || - refSignature.params.filter(({ optional }, j) => !optional && j > i).length > 0; + const testCase = `${fn.name}(${signature.params + .slice(0, i + 1) + .map((p, i) => + p.type === 'time_literal' ? '1 year,' : `${fieldNameFromType(p.type)}, ` + ) + .join('')} / )`; + + // @TODO: remove + console.log(`--@@testCase`, testCase); + if (testedCases.has(testCase)) { + continue; + } - const allParamDefs = fn.signatures - .map((s) => getParamAtPosition(s, i)) - .filter(nonNullable); + testedCases.add(testCase); - // get all possible types for this param - const [constantOnlyParamDefs, acceptsFieldParamDefs] = partition( - allParamDefs, - (p) => p.constantOnly || /_literal/.test(p.type as string) - ); + const typesToSuggestNext = narrowDownCompatibleTypesToSuggestNext( + fn, + enrichedArgs, + // +1 for where the cursor is + i + 1 + ); + const requiresMoreArgs = + typesToSuggestNext.filter(({ optional }) => !optional).length > 0; + const constantOnlyParamDefs = typesToSuggestNext.filter( + (p) => p.constantOnly || /_literal/.test(p.type as string) + ); - const getTypesFromParamDefs = ( - paramDefs: FunctionParameter[] - ): SupportedDataType[] => - Array.from(new Set(paramDefs.map((p) => p.type))).filter(isSupportedDataType); + const suggestedConstants = uniq( + typesToSuggestNext + .map((d) => d.literalSuggestions || d.literalOptions) + .filter((d) => d) + .flat() + ); - const suggestedConstants = param.literalSuggestions || param.literalOptions; + const addCommaIfRequired = (s: string | PartialSuggestionWithText) => { + if (!requiresMoreArgs || s === '' || (typeof s === 'object' && s.text === '')) { + return s; + } + return typeof s === 'string' ? `${s}, ` : { ...s, text: `${s.text},` }; + }; - const addCommaIfRequired = (s: string | PartialSuggestionWithText) => { - // don't add commas to the empty string or if there are no more required args - if (!requiresMoreArgs || s === '' || (typeof s === 'object' && s.text === '')) { - return s; - } - return typeof s === 'string' ? `${s}, ` : { ...s, text: `${s.text},` }; - }; + const expected = suggestedConstants?.length + ? suggestedConstants.map((option) => `"${option}"${requiresMoreArgs ? ', ' : ''}`) + : [ + ...getDateLiteralsByFieldType( + getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) + ), + ...getFieldNamesByType( + getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) + ), + ...getFunctionSignaturesByReturnType( + 'eval', + getTypesFromParamDefs(typesToSuggestNext), + { scalar: true }, + undefined, + [fn.name] + ), + ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), + ].map(addCommaIfRequired); + await assertSuggestions(`from a | eval var0 = ${testCase}`, expected, { + triggerCharacter: ' ', + }); - await assertSuggestions( - `from a | eval ${fn.name}(${Array(i).fill('field').join(', ')}${ - i ? ',' : '' - } /)`, - suggestedConstants?.length - ? suggestedConstants.map( - (option) => `"${option}"${requiresMoreArgs ? ', ' : ''}` - ) - : [ - ...getDateLiteralsByFieldType( - getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) - ), - ...getFieldNamesByType( - getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) - ), - ...getFunctionSignaturesByReturnType( - 'eval', - getTypesFromParamDefs(acceptsFieldParamDefs), - { scalar: true }, - undefined, - [fn.name] - ), - ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), - ].map(addCommaIfRequired), - { triggerCharacter: ' ' } - ); - await assertSuggestions( - `from a | eval var0 = ${fn.name}(${Array(i).fill('field').join(', ')}${ - i ? ',' : '' - } /)`, - suggestedConstants?.length - ? suggestedConstants.map( - (option) => `"${option}"${requiresMoreArgs ? ', ' : ''}` - ) - : [ - ...getDateLiteralsByFieldType( - getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) - ), - ...getFieldNamesByType( - getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) - ), - ...getFunctionSignaturesByReturnType( - 'eval', - getTypesFromParamDefs(acceptsFieldParamDefs), - { scalar: true }, - undefined, - [fn.name] - ), - ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), - ].map(addCommaIfRequired), - { triggerCharacter: ' ' } - ); - } + // if (i < signature.params.length) { + // // This ref signature thing is probably wrong in a few cases, but it matches + // // the logic in getFunctionArgsSuggestions. They should both be updated + // const refSignature = fn.signatures[0]; + // const requiresMoreArgs = + // i + 1 < (refSignature.minParams ?? 0) || + // refSignature.params.filter(({ optional }, j) => !optional && j > i).length > 0; + + // const allParamDefs = fn.signatures + // .map((s) => getParamAtPosition(s, i)) + // .filter(nonNullable); + + // // get all possible types for this param + // const [constantOnlyParamDefs, acceptsFieldParamDefs] = partition( + // allParamDefs, + // (p) => p.constantOnly || /_literal/.test(p.type as string) + // ); + + // const getTypesFromParamDefs = ( + // paramDefs: FunctionParameter[] + // ): SupportedDataType[] => + // Array.from(new Set(paramDefs.map((p) => p.type))).filter(isSupportedDataType); + + // const suggestedConstants = param.literalSuggestions || param.literalOptions; + + // const addCommaIfRequired = (s: string | PartialSuggestionWithText) => { + // // don't add commas to the empty string or if there are no more required args + // if (!requiresMoreArgs || s === '' || (typeof s === 'object' && s.text === '')) { + // return s; + // } + // return typeof s === 'string' ? `${s}, ` : { ...s, text: `${s.text},` }; + // }; + + // await assertSuggestions( + // `from a | eval ${fn.name}(${Array(i).fill('field').join(', ')}${ + // i ? ',' : '' + // } /)`, + // suggestedConstants?.length + // ? suggestedConstants.map( + // (option) => `"${option}"${requiresMoreArgs ? ', ' : ''}` + // ) + // : [ + // ...getDateLiteralsByFieldType( + // getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) + // ), + // ...getFieldNamesByType( + // getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) + // ), + // ...getFunctionSignaturesByReturnType( + // 'eval', + // getTypesFromParamDefs(acceptsFieldParamDefs), + // { scalar: true }, + // undefined, + // [fn.name] + // ), + // ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), + // ].map(addCommaIfRequired), + // { triggerCharacter: ' ' } + // ); } } }); @@ -460,25 +514,25 @@ describe('autocomplete.suggest', () => { // DATE_EXTRACT(concat("aligned_day_","of_week_in_month"), date) to also be suggested // which is actually valid according to func signature // but currently, our autocomplete only suggests the literal suggestions - if (['date_extract', 'date_diff'].includes(fn.name)) { - test(`${fn.name}`, async () => { - const { assertSuggestions } = await setup(); - const firstParam = fn.signatures[0].params[0]; - const suggestedConstants = firstParam?.literalSuggestions || firstParam?.literalOptions; - const requiresMoreArgs = true; + // if (['date_extract', 'date_diff'].includes(fn.name)) { + // test(`${fn.name}`, async () => { + // const { assertSuggestions } = await setup(); + // const firstParam = fn.signatures[0].params[0]; + // const suggestedConstants = firstParam?.literalSuggestions || firstParam?.literalOptions; + // const requiresMoreArgs = true; - await assertSuggestions( - `from a | eval ${fn.name}(/`, - suggestedConstants?.length - ? [ - ...suggestedConstants.map( - (option) => `"${option}"${requiresMoreArgs ? ', ' : ''}` - ), - ] - : [] - ); - }); - } + // await assertSuggestions( + // `from a | eval ${fn.name}(/`, + // suggestedConstants?.length + // ? [ + // ...suggestedConstants.map( + // (option) => `"${option}"${requiresMoreArgs ? ', ' : ''}` + // ), + // ] + // : [] + // ); + // }); + // } } }); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts index 7e55c2722bc81..d7cb8f1f59b01 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -27,7 +27,6 @@ import { import { METADATA_FIELDS } from '../shared/constants'; import { ESQL_COMMON_NUMERIC_TYPES, ESQL_STRING_TYPES } from '../shared/esql_types'; -const roundParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; const powParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; const log10ParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 5eba5c34080e2..74db1b9425def 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import { uniqBy } from 'lodash'; +import { uniq, uniqBy } from 'lodash'; import type { AstProviderFn, ESQLAstItem, @@ -93,6 +93,7 @@ import { getSourcesFromCommands, getSupportedTypesForBinaryOperators, isAggFunctionUsedAlready, + narrowDownCompatibleTypesToSuggestNext, narrowDownRelevantFunctionSignatures, removeQuoteForSuggestedSources, } from './helper'; @@ -1215,61 +1216,23 @@ async function getFunctionArgsSuggestions( const arg: ESQLAstItem = enrichedArgs[argIndex]; // Retrieve unique of types that are compatiable for the current arg - const relevantFuncSignatures = narrowDownRelevantFunctionSignatures( + const typesToSuggestNext = narrowDownCompatibleTypesToSuggestNext( fnDefinition, enrichedArgs, argIndex ); - const compatibleTypesToSuggestForArg = uniqBy( - relevantFuncSignatures.map((f) => f.params[argIndex]).filter((d) => d), - (o) => `${o.type}-${o.constantOnly}` - ); + const hasMoreMandatoryArgs = typesToSuggestNext.filter(({ optional }) => !optional).length > 0; - // the first signature is used as reference - // TODO - take into consideration all signatures that match the current args - const refSignature = fnDefinition.signatures[0]; - - // @TODO: remove - const oldHasMoreMandatoryArgs = - (refSignature.params.length >= argIndex && - refSignature.params.filter(({ optional }, index) => !optional && index > argIndex).length > - 0) || - ('minParams' in refSignature && refSignature.minParams - ? refSignature.minParams - 1 > argIndex - : false); - - const needMoreArgsChecks = relevantFuncSignatures.map((signature) => { - return ( - (signature.params.length >= argIndex && - signature.params.filter(({ optional }, index) => !optional && index > argIndex).length > - 0) || - ('minParams' in signature && signature.minParams ? signature.minParams - 1 > argIndex : false) - ); - }); - const hasMoreMandatoryArgs = - needMoreArgsChecks.length === 0 ? false : needMoreArgsChecks.some((d) => d === false) === false; - - const canHaveMoreArgs = compatibleTypesToSuggestForArg.length > 0; + // Should prepend comma to suggestion string + // E.g. if true, "fieldName" -> "fieldName, " const shouldAddComma = hasMoreMandatoryArgs && fnDefinition.type !== 'builtin'; - const suggestedConstants = Array.from( - new Set( - fnDefinition.signatures.reduce((acc, signature) => { - const p = signature.params[argIndex]; - if (!p) { - return acc; - } - - const _suggestions: string[] = p.literalSuggestions - ? p.literalSuggestions - : p.literalOptions - ? p.literalOptions - : []; - - return acc.concat(_suggestions); - }, [] as string[]) - ) - ); + const suggestedConstants = uniq( + typesToSuggestNext + .map((d) => d.literalSuggestions || d.literalOptions) + .filter((d) => d) + .flat() + ) as string[]; if (suggestedConstants.length) { return buildValueDefinitions(suggestedConstants, { @@ -1381,7 +1344,7 @@ async function getFunctionArgsSuggestions( // Fields suggestions.push( ...pushItUpInTheList( - await getFieldsByType(getTypesFromParamDefs(paramDefsWhichSupportFields) as string[], [], { + await getFieldsByType(getTypesFromParamDefs(typesToSuggestNext) as string[], [], { addComma: shouldAddComma, advanceCursorAndOpenSuggestions: hasMoreMandatoryArgs, }), @@ -1393,7 +1356,7 @@ async function getFunctionArgsSuggestions( ...getCompatibleFunctionDefinition( command.name, option?.name, - getTypesFromParamDefs(paramDefsWhichSupportFields) as string[], + getTypesFromParamDefs(typesToSuggestNext) as string[], fnToIgnore ).map((suggestion) => ({ ...suggestion, @@ -1403,7 +1366,7 @@ async function getFunctionArgsSuggestions( // could also be in stats (bucket) but our autocomplete is not great yet if ( - getTypesFromParamDefs(paramDefsWhichSupportFields).includes('date') && + getTypesFromParamDefs(typesToSuggestNext).includes('date') && ['where', 'eval'].includes(command.name) ) suggestions.push( diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts b/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts index fada6bea88134..36ea9f408d711 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts @@ -72,5 +72,10 @@ export const compareTypesWithLiterals = ( if (b === 'string') { return isStringType(a); } + if (a === 'time_literal' || a === 'time_duration') return b === 'timeInterval'; + if (b === 'time_literal' || b === 'time_duration') return a === 'timeInterval'; + if (a === 'time_literal') return b === 'time_duration'; + if (b === 'time_literal') return a === 'time_duration'; + return false; }; From 496952c9b2486b214562edf88ff85a74ae48ea5c Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Mon, 19 Aug 2024 16:37:36 -0500 Subject: [PATCH 05/19] Update logic --- .../autocomplete.suggest.eval.test.ts | 46 ++++++--- .../src/autocomplete/__tests__/helpers.ts | 8 +- .../src/autocomplete/autocomplete.ts | 95 +++++++++---------- .../src/autocomplete/helper.ts | 6 +- 4 files changed, 87 insertions(+), 68 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts index 3d096976a106a..fdd8c3f8ba9a7 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts @@ -20,9 +20,12 @@ import { ESQL_COMMON_NUMERIC_TYPES } from '../../shared/esql_types'; import { evalFunctionDefinitions } from '../../definitions/functions'; import { timeUnitsToSuggest } from '../../definitions/literals'; import { getUnitDuration } from '../factories'; -import { getParamAtPosition, narrowDownCompatibleTypesToSuggestNext } from '../helper'; -import { nonNullable } from '../../shared/helpers'; -import { partition, uniq } from 'lodash'; +import { + getParamAtPosition, + getCompatibleTypesToSuggestNext, + getValidFunctionSignaturesForPreviousArgs, +} from '../helper'; +import { uniq } from 'lodash'; import { FunctionParameter, SupportedDataType, @@ -356,7 +359,7 @@ describe('autocomplete.suggest', () => { ]); }); - describe.only('eval functions', () => { + describe('eval functions', () => { const getTypesFromParamDefs = (paramDefs: FunctionParameter[]): SupportedDataType[] => Array.from(new Set(paramDefs.map((p) => p.type))).filter(isSupportedDataType); @@ -366,7 +369,7 @@ describe('autocomplete.suggest', () => { const testedCases = new Set(); // skip this fn for the moment as it's quite hard to test // if (!['bucket', 'date_extract', 'date_diff', 'case'].includes(fn.name)) { - if (['date_trunc'].includes(fn.name)) { + if (!['bucket', 'date_extract', 'date_diff', 'case'].includes(fn.name)) { test(`${fn.name}`, async () => { const { assertSuggestions } = await setup(); @@ -382,29 +385,42 @@ describe('autocomplete.suggest', () => { })); for (const [i, param] of signature.params.entries()) { + if (param.type === 'time_duration') { + continue; + } const testCase = `${fn.name}(${signature.params .slice(0, i + 1) - .map((p, i) => + .map((p) => p.type === 'time_literal' ? '1 year,' : `${fieldNameFromType(p.type)}, ` ) .join('')} / )`; - // @TODO: remove - console.log(`--@@testCase`, testCase); if (testedCases.has(testCase)) { continue; } testedCases.add(testCase); - const typesToSuggestNext = narrowDownCompatibleTypesToSuggestNext( + const validSignatures = getValidFunctionSignaturesForPreviousArgs( fn, enrichedArgs, - // +1 for where the cursor is i + 1 ); - const requiresMoreArgs = - typesToSuggestNext.filter(({ optional }) => !optional).length > 0; + // Retrieve unique of types that are compatiable for the current arg + const typesToSuggestNext = getCompatibleTypesToSuggestNext(fn, enrichedArgs, i + 1); + + const hasMoreMandatoryArgs = !validSignatures + // Types available to suggest next after this argument is completed + .map((sig) => getParamAtPosition(sig, i + 2)) + // when a param is null, it means param is optional + // If there's at least one param that is optional, then + // no need to suggest comma + .some((p) => p === null || p?.optional === true); + + // Wehther to prepend comma to suggestion string + // E.g. if true, "fieldName" -> "fieldName, " + const shouldAddComma = hasMoreMandatoryArgs && fn.type !== 'builtin'; + const constantOnlyParamDefs = typesToSuggestNext.filter( (p) => p.constantOnly || /_literal/.test(p.type as string) ); @@ -417,14 +433,16 @@ describe('autocomplete.suggest', () => { ); const addCommaIfRequired = (s: string | PartialSuggestionWithText) => { - if (!requiresMoreArgs || s === '' || (typeof s === 'object' && s.text === '')) { + if (!shouldAddComma || s === '' || (typeof s === 'object' && s.text === '')) { return s; } return typeof s === 'string' ? `${s}, ` : { ...s, text: `${s.text},` }; }; const expected = suggestedConstants?.length - ? suggestedConstants.map((option) => `"${option}"${requiresMoreArgs ? ', ' : ''}`) + ? suggestedConstants.map( + (option) => `"${option}"${hasMoreMandatoryArgs ? ', ' : ''}` + ) : [ ...getDateLiteralsByFieldType( getTypesFromParamDefs(typesToSuggestNext).filter(isFieldType) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts index 0774dcd29ddab..5e967f3eb3363 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts @@ -326,7 +326,13 @@ export const setup = async (caret = '/') => { .map((suggestion) => (typeof suggestion === 'string' ? suggestion : suggestion.text ?? '')) .sort(); - expect(resultTexts).toEqual(expectedTexts); + try { + expect(resultTexts).toEqual(expectedTexts); + } catch (error) { + // eslint-disable-next-line no-console + console.error(`${query}\nExpected:${expectedTexts}\nResult:${resultTexts}`); + throw error; + } const expectedNonStringSuggestions = expected.filter( (suggestion) => typeof suggestion !== 'string' diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 74db1b9425def..2fb84a540db51 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -16,7 +16,6 @@ import type { ESQLLiteral, ESQLSingleAstItem, } from '@kbn/esql-ast'; -import { partition } from 'lodash'; import { ESQL_NUMBER_TYPES, compareTypesWithLiterals, isNumericType } from '../shared/esql_types'; import type { EditorContext, SuggestionRawDefinition } from './types'; import { @@ -93,8 +92,7 @@ import { getSourcesFromCommands, getSupportedTypesForBinaryOperators, isAggFunctionUsedAlready, - narrowDownCompatibleTypesToSuggestNext, - narrowDownRelevantFunctionSignatures, + getCompatibleTypesToSuggestNext, removeQuoteForSuggestedSources, } from './helper'; import { FunctionParameter, FunctionReturnType, SupportedDataType } from '../definitions/types'; @@ -439,7 +437,7 @@ function areCurrentArgsValid( return true; } -function extractFinalTypeFromArg( +export function extractFinalTypeFromArg( arg: ESQLAstItem, references: Pick ): @@ -1214,16 +1212,40 @@ async function getFunctionArgsSuggestions( } const arg: ESQLAstItem = enrichedArgs[argIndex]; + const existingTypes = node.args + .map((nodeArg) => + extractFinalTypeFromArg(nodeArg, { + fields: fieldsMap, + variables: variablesExcludingCurrentCommandOnes, + }) + ) + .filter(nonNullable); + + const validSignatures = fnDefinition.signatures + // if existing arguments are preset already, use them to filter out incompatible signatures + .filter((signature) => { + if (existingTypes.length) { + return existingTypes.every((type, index) => + signature.params[index] + ? compareTypesWithLiterals(signature.params[index].type, type) + : false + ); + } + return true; + }); // Retrieve unique of types that are compatiable for the current arg - const typesToSuggestNext = narrowDownCompatibleTypesToSuggestNext( - fnDefinition, - enrichedArgs, - argIndex - ); - const hasMoreMandatoryArgs = typesToSuggestNext.filter(({ optional }) => !optional).length > 0; + const typesToSuggestNext = getCompatibleTypesToSuggestNext(fnDefinition, enrichedArgs, argIndex); + + const hasMoreMandatoryArgs = !validSignatures + // Types available to suggest next after this argument is completed + .map((signature) => getParamAtPosition(signature, argIndex + 1)) + // when a param is null, it means param is optional + // If there's at least one param that is optional, then + // no need to suggest comma + .some((p) => p === null || p?.optional === true); - // Should prepend comma to suggestion string + // Wehther to prepend comma to suggestion string // E.g. if true, "fieldName" -> "fieldName, " const shouldAddComma = hasMoreMandatoryArgs && fnDefinition.type !== 'builtin'; @@ -1236,7 +1258,7 @@ async function getFunctionArgsSuggestions( if (suggestedConstants.length) { return buildValueDefinitions(suggestedConstants, { - addComma: hasMoreMandatoryArgs, + addComma: shouldAddComma, advanceCursorAndOpenSuggestions: hasMoreMandatoryArgs, }); } @@ -1281,37 +1303,6 @@ async function getFunctionArgsSuggestions( : []) ); } - - const existingTypes = node.args - .map((nodeArg) => - extractFinalTypeFromArg(nodeArg, { - fields: fieldsMap, - variables: variablesExcludingCurrentCommandOnes, - }) - ) - .filter(nonNullable); - - const validSignatures = fnDefinition.signatures - // if existing arguments are preset already, use them to filter out incompatible signatures - .filter((signature) => { - if (existingTypes.length) { - return existingTypes.every((type, index) => - signature.params[index] - ? compareTypesWithLiterals(signature.params[index].type, type) - : false - ); - } - return true; - }); - - /** - * Get all parameter definitions across all function signatures - * for the current parameter position in the given function definition, - */ - const allParamDefinitionsForThisPosition = validSignatures - .map((signature) => getParamAtPosition(signature, argIndex)) - .filter(nonNullable); - // Separate the param definitions into two groups: // fields should only be suggested if the param isn't constant-only, // and constant suggestions should only be given if it is. @@ -1322,9 +1313,8 @@ async function getFunctionArgsSuggestions( // (e.g. if func1's first parameter is constant-only, any nested functions should // inherit that constraint: func1(func2(shouldBeConstantOnly))) // - const [constantOnlyParamDefs, paramDefsWhichSupportFields] = partition( - allParamDefinitionsForThisPosition, - (paramDef) => paramDef.constantOnly || /_literal$/.test(paramDef.type as string) + const constantOnlyParamDefs = typesToSuggestNext.filter( + (p) => p.constantOnly || /_literal/.test(p.type as string) ); const getTypesFromParamDefs = (paramDefs: FunctionParameter[]) => { @@ -1344,10 +1334,15 @@ async function getFunctionArgsSuggestions( // Fields suggestions.push( ...pushItUpInTheList( - await getFieldsByType(getTypesFromParamDefs(typesToSuggestNext) as string[], [], { - addComma: shouldAddComma, - advanceCursorAndOpenSuggestions: hasMoreMandatoryArgs, - }), + await getFieldsByType( + // @TODO: have a way to better suggest constant only params + getTypesFromParamDefs(typesToSuggestNext.filter((d) => !d.constantOnly)) as string[], + [], + { + addComma: shouldAddComma, + advanceCursorAndOpenSuggestions: hasMoreMandatoryArgs, + } + ), true ) ); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts index 6f80396c933af..d5fd0eb360942 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts @@ -100,7 +100,7 @@ export function getSupportedTypesForBinaryOperators( // e.g. BUCKET(longField, /) => all signatures with first param as long column type // or BUCKET(longField, 2, /) => all signatures with (longField, integer, ...) -export function narrowDownRelevantFunctionSignatures( +export function getValidFunctionSignaturesForPreviousArgs( fnDefinition: FunctionDefinition, enrichedArgs: Array< ESQLAstItem & { @@ -130,7 +130,7 @@ export function narrowDownRelevantFunctionSignatures( * @param argIndex * @returns */ -export function narrowDownCompatibleTypesToSuggestNext( +export function getCompatibleTypesToSuggestNext( fnDefinition: FunctionDefinition, enrichedArgs: Array< ESQLAstItem & { @@ -139,7 +139,7 @@ export function narrowDownCompatibleTypesToSuggestNext( >, argIndex: number ) { - const relevantFuncSignatures = narrowDownRelevantFunctionSignatures( + const relevantFuncSignatures = getValidFunctionSignaturesForPreviousArgs( fnDefinition, enrichedArgs, argIndex From 7a160b53e7e35330f080144590ee27e3720fa3b6 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Tue, 20 Aug 2024 09:46:38 -0500 Subject: [PATCH 06/19] Update logic --- .../autocomplete.command.stats.test.ts | 2 +- .../autocomplete.suggest.eval.test.ts | 6 ++-- .../src/autocomplete/__tests__/helpers.ts | 33 +++++++++---------- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts index cdf4a6239a9ab..d2ca0925acb74 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts @@ -7,6 +7,7 @@ */ import { ESQL_COMMON_NUMERIC_TYPES, ESQL_NUMBER_TYPES } from '../../shared/esql_types'; +import { roundParameterTypes } from './constants'; import { setup, getFunctionSignaturesByReturnType, getFieldNamesByType } from './helpers'; const allAggFunctions = getFunctionSignaturesByReturnType('stats', 'any', { @@ -82,7 +83,6 @@ describe('autocomplete.suggest', () => { scalar: true, }).map((s) => ({ ...s, text: `${s.text},` })), ]); - const roundParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; await assertSuggestions('from a | stats round(/', [ ...getFunctionSignaturesByReturnType('stats', roundParameterTypes, { agg: true, diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts index fdd8c3f8ba9a7..7903fcf28d22b 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts @@ -11,7 +11,6 @@ import { getFunctionSignaturesByReturnType, getFieldNamesByType, createCustomCallbackMocks, - roundParameterTypes, getLiteralsByType, PartialSuggestionWithText, getDateLiteralsByFieldType, @@ -19,7 +18,6 @@ import { import { ESQL_COMMON_NUMERIC_TYPES } from '../../shared/esql_types'; import { evalFunctionDefinitions } from '../../definitions/functions'; import { timeUnitsToSuggest } from '../../definitions/literals'; -import { getUnitDuration } from '../factories'; import { getParamAtPosition, getCompatibleTypesToSuggestNext, @@ -34,6 +32,7 @@ import { } from '../../definitions/types'; import { fieldNameFromType } from '../../validation/validation.test'; import { ESQLAstItem } from '@kbn/esql-ast'; +import { roundParameterTypes } from './constants'; describe('autocomplete.suggest', () => { describe('eval', () => { @@ -558,7 +557,8 @@ describe('autocomplete.suggest', () => { const { assertSuggestions } = await setup(); const dateSuggestions = timeUnitsToSuggest.map(({ name }) => name); - await assertSuggestions('from a | eval var0 = bucket(@timestamp, /', getUnitDuration(1), { + // Eval bucket is not a valid expression + await assertSuggestions('from a | eval var0 = bucket(@timestamp, /', [], { triggerCharacter: ' ', }); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts index 5e967f3eb3363..96cb89c774776 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts @@ -319,29 +319,29 @@ export const setup = async (caret = '/') => { expected: Array, opts?: SuggestOptions ) => { - const result = await suggest(query, opts); - const resultTexts = [...result.map((suggestion) => suggestion.text)].sort(); + try { + const result = await suggest(query, opts); + const resultTexts = [...result.map((suggestion) => suggestion.text)].sort(); - const expectedTexts = expected - .map((suggestion) => (typeof suggestion === 'string' ? suggestion : suggestion.text ?? '')) - .sort(); + const expectedTexts = expected + .map((suggestion) => (typeof suggestion === 'string' ? suggestion : suggestion.text ?? '')) + .sort(); - try { expect(resultTexts).toEqual(expectedTexts); + + const expectedNonStringSuggestions = expected.filter( + (suggestion) => typeof suggestion !== 'string' + ) as PartialSuggestionWithText[]; + + for (const expectedSuggestion of expectedNonStringSuggestions) { + const suggestion = result.find((s) => s.text === expectedSuggestion.text); + expect(suggestion).toEqual(expect.objectContaining(expectedSuggestion)); + } } catch (error) { // eslint-disable-next-line no-console - console.error(`${query}\nExpected:${expectedTexts}\nResult:${resultTexts}`); + console.error(`Failed query\n-------------\n${query}`); throw error; } - - const expectedNonStringSuggestions = expected.filter( - (suggestion) => typeof suggestion !== 'string' - ) as PartialSuggestionWithText[]; - - for (const expectedSuggestion of expectedNonStringSuggestions) { - const suggestion = result.find((s) => s.text === expectedSuggestion.text); - expect(suggestion).toEqual(expect.objectContaining(expectedSuggestion)); - } }; return { @@ -350,4 +350,3 @@ export const setup = async (caret = '/') => { assertSuggestions, }; }; -export const roundParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; From f8ccdec8502c42f0bab55f56555a1c3978a0ce26 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Tue, 20 Aug 2024 11:27:29 -0500 Subject: [PATCH 07/19] Update --- .../autocomplete.suggest.eval.test.ts | 17 +++------- .../src/autocomplete/__tests__/constants.ts | 11 ++++++ .../src/autocomplete/autocomplete.ts | 34 +++++-------------- .../src/autocomplete/helper.ts | 7 ++++ 4 files changed, 32 insertions(+), 37 deletions(-) create mode 100644 packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/constants.ts diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts index 7903fcf28d22b..5c442ee7dea1e 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts @@ -19,9 +19,9 @@ import { ESQL_COMMON_NUMERIC_TYPES } from '../../shared/esql_types'; import { evalFunctionDefinitions } from '../../definitions/functions'; import { timeUnitsToSuggest } from '../../definitions/literals'; import { - getParamAtPosition, getCompatibleTypesToSuggestNext, getValidFunctionSignaturesForPreviousArgs, + strictlyGetParamAtPosition, } from '../helper'; import { uniq } from 'lodash'; import { @@ -283,16 +283,9 @@ describe('autocomplete.suggest', () => { { triggerCharacter: ' ' } ); // test that the arg type is correct after minParams - await assertSuggestions( - 'from a | eval a=cidr_match(ipField, textField, /', - [ - ...getFieldNamesByType('text'), - ...getFunctionSignaturesByReturnType('eval', 'text', { scalar: true }, undefined, [ - 'cidr_match', - ]), - ], - { triggerCharacter: ' ' } - ); + await assertSuggestions('from a | eval a=cidr_match(ipField, textField, /', [], { + triggerCharacter: ' ', + }); // test that comma is correctly added to the suggestions if minParams is not reached yet await assertSuggestions('from a | eval a=cidr_match(/', [ ...getFieldNamesByType('ip').map((v) => `${v}, `), @@ -410,7 +403,7 @@ describe('autocomplete.suggest', () => { const hasMoreMandatoryArgs = !validSignatures // Types available to suggest next after this argument is completed - .map((sig) => getParamAtPosition(sig, i + 2)) + .map((sig) => strictlyGetParamAtPosition(sig, i + 2)) // when a param is null, it means param is optional // If there's at least one param that is optional, then // no need to suggest comma diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/constants.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/constants.ts new file mode 100644 index 0000000000000..10079be511da9 --- /dev/null +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/constants.ts @@ -0,0 +1,11 @@ +/* + * 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 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 or the Server + * Side Public License, v 1. + */ + +export const roundParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; +export const powParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; +export const log10ParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 2fb84a540db51..0253460997f78 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -16,7 +16,7 @@ import type { ESQLLiteral, ESQLSingleAstItem, } from '@kbn/esql-ast'; -import { ESQL_NUMBER_TYPES, compareTypesWithLiterals, isNumericType } from '../shared/esql_types'; +import { ESQL_NUMBER_TYPES, isNumericType } from '../shared/esql_types'; import type { EditorContext, SuggestionRawDefinition } from './types'; import { lookupColumn, @@ -87,13 +87,14 @@ import { import { ESQLCallbacks } from '../shared/types'; import { getFunctionsToIgnoreForStats, - getParamAtPosition, getQueryForFields, getSourcesFromCommands, getSupportedTypesForBinaryOperators, isAggFunctionUsedAlready, getCompatibleTypesToSuggestNext, removeQuoteForSuggestedSources, + getValidFunctionSignaturesForPreviousArgs, + strictlyGetParamAtPosition, } from './helper'; import { FunctionParameter, FunctionReturnType, SupportedDataType } from '../definitions/types'; @@ -1212,34 +1213,17 @@ async function getFunctionArgsSuggestions( } const arg: ESQLAstItem = enrichedArgs[argIndex]; - const existingTypes = node.args - .map((nodeArg) => - extractFinalTypeFromArg(nodeArg, { - fields: fieldsMap, - variables: variablesExcludingCurrentCommandOnes, - }) - ) - .filter(nonNullable); - - const validSignatures = fnDefinition.signatures - // if existing arguments are preset already, use them to filter out incompatible signatures - .filter((signature) => { - if (existingTypes.length) { - return existingTypes.every((type, index) => - signature.params[index] - ? compareTypesWithLiterals(signature.params[index].type, type) - : false - ); - } - return true; - }); + const validSignatures = getValidFunctionSignaturesForPreviousArgs( + fnDefinition, + enrichedArgs, + argIndex + ); // Retrieve unique of types that are compatiable for the current arg const typesToSuggestNext = getCompatibleTypesToSuggestNext(fnDefinition, enrichedArgs, argIndex); - const hasMoreMandatoryArgs = !validSignatures // Types available to suggest next after this argument is completed - .map((signature) => getParamAtPosition(signature, argIndex + 1)) + .map((signature) => strictlyGetParamAtPosition(signature, argIndex + 1)) // when a param is null, it means param is optional // If there's at least one param that is optional, then // no need to suggest comma diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts index d5fd0eb360942..ef140153b7c07 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts @@ -59,6 +59,13 @@ export function getParamAtPosition( return params.length > position ? params[position] : minParams ? params[params.length - 1] : null; } +export function strictlyGetParamAtPosition( + { params, minParams }: FunctionDefinition['signatures'][number], + position: number +) { + return params[position] ? params[position] : null; +} + export function getQueryForFields(queryString: string, commands: ESQLCommand[]) { // If there is only one source command and it does not require fields, do not // fetch fields, hence return an empty string. From 2780de7d9a38c39bf980ddeaa25795dc0553263b Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Tue, 20 Aug 2024 11:52:54 -0500 Subject: [PATCH 08/19] Revert cookieAuth change --- .../core-http-server-internal/src/cookie_session_storage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/http/core-http-server-internal/src/cookie_session_storage.ts b/packages/core/http/core-http-server-internal/src/cookie_session_storage.ts index f006f658d5bbc..7962e0b40777a 100644 --- a/packages/core/http/core-http-server-internal/src/cookie_session_storage.ts +++ b/packages/core/http/core-http-server-internal/src/cookie_session_storage.ts @@ -56,7 +56,7 @@ class ScopedCookieSessionStorage implements SessionStorage } public clear() { - return this.request.cookieAuth?.clear(); + return this.request.cookieAuth.clear(); } } From 928f8aab0fa74504c0a808c18fb27a0f814dd610 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Tue, 20 Aug 2024 14:04:31 -0500 Subject: [PATCH 09/19] Add better suggestion for named parameters --- .../src/autocomplete/autocomplete.ts | 16 ++++++++++++---- .../src/autocomplete/helper.ts | 13 ++++++------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index ef8e01e1b006d..41912168ce14d 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -75,6 +75,7 @@ import { buildValueDefinitions, getDateLiterals, buildFieldsDefinitionsWithMetadata, + TIME_SYSTEM_PARAMS, } from './factories'; import { EDITOR_MARKER, SINGLE_BACKTICK, METADATA_FIELDS } from '../shared/constants'; import { getAstContext, removeMarkerArgFromArgsList } from '../shared/context'; @@ -1229,7 +1230,13 @@ async function getFunctionArgsSuggestions( }; const enrichedArgs = node.args.map((nodeArg) => { - const esType = extractFinalTypeFromArg(nodeArg, references); + let esType = extractFinalTypeFromArg(nodeArg, references); + + // For named system time parameters ?start and ?end, make sure it's compatiable + if (isLiteralItem(nodeArg) && TIME_SYSTEM_PARAMS.includes(nodeArg.text)) { + esType = 'date'; + } + return { ...nodeArg, esType } as ESQLAstItem & { esType: string }; }); @@ -1379,8 +1386,10 @@ async function getFunctionArgsSuggestions( // could also be in stats (bucket) but our autocomplete is not great yet if ( - getTypesFromParamDefs(typesToSuggestNext).includes('date') && - ['where', 'eval'].includes(command.name) + (getTypesFromParamDefs(typesToSuggestNext).includes('date') && + ['where', 'eval'].includes(command.name)) || + (command.name === 'stats' && + typesToSuggestNext.some((t) => t && t.type === 'date' && t.constantOnly === true)) ) suggestions.push( ...getDateLiterals({ @@ -1389,7 +1398,6 @@ async function getFunctionArgsSuggestions( }) ); } - // for eval and row commands try also to complete numeric literals with time intervals where possible if (arg) { if (command.name !== 'stats') { diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts index a28632a54be40..51efd32441647 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts @@ -117,13 +117,12 @@ export function getValidFunctionSignaturesForPreviousArgs( const relevantFuncSignatures = fnDefinition.signatures.filter( (s) => s.params?.length >= argIndex && - s.params - .slice(0, argIndex) - .every( - ({ type: esType }, idx) => - esType === enrichedArgs[idx].esType || - compareTypesWithLiterals(esType, enrichedArgs[idx].esType) - ) + s.params.slice(0, argIndex).every(({ type: esType }, idx) => { + return ( + esType === enrichedArgs[idx].esType || + compareTypesWithLiterals(esType, enrichedArgs[idx].esType) + ); + }) ); return relevantFuncSignatures; } From b291d1a56e5b33fafccd028127c0bf00711975c7 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Tue, 20 Aug 2024 14:57:20 -0500 Subject: [PATCH 10/19] Update tests --- .../autocomplete.suggest.eval.test.ts | 124 ++++++------------ 1 file changed, 38 insertions(+), 86 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts index 5c442ee7dea1e..0c290a9a2cb0a 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts @@ -357,16 +357,16 @@ describe('autocomplete.suggest', () => { // // Test suggestions for each possible param, within each signature variation, for each function for (const fn of evalFunctionDefinitions) { - // @TODO: add test case eval(fn.name /) - const testedCases = new Set(); // skip this fn for the moment as it's quite hard to test // if (!['bucket', 'date_extract', 'date_diff', 'case'].includes(fn.name)) { if (!['bucket', 'date_extract', 'date_diff', 'case'].includes(fn.name)) { test(`${fn.name}`, async () => { + const testedCases = new Set(); + const { assertSuggestions } = await setup(); for (const signature of fn.signatures) { - // @ts-ignore Partial + // @ts-expect-error Partial type const enrichedArgs: Array< ESQLAstItem & { esType: string; @@ -376,21 +376,29 @@ describe('autocomplete.suggest', () => { esType: type, })); - for (const [i, param] of signature.params.entries()) { - if (param.type === 'time_duration') { + // Starting at -1 to include empty case e.g. to_string( / ) + for (let i = -1; i < signature.params.length; i++) { + const param = signature.params[i]; + if (param?.type === 'time_duration') { continue; } const testCase = `${fn.name}(${signature.params .slice(0, i + 1) .map((p) => - p.type === 'time_literal' ? '1 year,' : `${fieldNameFromType(p.type)}, ` + p.type === 'time_literal' + ? '1 year,' + : `${ + typeof p.type === 'string' && isFieldType(p.type) + ? fieldNameFromType(p.type) + : 'field' + }, ` ) .join('')} / )`; + // Avoid duplicate test cases that might start with first params that are exactly the same if (testedCases.has(testCase)) { continue; } - testedCases.add(testCase); const validSignatures = getValidFunctionSignaturesForPreviousArgs( @@ -451,98 +459,42 @@ describe('autocomplete.suggest', () => { ), ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), ].map(addCommaIfRequired); - await assertSuggestions(`from a | eval var0 = ${testCase}`, expected, { + + await assertSuggestions(`from a | eval ${testCase}`, expected, { triggerCharacter: ' ', }); - // if (i < signature.params.length) { - // // This ref signature thing is probably wrong in a few cases, but it matches - // // the logic in getFunctionArgsSuggestions. They should both be updated - // const refSignature = fn.signatures[0]; - // const requiresMoreArgs = - // i + 1 < (refSignature.minParams ?? 0) || - // refSignature.params.filter(({ optional }, j) => !optional && j > i).length > 0; - - // const allParamDefs = fn.signatures - // .map((s) => getParamAtPosition(s, i)) - // .filter(nonNullable); - - // // get all possible types for this param - // const [constantOnlyParamDefs, acceptsFieldParamDefs] = partition( - // allParamDefs, - // (p) => p.constantOnly || /_literal/.test(p.type as string) - // ); - - // const getTypesFromParamDefs = ( - // paramDefs: FunctionParameter[] - // ): SupportedDataType[] => - // Array.from(new Set(paramDefs.map((p) => p.type))).filter(isSupportedDataType); - - // const suggestedConstants = param.literalSuggestions || param.literalOptions; - - // const addCommaIfRequired = (s: string | PartialSuggestionWithText) => { - // // don't add commas to the empty string or if there are no more required args - // if (!requiresMoreArgs || s === '' || (typeof s === 'object' && s.text === '')) { - // return s; - // } - // return typeof s === 'string' ? `${s}, ` : { ...s, text: `${s.text},` }; - // }; - - // await assertSuggestions( - // `from a | eval ${fn.name}(${Array(i).fill('field').join(', ')}${ - // i ? ',' : '' - // } /)`, - // suggestedConstants?.length - // ? suggestedConstants.map( - // (option) => `"${option}"${requiresMoreArgs ? ', ' : ''}` - // ) - // : [ - // ...getDateLiteralsByFieldType( - // getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) - // ), - // ...getFieldNamesByType( - // getTypesFromParamDefs(acceptsFieldParamDefs).filter(isFieldType) - // ), - // ...getFunctionSignaturesByReturnType( - // 'eval', - // getTypesFromParamDefs(acceptsFieldParamDefs), - // { scalar: true }, - // undefined, - // [fn.name] - // ), - // ...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)), - // ].map(addCommaIfRequired), - // { triggerCharacter: ' ' } - // ); + await assertSuggestions(`from a | eval var0 = ${testCase}`, expected, { + triggerCharacter: ' ', + }); } } }); } - // @TODO // The above test fails cause it expects nested functions like // DATE_EXTRACT(concat("aligned_day_","of_week_in_month"), date) to also be suggested // which is actually valid according to func signature // but currently, our autocomplete only suggests the literal suggestions - // if (['date_extract', 'date_diff'].includes(fn.name)) { - // test(`${fn.name}`, async () => { - // const { assertSuggestions } = await setup(); - // const firstParam = fn.signatures[0].params[0]; - // const suggestedConstants = firstParam?.literalSuggestions || firstParam?.literalOptions; - // const requiresMoreArgs = true; + if (['date_extract', 'date_diff'].includes(fn.name)) { + test(`${fn.name}`, async () => { + const { assertSuggestions } = await setup(); + const firstParam = fn.signatures[0].params[0]; + const suggestedConstants = firstParam?.literalSuggestions || firstParam?.literalOptions; + const requiresMoreArgs = true; - // await assertSuggestions( - // `from a | eval ${fn.name}(/`, - // suggestedConstants?.length - // ? [ - // ...suggestedConstants.map( - // (option) => `"${option}"${requiresMoreArgs ? ', ' : ''}` - // ), - // ] - // : [] - // ); - // }); - // } + await assertSuggestions( + `from a | eval ${fn.name}(/`, + suggestedConstants?.length + ? [ + ...suggestedConstants.map( + (option) => `"${option}"${requiresMoreArgs ? ', ' : ''}` + ), + ] + : [] + ); + }); + } } }); From 347ab091edb9e0bc6c936198407babd1776b8194 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Tue, 20 Aug 2024 15:05:29 -0500 Subject: [PATCH 11/19] Move around imports --- .../src/autocomplete/autocomplete.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts index b7e10871110dc..66073feaf7e19 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -26,9 +26,7 @@ import { } from './__tests__/helpers'; import { METADATA_FIELDS } from '../shared/constants'; import { ESQL_COMMON_NUMERIC_TYPES, ESQL_STRING_TYPES } from '../shared/esql_types'; - -const powParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; -const log10ParameterTypes = ['double', 'integer', 'long', 'unsigned_long'] as const; +import { log10ParameterTypes, powParameterTypes } from './__tests__/constants'; const commandDefinitions = unmodifiedCommandDefinitions.filter(({ hidden }) => !hidden); describe('autocomplete', () => { From 8a27505abd2e4b960f31a48262af37ff2991355c Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Fri, 23 Aug 2024 13:47:26 -0500 Subject: [PATCH 12/19] Update tests --- .../autocomplete.suggest.eval.test.ts | 22 +- .../esql_validation_meta_tests.json | 301 ++++++++++++++++++ 2 files changed, 312 insertions(+), 11 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts index 0c290a9a2cb0a..50ed43cf79b4d 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts @@ -26,14 +26,21 @@ import { import { uniq } from 'lodash'; import { FunctionParameter, + FunctionReturnType, SupportedDataType, isFieldType, + isReturnType, isSupportedDataType, } from '../../definitions/types'; import { fieldNameFromType } from '../../validation/validation.test'; import { ESQLAstItem } from '@kbn/esql-ast'; import { roundParameterTypes } from './constants'; +const getTypesFromParamDefs = (paramDefs: FunctionParameter[]): SupportedDataType[] => + Array.from(new Set(paramDefs.map((p) => p.type))).filter( + isSupportedDataType + ) as SupportedDataType[]; + describe('autocomplete.suggest', () => { describe('eval', () => { test('suggestions', async () => { @@ -352,9 +359,6 @@ describe('autocomplete.suggest', () => { }); describe('eval functions', () => { - const getTypesFromParamDefs = (paramDefs: FunctionParameter[]): SupportedDataType[] => - Array.from(new Set(paramDefs.map((p) => p.type))).filter(isSupportedDataType); - // // Test suggestions for each possible param, within each signature variation, for each function for (const fn of evalFunctionDefinitions) { // skip this fn for the moment as it's quite hard to test @@ -452,7 +456,9 @@ describe('autocomplete.suggest', () => { ), ...getFunctionSignaturesByReturnType( 'eval', - getTypesFromParamDefs(typesToSuggestNext), + getTypesFromParamDefs(typesToSuggestNext).filter( + isReturnType + ) as FunctionReturnType[], { scalar: true }, undefined, [fn.name] @@ -520,13 +526,7 @@ describe('autocomplete.suggest', () => { ], { triggerCharacter: ' ' } ); - await assertSuggestions('from a | eval a = 1 year /', [ - ',', - '| ', - ...getFunctionSignaturesByReturnType('eval', 'any', { builtin: true, skipAssign: true }, [ - 'time_interval', - ]), - ]); + await assertSuggestions('from a | eval a = 1 year /', [',', '| ', 'IS NOT NULL', 'IS NULL']); await assertSuggestions('from a | eval a = 1 day + 2 /', [',', '| ']); await assertSuggestions( 'from a | eval 1 day + 2 /', diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json b/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json index d290bd04b64e5..e787c48abc68d 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json +++ b/packages/kbn-esql-validation-autocomplete/src/validation/esql_validation_meta_tests.json @@ -37368,6 +37368,307 @@ "error": [], "warning": [] }, + { + "query": "row var = mv_percentile(5.5, 5.5)", + "error": [], + "warning": [] + }, + { + "query": "row mv_percentile(5.5, 5.5)", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_percentile(to_double(true), to_double(true))", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_percentile(5.5, 5)", + "error": [], + "warning": [] + }, + { + "query": "row mv_percentile(5.5, 5)", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_percentile(to_double(true), to_integer(true))", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_percentile(to_double(true), 5)", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_percentile(5, 5.5)", + "error": [], + "warning": [] + }, + { + "query": "row mv_percentile(5, 5.5)", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_percentile(to_integer(true), to_double(true))", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_percentile(5, 5)", + "error": [], + "warning": [] + }, + { + "query": "row mv_percentile(5, 5)", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_percentile(to_integer(true), to_integer(true))", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_percentile(to_integer(true), 5)", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_percentile(5, to_double(true))", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_percentile(5, to_integer(true))", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_percentile(true, true)", + "error": [ + "Argument of [mv_percentile] must be [double], found value [true] type [boolean]", + "Argument of [mv_percentile] must be [double], found value [true] type [boolean]" + ], + "warning": [] + }, + { + "query": "from a_index | where mv_percentile(doubleField, doubleField) > 0", + "error": [], + "warning": [] + }, + { + "query": "from a_index | where mv_percentile(booleanField, booleanField) > 0", + "error": [ + "Argument of [mv_percentile] must be [double], found value [booleanField] type [boolean]", + "Argument of [mv_percentile] must be [double], found value [booleanField] type [boolean]" + ], + "warning": [] + }, + { + "query": "from a_index | where mv_percentile(doubleField, integerField) > 0", + "error": [], + "warning": [] + }, + { + "query": "from a_index | where mv_percentile(doubleField, longField) > 0", + "error": [], + "warning": [] + }, + { + "query": "from a_index | where mv_percentile(integerField, doubleField) > 0", + "error": [], + "warning": [] + }, + { + "query": "from a_index | where mv_percentile(integerField, integerField) > 0", + "error": [], + "warning": [] + }, + { + "query": "from a_index | where mv_percentile(integerField, longField) > 0", + "error": [], + "warning": [] + }, + { + "query": "from a_index | where mv_percentile(longField, doubleField) > 0", + "error": [], + "warning": [] + }, + { + "query": "from a_index | where mv_percentile(longField, integerField) > 0", + "error": [], + "warning": [] + }, + { + "query": "from a_index | where mv_percentile(longField, longField) > 0", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(doubleField, doubleField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_percentile(doubleField, doubleField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(to_double(booleanField), to_double(booleanField))", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_percentile(booleanField, booleanField)", + "error": [ + "Argument of [mv_percentile] must be [double], found value [booleanField] type [boolean]", + "Argument of [mv_percentile] must be [double], found value [booleanField] type [boolean]" + ], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(doubleField, integerField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_percentile(doubleField, integerField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(to_double(booleanField), to_integer(booleanField))", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(doubleField, longField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_percentile(doubleField, longField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(to_double(booleanField), longField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(integerField, doubleField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_percentile(integerField, doubleField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(to_integer(booleanField), to_double(booleanField))", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(integerField, integerField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_percentile(integerField, integerField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(to_integer(booleanField), to_integer(booleanField))", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(integerField, longField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_percentile(integerField, longField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(to_integer(booleanField), longField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(longField, doubleField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_percentile(longField, doubleField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(longField, to_double(booleanField))", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(longField, integerField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_percentile(longField, integerField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(longField, to_integer(booleanField))", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval var = mv_percentile(longField, longField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_percentile(longField, longField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_percentile(doubleField, doubleField, extraArg)", + "error": [ + "Error: [mv_percentile] function expects exactly 2 arguments, got 3." + ], + "warning": [] + }, + { + "query": "from a_index | sort mv_percentile(doubleField, doubleField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_percentile(null, null)", + "error": [], + "warning": [] + }, + { + "query": "row nullVar = null | eval mv_percentile(nullVar, nullVar)", + "error": [], + "warning": [] + }, { "query": "f", "error": [ From abe8c3e4d5abc4bc1ed4554c9cb702b199b07418 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Wed, 28 Aug 2024 15:25:29 -0500 Subject: [PATCH 13/19] Fix date picker string --- .../src/autocomplete/autocomplete.ts | 4 +-- .../src/autocomplete/helper.ts | 29 ++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index f28fda7649265..61f94c952e603 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -77,7 +77,6 @@ import { getDateLiterals, buildFieldsDefinitionsWithMetadata, TRIGGER_SUGGESTION_COMMAND, - TIME_SYSTEM_PARAMS, } from './factories'; import { EDITOR_MARKER, SINGLE_BACKTICK, METADATA_FIELDS } from '../shared/constants'; import { getAstContext, removeMarkerArgFromArgsList } from '../shared/context'; @@ -99,6 +98,7 @@ import { removeQuoteForSuggestedSources, getValidFunctionSignaturesForPreviousArgs, strictlyGetParamAtPosition, + isLiteralDateItem, } from './helper'; import { FunctionParameter, @@ -1275,7 +1275,7 @@ async function getFunctionArgsSuggestions( let esType = extractFinalTypeFromArg(nodeArg, references); // For named system time parameters ?start and ?end, make sure it's compatiable - if (isLiteralItem(nodeArg) && TIME_SYSTEM_PARAMS.includes(nodeArg.text)) { + if (isLiteralDateItem(nodeArg)) { esType = 'date'; } diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts index 51efd32441647..a41cdbafbe97c 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts @@ -9,9 +9,15 @@ import type { ESQLAstItem, ESQLCommand, ESQLFunction, ESQLSource } from '@kbn/esql-ast'; import { uniqBy } from 'lodash'; import type { FunctionDefinition } from '../definitions/types'; -import { getFunctionDefinition, isAssignment, isFunctionItem } from '../shared/helpers'; +import { + getFunctionDefinition, + isAssignment, + isFunctionItem, + isLiteralItem, +} from '../shared/helpers'; import type { SuggestionRawDefinition } from './types'; import { compareTypesWithLiterals } from '../shared/esql_types'; +import { TIME_SYSTEM_PARAMS } from './factories'; function extractFunctionArgs(args: ESQLAstItem[]): ESQLFunction[] { return args.flatMap((arg) => (isAssignment(arg) ? arg.args[1] : arg)).filter(isFunctionItem); @@ -193,3 +199,24 @@ export function getOverlapRange( end: query.length, }; } + +function isValidDateString(dateString: string): boolean { + const timestamp = Date.parse(dateString.replace(/\"/g, '')); + return !isNaN(timestamp); +} + +/** + * Returns true is node is a valid literal that represents a date + * either a system time parameter or a date string generated by date picker + * @param dateString + * @returns + */ +export function isLiteralDateItem(nodeArg: ESQLAstItem): boolean { + return ( + isLiteralItem(nodeArg) && + // If text is ?start or ?end, it's a system time parameter + (TIME_SYSTEM_PARAMS.includes(nodeArg.text) || + // Or if it's a string generated by date picker + isValidDateString(nodeArg.text)) + ); +} From a0a67d8af7f81ce410e5996ced11e0baa4aa2909 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Wed, 28 Aug 2024 16:00:41 -0500 Subject: [PATCH 14/19] Update types, comments, add clarity --- .../autocomplete.suggest.eval.test.ts | 4 +-- .../src/autocomplete/autocomplete.ts | 32 +++++++++---------- .../src/autocomplete/helper.ts | 29 +++++++++++------ .../src/shared/esql_types.ts | 6 ++++ 4 files changed, 44 insertions(+), 27 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts index 50ed43cf79b4d..a73109a9ad3dc 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.suggest.eval.test.ts @@ -373,11 +373,11 @@ describe('autocomplete.suggest', () => { // @ts-expect-error Partial type const enrichedArgs: Array< ESQLAstItem & { - esType: string; + dataType: string; } > = signature.params.map(({ type }) => ({ type: 'column', - esType: type, + dataType: type, })); // Starting at -1 to include empty case e.g. to_string( / ) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 61f94c952e603..a70ace1c412c9 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -432,7 +432,7 @@ function areCurrentArgsValid( return false; } else { return ( - extractFinalTypeFromArg(node, references) === + extractTypeFromASTArg(node, references) === getCommandDefinition(command.name).signature.params[0].type ); } @@ -448,7 +448,7 @@ function areCurrentArgsValid( return true; } -export function extractFinalTypeFromArg( +export function extractTypeFromASTArg( arg: ESQLAstItem, references: Pick ): @@ -459,7 +459,7 @@ export function extractFinalTypeFromArg( | string // @TODO remove this | undefined { if (Array.isArray(arg)) { - return extractFinalTypeFromArg(arg[0], references); + return extractTypeFromASTArg(arg[0], references); } if (isColumnItem(arg) || isLiteralItem(arg)) { if (isLiteralItem(arg)) { @@ -512,7 +512,7 @@ function isFunctionArgComplete( } const hasCorrectTypes = fnDefinition.signatures.some((def) => { return arg.args.every((a, index) => { - return def.params[index].type === extractFinalTypeFromArg(a, references); + return def.params[index].type === extractTypeFromASTArg(a, references); }); }); if (!hasCorrectTypes) { @@ -748,7 +748,7 @@ async function getExpressionSuggestionsByType( if (isColumnItem(nodeArg)) { // ... | STATS a // ... | EVAL a - const nodeArgType = extractFinalTypeFromArg(nodeArg, references); + const nodeArgType = extractTypeFromASTArg(nodeArg, references); if (isParameterType(nodeArgType)) { suggestions.push( ...getBuiltinCompatibleFunctionDefinition( @@ -802,7 +802,7 @@ async function getExpressionSuggestionsByType( if (!isNewExpression) { if (isAssignment(nodeArg) && isAssignmentComplete(nodeArg)) { const [rightArg] = nodeArg.args[1] as [ESQLSingleAstItem]; - const nodeArgType = extractFinalTypeFromArg(rightArg, references); + const nodeArgType = extractTypeFromASTArg(rightArg, references); suggestions.push( ...getBuiltinCompatibleFunctionDefinition( command.name, @@ -819,7 +819,7 @@ async function getExpressionSuggestionsByType( if (isFunctionItem(rightArg)) { if (rightArg.args.some(isTimeIntervalItem)) { const lastFnArg = rightArg.args[rightArg.args.length - 1]; - const lastFnArgType = extractFinalTypeFromArg(lastFnArg, references); + const lastFnArgType = extractTypeFromASTArg(lastFnArg, references); if (isNumericType(lastFnArgType) && isLiteralItem(lastFnArg)) // ... EVAL var = 1 year + 2 suggestions.push(...getCompatibleLiterals(command.name, ['time_literal_unit'])); @@ -842,7 +842,7 @@ async function getExpressionSuggestionsByType( )) ); } else { - const nodeArgType = extractFinalTypeFromArg(nodeArg, references); + const nodeArgType = extractTypeFromASTArg(nodeArg, references); suggestions.push( ...(await getBuiltinFunctionNextArgument( innerText, @@ -857,7 +857,7 @@ async function getExpressionSuggestionsByType( ); if (nodeArg.args.some(isTimeIntervalItem)) { const lastFnArg = nodeArg.args[nodeArg.args.length - 1]; - const lastFnArgType = extractFinalTypeFromArg(lastFnArg, references); + const lastFnArgType = extractTypeFromASTArg(lastFnArg, references); if (isNumericType(lastFnArgType) && isLiteralItem(lastFnArg)) // ... EVAL var = 1 year + 2 suggestions.push(...getCompatibleLiterals(command.name, ['time_literal_unit'])); @@ -914,7 +914,7 @@ async function getExpressionSuggestionsByType( } } else { // if something is already present, leverage its type to suggest something in context - const nodeArgType = extractFinalTypeFromArg(nodeArg, references); + const nodeArgType = extractTypeFromASTArg(nodeArg, references); // These cases can happen here, so need to identify each and provide the right suggestion // i.e. ... | field // i.e. ... | field + @@ -1085,7 +1085,7 @@ async function getBuiltinFunctionNextArgument( // pick the last arg and check its type to verify whether is incomplete for the given function const cleanedArgs = removeMarkerArgFromArgsList(nodeArg)!.args; - const nestedType = extractFinalTypeFromArg(nodeArg.args[cleanedArgs.length - 1], references); + const nestedType = extractTypeFromASTArg(nodeArg.args[cleanedArgs.length - 1], references); if (isFnComplete.reason === 'fewArgs') { const fnDef = getFunctionDefinition(nodeArg.name); @@ -1272,14 +1272,14 @@ async function getFunctionArgsSuggestions( }; const enrichedArgs = node.args.map((nodeArg) => { - let esType = extractFinalTypeFromArg(nodeArg, references); + let dataType = extractTypeFromASTArg(nodeArg, references); // For named system time parameters ?start and ?end, make sure it's compatiable if (isLiteralDateItem(nodeArg)) { - esType = 'date'; + dataType = 'date'; } - return { ...nodeArg, esType } as ESQLAstItem & { esType: string }; + return { ...nodeArg, dataType } as ESQLAstItem & { dataType: string }; }); const variablesExcludingCurrentCommandOnes = excludeVariablesFromCurrentCommand( @@ -1492,7 +1492,7 @@ async function getListArgsSuggestions( }); const [firstArg] = node.args; if (isColumnItem(firstArg)) { - const argType = extractFinalTypeFromArg(firstArg, { + const argType = extractTypeFromASTArg(firstArg, { fields: fieldsMap, variables: anyVariables, }); @@ -1692,7 +1692,7 @@ async function getOptionArgsSuggestions( if (command.name === 'stats') { const argDef = optionDef?.signature.params[argIndex]; - const nodeArgType = extractFinalTypeFromArg(nodeArg, references); + const nodeArgType = extractTypeFromASTArg(nodeArg, references); // These cases can happen here, so need to identify each and provide the right suggestion // i.e. ... | STATS ... BY field + // i.e. ... | STATS ... BY field >= diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts index a41cdbafbe97c..ab85dcc353d9f 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/helper.ts @@ -65,8 +65,15 @@ export function getParamAtPosition( return params.length > position ? params[position] : minParams ? params[params.length - 1] : null; } +/** + * Given a function signature, returns the parameter at the given position, even if it's undefined or null + * + * @param {params} + * @param position + * @returns + */ export function strictlyGetParamAtPosition( - { params, minParams }: FunctionDefinition['signatures'][number], + { params }: FunctionDefinition['signatures'][number], position: number ) { return params[position] ? params[position] : null; @@ -112,7 +119,7 @@ export function getValidFunctionSignaturesForPreviousArgs( fnDefinition: FunctionDefinition, enrichedArgs: Array< ESQLAstItem & { - esType: string; + dataType: string; } >, argIndex: number @@ -123,10 +130,10 @@ export function getValidFunctionSignaturesForPreviousArgs( const relevantFuncSignatures = fnDefinition.signatures.filter( (s) => s.params?.length >= argIndex && - s.params.slice(0, argIndex).every(({ type: esType }, idx) => { + s.params.slice(0, argIndex).every(({ type: dataType }, idx) => { return ( - esType === enrichedArgs[idx].esType || - compareTypesWithLiterals(esType, enrichedArgs[idx].esType) + dataType === enrichedArgs[idx].dataType || + compareTypesWithLiterals(dataType, enrichedArgs[idx].dataType) ); }) ); @@ -134,26 +141,30 @@ export function getValidFunctionSignaturesForPreviousArgs( } /** + * Given a function signature, returns the compatible types to suggest for the next argument * - * @param fnDefinition - * @param enrichedArgs - * @param argIndex + * @param fnDefinition: the function definition + * @param enrichedArgs: AST args with enriched esType info to match with function signatures + * @param argIndex: the index of the argument to suggest for * @returns */ export function getCompatibleTypesToSuggestNext( fnDefinition: FunctionDefinition, enrichedArgs: Array< ESQLAstItem & { - esType: string; + dataType: string; } >, argIndex: number ) { + // First, narrow down to valid function signatures based on previous arguments const relevantFuncSignatures = getValidFunctionSignaturesForPreviousArgs( fnDefinition, enrichedArgs, argIndex ); + + // Then, get the compatible types to suggest for the next argument const compatibleTypesToSuggestForArg = uniqBy( relevantFuncSignatures.map((f) => f.params[argIndex]).filter((d) => d), (o) => `${o.type}-${o.constantOnly}` diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts b/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts index 36ea9f408d711..590541fd50719 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/esql_types.ts @@ -72,6 +72,12 @@ export const compareTypesWithLiterals = ( if (b === 'string') { return isStringType(a); } + + // In Elasticsearch function definitions, time_literal and time_duration are used + // time_duration is seconds/min/hour interval + // date_period is day/week/month/year interval + // time_literal includes time_duration and date_period + // So they are equivalent AST's 'timeInterval' (a date unit constant: e.g. 1 year, 15 month) if (a === 'time_literal' || a === 'time_duration') return b === 'timeInterval'; if (b === 'time_literal' || b === 'time_duration') return a === 'timeInterval'; if (a === 'time_literal') return b === 'time_duration'; From 4e3aa273e4f4db67d625a8c83bff0bcee1daaf62 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Thu, 29 Aug 2024 15:12:56 -0500 Subject: [PATCH 15/19] Add date histogram --- .../src/autocomplete/autocomplete.ts | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index a70ace1c412c9..0197efec74ef3 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -16,6 +16,7 @@ import type { ESQLLiteral, ESQLSingleAstItem, } from '@kbn/esql-ast'; +import { i18n } from '@kbn/i18n'; import { ESQL_NUMBER_TYPES, isNumericType } from '../shared/esql_types'; import type { EditorContext, SuggestionRawDefinition } from './types'; import { @@ -1426,7 +1427,6 @@ async function getFunctionArgsSuggestions( text: addCommaIf(shouldAddComma, suggestion.text), })) ); - // could also be in stats (bucket) but our autocomplete is not great yet if ( (getTypesFromParamDefs(typesToSuggestNext).includes('date') && @@ -1758,6 +1758,31 @@ async function getOptionArgsSuggestions( ); if (option.name === 'by') { + // Add quick snippet for for stats ... by bucket(<>) + if (command.name === 'stats') { + suggestions.push({ + label: i18n.translate( + 'kbn-esql-validation-autocomplete.esql.autocomplete.addDateHistogram', + { + defaultMessage: 'Add date histogram', + } + ), + text: `BUCKET($0, 20, ?start, ?end)`, + asSnippet: true, + kind: 'Function', + detail: i18n.translate( + 'kbn-esql-validation-autocomplete.esql.autocomplete.addDateHistogramDetail', + { + defaultMessage: 'Add date histogram using bucket()', + } + ), + documentation: { + value: '', + }, + sortText: '1A', + }); + } + suggestions.push( ...(await getFieldsOrFunctionsSuggestions( types[0] === 'column' ? ['any'] : types, From cfa7363c6e35119e3b2f741b3ef8f54389ad0ec4 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Thu, 29 Aug 2024 15:22:34 -0500 Subject: [PATCH 16/19] Update tests --- .../__tests__/autocomplete.command.stats.test.ts | 6 ++++++ .../src/autocomplete/autocomplete.test.ts | 10 +++++++++- .../src/autocomplete/autocomplete.ts | 3 ++- .../src/autocomplete/factories.ts | 1 + 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts index dba7785c7ae58..1f6bd93359311 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.stats.test.ts @@ -7,6 +7,7 @@ */ import { ESQL_COMMON_NUMERIC_TYPES, ESQL_NUMBER_TYPES } from '../../shared/esql_types'; +import { ADD_DATE_HISTOGRAM_SNIPPET } from '../factories'; import { roundParameterTypes } from './constants'; import { setup, getFunctionSignaturesByReturnType, getFieldNamesByType } from './helpers'; @@ -213,6 +214,7 @@ describe('autocomplete.suggest', () => { const { assertSuggestions } = await setup(); const expected = [ 'var0 = ', + ADD_DATE_HISTOGRAM_SNIPPET, ...getFieldNamesByType('any').map((field) => `${field} `), ...allEvaFunctions, ...allGroupingFunctions, @@ -235,6 +237,7 @@ describe('autocomplete.suggest', () => { const fields = getFieldNamesByType('any').map((field) => `${field} `); await assertSuggestions('from a | stats a=c by d, /', [ 'var0 = ', + ADD_DATE_HISTOGRAM_SNIPPET, ...fields, ...allEvaFunctions, ...allGroupingFunctions, @@ -246,6 +249,7 @@ describe('autocomplete.suggest', () => { ]); await assertSuggestions('from a | stats avg(b) by c, /', [ 'var0 = ', + ADD_DATE_HISTOGRAM_SNIPPET, ...fields, ...getFunctionSignaturesByReturnType('eval', 'any', { scalar: true }), ...allGroupingFunctions, @@ -267,11 +271,13 @@ describe('autocomplete.suggest', () => { ...allGroupingFunctions, ]); await assertSuggestions('from a | stats avg(b) by var0 = /', [ + ADD_DATE_HISTOGRAM_SNIPPET, ...getFieldNamesByType('any').map((field) => `${field} `), ...allEvaFunctions, ...allGroupingFunctions, ]); await assertSuggestions('from a | stats avg(b) by c, var0 = /', [ + ADD_DATE_HISTOGRAM_SNIPPET, ...getFieldNamesByType('any').map((field) => `${field} `), ...allEvaFunctions, ...allGroupingFunctions, diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts index a213cbf96de1c..5ca845a8c586c 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -10,7 +10,12 @@ import { suggest } from './autocomplete'; import { evalFunctionDefinitions } from '../definitions/functions'; import { timeUnitsToSuggest } from '../definitions/literals'; import { commandDefinitions as unmodifiedCommandDefinitions } from '../definitions/commands'; -import { getSafeInsertText, TIME_SYSTEM_PARAMS, TRIGGER_SUGGESTION_COMMAND } from './factories'; +import { + ADD_DATE_HISTOGRAM_SNIPPET, + getSafeInsertText, + TIME_SYSTEM_PARAMS, + TRIGGER_SUGGESTION_COMMAND, +} from './factories'; import { camelCase } from 'lodash'; import { getAstAndSyntaxErrors } from '@kbn/esql-ast'; import { @@ -638,6 +643,7 @@ describe('autocomplete', () => { // STATS argument BY expression testSuggestions('FROM index1 | STATS field BY f/', [ 'var0 = ', + ADD_DATE_HISTOGRAM_SNIPPET, ...getFunctionSignaturesByReturnType('stats', 'any', { grouping: true, scalar: true }), ...getFieldNamesByType('any').map((field) => `${field} `), ]); @@ -850,6 +856,7 @@ describe('autocomplete', () => { 'by' ); testSuggestions('FROM a | STATS AVG(numberField) BY /', [ + 'BUCKET($0, 20, ?start, ?end)', attachTriggerCommand('var0 = '), ...getFieldNamesByType('any') .map((field) => `${field} `) @@ -859,6 +866,7 @@ describe('autocomplete', () => { // STATS argument BY assignment (checking field suggestions) testSuggestions('FROM a | STATS AVG(numberField) BY var0 = /', [ + 'BUCKET($0, 20, ?start, ?end)', ...getFieldNamesByType('any') .map((field) => `${field} `) .map(attachTriggerCommand), diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 0197efec74ef3..d6d465e27d630 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -78,6 +78,7 @@ import { getDateLiterals, buildFieldsDefinitionsWithMetadata, TRIGGER_SUGGESTION_COMMAND, + ADD_DATE_HISTOGRAM_SNIPPET, } from './factories'; import { EDITOR_MARKER, SINGLE_BACKTICK, METADATA_FIELDS } from '../shared/constants'; import { getAstContext, removeMarkerArgFromArgsList } from '../shared/context'; @@ -1767,7 +1768,7 @@ async function getOptionArgsSuggestions( defaultMessage: 'Add date histogram', } ), - text: `BUCKET($0, 20, ?start, ?end)`, + text: ADD_DATE_HISTOGRAM_SNIPPET, asSnippet: true, kind: 'Function', detail: i18n.translate( diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts index 4ac3b3b24d1f6..06238a8feb335 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts @@ -30,6 +30,7 @@ const allFunctions = statsAggregationFunctionDefinitions .concat(groupingFunctionDefinitions); export const TIME_SYSTEM_PARAMS = ['?t_start', '?t_end']; +export const ADD_DATE_HISTOGRAM_SNIPPET = 'BUCKET($0, 20, ?start, ?end)'; export const TRIGGER_SUGGESTION_COMMAND = { title: 'Trigger Suggestion Dialog', From abcaab41d657065507a335584f0f327e452509e3 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Fri, 30 Aug 2024 09:53:22 -0500 Subject: [PATCH 17/19] Make it so it doesn't add comma when completing an arg from a snippet --- .../src/autocomplete/autocomplete.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index d6d465e27d630..8fbe915e456a0 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -309,7 +309,9 @@ export async function suggest( astContext, getFieldsByType, getFieldsMap, - getPolicyMetadata + getPolicyMetadata, + fullText, + offset ); } if (astContext.type === 'list') { @@ -1258,7 +1260,9 @@ async function getFunctionArgsSuggestions( }, getFieldsByType: GetFieldsByTypeFn, getFieldsMap: GetFieldsMapFn, - getPolicyMetadata: GetPolicyMetadataFn + getPolicyMetadata: GetPolicyMetadataFn, + fullText: string, + offset: number ): Promise { const fnDefinition = getFunctionDefinition(node.name); // early exit on no hit @@ -1314,9 +1318,11 @@ async function getFunctionArgsSuggestions( // no need to suggest comma .some((p) => p === null || p?.optional === true); - // Wehther to prepend comma to suggestion string + // Whether to prepend comma to suggestion string // E.g. if true, "fieldName" -> "fieldName, " - const shouldAddComma = hasMoreMandatoryArgs && fnDefinition.type !== 'builtin'; + const alreadyHasComma = fullText ? fullText[offset] === ',' : false; + const shouldAddComma = + hasMoreMandatoryArgs && fnDefinition.type !== 'builtin' && !alreadyHasComma; const suggestedConstants = uniq( typesToSuggestNext @@ -1777,11 +1783,8 @@ async function getOptionArgsSuggestions( defaultMessage: 'Add date histogram using bucket()', } ), - documentation: { - value: '', - }, sortText: '1A', - }); + } as SuggestionRawDefinition); } suggestions.push( From 558cb4e5170a95f7046f18ead978919b7f3ee655 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Fri, 30 Aug 2024 12:26:42 -0500 Subject: [PATCH 18/19] Fix not opening suggestions for snippets --- .../src/autocomplete/autocomplete.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 8fbe915e456a0..bad01739d3d6d 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -1415,8 +1415,8 @@ async function getFunctionArgsSuggestions( [], { addComma: shouldAddComma, - advanceCursor: hasMoreMandatoryArgs, - openSuggestions: hasMoreMandatoryArgs, + advanceCursor: shouldAddComma, + openSuggestions: shouldAddComma, } ), true From ca2d8e773a2aabed6ffd1c3668339ea851d12d2a Mon Sep 17 00:00:00 2001 From: Quynh Nguyen Date: Fri, 30 Aug 2024 13:28:45 -0500 Subject: [PATCH 19/19] Update tests and bucket number --- .../src/autocomplete/autocomplete.test.ts | 4 ++-- .../src/autocomplete/autocomplete.ts | 1 + .../src/autocomplete/factories.ts | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts index 5ca845a8c586c..9f558c7c17470 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -856,7 +856,7 @@ describe('autocomplete', () => { 'by' ); testSuggestions('FROM a | STATS AVG(numberField) BY /', [ - 'BUCKET($0, 20, ?start, ?end)', + ADD_DATE_HISTOGRAM_SNIPPET, attachTriggerCommand('var0 = '), ...getFieldNamesByType('any') .map((field) => `${field} `) @@ -866,7 +866,7 @@ describe('autocomplete', () => { // STATS argument BY assignment (checking field suggestions) testSuggestions('FROM a | STATS AVG(numberField) BY var0 = /', [ - 'BUCKET($0, 20, ?start, ?end)', + ADD_DATE_HISTOGRAM_SNIPPET, ...getFieldNamesByType('any') .map((field) => `${field} `) .map(attachTriggerCommand), diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index bad01739d3d6d..b2808ee2c1156 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -1784,6 +1784,7 @@ async function getOptionArgsSuggestions( } ), sortText: '1A', + command: TRIGGER_SUGGESTION_COMMAND, } as SuggestionRawDefinition); } diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts index 06238a8feb335..ac405084bb253 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts @@ -30,7 +30,7 @@ const allFunctions = statsAggregationFunctionDefinitions .concat(groupingFunctionDefinitions); export const TIME_SYSTEM_PARAMS = ['?t_start', '?t_end']; -export const ADD_DATE_HISTOGRAM_SNIPPET = 'BUCKET($0, 20, ?start, ?end)'; +export const ADD_DATE_HISTOGRAM_SNIPPET = 'BUCKET($0, 10, ?t_start, ?t_end)'; export const TRIGGER_SUGGESTION_COMMAND = { title: 'Trigger Suggestion Dialog',