github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 01:38:56 -04:00
Code Issues Projects Releases Packages Wiki Activity

[ES|QL] Improve handling of functions for variadic signatures with minParams (#177165)

Browse source 
## Summary

Fix an issue with functions with the `minParams` configuration
(`concat`, `case`, `cidr_match`).

Validation fix:
* now validation engine understands the minimum number of args and
provide a better message based on the function signature
* if the function has a single exact signature, then make it explicit
the `exact` nature
<img width="442" alt="Screenshot 2024-02-19 at 11 46 59"
src="14aa9cb4-bce2-404b-936e-cb92ec966c2d">

<img width="227" alt="Screenshot 2024-02-19 at 11 45 18"
src="94b2a051-5cd2-4f60-b65a-c3ac77c17b85">

* if the function has some optional args in the signature, then make it
explicit that there are too few or too many args
  
<img width="441" alt="Screenshot 2024-02-19 at 11 48 00"
src="55acf5f7-ce6b-452d-ba49-cd38ac05120e">

<img width="443" alt="Screenshot 2024-02-19 at 11 45 03"
src="653f62d3-7ee5-44d2-91e9-aae812f08394">

* if the function has a minParams configuration, then it should make it
explicit that there are too few args:
<img width="467" alt="Screenshot 2024-02-19 at 11 41 46"
src="8a4030e0-317d-4371-abd0-11b333ad26d9">

<img width="441" alt="Screenshot 2024-02-19 at 11 41 16"
src="d8f8048e-edda-44c2-b84e-b908cd7b29a5">

<img width="446" alt="Screenshot 2024-02-19 at 11 41 04"
src="c6ac58ef-a1b7-48a6-b2ad-a8994ae2b885">

* autocomplete now understand the `minParams` and suggest the right
value in the right place

![esql_min_params_autocomplete](fd21a819-8310-4f94-88e6-3a2967c7a8bd)

* signature hover tooltip now provides a better signature for functions
with minParams (arg should not be optional, rather mandatory until the
#minParams, optional after)


![esql_min_params_signature](421f22f3-8d00-4acb-a65c-495a05b8d400)

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
This commit is contained in:
Marco Liberati 2024-02-21 09:07:44 +01:00 • committed by GitHub
parent 41f5b45adf
commit eb708f5f4e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 277 additions and 102 deletions
Download patch file Download diff file Expand all files Collapse all files

33
packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.test.ts
View file

@ -975,6 +975,39 @@ describe('autocomplete', () => {
],
'('
);
// test that comma is correctly added to the suggestions if minParams is not reached yet
testSuggestions('from a | eval a=concat( ', [
...getFieldNamesByType('string').map((v) => `${v},`),
...getFunctionSignaturesByReturnType('eval', 'string', { evalMath: true }, undefined, [
'concat',
]).map((v) => `${v},`),
]);
testSuggestions('from a | eval a=concat(stringField, ', [
...getFieldNamesByType('string'),
...getFunctionSignaturesByReturnType('eval', 'string', { evalMath: true }, undefined, [
'concat',
]),
]);
// test that the arg type is correct after minParams
testSuggestions('from a | eval a=cidr_match(ipField, stringField,', [
...getFieldNamesByType('string'),
...getFunctionSignaturesByReturnType('eval', 'string', { evalMath: 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', { evalMath: true }, undefined, [
'cidr_match',
]).map((v) => `${v},`),
]);
testSuggestions('from a | eval a=cidr_match(ipField, ', [
...getFieldNamesByType('string'),
...getFunctionSignaturesByReturnType('eval', 'string', { evalMath: true }, undefined, [
'cidr_match',
]),
]);
// test deep function nesting suggestions (and check that the same function is not suggested)
// round(round(
// round(round(round(

17
packages/kbn-monaco/src/esql/lib/ast/autocomplete/autocomplete.ts
View file

@ -173,7 +173,8 @@ export async function suggest(
unclosedRoundBrackets === 0 &&
getLastCharFromTrimmed(innerText) !== '_') ||
(context.triggerCharacter === ' ' &&
(isMathFunction(innerText, offset) || isComma(innerText[offset - 2])))
(isMathFunction(innerText, offset) ||
isComma(innerText.trimEnd()[innerText.trimEnd().length - 1])))
) {
finalText = `${innerText.substring(0, offset)}${EDITOR_MARKER}${innerText.substring(offset)}`;
}
@ -1078,17 +1079,23 @@ async function getFunctionArgsSuggestions(
if (signature.params.length > argIndex) {
return signature.params[argIndex].type;
}
if (signature.infiniteParams) {
return signature.params[0].type;
if (signature.infiniteParams || signature.minParams) {
return signature.params[signature.params.length - 1].type;
}
return [];
});
const arg = node.args[argIndex];
// the first signature is used as reference
const refSignature = fnDefinition.signatures[0];
const hasMoreMandatoryArgs =
fnDefinition.signatures[0].params.filter(({ optional }, index) => !optional && index > argIndex)
.length > argIndex;
refSignature.params.filter(({ optional }, index) => !optional && index > argIndex).length >
argIndex ||
('minParams' in refSignature && refSignature.minParams
? refSignature.minParams - 1 > argIndex
: false);
const suggestions = [];
const noArgDefined = !arg;

44
packages/kbn-monaco/src/esql/lib/ast/definitions/helpers.ts
View file

@ -12,12 +12,44 @@ export function getFunctionSignatures(
{ name, signatures }: FunctionDefinition,
{ withTypes }: { withTypes: boolean } = { withTypes: true }
) {
return signatures.map(({ params, returnType, infiniteParams, examples }) => ({
declaration: `${name}(${params.map((arg) => printArguments(arg, withTypes)).join(', ')}${
infiniteParams ? ` ,[... ${params.map((arg) => printArguments(arg, withTypes))}]` : ''
})${withTypes ? `: ${returnType}` : ''}`,
examples,
}));
return signatures.map(({ params, returnType, infiniteParams, minParams, examples }) => {
// for functions with a minimum number of args, repeat the last arg multiple times
// just make sure to compute the right number of args to add
const minParamsToAdd = Math.max((minParams || 0) - params.length, 0);
const hasMoreOptionalArgs = !!infiniteParams || !!minParams;
const extraArg = Array(minParamsToAdd || 1).fill(params[Math.max(params.length - 1, 0)]);
return {
declaration: `${name}(${params
.map((arg) => printArguments(arg, withTypes))
.join(', ')}${handleAdditionalArgs(
minParamsToAdd > 0,
extraArg,
withTypes,
false
)}${handleAdditionalArgs(hasMoreOptionalArgs, extraArg, withTypes)})${
withTypes ? `: ${returnType}` : ''
}`,
examples,
};
});
}
function handleAdditionalArgs(
criteria: boolean,
additionalArgs: Array<{
name: string;
type: string | string[];
optional?: boolean;
reference?: string;
}>,
withTypes: boolean,
optionalArg: boolean = true
) {
return criteria
? `${withTypes && optionalArg ? ' ,[... ' : ', '}${additionalArgs
.map((arg) => printArguments(arg, withTypes))
.join(', ')}${withTypes && optionalArg ? ']' : ''}`
: '';
}
export function getCommandSignature(

2
packages/kbn-monaco/src/esql/lib/ast/shared/helpers.ts
View file

@ -91,7 +91,7 @@ export function isIncompleteItem(arg: ESQLAstItem): boolean {
}
export function isMathFunction(query: string, offset: number) {
const queryTrimmed = query.substring(0, offset).trimEnd();
const queryTrimmed = query.trimEnd();
// try to get the full operation token (e.g. "+", "in", "like", etc...) but it requires the token
// to be spaced out from a field/function (e.g. "field + ") so it is subject to issues
const [opString] = queryTrimmed.split(' ').reverse();

31
packages/kbn-monaco/src/esql/lib/ast/validation/errors.ts
View file

@ -57,14 +57,39 @@ function getMessageAndTypeFromId<K extends ErrorTypes>({
};
case 'wrongArgumentNumber':
return {
message: i18n.translate('monaco.esql.validation.wrongArgumentNumber', {
message: i18n.translate('monaco.esql.validation.wrongArgumentExactNumber', {
defaultMessage:
'Error building [{fn}]: expects {canHaveMoreArgs, plural, =0 {exactly } other {}}{numArgs, plural, one {one argument} other {{numArgs} arguments}}, passed {passedArgs} instead.',
'Error building [{fn}]: expects exactly {numArgs, plural, one {one argument} other {{numArgs} arguments}}, passed {passedArgs} instead.',
values: {
fn: out.fn,
numArgs: out.numArgs,
passedArgs: out.passedArgs,
canHaveMoreArgs: out.exactly,
},
}),
};
case 'wrongArgumentNumberTooMany':
return {
message: i18n.translate('monaco.esql.validation.wrongArgumentTooManyNumber', {
defaultMessage:
'Error building [{fn}]: expects {extraArgs, plural, =0 {} other {no more than }}{numArgs, plural, one {one argument} other {{numArgs} arguments}}, passed {passedArgs} instead.',
values: {
fn: out.fn,
numArgs: out.numArgs,
passedArgs: out.passedArgs,
extraArgs: out.extraArgs,
},
}),
};
case 'wrongArgumentNumberTooFew':
return {
message: i18n.translate('monaco.esql.validation.wrongArgumentTooFewNumber', {
defaultMessage:
'Error building [{fn}]: expects {missingArgs, plural, =0 {} other {at least }}{numArgs, plural, one {one argument} other {{numArgs} arguments}}, passed {passedArgs} instead.',
values: {
fn: out.fn,
numArgs: out.numArgs,
passedArgs: out.passedArgs,
missingArgs: out.missingArgs,
},
}),
};

48
packages/kbn-monaco/src/esql/lib/ast/validation/esql_validation_meta_tests.json
View file

@ -519,11 +519,11 @@
"error": false
},
{
"query": "row var = case(true, \"a\")",
"query": "row var = case(true, \"a\", \"a\", \"a\")",
"error": false
},
{
"query": "row case(true, \"a\")",
"query": "row case(true, \"a\", \"a\", \"a\")",
"error": false
},
{
@ -543,19 +543,19 @@
"error": true
},
{
"query": "row var = cidr_match(to_ip(\"127.0.0.1\"), \"a\")",
"query": "row var = cidr_match(to_ip(\"127.0.0.1\"), \"a\", \"a\")",
"error": false
},
{
"query": "row cidr_match(to_ip(\"127.0.0.1\"), \"a\")",
"query": "row cidr_match(to_ip(\"127.0.0.1\"), \"a\", \"a\")",
"error": false
},
{
"query": "row var = cidr_match(to_ip(\"a\"), to_string(\"a\"))",
"query": "row var = cidr_match(to_ip(\"a\"), to_string(\"a\"), to_string(\"a\"))",
"error": false
},
{
"query": "row var = cidr_match(\"a\", 5)",
"query": "row var = cidr_match(\"a\", 5, 5)",
"error": true
},
{
@ -567,19 +567,19 @@
"error": false
},
{
"query": "row var = concat(\"a\")",
"query": "row var = concat(\"a\", \"a\", \"a\")",
"error": false
},
{
"query": "row concat(\"a\")",
"query": "row concat(\"a\", \"a\", \"a\")",
"error": false
},
{
"query": "row var = concat(to_string(\"a\"))",
"query": "row var = concat(to_string(\"a\"), to_string(\"a\"), to_string(\"a\"))",
"error": false
},
{
"query": "row var = concat(5)",
"query": "row var = concat(5, 5, 5)",
"error": true
},
{
@ -3535,11 +3535,11 @@
"error": true
},
{
"query": "from a_index | where length(concat(stringField)) > 0",
"query": "from a_index | where length(concat(stringField, stringField, stringField)) > 0",
"error": false
},
{
"query": "from a_index | where length(concat(numberField)) > 0",
"query": "from a_index | where length(concat(numberField, numberField, numberField)) > 0",
"error": true
},
{
@ -4567,11 +4567,11 @@
"error": true
},
{
"query": "from a_index | eval var = case(booleanField, stringField)",
"query": "from a_index | eval var = case(booleanField, stringField, stringField, stringField)",
"error": false
},
{
"query": "from a_index | eval case(booleanField, stringField)",
"query": "from a_index | eval case(booleanField, stringField, stringField, stringField)",
"error": false
},
{
@ -4599,19 +4599,19 @@
"error": true
},
{
"query": "from a_index | eval var = cidr_match(ipField, stringField)",
"query": "from a_index | eval var = cidr_match(ipField, stringField, stringField)",
"error": false
},
{
"query": "from a_index | eval cidr_match(ipField, stringField)",
"query": "from a_index | eval cidr_match(ipField, stringField, stringField)",
"error": false
},
{
"query": "from a_index | eval var = cidr_match(to_ip(stringField), to_string(stringField))",
"query": "from a_index | eval var = cidr_match(to_ip(stringField), to_string(stringField), to_string(stringField))",
"error": false
},
{
"query": "from a_index | eval cidr_match(stringField, numberField)",
"query": "from a_index | eval cidr_match(stringField, numberField, numberField)",
"error": true
},
{
@ -4627,23 +4627,19 @@
"error": true
},
{
"query": "from a_index | eval var = concat(stringField)",
"query": "from a_index | eval var = concat(stringField, stringField, stringField)",
"error": false
},
{
"query": "from a_index | eval concat(stringField)",
"query": "from a_index | eval concat(stringField, stringField, stringField)",
"error": false
},
{
"query": "from a_index | eval var = concat(to_string(stringField))",
"query": "from a_index | eval var = concat(to_string(stringField), to_string(stringField), to_string(stringField))",
"error": false
},
{
"query": "from a_index | eval concat(numberField)",
"error": true
},
{
"query": "from a_index | eval var = concat(*)",
"query": "from a_index | eval concat(numberField, numberField, numberField)",
"error": true
},
{

24
packages/kbn-monaco/src/esql/lib/ast/validation/types.ts
View file

@ -47,7 +47,29 @@ export interface ValidationErrors {
};
wrongArgumentNumber: {
message: string;
type: { fn: string; numArgs: number; passedArgs: number; exactly: number };
type: {
fn: string;
numArgs: number;
passedArgs: number;
};
};
wrongArgumentNumberTooMany: {
message: string;
type: {
fn: string;
numArgs: number;
passedArgs: number;
extraArgs: number;
};
};
wrongArgumentNumberTooFew: {
message: string;
type: {
fn: string;
numArgs: number;
passedArgs: number;
missingArgs: number;
};
};
unknownColumn: {
message: string;

104
packages/kbn-monaco/src/esql/lib/ast/validation/validation.test.ts
View file

@ -503,11 +503,11 @@ describe('validation logic', () => {
}
for (const { name, alias, signatures, ...defRest } of evalFunctionsDefinitions) {
for (const { params, returnType } of signatures) {
for (const { params, infiniteParams, ...signRest } of signatures) {
const fieldMapping = getFieldMapping(params);
const signatureStringCorrect = tweakSignatureForRowCommand(
getFunctionSignatures(
{ name, ...defRest, signatures: [{ params: fieldMapping, returnType }] },
{ name, ...defRest, signatures: [{ params: fieldMapping, ...signRest }] },
{ withTypes: false }
)[0].declaration
);
@ -519,7 +519,11 @@ describe('validation logic', () => {
for (const otherName of alias) {
const signatureStringWithAlias = tweakSignatureForRowCommand(
getFunctionSignatures(
{ name: otherName, ...defRest, signatures: [{ params: fieldMapping, returnType }] },
{
name: otherName,
...defRest,
signatures: [{ params: fieldMapping, ...signRest }],
},
{ withTypes: false }
)[0].declaration
);
@ -545,7 +549,7 @@ describe('validation logic', () => {
{
name,
...defRest,
signatures: [{ params: fieldMappingWithNestedFunctions, returnType }],
signatures: [{ params: fieldMappingWithNestedFunctions, ...signRest }],
},
{ withTypes: false }
)[0].declaration
@ -561,7 +565,7 @@ describe('validation logic', () => {
);
const wrongSignatureString = tweakSignatureForRowCommand(
getFunctionSignatures(
{ name, ...defRest, signatures: [{ params: wrongFieldMapping, returnType }] },
{ name, ...defRest, signatures: [{ params: wrongFieldMapping, ...signRest }] },
{ withTypes: false }
)[0].declaration
);
@ -1024,7 +1028,7 @@ describe('validation logic', () => {
}
testErrorsAndWarnings(`from a_index | where cidr_match(ipField)`, [
`Error building [cidr_match]: expects exactly 2 arguments, passed 1 instead.`,
`Error building [cidr_match]: expects at least 2 arguments, passed 1 instead.`,
]);
testErrorsAndWarnings(
`from a_index | eval cidr = "172.0.0.1/30" | where cidr_match(ipField, "172.0.0.1/30", cidr)`,
@ -1054,7 +1058,7 @@ describe('validation logic', () => {
const supportedSignatures = signatures.filter(({ returnType }) =>
['number', 'string'].includes(returnType)
);
for (const { params, returnType } of supportedSignatures) {
for (const { params, returnType, ...restSign } of supportedSignatures) {
const correctMapping = params
.filter(({ optional }) => !optional)
.map(({ type }) =>
@ -1066,7 +1070,7 @@ describe('validation logic', () => {
`from a_index | where ${returnType !== 'number' ? 'length(' : ''}${
// hijacking a bit this function to produce a function call
getFunctionSignatures(
{ name, ...rest, signatures: [{ params: correctMapping, returnType }] },
{ name, ...rest, signatures: [{ params: correctMapping, returnType, ...restSign }] },
{ withTypes: false }
)[0].declaration
}${returnType !== 'number' ? ')' : ''} > 0`,
@ -1083,7 +1087,11 @@ describe('validation logic', () => {
`from a_index | where ${returnType !== 'number' ? 'length(' : ''}${
// hijacking a bit this function to produce a function call
getFunctionSignatures(
{ name, ...rest, signatures: [{ params: wrongFieldMapping, returnType }] },
{
name,
...rest,
signatures: [{ params: wrongFieldMapping, returnType, ...restSign }],
},
{ withTypes: false }
)[0].declaration
}${returnType !== 'number' ? ')' : ''} > 0`,
@ -1182,12 +1190,16 @@ describe('validation logic', () => {
}
for (const { name, alias, signatures, ...defRest } of evalFunctionsDefinitions) {
for (const { params, returnType, infiniteParams, minParams } of signatures) {
for (const { params, infiniteParams, ...signRest } of signatures) {
const fieldMapping = getFieldMapping(params);
testErrorsAndWarnings(
`from a_index | eval var = ${
getFunctionSignatures(
{ name, ...defRest, signatures: [{ params: fieldMapping, returnType }] },
{
name,
...defRest,
signatures: [{ params: fieldMapping, ...signRest }],
},
{ withTypes: false }
)[0].declaration
}`,
@ -1196,7 +1208,7 @@ describe('validation logic', () => {
testErrorsAndWarnings(
`from a_index | eval ${
getFunctionSignatures(
{ name, ...defRest, signatures: [{ params: fieldMapping, returnType }] },
{ name, ...defRest, signatures: [{ params: fieldMapping, ...signRest }] },
{ withTypes: false }
)[0].declaration
}`,
@ -1218,7 +1230,7 @@ describe('validation logic', () => {
{
name,
...defRest,
signatures: [{ params: fieldMappingWithoutLiterals, returnType }],
signatures: [{ params: fieldMappingWithoutLiterals, ...signRest }],
},
{ withTypes: false }
)[0].declaration
@ -1232,7 +1244,11 @@ describe('validation logic', () => {
if (alias) {
for (const otherName of alias) {
const signatureStringWithAlias = getFunctionSignatures(
{ name: otherName, ...defRest, signatures: [{ params: fieldMapping, returnType }] },
{
name: otherName,
...defRest,
signatures: [{ params: fieldMapping, ...signRest }],
},
{ withTypes: false }
)[0].declaration;
@ -1258,7 +1274,7 @@ describe('validation logic', () => {
{
name,
...defRest,
signatures: [{ params: fieldMappingWithNestedFunctions, returnType }],
signatures: [{ params: fieldMappingWithNestedFunctions, ...signRest }],
},
{ withTypes: false }
)[0].declaration
@ -1274,42 +1290,52 @@ describe('validation logic', () => {
testErrorsAndWarnings(
`from a_index | eval ${
getFunctionSignatures(
{ name, ...defRest, signatures: [{ params: wrongFieldMapping, returnType }] },
{ name, ...defRest, signatures: [{ params: wrongFieldMapping, ...signRest }] },
{ withTypes: false }
)[0].declaration
}`,
expectedErrors
);
if (!infiniteParams && !minParams) {
if (!infiniteParams && !signRest.minParams) {
// test that additional args are spotted
const fieldMappingWithOneExtraArg = getFieldMapping(params).concat({
name: 'extraArg',
type: 'number',
});
const refSignature = signatures[0];
// get the expected args from the first signature in case of errors
const expectedArgs = signatures[0].params.filter(({ optional }) => !optional).length;
const shouldBeExactly = signatures[0].params.length;
const minNumberOfArgs = refSignature.params.filter(({ optional }) => !optional).length;
const fullNumberOfArgs = refSignature.params.length;
const hasOptionalArgs = minNumberOfArgs < fullNumberOfArgs;
const hasTooManyArgs = fieldMappingWithOneExtraArg.length > fullNumberOfArgs;
// the validation engine tries to be smart about signatures with optional args
let messageQuantifier = 'exactly ';
if (hasOptionalArgs && hasTooManyArgs) {
messageQuantifier = 'no more than ';
}
if (!hasOptionalArgs && !hasTooManyArgs) {
messageQuantifier = 'at least ';
}
testErrorsAndWarnings(
`from a_index | eval ${
getFunctionSignatures(
{
name,
...defRest,
signatures: [{ params: fieldMappingWithOneExtraArg, returnType }],
signatures: [{ params: fieldMappingWithOneExtraArg, ...signRest }],
},
{ withTypes: false }
)[0].declaration
}`,
[
`Error building [${name}]: expects ${
shouldBeExactly - expectedArgs === 0 ? 'exactly ' : ''
}${
expectedArgs === 1
`Error building [${name}]: expects ${messageQuantifier}${
fullNumberOfArgs === 1
? 'one argument'
: expectedArgs === 0
: fullNumberOfArgs === 0
? '0 arguments'
: `${expectedArgs} arguments`
: `${fullNumberOfArgs} arguments`
}, passed ${fieldMappingWithOneExtraArg.length} instead.`,
]
);
@ -1317,7 +1343,7 @@ describe('validation logic', () => {
}
// test that wildcard won't work as arg
if (fieldMapping.length === 1) {
if (fieldMapping.length === 1 && !signRest.minParams) {
const fieldMappingWithWildcard = [...fieldMapping];
fieldMappingWithWildcard[0].name = '*';
@ -1327,7 +1353,7 @@ describe('validation logic', () => {
{
name,
...defRest,
signatures: [{ params: fieldMappingWithWildcard, returnType }],
signatures: [{ params: fieldMappingWithWildcard, ...signRest }],
},
{ withTypes: false }
)[0].declaration
@ -1692,17 +1718,17 @@ describe('validation logic', () => {
]);
for (const { name, alias, signatures, ...defRest } of statsAggregationFunctionDefinitions) {
for (const { params, returnType } of signatures) {
for (const { params, infiniteParams, ...signRest } of signatures) {
const fieldMapping = getFieldMapping(params);
const correctSignature = getFunctionSignatures(
{ name, ...defRest, signatures: [{ params: fieldMapping, returnType }] },
{ name, ...defRest, signatures: [{ params: fieldMapping, ...signRest }] },
{ withTypes: false }
)[0].declaration;
testErrorsAndWarnings(`from a_index | stats var = ${correctSignature}`, []);
testErrorsAndWarnings(`from a_index | stats ${correctSignature}`, []);
if (returnType === 'number') {
if (signRest.returnType === 'number') {
testErrorsAndWarnings(`from a_index | stats var = round(${correctSignature})`, []);
testErrorsAndWarnings(`from a_index | stats round(${correctSignature})`, []);
testErrorsAndWarnings(
@ -1731,7 +1757,7 @@ describe('validation logic', () => {
{
name,
...defRest,
signatures: [{ params: fieldMappingWithoutLiterals, returnType }],
signatures: [{ params: fieldMappingWithoutLiterals, ...signRest }],
},
{ withTypes: false }
)[0].declaration
@ -1745,7 +1771,7 @@ describe('validation logic', () => {
if (alias) {
for (const otherName of alias) {
const signatureStringWithAlias = getFunctionSignatures(
{ name: otherName, ...defRest, signatures: [{ params: fieldMapping, returnType }] },
{ name: otherName, ...defRest, signatures: [{ params: fieldMapping, ...signRest }] },
{ withTypes: false }
)[0].declaration;
@ -1763,7 +1789,7 @@ describe('validation logic', () => {
{
name,
...defRest,
signatures: [{ params: fieldMappingWithNestedBuiltinFunctions, returnType }],
signatures: [{ params: fieldMappingWithNestedBuiltinFunctions, ...signRest }],
},
{ withTypes: false }
)[0].declaration;
@ -1787,7 +1813,7 @@ describe('validation logic', () => {
{
name,
...defRest,
signatures: [{ params: fieldMappingWithNestedEvalAndBuiltinFunctions, returnType }],
signatures: [{ params: fieldMappingWithNestedEvalAndBuiltinFunctions, ...signRest }],
},
{ withTypes: false }
)[0].declaration;
@ -1856,7 +1882,7 @@ describe('validation logic', () => {
{
name,
...defRest,
signatures: [{ params: fieldMappingWithNestedAggsFunctions, returnType }],
signatures: [{ params: fieldMappingWithNestedAggsFunctions, ...signRest }],
},
{ withTypes: false }
)[0].declaration
@ -1869,7 +1895,7 @@ describe('validation logic', () => {
{
name,
...defRest,
signatures: [{ params: fieldMappingWithNestedAggsFunctions, returnType }],
signatures: [{ params: fieldMappingWithNestedAggsFunctions, ...signRest }],
},
{ withTypes: false }
)[0].declaration
@ -1886,7 +1912,7 @@ describe('validation logic', () => {
testErrorsAndWarnings(
`from a_index | stats ${
getFunctionSignatures(
{ name, ...defRest, signatures: [{ params: wrongFieldMapping, returnType }] },
{ name, ...defRest, signatures: [{ params: wrongFieldMapping, ...signRest }] },
{ withTypes: false }
)[0].declaration
}`,
@ -1905,7 +1931,7 @@ describe('validation logic', () => {
{
name,
...defRest,
signatures: [{ params: fieldMappingWithWildcard, returnType }],
signatures: [{ params: fieldMappingWithWildcard, ...signRest }],
},
{ withTypes: false }
)[0].declaration

76
packages/kbn-monaco/src/esql/lib/ast/validation/validation.ts
View file

@ -255,16 +255,16 @@ function extractCompatibleSignaturesForFunction(
astFunction: ESQLFunction
) {
return fnDef.signatures.filter((def) => {
if (def.infiniteParams && astFunction.args.length > 0) {
return true;
if (def.infiniteParams) {
return astFunction.args.length > 0;
}
if (def.minParams && astFunction.args.length >= def.minParams) {
return true;
if (def.minParams) {
return astFunction.args.length >= def.minParams;
}
if (astFunction.args.length === def.params.length) {
return true;
}
return astFunction.args.length === def.params.filter(({ optional }) => !optional).length;
return (
astFunction.args.length >= def.params.filter(({ optional }) => !optional).length &&
astFunction.args.length <= def.params.length
);
});
}
@ -322,19 +322,53 @@ function validateFunction(
}
const matchingSignatures = extractCompatibleSignaturesForFunction(fnDefinition, astFunction);
if (!matchingSignatures.length) {
const numArgs = fnDefinition.signatures[0].params.filter(({ optional }) => !optional).length;
messages.push(
getMessageFromId({
messageId: 'wrongArgumentNumber',
values: {
fn: astFunction.name,
numArgs,
passedArgs: astFunction.args.length,
exactly: fnDefinition.signatures[0].params.length - numArgs,
},
locations: astFunction.location,
})
);
const refSignature = fnDefinition.signatures[0];
const numArgs =
refSignature.minParams ?? refSignature.params.filter(({ optional }) => !optional).length;
if (
!refSignature.minParams &&
refSignature.params.filter(({ optional }) => !optional).length === refSignature.params.length
) {
messages.push(
getMessageFromId({
messageId: 'wrongArgumentNumber',
values: {
fn: astFunction.name,
numArgs:
refSignature.minParams ??
refSignature.params.filter(({ optional }) => !optional).length,
passedArgs: astFunction.args.length,
},
locations: astFunction.location,
})
);
} else if (Math.max(astFunction.args.length - refSignature.params.length, 0) > 0) {
messages.push(
getMessageFromId({
messageId: 'wrongArgumentNumberTooMany',
values: {
fn: astFunction.name,
numArgs: refSignature.params.length,
passedArgs: astFunction.args.length,
extraArgs: Math.max(astFunction.args.length - refSignature.params.length, 0),
},
locations: astFunction.location,
})
);
} else {
messages.push(
getMessageFromId({
messageId: 'wrongArgumentNumberTooFew',
values: {
fn: astFunction.name,
numArgs,
passedArgs: astFunction.args.length,
missingArgs: Math.max(numArgs - astFunction.args.length, 0),
},
locations: astFunction.location,
})
);
}
}
// now perform the same check on all functions args
for (const arg of astFunction.args) {