From 7915807ce73f0fac222a8516b620adf1a0bec9ee Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Tue, 2 Apr 2024 15:08:45 -0600 Subject: [PATCH 01/17] add values definition --- .../src/definitions/aggs.ts | 18 +++++++++ .../esql_validation_meta_tests.json | 38 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/aggs.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/aggs.ts index 94ab3106036fd..9c95dcd4d072e 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/aggs.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/aggs.ts @@ -175,4 +175,22 @@ export const statsAggregationFunctionDefinitions: FunctionDefinition[] = [ }, ], }, + { + name: 'values', + type: 'agg', + description: i18n.translate('kbn-esql-validation-autocomplete.esql.definitions.values', { + defaultMessage: 'Returns all values in a group as an array.', + }), + supportedCommands: ['stats'], + signatures: [ + { + params: [{ name: 'expression', type: 'any', noNestingFunctions: true }], + returnType: 'any[]', + examples: [ + 'from index | stats all_agents=values(agents.keyword)', + 'from index | stats all_sorted_agents=mv_sort(values(agents.keyword))', + ], + }, + ], + }, ]); 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 0db7e695f8a62..56285f448a2c6 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 @@ -6838,6 +6838,34 @@ ], "warning": [] }, + { + "query": "from a_index | eval var = values(stringField)", + "error": [ + "EVAL does not support function values" + ], + "warning": [] + }, + { + "query": "from a_index | eval var = values(stringField) > 0", + "error": [ + "EVAL does not support function values" + ], + "warning": [] + }, + { + "query": "from a_index | eval values(stringField)", + "error": [ + "EVAL does not support function values" + ], + "warning": [] + }, + { + "query": "from a_index | eval values(stringField) > 0", + "error": [ + "EVAL does not support function values" + ], + "warning": [] + }, { "query": "from a_index | eval var = abs(numberField)", "error": [], @@ -12125,6 +12153,16 @@ ], "warning": [] }, + { + "query": "from a_index | stats var = values(stringField)", + "error": [], + "warning": [] + }, + { + "query": "from a_index | stats values(stringField)", + "error": [], + "warning": [] + }, { "query": "FROM index\n | EVAL numberField * 3.281\n | STATS avg_numberField = AVG(`numberField * 3.281`)", "error": [], From 6a701899b1c4c26333c743ec809bdd7eefcb61d1 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Wed, 3 Apr 2024 12:34:29 -0600 Subject: [PATCH 02/17] add validation for mv_sort --- packages/kbn-esql-ast/src/types.ts | 11 ++++++++ .../src/definitions/aggs.ts | 2 +- .../src/definitions/functions.ts | 24 ++++++++++++++++++ .../src/definitions/types.ts | 17 +++++++++++++ .../src/validation/errors.ts | 16 ++++++++++++ .../src/validation/types.ts | 4 +++ .../src/validation/validation.ts | 25 ++++++++++++++++++- 7 files changed, 97 insertions(+), 2 deletions(-) diff --git a/packages/kbn-esql-ast/src/types.ts b/packages/kbn-esql-ast/src/types.ts index 4bce46d776671..616c718bdd274 100644 --- a/packages/kbn-esql-ast/src/types.ts +++ b/packages/kbn-esql-ast/src/types.ts @@ -78,6 +78,17 @@ export interface ESQLLiteral extends ESQLAstBaseItem { value: string | number; } +export interface ESQLStringLiteral extends ESQLLiteral { + type: 'literal'; + literalType: 'string'; + value: string; +} + +export const isESQLStringLiteral = (literal: ESQLLiteral): literal is ESQLStringLiteral => + literal.literalType === 'string'; + +export const unwrapStringLiteralQuotes = (value: string) => value.slice(1, -1); + export interface ESQLMessage { type: 'error' | 'warning'; text: string; diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/aggs.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/aggs.ts index 9c95dcd4d072e..169ae23052ebc 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/aggs.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/aggs.ts @@ -185,7 +185,7 @@ export const statsAggregationFunctionDefinitions: FunctionDefinition[] = [ signatures: [ { params: [{ name: 'expression', type: 'any', noNestingFunctions: true }], - returnType: 'any[]', + returnType: 'any', examples: [ 'from index | stats all_agents=values(agents.keyword)', 'from index | stats all_sorted_agents=mv_sort(values(agents.keyword))', diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts index 99f373879dbb6..f9bdec9bde3b1 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts @@ -924,6 +924,30 @@ export const evalFunctionsDefinitions: FunctionDefinition[] = [ }, ], }, + { + name: 'mv_sort', + description: i18n.translate('kbn-esql-validation-autocomplete.esql.definitions.mvAvgDoc', { + defaultMessage: 'Sorts a multivalue expression in lexicographical order.', + }), + signatures: [ + { + params: [ + { name: 'field', type: 'any' }, + { + name: 'order', + type: 'string', + optional: true, + literalOptions: ['asc', 'desc'], + }, + ], + returnType: 'any', + examples: [ + 'row a = [4, 2, -3, 2] | eval sorted = mv_sort(a)', + 'row a = ["b", "c", "a"] | sorted = mv_sort(a, "DESC")', + ], + }, + ], + }, { name: 'mv_avg', description: i18n.translate('kbn-esql-validation-autocomplete.esql.definitions.mvAvgDoc', { diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts index 111846f1f515f..cc9065654dbdb 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts @@ -23,7 +23,22 @@ export interface FunctionDefinition { optional?: boolean; noNestingFunctions?: boolean; supportsWildcard?: boolean; + /** + * if set this indicates that the value must be a literal + * but can be any literal of the correct type + */ literalOnly?: boolean; + /** + * if provided this means that the value must be one + * of the options in the array iff the value is a literal. + * + * String values are case insensitive. + * + * If the value is not a literal, this field is ignored because + * we can't check the return value of a function to see if it + * matches one of the options prior to runtime. + */ + literalOptions?: string[]; }>; minParams?: number; returnType: string; @@ -87,3 +102,5 @@ export type SignatureType = | FunctionDefinition['signatures'][number] | CommandOptionsDefinition['signature']; export type SignatureArgType = SignatureType['params'][number]; + +export type FunctionArg = FunctionDefinition['signatures'][number]['params'][number]; diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/errors.ts b/packages/kbn-esql-validation-autocomplete/src/validation/errors.ts index b7e02f10683f3..5c8608e37ea7c 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/errors.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/errors.ts @@ -300,6 +300,22 @@ function getMessageAndTypeFromId({ ), type: 'error', }; + case 'unsupportedLiteralOption': + return { + message: i18n.translate( + 'kbn-esql-validation-autocomplete.esql.validation.unsupportedLiteralOption', + { + defaultMessage: + 'Invalid option [{value}] for {name}. Supported options: [{supportedOptions}].', + values: { + name: out.name, + value: out.value, + supportedOptions: out.supportedOptions, + }, + } + ), + type: 'warning', + }; case 'expectedConstant': return { message: i18n.translate( diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/types.ts b/packages/kbn-esql-validation-autocomplete/src/validation/types.ts index 0aec2b79eaa89..aaf98773eca2c 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/types.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/types.ts @@ -96,6 +96,10 @@ export interface ValidationErrors { message: string; type: { name: string; command: string; option: string }; }; + unsupportedLiteralOption: { + message: string; + type: { name: string; value: string; supportedOptions: string }; + }; shadowFieldType: { message: string; type: { field: string; fieldType: string; newType: string }; diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts index c8e60bf4c7e96..eb0aca089816b 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts @@ -19,9 +19,11 @@ import type { ESQLSingleAstItem, ESQLSource, } from '@kbn/esql-ast'; +import { isESQLStringLiteral, unwrapStringLiteralQuotes } from '@kbn/esql-ast/src/types'; import { CommandModeDefinition, CommandOptionsDefinition, + FunctionArg, FunctionDefinition, SignatureArgType, } from '../definitions/types'; @@ -75,12 +77,33 @@ import { function validateFunctionLiteralArg( astFunction: ESQLFunction, actualArg: ESQLAstItem, - argDef: SignatureArgType, + argDef: FunctionArg, references: ReferenceMaps, parentCommand: string ) { const messages: ESQLMessage[] = []; if (isLiteralItem(actualArg)) { + if ( + // currently only supporting literalOptions with string literals + isESQLStringLiteral(actualArg) && + argDef.literalOptions && + !argDef.literalOptions + .map((option) => option.toLowerCase()) + .includes(unwrapStringLiteralQuotes(actualArg.value).toLowerCase()) + ) { + messages.push( + getMessageFromId({ + messageId: 'unsupportedLiteralOption', + values: { + name: astFunction.name, + value: actualArg.value, + supportedOptions: argDef.literalOptions.map((option) => `"${option}"`).join(', '), + }, + locations: actualArg.location, + }) + ); + } + if (!isEqualType(actualArg, argDef, references, parentCommand)) { messages.push( getMessageFromId({ From ae67251ec52dd304c16f9957d7c5402ae2991fa9 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Wed, 3 Apr 2024 15:29:36 -0600 Subject: [PATCH 03/17] add validation tests --- .../src/definitions/functions.ts | 15 ++- .../esql_validation_meta_tests.json | 34 +++++++ .../src/validation/validation.test.ts | 91 ++++++++++++------- 3 files changed, 106 insertions(+), 34 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts index f9bdec9bde3b1..1004abfc50326 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts @@ -97,7 +97,6 @@ export const evalFunctionsDefinitions: FunctionDefinition[] = [ ], validate: validateLogFunctions, }, - { name: 'log', description: i18n.translate('kbn-esql-validation-autocomplete.esql.definitions.logDoc', { @@ -932,7 +931,7 @@ export const evalFunctionsDefinitions: FunctionDefinition[] = [ signatures: [ { params: [ - { name: 'field', type: 'any' }, + { name: 'field', type: 'any[]' }, { name: 'order', type: 'string', @@ -946,6 +945,18 @@ export const evalFunctionsDefinitions: FunctionDefinition[] = [ 'row a = ["b", "c", "a"] | sorted = mv_sort(a, "DESC")', ], }, + { + params: [ + { name: 'field', type: 'any' }, + { + name: 'order', + type: 'string', + optional: true, + literalOptions: ['asc', 'desc'], + }, + ], + returnType: 'any', + }, ], }, { 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 9d981bbb6c63e..35f366d5c5fab 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 @@ -1370,6 +1370,16 @@ "error": [], "warning": [] }, + { + "query": "row var = mv_sort([\"a\", \"b\", \"c\"], \"asc\")", + "error": [], + "warning": [] + }, + { + "query": "row mv_sort([\"a\", \"b\", \"c\"], \"asc\")", + "error": [], + "warning": [] + }, { "query": "row var = mv_sum(5)", "error": [], @@ -2333,6 +2343,13 @@ ], "warning": [] }, + { + "query": "row var = mv_sort([\"a\", \"b\"], \"bogus\")", + "error": [], + "warning": [ + "Invalid option [\"bogus\"] for mv_sort. Supported options: [\"asc\", \"desc\"]." + ] + }, { "query": "row 1 anno", "error": [ @@ -7899,6 +7916,16 @@ ], "warning": [] }, + { + "query": "from a_index | eval var = mv_sort([\"a\", \"b\", \"c\"], \"asc\")", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_sort([\"a\", \"b\", \"c\"], \"asc\")", + "error": [], + "warning": [] + }, { "query": "from a_index | eval var = mv_sum(numberField)", "error": [], @@ -9441,6 +9468,13 @@ ], "warning": [] }, + { + "query": "from a_index | eval mv_sort([\"a\", \"b\"], \"bogus\")", + "error": [], + "warning": [ + "Invalid option [\"bogus\"] for mv_sort. Supported options: [\"asc\", \"desc\"]." + ] + }, { "query": "from a_index | eval 1 anno", "error": [ diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts index 7d19f4c23cad1..9330fb03b3253 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts @@ -164,9 +164,17 @@ function getFieldMapping( string: `"a"`, number: '5', }; - return params.map(({ name: _name, type, literalOnly, ...rest }) => { + return params.map(({ name: _name, type, literalOnly, literalOptions, ...rest }) => { const typeString: string = type; if (fieldTypes.includes(typeString)) { + if (useLiterals && literalOptions) { + return { + name: `"${literalOptions[0]}"`, + type, + ...rest, + }; + } + const fieldName = literalOnly && typeString in literalValues ? literalValues[typeString as keyof typeof literalValues]! @@ -187,7 +195,7 @@ function getFieldMapping( ...rest, }; } - if (/[]$/.test(typeString)) { + if (/\[\]$/.test(typeString)) { return { name: getMultiValue(typeString), type, @@ -198,7 +206,7 @@ function getFieldMapping( }); } -function generateWrongMappingForArgs( +function generateIncorrectlyTypedParameters( name: string, signatures: FunctionDefinition['signatures'], currentParams: FunctionDefinition['signatures'][number]['params'], @@ -208,28 +216,30 @@ function generateWrongMappingForArgs( string: `"a"`, number: '5', }; - const wrongFieldMapping = currentParams.map(({ name: _name, literalOnly, type, ...rest }, i) => { - // this thing is complex enough, let's not make it harder for constants - if (literalOnly) { - return { name: literalValues[type as keyof typeof literalValues], type, ...rest }; + const wrongFieldMapping = currentParams.map( + ({ name: _name, literalOnly, literalOptions, type, ...rest }, i) => { + // this thing is complex enough, let's not make it harder for constants + if (literalOnly) { + return { name: literalValues[type as keyof typeof literalValues], type, ...rest }; + } + const canBeFieldButNotString = Boolean( + fieldTypes.filter((t) => t !== 'string').includes(type) && + signatures.every(({ params: fnParams }) => fnParams[i].type !== 'string') + ); + const canBeFieldButNotNumber = + fieldTypes.filter((t) => t !== 'number').includes(type) && + signatures.every(({ params: fnParams }) => fnParams[i].type !== 'number'); + const isLiteralType = /literal$/.test(type); + // pick a field name purposely wrong + const nameValue = + canBeFieldButNotString || isLiteralType + ? values.stringField + : canBeFieldButNotNumber + ? values.numberField + : values.booleanField; + return { name: nameValue, type, ...rest }; } - const canBeFieldButNotString = Boolean( - fieldTypes.filter((t) => t !== 'string').includes(type) && - signatures.every(({ params: fnParams }) => fnParams[i].type !== 'string') - ); - const canBeFieldButNotNumber = - fieldTypes.filter((t) => t !== 'number').includes(type) && - signatures.every(({ params: fnParams }) => fnParams[i].type !== 'number'); - const isLiteralType = /literal$/.test(type); - // pick a field name purposely wrong - const nameValue = - canBeFieldButNotString || isLiteralType - ? values.stringField - : canBeFieldButNotNumber - ? values.numberField - : values.booleanField; - return { name: nameValue, type, ...rest }; - }); + ); const generatedFieldTypes = { [values.stringField]: 'string', @@ -250,6 +260,7 @@ function generateWrongMappingForArgs( return `Argument of [${name}] must be [${type}], found value [${fieldName}] type [${generatedFieldTypes[fieldName]}]`; }) .filter(nonNullable); + return { wrongFieldMapping, expectedErrors }; } @@ -565,7 +576,7 @@ describe('validation logic', () => { // the right error message if ( params.every(({ type }) => type !== 'any') && - !['auto_bucket', 'to_version'].includes(name) + !['auto_bucket', 'to_version', 'mv_sort'].includes(name) ) { // now test nested functions const fieldMappingWithNestedFunctions = getFieldMapping(params, { @@ -585,11 +596,15 @@ describe('validation logic', () => { testErrorsAndWarnings(`row var = ${signatureString}`, []); - const { wrongFieldMapping, expectedErrors } = generateWrongMappingForArgs( + const { wrongFieldMapping, expectedErrors } = generateIncorrectlyTypedParameters( name, signatures, params, - { stringField: '"a"', numberField: '5', booleanField: 'true' } + { + stringField: '"a"', + numberField: '5', + booleanField: 'true', + } ); const wrongSignatureString = tweakSignatureForRowCommand( getFunctionSignatures( @@ -634,6 +649,12 @@ describe('validation logic', () => { ]); } + testErrorsAndWarnings( + `row var = mv_sort(["a", "b"], "bogus")`, + [], + ['Invalid option ["bogus"] for mv_sort. Supported options: ["asc", "desc"].'] + ); + describe('date math', () => { testErrorsAndWarnings('row 1 anno', [ 'ROW does not support [date_period] in expression [1 anno]', @@ -1187,7 +1208,7 @@ describe('validation logic', () => { [] ); - const { wrongFieldMapping, expectedErrors } = generateWrongMappingForArgs( + const { wrongFieldMapping, expectedErrors } = generateIncorrectlyTypedParameters( name, signatures, params, @@ -1435,7 +1456,7 @@ describe('validation logic', () => { // the right error message if ( params.every(({ type }) => type !== 'any') && - !['auto_bucket', 'to_version'].includes(name) + !['auto_bucket', 'to_version', 'mv_sort'].includes(name) ) { // now test nested functions const fieldMappingWithNestedFunctions = getFieldMapping(params, { @@ -1455,7 +1476,7 @@ describe('validation logic', () => { }` ); - const { wrongFieldMapping, expectedErrors } = generateWrongMappingForArgs( + const { wrongFieldMapping, expectedErrors } = generateIncorrectlyTypedParameters( name, signatures, params, @@ -1676,6 +1697,12 @@ describe('validation logic', () => { "SyntaxError: mismatched input '' expecting {',', ')'}", ]); + testErrorsAndWarnings( + 'from a_index | eval mv_sort(["a", "b"], "bogus")', + [], + ['Invalid option ["bogus"] for mv_sort. Supported options: ["asc", "desc"].'] + ); + describe('date math', () => { testErrorsAndWarnings('from a_index | eval 1 anno', [ 'EVAL does not support [date_period] in expression [1 anno]', @@ -2064,7 +2091,7 @@ describe('validation logic', () => { // the right error message if ( params.every(({ type }) => type !== 'any') && - !['auto_bucket', 'to_version'].includes(name) + !['auto_bucket', 'to_version', 'mv_sort'].includes(name) ) { // now test nested functions const fieldMappingWithNestedAggsFunctions = getFieldMapping(params, { @@ -2103,7 +2130,7 @@ describe('validation logic', () => { }`, nestedAggsExpectedErrors ); - const { wrongFieldMapping, expectedErrors } = generateWrongMappingForArgs( + const { wrongFieldMapping, expectedErrors } = generateIncorrectlyTypedParameters( name, signatures, params, From 881c265f677a5ac5a27f74be4a16056e272bff50 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Fri, 5 Apr 2024 09:12:36 -0600 Subject: [PATCH 04/17] fix i18n string --- .../src/definitions/functions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts index 1004abfc50326..6ff3d0c0ae810 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts @@ -925,7 +925,7 @@ export const evalFunctionsDefinitions: FunctionDefinition[] = [ }, { name: 'mv_sort', - description: i18n.translate('kbn-esql-validation-autocomplete.esql.definitions.mvAvgDoc', { + description: i18n.translate('kbn-esql-validation-autocomplete.esql.definitions.mvSortDoc', { defaultMessage: 'Sorts a multivalue expression in lexicographical order.', }), signatures: [ From 9778c7db7acdb6fa76758dacab1ae62aa55eecde Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 8 Apr 2024 11:44:07 -0600 Subject: [PATCH 05/17] move type helpers --- .../kbn-esql-validation-autocomplete/src/shared/helpers.ts | 6 ++++++ .../src/validation/validation.ts | 3 ++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts index 8eab378214a47..6c983de82ec1d 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts @@ -17,6 +17,7 @@ import type { ESQLSource, ESQLTimeInterval, } from '@kbn/esql-ast'; +import { ESQLStringLiteral } from '@kbn/esql-ast/src/types'; import { statsAggregationFunctionDefinitions } from '../definitions/aggs'; import { builtinFunctions } from '../definitions/builtin'; import { commandDefinitions } from '../definitions/commands'; @@ -72,6 +73,9 @@ export function isLiteralItem(arg: ESQLAstItem): arg is ESQLLiteral { return isSingleItem(arg) && arg.type === 'literal'; } +export const isESQLStringLiteral = (literal: ESQLLiteral): literal is ESQLStringLiteral => + literal.literalType === 'string'; + export function isTimeIntervalItem(arg: ESQLAstItem): arg is ESQLTimeInterval { return isSingleItem(arg) && arg.type === 'timeInterval'; } @@ -179,6 +183,8 @@ export function getFunctionDefinition(name: string) { return buildFunctionLookup().get(name.toLowerCase()); } +export const unwrapStringLiteralQuotes = (value: string) => value.slice(1, -1); + function buildCommandLookup() { if (!commandLookups) { commandLookups = commandDefinitions.reduce((memo, def) => { diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts index eb0aca089816b..f3670a61980e4 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts @@ -19,7 +19,6 @@ import type { ESQLSingleAstItem, ESQLSource, } from '@kbn/esql-ast'; -import { isESQLStringLiteral, unwrapStringLiteralQuotes } from '@kbn/esql-ast/src/types'; import { CommandModeDefinition, CommandOptionsDefinition, @@ -38,6 +37,7 @@ import { isArrayType, isColumnItem, isEqualType, + isESQLStringLiteral, isFunctionItem, isLiteralItem, isOptionItem, @@ -53,6 +53,7 @@ import { isSettingItem, isAssignment, isVariable, + unwrapStringLiteralQuotes, } from '../shared/helpers'; import { collectVariables } from '../shared/variables'; import { getMessageFromId, getUnknownTypeLabel } from './errors'; From 42d383256a179b726a343859e5e3fef9826b645a Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 8 Apr 2024 14:04:05 -0600 Subject: [PATCH 06/17] add autocomplete suggestions --- .../src/autocomplete/autocomplete.test.ts | 56 +++++++++++-------- .../src/autocomplete/autocomplete.ts | 14 +++++ .../src/definitions/functions.ts | 14 +---- 3 files changed, 47 insertions(+), 37 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 6afe146560057..6cc7d52fee97f 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -245,7 +245,7 @@ describe('autocomplete', () => { // simulate the editor behaviour for sorting suggestions .sort((a, b) => (a.sortText || '').localeCompare(b.sortText || '')); for (const [index, receivedSuggestion] of suggestionInertTextSorted.entries()) { - if (typeof expected[index] === 'string') { + if (typeof expected[index] !== 'object') { expect(receivedSuggestion.text).toEqual(expected[index]); } else { // check all properties that are defined in the expected suggestion @@ -1054,38 +1054,46 @@ describe('autocomplete', () => { if (fn.name !== 'auto_bucket') { for (const signature of fn.signatures) { signature.params.forEach((param, i) => { - if (i < signature.params.length - 1) { + if (i < signature.params.length) { const canHaveMoreArgs = signature.params.filter(({ optional }, j) => !optional && j > i).length > i; testSuggestions( `from a | eval ${fn.name}(${Array(i).fill('field').join(', ')}${i ? ',' : ''} )`, - [ - ...getFieldNamesByType(param.type).map((f) => (canHaveMoreArgs ? `${f},` : f)), - ...getFunctionSignaturesByReturnType( - 'eval', - param.type, - { evalMath: true }, - undefined, - [fn.name] - ).map((l) => (canHaveMoreArgs ? `${l},` : l)), - ...getLiteralsByType(param.type).map((d) => (canHaveMoreArgs ? `${d},` : d)), - ] + param.literalOptions?.length + ? param.literalOptions.map((option) => `"${option}"${canHaveMoreArgs ? ',' : ''}`) + : [ + ...getFieldNamesByType(param.type).map((f) => + canHaveMoreArgs ? `${f},` : f + ), + ...getFunctionSignaturesByReturnType( + 'eval', + param.type, + { evalMath: true }, + undefined, + [fn.name] + ).map((l) => (canHaveMoreArgs ? `${l},` : l)), + ...getLiteralsByType(param.type).map((d) => (canHaveMoreArgs ? `${d},` : d)), + ] ); testSuggestions( `from a | eval var0 = ${fn.name}(${Array(i).fill('field').join(', ')}${ i ? ',' : '' } )`, - [ - ...getFieldNamesByType(param.type).map((f) => (canHaveMoreArgs ? `${f},` : f)), - ...getFunctionSignaturesByReturnType( - 'eval', - param.type, - { evalMath: true }, - undefined, - [fn.name] - ).map((l) => (canHaveMoreArgs ? `${l},` : l)), - ...getLiteralsByType(param.type).map((d) => (canHaveMoreArgs ? `${d},` : d)), - ] + param.literalOptions?.length + ? param.literalOptions.map((option) => `"${option}"${canHaveMoreArgs ? ',' : ''}`) + : [ + ...getFieldNamesByType(param.type).map((f) => + canHaveMoreArgs ? `${f},` : f + ), + ...getFunctionSignaturesByReturnType( + 'eval', + param.type, + { evalMath: true }, + undefined, + [fn.name] + ).map((l) => (canHaveMoreArgs ? `${l},` : l)), + ...getLiteralsByType(param.type).map((d) => (canHaveMoreArgs ? `${d},` : d)), + ] ); } }); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 84c5f59f49c31..b14c0c9516c9a 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -1082,6 +1082,20 @@ async function getFunctionArgsSuggestions( return []; }); + const literalOptions = fnDefinition.signatures.reduce((acc, signature) => { + const literalOptionsForThisParameter = signature.params[argIndex]?.literalOptions; + return literalOptionsForThisParameter ? acc.concat(literalOptionsForThisParameter) : acc; + }, [] as string[]); + + if (literalOptions.length) { + return literalOptions.map((literalOption) => ({ + label: literalOption, + text: `"${literalOption}"`, + detail: literalOption, + kind: 'Constant', + })); + } + const arg = node.args[argIndex]; // the first signature is used as reference diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts index 6ff3d0c0ae810..8b9a2e358c137 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts @@ -931,7 +931,7 @@ export const evalFunctionsDefinitions: FunctionDefinition[] = [ signatures: [ { params: [ - { name: 'field', type: 'any[]' }, + { name: 'field', type: 'any' }, { name: 'order', type: 'string', @@ -945,18 +945,6 @@ export const evalFunctionsDefinitions: FunctionDefinition[] = [ 'row a = ["b", "c", "a"] | sorted = mv_sort(a, "DESC")', ], }, - { - params: [ - { name: 'field', type: 'any' }, - { - name: 'order', - type: 'string', - optional: true, - literalOptions: ['asc', 'desc'], - }, - ], - returnType: 'any', - }, ], }, { From 10e1c0096f43698b39842ca02d55d866e2f406ef Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 8 Apr 2024 14:30:39 -0600 Subject: [PATCH 07/17] account for minParams in autocomplete tests --- .../src/autocomplete/autocomplete.test.ts | 1 + 1 file changed, 1 insertion(+) 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 6cc7d52fee97f..c51173cae68ce 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -1056,6 +1056,7 @@ describe('autocomplete', () => { signature.params.forEach((param, i) => { if (i < signature.params.length) { const canHaveMoreArgs = + i + 1 < (signature.minParams ?? 0) || signature.params.filter(({ optional }, j) => !optional && j > i).length > i; testSuggestions( `from a | eval ${fn.name}(${Array(i).fill('field').join(', ')}${i ? ',' : ''} )`, From 954d4b56f33b11049aff0414867236d10cd52750 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 8 Apr 2024 15:02:52 -0600 Subject: [PATCH 08/17] fix to_version definition --- .../src/definitions/functions.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts index 8b9a2e358c137..f5badd2693234 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/functions.ts @@ -480,14 +480,9 @@ export const evalFunctionsDefinitions: FunctionDefinition[] = [ signatures: [ { params: [{ name: 'field', type: 'string' }], - returnType: 'version', + returnType: 'string', examples: [`from index | EVAL version = to_version(stringField)`], }, - { - params: [{ name: 'field', type: 'version' }], - returnType: 'version', - examples: [`from index | EVAL version = to_version(versionField)`], - }, ], }, { From 9bdde375e1e7edbea45a2ae574f289b50038afa8 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 8 Apr 2024 15:09:38 -0600 Subject: [PATCH 09/17] leverage existing factory --- .../src/autocomplete/autocomplete.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index b14c0c9516c9a..53e43b66bf6ce 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -1088,12 +1088,7 @@ async function getFunctionArgsSuggestions( }, [] as string[]); if (literalOptions.length) { - return literalOptions.map((literalOption) => ({ - label: literalOption, - text: `"${literalOption}"`, - detail: literalOption, - kind: 'Constant', - })); + return buildConstantsDefinitions(literalOptions); } const arg = node.args[argIndex]; From 4b9feaf21ee571aff9f2d5edaf8a5668244505b5 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 8 Apr 2024 15:20:07 -0600 Subject: [PATCH 10/17] update literal type --- packages/kbn-esql-ast/src/types.ts | 29 ++++++++++++++----- .../src/shared/helpers.ts | 4 --- .../src/validation/validation.ts | 3 +- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/packages/kbn-esql-ast/src/types.ts b/packages/kbn-esql-ast/src/types.ts index 616c718bdd274..f795b4412d97a 100644 --- a/packages/kbn-esql-ast/src/types.ts +++ b/packages/kbn-esql-ast/src/types.ts @@ -72,22 +72,35 @@ export interface ESQLList extends ESQLAstBaseItem { values: ESQLLiteral[]; } -export interface ESQLLiteral extends ESQLAstBaseItem { +export type ESQLLiteral = + | ESQLNumberLiteral + | ESQLBooleanLiteral + | ESQLNullLiteral + | ESQLStringLiteral; + +interface ESQLNumberLiteral extends ESQLAstBaseItem { type: 'literal'; - literalType: 'string' | 'number' | 'boolean' | 'null'; - value: string | number; + literalType: 'number'; + value: number; } -export interface ESQLStringLiteral extends ESQLLiteral { +interface ESQLBooleanLiteral extends ESQLAstBaseItem { type: 'literal'; - literalType: 'string'; + literalType: 'boolean'; value: string; } -export const isESQLStringLiteral = (literal: ESQLLiteral): literal is ESQLStringLiteral => - literal.literalType === 'string'; +interface ESQLNullLiteral extends ESQLAstBaseItem { + type: 'literal'; + literalType: 'null'; + value: string; +} -export const unwrapStringLiteralQuotes = (value: string) => value.slice(1, -1); +interface ESQLStringLiteral extends ESQLAstBaseItem { + type: 'literal'; + literalType: 'string'; + value: string; +} export interface ESQLMessage { type: 'error' | 'warning'; diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts index 6c983de82ec1d..a436ca6f9eb5b 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts @@ -17,7 +17,6 @@ import type { ESQLSource, ESQLTimeInterval, } from '@kbn/esql-ast'; -import { ESQLStringLiteral } from '@kbn/esql-ast/src/types'; import { statsAggregationFunctionDefinitions } from '../definitions/aggs'; import { builtinFunctions } from '../definitions/builtin'; import { commandDefinitions } from '../definitions/commands'; @@ -73,9 +72,6 @@ export function isLiteralItem(arg: ESQLAstItem): arg is ESQLLiteral { return isSingleItem(arg) && arg.type === 'literal'; } -export const isESQLStringLiteral = (literal: ESQLLiteral): literal is ESQLStringLiteral => - literal.literalType === 'string'; - export function isTimeIntervalItem(arg: ESQLAstItem): arg is ESQLTimeInterval { return isSingleItem(arg) && arg.type === 'timeInterval'; } diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts index f3670a61980e4..62a39a2c22705 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts @@ -37,7 +37,6 @@ import { isArrayType, isColumnItem, isEqualType, - isESQLStringLiteral, isFunctionItem, isLiteralItem, isOptionItem, @@ -86,7 +85,7 @@ function validateFunctionLiteralArg( if (isLiteralItem(actualArg)) { if ( // currently only supporting literalOptions with string literals - isESQLStringLiteral(actualArg) && + actualArg.literalType === 'string' && argDef.literalOptions && !argDef.literalOptions .map((option) => option.toLowerCase()) From 90baa5b08bc375e60c3868f15888bb769f436398 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 8 Apr 2024 16:15:07 -0600 Subject: [PATCH 11/17] fix typings --- packages/kbn-esql-ast/src/ast_helpers.ts | 19 +++++++++++++++---- packages/kbn-esql-ast/src/types.ts | 8 ++++---- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/kbn-esql-ast/src/ast_helpers.ts b/packages/kbn-esql-ast/src/ast_helpers.ts index 0f85e30b96c45..8ea9313ac9dec 100644 --- a/packages/kbn-esql-ast/src/ast_helpers.ts +++ b/packages/kbn-esql-ast/src/ast_helpers.ts @@ -113,14 +113,25 @@ export function createLiteral( return; } const text = node.getText(); - return { + + const partialLiteral: Pick = { type: 'literal', - literalType: type, text, name: text, - value: type === 'number' ? Number(text) : text, location: getPosition(node.symbol), - incomplete: isMissingText(node.getText()), + incomplete: isMissingText(text), + }; + if (type === 'number') { + return { + ...partialLiteral, + literalType: type, + value: Number(text), + }; + } + return { + ...partialLiteral, + literalType: type, + value: text, }; } diff --git a/packages/kbn-esql-ast/src/types.ts b/packages/kbn-esql-ast/src/types.ts index f795b4412d97a..abb6f8090e7b8 100644 --- a/packages/kbn-esql-ast/src/types.ts +++ b/packages/kbn-esql-ast/src/types.ts @@ -78,25 +78,25 @@ export type ESQLLiteral = | ESQLNullLiteral | ESQLStringLiteral; -interface ESQLNumberLiteral extends ESQLAstBaseItem { +export interface ESQLNumberLiteral extends ESQLAstBaseItem { type: 'literal'; literalType: 'number'; value: number; } -interface ESQLBooleanLiteral extends ESQLAstBaseItem { +export interface ESQLBooleanLiteral extends ESQLAstBaseItem { type: 'literal'; literalType: 'boolean'; value: string; } -interface ESQLNullLiteral extends ESQLAstBaseItem { +export interface ESQLNullLiteral extends ESQLAstBaseItem { type: 'literal'; literalType: 'null'; value: string; } -interface ESQLStringLiteral extends ESQLAstBaseItem { +export interface ESQLStringLiteral extends ESQLAstBaseItem { type: 'literal'; literalType: 'string'; value: string; From e21e5dd685cebc4ee29d45ec488e3c4e06210a41 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Tue, 9 Apr 2024 07:23:36 -0600 Subject: [PATCH 12/17] fix value suggestions --- .../src/autocomplete/autocomplete.ts | 3 ++- .../src/autocomplete/factories.ts | 10 ++++++++++ 2 files changed, 12 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 53e43b66bf6ce..ce9b2d2e0acf2 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -68,6 +68,7 @@ import { buildVariablesDefinitions, buildOptionDefinition, buildSettingDefinitions, + buildValueDefinitions, } from './factories'; import { EDITOR_MARKER, SINGLE_BACKTICK } from '../shared/constants'; import { getAstContext, removeMarkerArgFromArgsList } from '../shared/context'; @@ -1088,7 +1089,7 @@ async function getFunctionArgsSuggestions( }, [] as string[]); if (literalOptions.length) { - return buildConstantsDefinitions(literalOptions); + return buildValueDefinitions(literalOptions); } const arg = node.args[argIndex]; diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts index 3e243618ed7fa..2818634c58188 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/factories.ts @@ -165,6 +165,16 @@ export const buildConstantsDefinitions = ( sortText: 'A', })); +export const buildValueDefinitions = (values: string[]): SuggestionRawDefinition[] => + values.map((value) => ({ + label: `"${value}"`, + text: `"${value}"`, + detail: i18n.translate('kbn-esql-validation-autocomplete.esql.autocomplete.valueDefinition', { + defaultMessage: 'Literal value', + }), + kind: 'Value', + })); + export const buildNewVarDefinition = (label: string): SuggestionRawDefinition => { return { label, From acff5e740fa462453f277adae72e1ecaa053e537 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Tue, 9 Apr 2024 10:23:12 -0600 Subject: [PATCH 13/17] mark type-specific literals as internal --- packages/kbn-esql-ast/src/types.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/kbn-esql-ast/src/types.ts b/packages/kbn-esql-ast/src/types.ts index abb6f8090e7b8..f167276ae84a5 100644 --- a/packages/kbn-esql-ast/src/types.ts +++ b/packages/kbn-esql-ast/src/types.ts @@ -78,24 +78,28 @@ export type ESQLLiteral = | ESQLNullLiteral | ESQLStringLiteral; +// @internal export interface ESQLNumberLiteral extends ESQLAstBaseItem { type: 'literal'; literalType: 'number'; value: number; } +// @internal export interface ESQLBooleanLiteral extends ESQLAstBaseItem { type: 'literal'; literalType: 'boolean'; value: string; } +// @internal export interface ESQLNullLiteral extends ESQLAstBaseItem { type: 'literal'; literalType: 'null'; value: string; } +// @internal export interface ESQLStringLiteral extends ESQLAstBaseItem { type: 'literal'; literalType: 'string'; From 7de22dedf2dd9734d46ff4983e7b629906d81829 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Tue, 9 Apr 2024 10:52:02 -0600 Subject: [PATCH 14/17] pick => omit --- packages/kbn-esql-ast/src/ast_helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kbn-esql-ast/src/ast_helpers.ts b/packages/kbn-esql-ast/src/ast_helpers.ts index 8ea9313ac9dec..0b3c3eff25ada 100644 --- a/packages/kbn-esql-ast/src/ast_helpers.ts +++ b/packages/kbn-esql-ast/src/ast_helpers.ts @@ -114,7 +114,7 @@ export function createLiteral( } const text = node.getText(); - const partialLiteral: Pick = { + const partialLiteral: Omit = { type: 'literal', text, name: text, From ce13b6c4c1c5eb4748696c9f756f53e234f11b3d Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Tue, 9 Apr 2024 12:08:34 -0600 Subject: [PATCH 15/17] move some code into a separate function --- .../src/definitions/types.ts | 2 +- .../src/shared/helpers.ts | 41 +++++++++---- .../esql_validation_meta_tests.json | 57 ++++++------------- .../src/validation/validation.ts | 13 ++--- 4 files changed, 52 insertions(+), 61 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts b/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts index cc9065654dbdb..6d3488aacc003 100644 --- a/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts +++ b/packages/kbn-esql-validation-autocomplete/src/definitions/types.ts @@ -103,4 +103,4 @@ export type SignatureType = | CommandOptionsDefinition['signature']; export type SignatureArgType = SignatureType['params'][number]; -export type FunctionArg = FunctionDefinition['signatures'][number]['params'][number]; +export type FunctionArgSignature = FunctionDefinition['signatures'][number]['params'][number]; diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts index a436ca6f9eb5b..f19fcf2d400fe 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts @@ -34,6 +34,7 @@ import { import type { CommandDefinition, CommandOptionsDefinition, + FunctionArgSignature, FunctionDefinition, SignatureArgType, } from '../definitions/types'; @@ -179,7 +180,7 @@ export function getFunctionDefinition(name: string) { return buildFunctionLookup().get(name.toLowerCase()); } -export const unwrapStringLiteralQuotes = (value: string) => value.slice(1, -1); +const unwrapStringLiteralQuotes = (value: string) => value.slice(1, -1); function buildCommandLookup() { if (!commandLookups) { @@ -339,8 +340,26 @@ export function inKnownTimeInterval(item: ESQLTimeInterval): boolean { return timeLiterals.some(({ name }) => name === item.unit.toLowerCase()); } +/** + * Checks if this argument is one of the possible options + * if they are defined on the arg definition. + */ +export function isValidLiteralOption(arg: ESQLLiteral, argDef: FunctionArgSignature) { + return ( + arg.literalType === 'string' && + argDef.literalOptions && + !argDef.literalOptions + .map((option) => option.toLowerCase()) + .includes(unwrapStringLiteralQuotes(arg.value).toLowerCase()) + ); +} + +/** + * Checks if an AST argument is of the correct type + * given the definition. + */ export function isEqualType( - item: ESQLSingleAstItem, + arg: ESQLSingleAstItem, argDef: SignatureArgType, references: ReferenceMaps, parentCommand?: string, @@ -350,24 +369,24 @@ export function isEqualType( if (argType === 'any') { return true; } - if (item.type === 'literal') { - return compareLiteralType(argType, item); + if (arg.type === 'literal') { + return compareLiteralType(argType, arg); } - if (item.type === 'function') { - if (isSupportedFunction(item.name, parentCommand).supported) { - const fnDef = buildFunctionLookup().get(item.name)!; + if (arg.type === 'function') { + if (isSupportedFunction(arg.name, parentCommand).supported) { + const fnDef = buildFunctionLookup().get(arg.name)!; return fnDef.signatures.some((signature) => argType === signature.returnType); } } - if (item.type === 'timeInterval') { - return argType === 'time_literal' && inKnownTimeInterval(item); + if (arg.type === 'timeInterval') { + return argType === 'time_literal' && inKnownTimeInterval(arg); } - if (item.type === 'column') { + if (arg.type === 'column') { if (argType === 'column') { // anything goes, so avoid any effort here return true; } - const hit = getColumnHit(nameHit ?? item.name, references); + const hit = getColumnHit(nameHit ?? arg.name, references); const validHit = hit; if (!validHit) { return false; 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 35f366d5c5fab..31ea257f45a52 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 @@ -1371,12 +1371,12 @@ "warning": [] }, { - "query": "row var = mv_sort([\"a\", \"b\", \"c\"], \"asc\")", + "query": "row var = mv_sort(\"a\", \"asc\")", "error": [], "warning": [] }, { - "query": "row mv_sort([\"a\", \"b\", \"c\"], \"asc\")", + "query": "row mv_sort(\"a\", \"asc\")", "error": [], "warning": [] }, @@ -2015,21 +2015,6 @@ "error": [], "warning": [] }, - { - "query": "row var = to_version(\"a\")", - "error": [], - "warning": [] - }, - { - "query": "row to_version(\"a\")", - "error": [], - "warning": [] - }, - { - "query": "row var = to_ver(\"a\")", - "error": [], - "warning": [] - }, { "query": "row var = trim(\"a\")", "error": [], @@ -5768,6 +5753,18 @@ ], "warning": [] }, + { + "query": "from a_index | where length(to_version(stringField)) > 0", + "error": [], + "warning": [] + }, + { + "query": "from a_index | where length(to_version(numberField)) > 0", + "error": [ + "Argument of [to_version] must be [string], found value [numberField] type [number]" + ], + "warning": [] + }, { "query": "from a_index | where length(trim(stringField)) > 0", "error": [], @@ -7917,12 +7914,12 @@ "warning": [] }, { - "query": "from a_index | eval var = mv_sort([\"a\", \"b\", \"c\"], \"asc\")", + "query": "from a_index | eval var = mv_sort(stringField, \"asc\")", "error": [], "warning": [] }, { - "query": "from a_index | eval mv_sort([\"a\", \"b\", \"c\"], \"asc\")", + "query": "from a_index | eval mv_sort(stringField, \"asc\")", "error": [], "warning": [] }, @@ -8876,28 +8873,6 @@ ], "warning": [] }, - { - "query": "from a_index | eval var = to_version(stringField)", - "error": [], - "warning": [] - }, - { - "query": "from a_index | eval to_version(stringField)", - "error": [], - "warning": [] - }, - { - "query": "from a_index | eval var = to_ver(stringField)", - "error": [], - "warning": [] - }, - { - "query": "from a_index | eval var = to_version(*)", - "error": [ - "Using wildcards (*) in to_version is not allowed" - ], - "warning": [] - }, { "query": "from a_index | eval var = trim(stringField)", "error": [], diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts index 62a39a2c22705..3d508dc93389f 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.ts @@ -22,7 +22,7 @@ import type { import { CommandModeDefinition, CommandOptionsDefinition, - FunctionArg, + FunctionArgSignature, FunctionDefinition, SignatureArgType, } from '../definitions/types'; @@ -52,7 +52,7 @@ import { isSettingItem, isAssignment, isVariable, - unwrapStringLiteralQuotes, + isValidLiteralOption, } from '../shared/helpers'; import { collectVariables } from '../shared/variables'; import { getMessageFromId, getUnknownTypeLabel } from './errors'; @@ -77,19 +77,16 @@ import { function validateFunctionLiteralArg( astFunction: ESQLFunction, actualArg: ESQLAstItem, - argDef: FunctionArg, + argDef: FunctionArgSignature, references: ReferenceMaps, parentCommand: string ) { const messages: ESQLMessage[] = []; if (isLiteralItem(actualArg)) { if ( - // currently only supporting literalOptions with string literals actualArg.literalType === 'string' && argDef.literalOptions && - !argDef.literalOptions - .map((option) => option.toLowerCase()) - .includes(unwrapStringLiteralQuotes(actualArg.value).toLowerCase()) + isValidLiteralOption(actualArg, argDef) ) { messages.push( getMessageFromId({ @@ -97,7 +94,7 @@ function validateFunctionLiteralArg( values: { name: astFunction.name, value: actualArg.value, - supportedOptions: argDef.literalOptions.map((option) => `"${option}"`).join(', '), + supportedOptions: argDef.literalOptions?.map((option) => `"${option}"`).join(', '), }, locations: actualArg.location, }) From 2ac7a3aa12d23d753f10ab4003811c51606da4b6 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Tue, 9 Apr 2024 12:10:19 -0600 Subject: [PATCH 16/17] Add todo --- packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts index f19fcf2d400fe..1d439aa3a0b9e 100644 --- a/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/shared/helpers.ts @@ -343,6 +343,8 @@ export function inKnownTimeInterval(item: ESQLTimeInterval): boolean { /** * Checks if this argument is one of the possible options * if they are defined on the arg definition. + * + * TODO - Consider merging with isEqualType to create a unified arg validation function */ export function isValidLiteralOption(arg: ESQLLiteral, argDef: FunctionArgSignature) { return ( From 98c6b5f02b72fe90041baadb3ada7171672c9d0b Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Wed, 10 Apr 2024 09:07:11 -0600 Subject: [PATCH 17/17] test with capitalized options --- .../esql_validation_meta_tests.json | 20 +++++++++++++++++++ .../src/validation/validation.test.ts | 6 ++++++ 2 files changed, 26 insertions(+) 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 31ea257f45a52..4adae6dc511eb 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 @@ -2335,6 +2335,16 @@ "Invalid option [\"bogus\"] for mv_sort. Supported options: [\"asc\", \"desc\"]." ] }, + { + "query": "row var = mv_sort([\"a\", \"b\"], \"ASC\")", + "error": [], + "warning": [] + }, + { + "query": "row var = mv_sort([\"a\", \"b\"], \"DESC\")", + "error": [], + "warning": [] + }, { "query": "row 1 anno", "error": [ @@ -9450,6 +9460,16 @@ "Invalid option [\"bogus\"] for mv_sort. Supported options: [\"asc\", \"desc\"]." ] }, + { + "query": "from a_index | eval mv_sort([\"a\", \"b\"], \"ASC\")", + "error": [], + "warning": [] + }, + { + "query": "from a_index | eval mv_sort([\"a\", \"b\"], \"DESC\")", + "error": [], + "warning": [] + }, { "query": "from a_index | eval 1 anno", "error": [ diff --git a/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts b/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts index 9330fb03b3253..0e48e60b9cc3d 100644 --- a/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/validation/validation.test.ts @@ -655,6 +655,9 @@ describe('validation logic', () => { ['Invalid option ["bogus"] for mv_sort. Supported options: ["asc", "desc"].'] ); + testErrorsAndWarnings(`row var = mv_sort(["a", "b"], "ASC")`, []); + testErrorsAndWarnings(`row var = mv_sort(["a", "b"], "DESC")`, []); + describe('date math', () => { testErrorsAndWarnings('row 1 anno', [ 'ROW does not support [date_period] in expression [1 anno]', @@ -1703,6 +1706,9 @@ describe('validation logic', () => { ['Invalid option ["bogus"] for mv_sort. Supported options: ["asc", "desc"].'] ); + testErrorsAndWarnings(`from a_index | eval mv_sort(["a", "b"], "ASC")`, []); + testErrorsAndWarnings(`from a_index | eval mv_sort(["a", "b"], "DESC")`, []); + describe('date math', () => { testErrorsAndWarnings('from a_index | eval 1 anno', [ 'EVAL does not support [date_period] in expression [1 anno]',