Skip to content

Commit

Permalink
[8.15] [ES|QL] capitalize function and operator signatures (#189109) (#…
Browse files Browse the repository at this point in the history
…189216)

# Backport

This will backport the following commits from `main` to `8.15`:
- [[ES|QL] capitalize function and operator signatures
(#189109)](#189109)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Drew
Tate","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-07-25T14:32:52Z","message":"[ES|QL]
capitalize function and operator signatures (#189109)\n\n##
Summary\r\n\r\nThis is a follow-up to
#186340. I\r\nforgot to capitalize
the labels for functions and operators. Fixed
now.\r\n\r\n**Before**\r\n<img width=\"1321\" alt=\"Screenshot
2024-07-24 at 10 26
52 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/05102828-0469-4b14-a2f9-34a160edbd18\">\r\n\r\n**After**\r\n<img
width=\"1286\" alt=\"Screenshot 2024-07-24 at 10 23
59 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/dd01ccde-ec12-4ad3-98f7-561e5d233e52\">\r\n\r\nAlso,
this PR changes the validation error messages to actually
reflect\r\nwhat the user has typed:\r\n\r\n**Before**\r\n<img
width=\"697\" alt=\"Screenshot 2024-07-24 at 11 43
54 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/28cbbd4c-1b3e-4d22-a1af-33b2f17af597\">\r\n\r\n**After**\r\n<img
width=\"709\" alt=\"Screenshot 2024-07-24 at 11 40
48 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/dffd4a29-d348-44d0-ba58-00a0a70d04b3\">\r\n<img
width=\"663\" alt=\"Screenshot 2024-07-24 at 11 39
54 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/dff0c726-d028-4826-a1cd-030e733987a7\">\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"05b6d7eacc7dfa0f0419982cd002e83e8be6c1b0","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:prev-minor","Feature:ES|QL","Team:ESQL","v8.15.0","v8.16.0"],"number":189109,"url":"https://github.com/elastic/kibana/pull/189109","mergeCommit":{"message":"[ES|QL]
capitalize function and operator signatures (#189109)\n\n##
Summary\r\n\r\nThis is a follow-up to
#186340. I\r\nforgot to capitalize
the labels for functions and operators. Fixed
now.\r\n\r\n**Before**\r\n<img width=\"1321\" alt=\"Screenshot
2024-07-24 at 10 26
52 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/05102828-0469-4b14-a2f9-34a160edbd18\">\r\n\r\n**After**\r\n<img
width=\"1286\" alt=\"Screenshot 2024-07-24 at 10 23
59 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/dd01ccde-ec12-4ad3-98f7-561e5d233e52\">\r\n\r\nAlso,
this PR changes the validation error messages to actually
reflect\r\nwhat the user has typed:\r\n\r\n**Before**\r\n<img
width=\"697\" alt=\"Screenshot 2024-07-24 at 11 43
54 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/28cbbd4c-1b3e-4d22-a1af-33b2f17af597\">\r\n\r\n**After**\r\n<img
width=\"709\" alt=\"Screenshot 2024-07-24 at 11 40
48 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/dffd4a29-d348-44d0-ba58-00a0a70d04b3\">\r\n<img
width=\"663\" alt=\"Screenshot 2024-07-24 at 11 39
54 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/dff0c726-d028-4826-a1cd-030e733987a7\">\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"05b6d7eacc7dfa0f0419982cd002e83e8be6c1b0"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"8.15","label":"v8.15.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/189109","number":189109,"mergeCommit":{"message":"[ES|QL]
capitalize function and operator signatures (#189109)\n\n##
Summary\r\n\r\nThis is a follow-up to
#186340. I\r\nforgot to capitalize
the labels for functions and operators. Fixed
now.\r\n\r\n**Before**\r\n<img width=\"1321\" alt=\"Screenshot
2024-07-24 at 10 26
52 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/05102828-0469-4b14-a2f9-34a160edbd18\">\r\n\r\n**After**\r\n<img
width=\"1286\" alt=\"Screenshot 2024-07-24 at 10 23
59 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/dd01ccde-ec12-4ad3-98f7-561e5d233e52\">\r\n\r\nAlso,
this PR changes the validation error messages to actually
reflect\r\nwhat the user has typed:\r\n\r\n**Before**\r\n<img
width=\"697\" alt=\"Screenshot 2024-07-24 at 11 43
54 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/28cbbd4c-1b3e-4d22-a1af-33b2f17af597\">\r\n\r\n**After**\r\n<img
width=\"709\" alt=\"Screenshot 2024-07-24 at 11 40
48 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/dffd4a29-d348-44d0-ba58-00a0a70d04b3\">\r\n<img
width=\"663\" alt=\"Screenshot 2024-07-24 at 11 39
54 AM\"\r\nsrc=\"https://github.com/user-attachments/assets/dff0c726-d028-4826-a1cd-030e733987a7\">\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"05b6d7eacc7dfa0f0419982cd002e83e8be6c1b0"}}]}]
BACKPORT-->
  • Loading branch information
drewdaemon authored Jul 26, 2024
1 parent b030e14 commit 2bf9de9
Show file tree
Hide file tree
Showing 7 changed files with 188 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ import { FunctionParameter } from '../definitions/types';
import { getParamAtPosition } from './helper';
import { nonNullable } from '../shared/helpers';
import { METADATA_FIELDS } from '../shared/constants';
import { SuggestionRawDefinition } from './types';
import { getFunctionSignatures } from '../definitions/helpers';

export type PartialSuggestionWithText = Partial<SuggestionRawDefinition> & { text: string };

interface Integration {
name: string;
Expand Down Expand Up @@ -196,13 +200,25 @@ function getFunctionSignaturesByReturnType(
return true;
})
.sort(({ name: a }, { name: b }) => a.localeCompare(b))
.map(({ type, name, signatures }) => {
.map<PartialSuggestionWithText>((definition) => {
const { type, name, signatures } = definition;

if (type === 'builtin') {
return signatures.some(({ params }) => params.length > 1)
? `${name.toUpperCase()} $0`
: name.toUpperCase();
return {
text: signatures.some(({ params }) => params.length > 1)
? `${name.toUpperCase()} $0`
: name.toUpperCase(),
label: name.toUpperCase(),
};
}
return `${name.toUpperCase()}($0)`;
const printedSignatures = getFunctionSignatures(definition, {
withTypes: true,
capitalize: true,
});
return {
text: `${name.toUpperCase()}($0)`,
label: printedSignatures[0].declaration,
};
});
}

Expand Down Expand Up @@ -271,14 +287,14 @@ function getPolicyFields(policyName: string) {
describe('autocomplete', () => {
type TestArgs = [
string,
string[],
Array<string | PartialSuggestionWithText>,
(string | number)?,
Parameters<typeof createCustomCallbackMocks>?
];

const testSuggestionsFn = (
statement: string,
expected: string[],
expected: Array<string | PartialSuggestionWithText>,
triggerCharacter: string | number = '',
customCallbacksArgs: Parameters<typeof createCustomCallbackMocks> = [
undefined,
Expand Down Expand Up @@ -312,10 +328,22 @@ describe('autocomplete', () => {
callbackMocks
);

const sortedSuggestions = suggestions.map((suggestion) => suggestion.text).sort();
const sortedExpected = expected.sort();
const sortedSuggestionTexts = suggestions.map((suggestion) => suggestion.text).sort();
const sortedExpectedTexts = expected
.map((suggestion) =>
typeof suggestion === 'string' ? suggestion : suggestion.text ?? ''
)
.sort();

expect(sortedSuggestionTexts).toEqual(sortedExpectedTexts);
const expectedNonStringSuggestions = expected.filter(
(suggestion) => typeof suggestion !== 'string'
) as PartialSuggestionWithText[];

expect(sortedSuggestions).toEqual(sortedExpected);
for (const expectedSuggestion of expectedNonStringSuggestions) {
const suggestion = suggestions.find((s) => s.text === expectedSuggestion.text);
expect(suggestion).toEqual(expect.objectContaining(expectedSuggestion));
}
}
);
};
Expand Down Expand Up @@ -786,13 +814,12 @@ describe('autocomplete', () => {
]);

// expect "bucket" NOT to be suggested for its own parameter
testSuggestions(
'from a | stats by bucket(',
[
...getFieldNamesByType(['number', 'date']),
...getFunctionSignaturesByReturnType('eval', ['date', 'number'], { evalMath: true }),
].map((field) => `${field},`)
);
testSuggestions('from a | stats by bucket(', [
...getFieldNamesByType(['number', 'date']).map((field) => `${field},`),
...getFunctionSignaturesByReturnType('eval', ['date', 'number'], { evalMath: true }).map(
(s) => ({ ...s, text: s.text + ',' })
),
]);

testSuggestions('from a | stats avg(b) by numberField % 2 ', [',', '|']);

Expand Down Expand Up @@ -1104,7 +1131,7 @@ describe('autocomplete', () => {
...getFieldNamesByType('string').map((v) => `${v},`),
...getFunctionSignaturesByReturnType('eval', 'string', { evalMath: true }, undefined, [
'concat',
]).map((v) => `${v},`),
]).map((v) => ({ ...v, text: `${v.text},` })),
]);
testSuggestions('from a | eval a=concat(stringField, ', [
...getFieldNamesByType('string'),
Expand All @@ -1124,7 +1151,7 @@ describe('autocomplete', () => {
...getFieldNamesByType('ip').map((v) => `${v},`),
...getFunctionSignaturesByReturnType('eval', 'ip', { evalMath: true }, undefined, [
'cidr_match',
]).map((v) => `${v},`),
]).map((v) => ({ ...v, text: `${v.text},` })),
]);
testSuggestions('from a | eval a=cidr_match(ipField, ', [
...getFieldNamesByType('string'),
Expand Down Expand Up @@ -1201,25 +1228,29 @@ 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},` };
};

testSuggestions(
`from a | eval ${fn.name}(${Array(i).fill('field').join(', ')}${i ? ',' : ''} )`,
suggestedConstants?.length
? suggestedConstants.map((option) => `"${option}"${requiresMoreArgs ? ',' : ''}`)
: [
...getFieldNamesByType(getTypesFromParamDefs(acceptsFieldParamDefs)).map(
(f) => (requiresMoreArgs ? `${f},` : f)
),
...getFieldNamesByType(getTypesFromParamDefs(acceptsFieldParamDefs)),
...getFunctionSignaturesByReturnType(
'eval',
getTypesFromParamDefs(acceptsFieldParamDefs),
{ evalMath: true },
undefined,
[fn.name]
).map((l) => (requiresMoreArgs ? `${l},` : l)),
...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)).map((d) =>
requiresMoreArgs ? `${d},` : d
),
]
...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)),
].map(addCommaIfRequired)
);
testSuggestions(
`from a | eval var0 = ${fn.name}(${Array(i).fill('field').join(', ')}${
Expand All @@ -1228,20 +1259,16 @@ describe('autocomplete', () => {
suggestedConstants?.length
? suggestedConstants.map((option) => `"${option}"${requiresMoreArgs ? ',' : ''}`)
: [
...getFieldNamesByType(getTypesFromParamDefs(acceptsFieldParamDefs)).map(
(f) => (requiresMoreArgs ? `${f},` : f)
),
...getFieldNamesByType(getTypesFromParamDefs(acceptsFieldParamDefs)),
...getFunctionSignaturesByReturnType(
'eval',
getTypesFromParamDefs(acceptsFieldParamDefs),
{ evalMath: true },
undefined,
[fn.name]
).map((l) => (requiresMoreArgs ? `${l},` : l)),
...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)).map((d) =>
requiresMoreArgs ? `${d},` : d
),
]
...getLiteralsByType(getTypesFromParamDefs(constantOnlyParamDefs)),
].map(addCommaIfRequired)
);
}
});
Expand Down Expand Up @@ -1283,13 +1310,14 @@ describe('autocomplete', () => {
'number',
]),
]);

testSuggestions(
'from a | eval var0=date_trunc()',
[
...getLiteralsByType('time_literal').map((t) => `${t},`),
...getFunctionSignaturesByReturnType('eval', 'date', { evalMath: true }, undefined, [
'date_trunc',
]).map((t) => `${t},`),
]).map((t) => ({ ...t, text: `${t.text},` })),
...getFieldNamesByType('date').map((t) => `${t},`),
],
'('
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function getSafeInsertSourceText(text: string) {
}

export function getSuggestionFunctionDefinition(fn: FunctionDefinition): SuggestionRawDefinition {
const fullSignatures = getFunctionSignatures(fn);
const fullSignatures = getFunctionSignatures(fn, { capitalize: true, withTypes: true });
return {
label: fullSignatures[0].declaration,
text: `${fn.name.toUpperCase()}($0)`,
Expand All @@ -66,7 +66,7 @@ export function getSuggestionFunctionDefinition(fn: FunctionDefinition): Suggest
export function getSuggestionBuiltinDefinition(fn: FunctionDefinition): SuggestionRawDefinition {
const hasArgs = fn.signatures.some(({ params }) => params.length > 1);
return {
label: fn.name,
label: fn.name.toUpperCase(),
text: hasArgs ? `${fn.name.toUpperCase()} $0` : fn.name.toUpperCase(),
asSnippet: hasArgs,
kind: 'Operator',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,18 @@ import type { CommandDefinition, FunctionDefinition } from './types';
*/
export function getFunctionSignatures(
{ name, signatures }: FunctionDefinition,
{ withTypes }: { withTypes: boolean } = { withTypes: true }
{ withTypes, capitalize }: { withTypes: boolean; capitalize?: boolean } = {
withTypes: true,
capitalize: false,
}
) {
return signatures.map(({ params, returnType, minParams }) => {
// 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 extraArg = Array(minParamsToAdd || 1).fill(params[Math.max(params.length - 1, 0)]);
return {
declaration: `${name}(${params
declaration: `${capitalize ? name.toUpperCase() : name}(${params
.map((arg) => printArguments(arg, withTypes))
.join(', ')}${handleAdditionalArgs(minParamsToAdd > 0, extraArg, withTypes)})${
withTypes ? `: ${returnType}` : ''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,13 +290,12 @@ export function areFieldAndVariableTypesCompatible(
return fieldType === variableType;
}

export function printFunctionSignature(arg: ESQLFunction, useCaps = true): string {
export function printFunctionSignature(arg: ESQLFunction): string {
const fnDef = getFunctionDefinition(arg.name);
if (fnDef) {
const signature = getFunctionSignatures(
{
...fnDef,
name: useCaps ? fnDef.name.toUpperCase() : fnDef.name,
signatures: [
{
...fnDef?.signatures[0],
Expand All @@ -313,7 +312,7 @@ export function printFunctionSignature(arg: ESQLFunction, useCaps = true): strin
},
],
},
{ withTypes: false }
{ withTypes: false, capitalize: true }
);
return signature[0].declaration;
}
Expand Down
Loading

0 comments on commit 2bf9de9

Please sign in to comment.