Skip to content

Commit

Permalink
[ES|QL] Changes function definition type to enum (elastic#211782)
Browse files Browse the repository at this point in the history
  • Loading branch information
stratoula authored Feb 20, 2025
1 parent a4c7c8b commit 6b3fce9
Show file tree
Hide file tree
Showing 25 changed files with 312 additions and 263 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export {
} from './src/shared/helpers';
export { ENRICH_MODES } from './src/definitions/settings';
export { timeUnits } from './src/definitions/literals';
export { aggregationFunctionDefinitions } from './src/definitions/generated/aggregation_functions';
export { aggFunctionDefinitions } from './src/definitions/generated/aggregation_functions';
export { getFunctionSignatures } from './src/definitions/helpers';

export {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
FunctionParameterType,
FunctionReturnType,
Signature,
FunctionDefinitionTypes,
} from '../src/definitions/types';
import { FULL_TEXT_SEARCH_FUNCTIONS } from '../src/shared/constants';
const aliasTable: Record<string, string[]> = {
Expand Down Expand Up @@ -91,7 +92,7 @@ const excludedFunctions = new Set(['case']);

const extraFunctions: FunctionDefinition[] = [
{
type: 'scalar',
type: FunctionDefinitionTypes.SCALAR,
name: 'case',
description:
'Accepts pairs of conditions and values. The function returns the value that belongs to the first condition that evaluates to `true`. If the number of arguments is odd, the last argument is the default value which is returned when no condition matches.',
Expand Down Expand Up @@ -307,7 +308,7 @@ function getFunctionDefinition(ESFunctionDefinition: Record<string, any>): Funct
FunctionDefinition,
'supportedCommands' | 'supportedOptions'
> =
ESFunctionDefinition.type === 'scalar'
ESFunctionDefinition.type === FunctionDefinitionTypes.SCALAR
? scalarSupportedCommandsAndOptions
: aggregationSupportedCommandsAndOptions;

Expand Down Expand Up @@ -765,7 +766,7 @@ const enrichOperators = (
// so we are overriding to add proper support
supportedCommands,
supportedOptions,
type: 'operator' as const,
type: FunctionDefinitionTypes.OPERATOR,
validate: validators[op.name],
...(isNotOperator ? { ignoreAsSuggestion: true } : {}),
};
Expand All @@ -774,7 +775,7 @@ const enrichOperators = (

function printGeneratedFunctionsFile(
functionDefinitions: FunctionDefinition[],
functionsType: 'aggregation' | 'scalar' | 'operator' | 'grouping'
functionsType: FunctionDefinitionTypes
) {
/**
* Deals with asciidoc internal cross-references in the function descriptions
Expand Down Expand Up @@ -828,7 +829,7 @@ function printGeneratedFunctionsFile(
}
return `// Do not edit this manually... generated by scripts/generate_function_definitions.ts
const ${getDefinitionName(name)}: FunctionDefinition = {
type: '${type}',
type: FunctionDefinitionTypes.${type.toUpperCase()},
name: '${functionName}',
description: i18n.translate('kbn-esql-validation-autocomplete.esql.definitions.${name}', { defaultMessage: ${JSON.stringify(
removeAsciiDocInternalCrossReferences(removeInlineAsciiDocLinks(description), functionNames)
Expand Down Expand Up @@ -867,14 +868,18 @@ function printGeneratedFunctionsFile(
*/
import { i18n } from '@kbn/i18n';
import type { FunctionDefinition } from '../types';
import { type FunctionDefinition, FunctionDefinitionTypes } from '../types';
${
functionsType === 'scalar'
functionsType === FunctionDefinitionTypes.SCALAR
? `import type { ESQLFunction } from '@kbn/esql-ast';
import { isLiteralItem } from '../../shared/helpers';`
: ''
}
${functionsType === 'operator' ? `import { isNumericType } from '../../shared/esql_types';` : ''}
${
functionsType === FunctionDefinitionTypes.OPERATOR
? `import { isNumericType } from '../../shared/esql_types';`
: ''
}
Expand Down Expand Up @@ -927,17 +932,20 @@ ${functionsType === 'operator' ? `import { isNumericType } from '../../shared/es
const isLikeOperator = functionDefinition.name.toLowerCase().includes('like');

if (functionDefinition.name.toLowerCase() === 'match') {
scalarFunctionDefinitions.push({ ...functionDefinition, type: 'scalar' });
scalarFunctionDefinitions.push({
...functionDefinition,
type: FunctionDefinitionTypes.SCALAR,
});
continue;
}
if (functionDefinition.type === 'operator' || isLikeOperator) {
if (functionDefinition.type === FunctionDefinitionTypes.OPERATOR || isLikeOperator) {
operatorDefinitions.push(functionDefinition);
}
if (functionDefinition.type === 'scalar' && !isLikeOperator) {
if (functionDefinition.type === FunctionDefinitionTypes.SCALAR && !isLikeOperator) {
scalarFunctionDefinitions.push(functionDefinition);
} else if (functionDefinition.type === 'agg') {
} else if (functionDefinition.type === FunctionDefinitionTypes.AGG) {
aggFunctionDefinitions.push(functionDefinition);
} else if (functionDefinition.type === 'grouping') {
} else if (functionDefinition.type === FunctionDefinitionTypes.GROUPING) {
groupingFunctionDefinitions.push(functionDefinition);
}
}
Expand All @@ -946,18 +954,24 @@ ${functionsType === 'operator' ? `import { isNumericType } from '../../shared/es

await writeFile(
join(__dirname, '../src/definitions/generated/scalar_functions.ts'),
printGeneratedFunctionsFile(scalarFunctionDefinitions, 'scalar')
printGeneratedFunctionsFile(scalarFunctionDefinitions, FunctionDefinitionTypes.SCALAR)
);
await writeFile(
join(__dirname, '../src/definitions/generated/aggregation_functions.ts'),
printGeneratedFunctionsFile(aggFunctionDefinitions, 'aggregation')
printGeneratedFunctionsFile(aggFunctionDefinitions, FunctionDefinitionTypes.AGG)
);
await writeFile(
join(__dirname, '../src/definitions/generated/operators.ts'),
printGeneratedFunctionsFile(enrichOperators(operatorDefinitions), 'operator')
printGeneratedFunctionsFile(
enrichOperators(operatorDefinitions),
FunctionDefinitionTypes.OPERATOR
)
);
await writeFile(
join(__dirname, '../src/definitions/generated/grouping_functions.ts'),
printGeneratedFunctionsFile(enrichGrouping(groupingFunctionDefinitions), 'grouping')
printGeneratedFunctionsFile(
enrichGrouping(groupingFunctionDefinitions),
FunctionDefinitionTypes.GROUPING
)
);
})();
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
import { ESQL_COMMON_NUMERIC_TYPES } from '../../shared/esql_types';
import { scalarFunctionDefinitions } from '../../definitions/generated/scalar_functions';
import { timeUnitsToSuggest } from '../../definitions/literals';
import { FunctionDefinitionTypes } from '../../definitions/types';
import {
getCompatibleTypesToSuggestNext,
getValidFunctionSignaturesForPreviousArgs,
Expand Down Expand Up @@ -454,7 +455,8 @@ describe('autocomplete.suggest', () => {

// Wehther to prepend comma to suggestion string
// E.g. if true, "fieldName" -> "fieldName, "
const shouldAddComma = hasMoreMandatoryArgs && fn.type !== 'operator';
const shouldAddComma =
hasMoreMandatoryArgs && fn.type !== FunctionDefinitionTypes.OPERATOR;

const constantOnlyParamDefs = typesToSuggestNext.filter(
(p) => p.constantOnly || /_literal/.test(p.type as string)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ import { parse } from '@kbn/esql-ast';
import { scalarFunctionDefinitions } from '../../definitions/generated/scalar_functions';
import { operatorsDefinitions } from '../../definitions/all_operators';
import { NOT_SUGGESTED_TYPES } from '../../shared/resources_helpers';
import { aggregationFunctionDefinitions } from '../../definitions/generated/aggregation_functions';
import { aggFunctionDefinitions } from '../../definitions/generated/aggregation_functions';
import { timeUnitsToSuggest } from '../../definitions/literals';
import { FunctionDefinitionTypes } from '../../definitions/types';
import { groupingFunctionDefinitions } from '../../definitions/generated/grouping_functions';
import * as autocomplete from '../autocomplete';
import type { ESQLCallbacks } from '../../shared/types';
Expand Down Expand Up @@ -158,7 +159,7 @@ export function getFunctionSignaturesByReturnType(

const list = [];
if (agg) {
list.push(...aggregationFunctionDefinitions);
list.push(...aggFunctionDefinitions);
}
if (grouping) {
list.push(...groupingFunctionDefinitions);
Expand Down Expand Up @@ -217,7 +218,7 @@ export function getFunctionSignaturesByReturnType(
.map<PartialSuggestionWithText>((definition) => {
const { type, name, signatures, customParametersSnippet } = definition;

if (type === 'operator') {
if (type === FunctionDefinitionTypes.OPERATOR) {
return {
text: signatures.some(({ params }) => params.length > 1)
? `${name.toUpperCase()} $0`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import { setTestFunctions } from '../../shared/test_functions';
import { FunctionDefinitionTypes } from '../../definitions/types';
import { setup } from './helpers';

describe('hidden commands', () => {
Expand All @@ -28,15 +29,15 @@ describe('hidden functions', () => {
it('does not suggest hidden scalar functions', async () => {
setTestFunctions([
{
type: 'scalar',
type: FunctionDefinitionTypes.SCALAR,
name: 'HIDDEN_FUNCTION',
description: 'This is a hidden function',
signatures: [{ params: [], returnType: 'text' }],
supportedCommands: ['eval'],
ignoreAsSuggestion: true,
},
{
type: 'scalar',
type: FunctionDefinitionTypes.SCALAR,
name: 'VISIBLE_FUNCTION',
description: 'This is a visible function',
signatures: [{ params: [], returnType: 'text' }],
Expand All @@ -54,15 +55,15 @@ describe('hidden functions', () => {
it('does not suggest hidden agg functions', async () => {
setTestFunctions([
{
type: 'agg',
type: FunctionDefinitionTypes.AGG,
name: 'HIDDEN_FUNCTION',
description: 'This is a hidden function',
signatures: [{ params: [], returnType: 'text' }],
supportedCommands: ['stats'],
ignoreAsSuggestion: true,
},
{
type: 'agg',
type: FunctionDefinitionTypes.AGG,
name: 'VISIBLE_FUNCTION',
description: 'This is a visible function',
signatures: [{ params: [], returnType: 'text' }],
Expand All @@ -80,7 +81,7 @@ describe('hidden functions', () => {
it('does not suggest hidden operators', async () => {
setTestFunctions([
{
type: 'operator',
type: FunctionDefinitionTypes.OPERATOR,
name: 'HIDDEN_OPERATOR',
description: 'This is a hidden function',
supportedCommands: ['eval', 'where', 'row', 'sort'],
Expand All @@ -97,7 +98,7 @@ describe('hidden functions', () => {
],
},
{
type: 'operator',
type: FunctionDefinitionTypes.OPERATOR,
name: 'VISIBLE_OPERATOR',
description: 'This is a visible function',
supportedCommands: ['eval', 'where', 'row', 'sort'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ import {
getSuggestionsToRightOfOperatorExpression,
checkFunctionInvocationComplete,
} from './helper';
import { FunctionParameter, isParameterType } from '../definitions/types';
import { FunctionParameter, isParameterType, FunctionDefinitionTypes } from '../definitions/types';
import { comparisonFunctions } from '../definitions/all_operators';
import { getRecommendedQueriesSuggestions } from './recommended_queries/suggestions';

Expand Down Expand Up @@ -992,11 +992,13 @@ async function getFunctionArgsSuggestions(

const shouldAddComma =
hasMoreMandatoryArgs &&
fnDefinition.type !== 'operator' &&
fnDefinition.type !== FunctionDefinitionTypes.OPERATOR &&
!isCursorFollowedByComma &&
!canBeBooleanCondition;
const shouldAdvanceCursor =
hasMoreMandatoryArgs && fnDefinition.type !== 'operator' && !isCursorFollowedByComma;
hasMoreMandatoryArgs &&
fnDefinition.type !== FunctionDefinitionTypes.OPERATOR &&
!isCursorFollowedByComma;

const suggestedConstants = uniq(
typesToSuggestNext
Expand Down Expand Up @@ -1048,9 +1050,9 @@ async function getFunctionArgsSuggestions(
fnToIgnore.push(
...getFunctionsToIgnoreForStats(command, finalCommandArgIndex),
// ignore grouping functions, they are only used for grouping
...getAllFunctions({ type: 'grouping' }).map(({ name }) => name),
...getAllFunctions({ type: FunctionDefinitionTypes.GROUPING }).map(({ name }) => name),
...(isAggFunctionUsedAlready(command, finalCommandArgIndex)
? getAllFunctions({ type: 'agg' }).map(({ name }) => name)
? getAllFunctions({ type: FunctionDefinitionTypes.AGG }).map(({ name }) => name)
: [])
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { i18n } from '@kbn/i18n';
import { memoize } from 'lodash';
import { SuggestionRawDefinition } from './types';
import { groupingFunctionDefinitions } from '../definitions/generated/grouping_functions';
import { aggregationFunctionDefinitions } from '../definitions/generated/aggregation_functions';
import { aggFunctionDefinitions } from '../definitions/generated/aggregation_functions';
import { scalarFunctionDefinitions } from '../definitions/generated/scalar_functions';
import { getFunctionSignatures } from '../definitions/helpers';
import { timeUnitsToSuggest } from '../definitions/literals';
Expand All @@ -20,6 +20,7 @@ import {
CommandOptionsDefinition,
CommandModeDefinition,
FunctionParameterType,
FunctionDefinitionTypes,
} from '../definitions/types';
import { shouldBeQuotedSource, shouldBeQuotedText } from '../shared/helpers';
import { buildFunctionDocumentation } from './documentation_util';
Expand All @@ -39,7 +40,7 @@ const techPreviewLabel = i18n.translate(

const allFunctions = memoize(
() =>
aggregationFunctionDefinitions
aggFunctionDefinitions
.concat(scalarFunctionDefinitions)
.concat(groupingFunctionDefinitions)
.concat(getTestFunctions()),
Expand Down Expand Up @@ -87,7 +88,7 @@ export function getFunctionSuggestion(fn: FunctionDefinition): SuggestionRawDefi
value: buildFunctionDocumentation(fullSignatures, fn.examples),
},
// agg functgions have priority over everything else
sortText: fn.type === 'agg' ? '1A' : 'C',
sortText: fn.type === FunctionDefinitionTypes.AGG ? '1A' : 'C',
// trigger a suggestion follow up on selection
command: TRIGGER_SUGGESTION_COMMAND,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
type FunctionReturnType,
type SupportedDataType,
isReturnType,
FunctionDefinitionTypes,
} from '../definitions/types';
import {
findFinalWord,
Expand Down Expand Up @@ -56,7 +57,10 @@ function extractFunctionArgs(args: ESQLAstItem[]): ESQLFunction[] {

function checkContent(fn: ESQLFunction): boolean {
const fnDef = getFunctionDefinition(fn.name);
return (!!fnDef && fnDef.type === 'agg') || extractFunctionArgs(fn.args).some(checkContent);
return (
(!!fnDef && fnDef.type === FunctionDefinitionTypes.AGG) ||
extractFunctionArgs(fn.args).some(checkContent)
);
}

export function isAggFunctionUsedAlready(command: ESQLCommand, argIndex: number) {
Expand Down Expand Up @@ -291,7 +295,9 @@ export function getValidSignaturesAndTypesToSuggestNext(
// E.g. if true, "fieldName" -> "fieldName, "
const alreadyHasComma = fullText ? fullText[offset] === ',' : false;
const shouldAddComma =
hasMoreMandatoryArgs && fnDefinition.type !== 'operator' && !alreadyHasComma;
hasMoreMandatoryArgs &&
fnDefinition.type !== FunctionDefinitionTypes.OPERATOR &&
!alreadyHasComma;
const currentArg = enrichedArgs[argIndex];
return {
shouldAddComma,
Expand Down Expand Up @@ -610,7 +616,8 @@ export async function getSuggestionsToRightOfOperatorExpression({
// technically another boolean value should be suggested, but it is a better experience
// to actually suggest a wider set of fields/functions
const typeToUse =
finalType === 'boolean' && getFunctionDefinition(operator.name)?.type === 'operator'
finalType === 'boolean' &&
getFunctionDefinition(operator.name)?.type === FunctionDefinitionTypes.OPERATOR
? ['any']
: (supportedTypes as string[]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { getAllFunctions } from '../shared/helpers';
import { getAstAndSyntaxErrors } from '@kbn/esql-ast';
import type { CodeActionOptions } from './types';
import type { ESQLRealField } from '../validation/types';
import type { FieldType } from '../definitions/types';
import { type FieldType, FunctionDefinitionTypes } from '../definitions/types';
import type { ESQLCallbacks, PartialFieldsMetadataClient } from '../shared/types';
import { FULL_TEXT_SEARCH_FUNCTIONS } from '../shared/constants';

Expand Down Expand Up @@ -280,7 +280,7 @@ describe('quick fixes logic', () => {
{ relaxOnMissingCallbacks: false },
{ relaxOnMissingCallbacks: false },
]) {
for (const fn of getAllFunctions({ type: 'scalar' })) {
for (const fn of getAllFunctions({ type: FunctionDefinitionTypes.SCALAR })) {
if (FULL_TEXT_SEARCH_FUNCTIONS.includes(fn.name)) {
testQuickFixes(
`FROM index | WHERE ${BROKEN_PREFIX}${fn.name}()`,
Expand All @@ -289,7 +289,7 @@ describe('quick fixes logic', () => {
);
}
}
for (const fn of getAllFunctions({ type: 'scalar' })) {
for (const fn of getAllFunctions({ type: FunctionDefinitionTypes.SCALAR })) {
if (FULL_TEXT_SEARCH_FUNCTIONS.includes(fn.name)) continue;
// add an A to the function name to make it invalid
testQuickFixes(
Expand Down Expand Up @@ -318,7 +318,7 @@ describe('quick fixes logic', () => {
{ equalityCheck: 'include', ...options }
);
}
for (const fn of getAllFunctions({ type: 'agg' })) {
for (const fn of getAllFunctions({ type: FunctionDefinitionTypes.AGG })) {
if (FULL_TEXT_SEARCH_FUNCTIONS.includes(fn.name)) continue;

// add an A to the function name to make it invalid
Expand Down
Loading

0 comments on commit 6b3fce9

Please sign in to comment.