Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import { uniq } from 'lodash';
import type { LicenseType } from '@kbn/licensing-types';
import type { EsqlFieldType } from '@kbn/esql-types';
import { Parser } from '@elastic/esql';
import type { ESQLAstAllCommands } from '@elastic/esql/types';
import type {
ICommandCallbacks,
Expand Down Expand Up @@ -45,7 +44,7 @@ import { FunctionDefinitionTypes } from '../../commands/definitions/types';
import { mockContext, getMockCallbacks } from './context_fixtures';
import { getSafeInsertText } from '../../commands/definitions/utils';
import { timeUnitsToSuggest } from '../../commands/definitions/constants';
import { correctQuerySyntax, findAstPosition } from '../../commands/definitions/utils/ast';
import { findAutocompleteAstPosition } from '../../language/shared/parse_for_autocomplete_query';
import { FUNCTIONS_TO_IGNORE } from '../../commands/registry/eval/autocomplete';
import { attachReplacementRanges } from '../../language/autocomplete/utils/prefix_range';

Expand Down Expand Up @@ -87,24 +86,20 @@ export const suggest = async (
) => Promise<ISuggestionItem[]>,
offset?: number
): Promise<ISuggestionItem[]> => {
const innerText = query.substring(0, offset ?? query.length);
const correctedQuery = correctQuerySyntax(innerText);
const { root } = Parser.parse(correctedQuery, { withFormatting: true });
const headerConstruction = root?.header?.find((cmd) => cmd.name === commandName);

const cursorPosition = offset ?? query.length;
const { innerText, root, command } = findAutocompleteAstPosition(query, cursorPosition);
const headerConstruction = root?.header?.find((cmd) => cmd.name === commandName);
const targetCommand = headerConstruction ?? command;

const command = headerConstruction ?? findAstPosition(root, cursorPosition).command;

if (!command) {
if (!targetCommand) {
throw new Error(`${commandName.toUpperCase()} command not found in the parsed query`);
}

const contextWithRoot = { ...context, rootAst: root };

const suggestions = await autocomplete(
query,
command,
targetCommand,
mockCallbacks,
contextWithRoot,
cursorPosition
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,27 @@ export function removeMarkerArgFromArgsList<T extends ESQLSingleAstItem | ESQLAs
}

export function mapToNonMarkerNode(arg: ESQLAstItem): ESQLAstItem {
return Array.isArray(arg) ? arg.filter(isNotMarkerNodeOrArray).map(mapToNonMarkerNode) : arg;
if (Array.isArray(arg)) {
return arg.filter(isNotMarkerNodeOrArray).map(mapToNonMarkerNode);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the missing part of the task

if ('args' in arg) {
return {
...arg,
args: arg.args.filter(isNotMarkerNodeOrArray).map(mapToNonMarkerNode) as typeof arg.args,
} as typeof arg;
}

if ('values' in arg) {
return {
...arg,
values: arg.values
.filter((value) => !isMarkerNode(value))
.map((value) => mapToNonMarkerNode(value) as ESQLAstExpression) as typeof arg.values,
} as typeof arg;
}

return arg;
}

function cleanMarkerNode(node: ESQLSingleAstItem | undefined): ESQLSingleAstItem | undefined {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import { isList } from '@elastic/esql';
import type { ESQLSingleAstItem } from '@elastic/esql/types';
import { isMarkerNode } from '../../../ast';
import { getOperatorSuggestion } from '../../../operators';
import type { ISuggestionItem } from '../../../../../registry/types';
import { logicalOperators } from '../../../../all_operators';
Expand All @@ -34,11 +33,7 @@ export function endsWithIsOrIsNotToken(innerText: string): boolean {
}

export function isOperandMissing(operand: ESQLSingleAstItem | undefined): boolean {
return (
!operand ||
isMarkerNode(operand) ||
(operand?.type === 'unknown' && operand?.incomplete === true)
);
return !operand || (operand?.type === 'unknown' && operand?.incomplete === true);
}

/** Returns true if we should suggest opening a list for the right operand */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/
import { Parser } from '@elastic/esql';
import { isAssignment, Parser } from '@elastic/esql';
import type { SupportedDataType } from '../types';
import { FunctionDefinitionTypes } from '../types';
import type { ESQLColumnData } from '../../registry/types';
import { Location } from '../../registry/types';
import { buildPartialMatcher, getExpressionType } from './expressions';
import { buildPartialMatcher, getAssignmentExpressionRoot, getExpressionType } from './expressions';
import { setTestFunctions } from './test_functions';
import { TIME_SYSTEM_PARAMS } from './literals';
import { getAutocompleteCursorContext } from '../../../language/shared/parse_for_autocomplete_query';

describe('buildPartialMatcher', () => {
it('should build a partial matcher', () => {
Expand Down Expand Up @@ -458,3 +459,41 @@ describe('getExpressionType', () => {
);
});
});

describe('getAssignmentExpressionRoot', () => {
it('returns undefined for an incomplete assignment after autocomplete parsing cleanup', () => {
const query = 'FROM employees | EVAL total = ';
const { astContext } = getAutocompleteCursorContext(query, query.length);

if (astContext.type !== 'expression') {
throw new Error(`Expected expression context for query: ${query}`);
}

const assignment = astContext.command.args[astContext.command.args.length - 1];

if (!assignment || !isAssignment(assignment)) {
throw new Error(`Expected assignment expression for query: ${query}`);
}

expect(getAssignmentExpressionRoot(assignment)).toBeUndefined();
});

it('returns the RHS root for a complete assignment after autocomplete parsing cleanup', () => {
const query = 'FROM employees | EVAL total = salary';
const { astContext } = getAutocompleteCursorContext(query, query.length);

if (astContext.type !== 'expression') {
throw new Error(`Expected expression context for query: ${query}`);
}

const assignment = astContext.command.args[astContext.command.args.length - 1];

if (!assignment || !isAssignment(assignment)) {
throw new Error(`Expected assignment expression for query: ${query}`);
}

const root = getAssignmentExpressionRoot(assignment);

expect(root).toMatchObject({ type: 'column', name: 'salary' });
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import type {
import { suggestForExpression } from '../../definitions/utils';
import type { MapParameters } from '../../definitions/utils/autocomplete/map_expression';
import { getCommandMapExpressionSuggestions } from '../../definitions/utils/autocomplete/map_expression';
import { EDITOR_MARKER } from '../../definitions/constants';
import {
pipeCompleteItem,
assignCompletionItem,
Expand Down Expand Up @@ -85,7 +84,7 @@ function getPosition(
return { position: CompletionPosition.AFTER_COMMAND };
}

const expressionRoot = prompt?.text !== EDITOR_MARKER ? prompt : undefined;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this by hand, I found no side effects

const expressionRoot = prompt;

// (function, literal, or existing column) - handle as primaryExpression
if (isFunctionExpression(expressionRoot) || isLiteral(prompt) || isExistingColumn) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@
*/
import { i18n } from '@kbn/i18n';
import type { ESQLAstAllCommands } from '@elastic/esql/types';
import { Parser } from '@elastic/esql';
import { withAutoSuggest } from '../../definitions/utils/autocomplete/helpers';
import type { ICommandCallbacks } from '../types';
import { pipeCompleteItem, colonCompleteItem, semiColonCompleteItem } from '../complete_items';
import { type ISuggestionItem, type ICommandContext } from '../types';
import { buildConstantsDefinitions } from '../../definitions/utils/literals';
import { ESQL_STRING_TYPES } from '../../definitions/types';
import { correctQuerySyntax, findAstPosition } from '../../definitions/utils/ast';
import { findAutocompleteAstPosition } from '../../../language/shared/parse_for_autocomplete_query';

const appendSeparatorCompletionItem: ISuggestionItem = withAutoSuggest({
detail: i18n.translate('kbn-esql-language.esql.definitions.appendSeparatorDoc', {
Expand All @@ -39,9 +38,7 @@ export async function autocomplete(
const commandArgs = command.args.filter((arg) => !Array.isArray(arg) && arg.type !== 'unknown');

// If cursor is inside a string literal, don't suggest anything
const correctedQuery = correctQuerySyntax(innerText);
const { root } = Parser.parse(correctedQuery, { withFormatting: true });
const { node } = findAstPosition(root, innerText.length);
const { node } = findAutocompleteAstPosition(query, cursorPosition);

if (node?.type === 'literal' && node.literalType === 'keyword') {
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ import {
ESQL_STRING_TYPES,
ESQL_NUMBER_TYPES,
} from '../../definitions/types';
import { correctQuerySyntax, findAstPosition } from '../../definitions/utils/ast';
import { Parser } from '@elastic/esql';
import { findAutocompleteAstPosition } from '../../../language/shared/parse_for_autocomplete_query';

const allEvalFnsForWhere = getFunctionSignaturesByReturnType(Location.WHERE, 'any', {
scalar: true,
Expand Down Expand Up @@ -428,11 +427,8 @@ describe('FORK Autocomplete', () => {

it('suggests pipe after complete subcommands', async () => {
const assertSuggestsPipe = async (query: string) => {
const correctedQuery = correctQuerySyntax(query);
const { root } = Parser.parse(correctedQuery, { withFormatting: true });

const cursorPosition = query.length;
const { command } = findAstPosition(root, cursorPosition);
const { command } = findAutocompleteAstPosition(query, cursorPosition);
if (!command) {
throw new Error('Command not found in the parsed query');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ import {
} from '../options/recommended_queries';
import type { ICommandCallbacks } from '../types';
import { autocomplete } from './autocomplete';
import { correctQuerySyntax, findAstPosition } from '../../definitions/utils/ast';
import { Parser } from '@elastic/esql';
import { findAutocompleteAstPosition } from '../../../language/shared/parse_for_autocomplete_query';

const metadataFields = [...METADATA_FIELDS].sort();

Expand Down Expand Up @@ -80,12 +79,8 @@ describe('FROM Autocomplete', () => {
};

const suggest = async (query: string) => {
const correctedQuery = correctQuerySyntax(query);
const { root } = Parser.parse(correctedQuery, { withFormatting: true });

const cursorPosition = query.length;
const { command } = findAstPosition(root, cursorPosition);

const { command } = findAutocompleteAstPosition(query, cursorPosition);
return autocomplete(query, command!, mockCallbacks, mockContext, cursorPosition);
};

Expand Down Expand Up @@ -120,12 +115,8 @@ describe('FROM Autocomplete', () => {

test('suggests comma or pipe after complete index name', async () => {
const suggest = async (query: string) => {
const correctedQuery = correctQuerySyntax(query);
const { root } = Parser.parse(correctedQuery, { withFormatting: true });

const cursorPosition = query.length;
const { command } = findAstPosition(root, cursorPosition);

const { command } = findAutocompleteAstPosition(query, cursorPosition);
return autocomplete(query, command!, mockCallbacks, mockContext, cursorPosition);
};
const suggestions = (await suggest('from index')).map((s) => s.text);
Expand Down Expand Up @@ -176,10 +167,8 @@ describe('FROM Autocomplete', () => {
);
// View names appear when typing (fragment "my_")
const getSuggestions = async (query: string) => {
const correctedQuery = correctQuerySyntax(query);
const { root } = Parser.parse(correctedQuery, { withFormatting: true });
const cursorPosition = query.length;
const { command } = findAstPosition(root, cursorPosition);
const { command } = findAutocompleteAstPosition(query, cursorPosition);
return autocomplete(query, command!, mockCallbacks, contextWithViews, cursorPosition);
};
const suggestions = (await getSuggestions('FROM my_')).map((s) => s.text);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@
*/
import { i18n } from '@kbn/i18n';
import type { ESQLAstAllCommands } from '@elastic/esql/types';
import { Parser } from '@elastic/esql';
import { withAutoSuggest } from '../../definitions/utils/autocomplete/helpers';
import { commaCompleteItem, pipeCompleteItem } from '../complete_items';
import type { ICommandCallbacks } from '../types';
import type { ISuggestionItem, ICommandContext } from '../types';
import { buildConstantsDefinitions } from '../../definitions/utils/literals';
import { ESQL_STRING_TYPES } from '../../definitions/types';
import { correctQuerySyntax, findAstPosition } from '../../definitions/utils/ast';
import { findAutocompleteAstPosition } from '../../../language/shared/parse_for_autocomplete_query';

export async function autocomplete(
query: string,
Expand All @@ -28,9 +27,7 @@ export async function autocomplete(
const commandArgs = command.args.filter((arg) => !Array.isArray(arg) && arg.type !== 'unknown');

// If cursor is inside a string literal, don't suggest anything
const correctedQuery = correctQuerySyntax(innerText);
const { root } = Parser.parse(correctedQuery, { withFormatting: true });
const { node } = findAstPosition(root, innerText.length);
const { node } = findAutocompleteAstPosition(query, cursorPosition);

if (node?.type === 'literal' && node.literalType === 'keyword') {
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,19 @@ describe('RERANK Autocomplete', () => {
);
});

test('suggests text and keyword fields after an incomplete assignment in ON clause', async () => {
const query = `${buildRerankQuery({ query: '"search query"', onClause: 'col0 =' })} `;

await expectRerankSuggestions(
query,
{
contains: ['textField', 'keywordField'],
notContains: ['integerField'],
},
mockCallbacks
);
});

test('suggests field columns after a list of keyword fields and comma', async () => {
const query =
buildRerankQuery({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ import {
FunctionDefinitionTypes,
ESQL_COMMON_NUMERIC_TYPES,
} from '../../definitions/types';
import { correctQuerySyntax, findAstPosition } from '../../definitions/utils/ast';
import { Parser } from '@elastic/esql';
import { findAutocompleteAstPosition } from '../../../language/shared/parse_for_autocomplete_query';
import { setTestFunctions } from '../../definitions/utils/test_functions';
import {
getDateHistogramCompletionItem,
Expand Down Expand Up @@ -133,11 +132,8 @@ describe('STATS Autocomplete', () => {
});

const suggest = async (query: string) => {
const correctedQuery = correctQuerySyntax(query);
const { root } = Parser.parse(correctedQuery, { withFormatting: true });
const cursorPosition = query.length;
const innerText = query.substring(0, cursorPosition);
const { command } = findAstPosition(root, cursorPosition);
const { innerText, root, command } = findAutocompleteAstPosition(query, cursorPosition);
if (!command) {
throw new Error('Command not found in the parsed query');
}
Expand Down Expand Up @@ -542,6 +538,17 @@ describe('STATS Autocomplete', () => {
);
});

it('suggests opening a list after IN in WHERE', async () => {
await statsExpectSuggestions('FROM a | STATS MIN(b) WHERE keywordField IN ', ['($0)']);
});

it('suggests LIKE pattern values after LIKE in WHERE', async () => {
await statsExpectSuggestions('FROM a | STATS MIN(b) WHERE keywordField LIKE ', [
'"${0:*}"',
'"${0:?}"',
]);
});

describe('completed expression suggestions', () => {
const completedExpressionSuggestions = ['| ', ', ', 'BY '];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import {
import { getAllFunctions } from '../../definitions/utils/functions';
import { FunctionDefinitionTypes } from '../../definitions/types';
import { getPosition, getCommaAndPipe, rightAfterColumn } from './utils';
import { isMarkerNode, findAstPosition } from '../../definitions/utils/ast';
import { findAstPosition } from '../../definitions/utils/ast';
import { getAssignmentExpressionRoot } from '../../definitions/utils/expressions';
import { inOperators, nullCheckOperators } from '../../definitions/all_operators';
import { buildExpressionFunctionParameterContext } from '../../definitions/utils';
Expand Down Expand Up @@ -181,8 +181,7 @@ export async function autocomplete(

case 'after_where': {
const whereFn = command.args[command.args.length - 1] as ESQLFunction;
// TODO do we still need this check?
const expressionRoot = isMarkerNode(whereFn.args[1]) ? undefined : whereFn.args[1]!;
const expressionRoot = whereFn.args[1];
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some testing by hand but the unit tests already covered it well


if (expressionRoot && !!Array.isArray(expressionRoot)) {
return [];
Expand Down
Loading
Loading