Skip to content

Commit

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

This is a follow-up to #186340. I
forgot to capitalize the labels for functions and operators. Fixed now.

**Before**
<img width="1321" alt="Screenshot 2024-07-24 at 10 26 52 AM"
src="https://github.com/user-attachments/assets/05102828-0469-4b14-a2f9-34a160edbd18">

**After**
<img width="1286" alt="Screenshot 2024-07-24 at 10 23 59 AM"
src="https://github.com/user-attachments/assets/dd01ccde-ec12-4ad3-98f7-561e5d233e52">

Also, this PR changes the validation error messages to actually reflect
what the user has typed:

**Before**
<img width="697" alt="Screenshot 2024-07-24 at 11 43 54 AM"
src="https://github.com/user-attachments/assets/28cbbd4c-1b3e-4d22-a1af-33b2f17af597">

**After**
<img width="709" alt="Screenshot 2024-07-24 at 11 40 48 AM"
src="https://github.com/user-attachments/assets/dffd4a29-d348-44d0-ba58-00a0a70d04b3">
<img width="663" alt="Screenshot 2024-07-24 at 11 39 54 AM"
src="https://github.com/user-attachments/assets/dff0c726-d028-4826-a1cd-030e733987a7">


### 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
  • Loading branch information
drewdaemon authored Jul 25, 2024
1 parent c5a00f6 commit 05b6d7e
Show file tree
Hide file tree
Showing 9 changed files with 173 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,12 @@ describe('autocomplete.suggest', () => {
test('on function left paren', async () => {
const { assertSuggestions } = await setup();

await assertSuggestions(
'from a | stats by bucket(/',
[
...getFieldNamesByType(['number', 'date']),
...getFunctionSignaturesByReturnType('eval', ['date', 'number'], { scalar: true }),
].map((field) => `${field},`)
);
await assertSuggestions('from a | stats by bucket(/', [
...getFieldNamesByType(['number', 'date']).map((field) => `${field},`),
...getFunctionSignaturesByReturnType('eval', ['date', 'number'], { scalar: true }).map(
(s) => ({ ...s, text: `${s.text},` })
),
]);

await assertSuggestions('from a | stats round(/', [
...getFunctionSignaturesByReturnType('stats', 'number', { agg: true, grouping: true }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import * as autocomplete from '../autocomplete';
import type { ESQLCallbacks } from '../../shared/types';
import type { EditorContext, SuggestionRawDefinition } from '../types';
import { TIME_SYSTEM_PARAMS } from '../factories';
import { getFunctionSignatures } from '../../definitions/helpers';

export interface Integration {
name: string;
Expand Down Expand Up @@ -142,7 +143,7 @@ export function getFunctionSignaturesByReturnType(
paramsTypes?: string[],
ignored?: string[],
option?: string
) {
): PartialSuggestionWithText[] {
const expectedReturnType = Array.isArray(_expectedReturnType)
? _expectedReturnType
: [_expectedReturnType];
Expand Down Expand Up @@ -203,13 +204,25 @@ export 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 @@ -300,11 +313,28 @@ export const setup = async (caret = '/') => {
);
};

const assertSuggestions = async (query: string, expected: string[], opts?: SuggestOptions) => {
const assertSuggestions = async (
query: string,
expected: Array<string | PartialSuggestionWithText>,
opts?: SuggestOptions
) => {
const result = await suggest(query, opts);
const resultTexts = [...result.map((suggestion) => suggestion.text)].sort();

expect(resultTexts).toEqual([...expected].sort());
const expectedTexts = expected
.map((suggestion) => (typeof suggestion === 'string' ? suggestion : suggestion.text ?? ''))
.sort();

expect(resultTexts).toEqual(expectedTexts);

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

for (const expectedSuggestion of expectedNonStringSuggestions) {
const suggestion = result.find((s) => s.text === expectedSuggestion.text);
expect(suggestion).toEqual(expect.objectContaining(expectedSuggestion));
}
};

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ describe('autocomplete', () => {
...getFieldNamesByType('string').map((v) => `${v},`),
...getFunctionSignaturesByReturnType('eval', 'string', { scalar: true }, undefined, [
'concat',
]).map((v) => `${v},`),
]).map((v) => ({ ...v, text: `${v.text},` })),
]);
testSuggestions(
'from a | eval a=concat(stringField, ',
Expand Down Expand Up @@ -696,7 +696,7 @@ describe('autocomplete', () => {
...getFieldNamesByType('ip').map((v) => `${v},`),
...getFunctionSignaturesByReturnType('eval', 'ip', { scalar: true }, undefined, [
'cidr_match',
]).map((v) => `${v},`),
]).map((v) => ({ ...v, text: `${v.text},` })),
]);
testSuggestions(
'from a | eval a=cidr_match(ipField, ',
Expand Down Expand Up @@ -884,7 +884,7 @@ describe('autocomplete', () => {
...getLiteralsByType('time_literal').map((t) => `${t},`),
...getFunctionSignaturesByReturnType('eval', 'date', { scalar: true }, undefined, [
'date_trunc',
]).map((t) => `${t},`),
]).map((t) => ({ ...t, text: `${t.text},` })),
...getFieldNamesByType('date').map((t) => `${t},`),
TIME_PICKER_SUGGESTION,
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,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 @@ -69,7 +69,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 @@ -287,13 +287,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 @@ -310,7 +309,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 05b6d7e

Please sign in to comment.