Skip to content

Commit 28b7e62

Browse files
authored
[ES|QL] Wrap the fork subcommands inside the parens node (elastic#242369)
## Summary elastic#242225 Now the fork command is wrapped inside a new node called `parens`, which changes the ast structure. So now the branches are children of `parens`.
1 parent e097dcc commit 28b7e62

File tree

17 files changed

+190
-76
lines changed

17 files changed

+190
-76
lines changed

src/platform/packages/shared/kbn-esql-ast/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export type {
2929
ESQLInlineCast,
3030
ESQLAstBaseItem,
3131
ESQLAstChangePointCommand,
32+
ESQLAstForkCommand,
33+
ESQLForkParens,
3234
} from './src/types';
3335

3436
export * from './src/ast/is';

src/platform/packages/shared/kbn-esql-ast/src/commands_registry/commands/fork/autocomplete.ts

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@
88
*/
99
import { i18n } from '@kbn/i18n';
1010
import { withAutoSuggest } from '../../../definitions/utils/autocomplete/helpers';
11-
import type { ESQLAstAllCommands, ESQLCommand } from '../../../types';
11+
import type {
12+
ESQLAstAllCommands,
13+
ESQLAstForkCommand,
14+
ESQLAstQueryExpression,
15+
} from '../../../types';
1216
import { pipeCompleteItem, getCommandAutocompleteDefinitions } from '../../complete_items';
1317
import { pipePrecedesCurrentWord } from '../../../definitions/utils/shared';
1418
import type { ICommandCallbacks } from '../../types';
@@ -22,20 +26,22 @@ export async function autocomplete(
2226
context?: ICommandContext,
2327
cursorPosition: number = query.length
2428
): Promise<ISuggestionItem[]> {
29+
const forkCommand = command as ESQLAstForkCommand;
30+
2531
const innerText = query.substring(0, cursorPosition);
2632
if (/FORK\s+$/i.test(innerText)) {
2733
return [newBranchSuggestion];
2834
}
2935

30-
const activeBranch = getActiveBranch(command);
36+
const activeBranch = getActiveBranch(forkCommand);
3137
const withinActiveBranch =
3238
activeBranch &&
3339
activeBranch.location.min <= innerText.length &&
3440
activeBranch.location.max >= innerText.length;
3541

3642
if (!withinActiveBranch && /\)\s+$/i.test(innerText)) {
3743
const suggestions = [newBranchSuggestion];
38-
if (command.args.length > 1) {
44+
if (forkCommand.args.length > 1) {
3945
suggestions.push(pipeCompleteItem);
4046
}
4147
return suggestions;
@@ -58,13 +64,7 @@ export async function autocomplete(
5864

5965
const subCommandMethods = esqlCommandRegistry.getCommandMethods(subCommand.name);
6066
return (
61-
subCommandMethods?.autocomplete(
62-
innerText,
63-
subCommand as ESQLCommand,
64-
callbacks,
65-
context,
66-
cursorPosition
67-
) || []
67+
subCommandMethods?.autocomplete(innerText, subCommand, callbacks, context, cursorPosition) || []
6868
);
6969
}
7070

@@ -80,13 +80,12 @@ const newBranchSuggestion: ISuggestionItem = withAutoSuggest({
8080
asSnippet: true,
8181
});
8282

83-
const getActiveBranch = (command: ESQLAstAllCommands) => {
83+
const getActiveBranch = (command: ESQLAstForkCommand): ESQLAstQueryExpression | undefined => {
8484
const finalBranch = command.args[command.args.length - 1];
8585

86-
if (Array.isArray(finalBranch) || finalBranch.type !== 'query') {
87-
// should never happen
86+
if (!finalBranch) {
8887
return;
8988
}
9089

91-
return finalBranch;
90+
return finalBranch.child;
9291
};

src/platform/packages/shared/kbn-esql-ast/src/commands_registry/commands/fork/columns_after.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,18 @@
88
*/
99
import { uniqBy } from 'lodash';
1010
import { esqlCommandRegistry } from '../../../..';
11-
import type { ESQLAstQueryExpression } from '../../../types';
12-
import { type ESQLCommand } from '../../../types';
11+
import type { ESQLAstAllCommands, ESQLAstForkCommand } from '../../../types';
1312
import type { ESQLColumnData, ESQLFieldWithMetadata } from '../../types';
1413
import type { IAdditionalFields } from '../../registry';
1514

1615
export const columnsAfter = async (
17-
command: ESQLCommand,
16+
command: ESQLAstAllCommands,
1817
previousColumns: ESQLColumnData[],
1918
query: string,
2019
additionalFields: IAdditionalFields
2120
) => {
22-
const branches = command.args as ESQLAstQueryExpression[];
21+
const forkCommand = command as ESQLAstForkCommand;
22+
const branches = forkCommand.args.map((parens) => parens.child);
2323

2424
const columnsFromBranches = [];
2525

src/platform/packages/shared/kbn-esql-ast/src/commands_registry/commands/fork/validate.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* your election, the "Elastic License 2.0", the "GNU Affero General Public
77
* License v3.0 only", or the "Server Side Public License, v 1".
88
*/
9-
import type { ESQLAst, ESQLAstAllCommands, ESQLMessage } from '../../../types';
9+
import type { ESQLAst, ESQLAstAllCommands, ESQLAstForkCommand, ESQLMessage } from '../../../types';
1010
import { Walker } from '../../../walker';
1111
import type { ICommandContext, ICommandCallbacks } from '../../types';
1212
import { validateCommandArguments } from '../../../definitions/utils/validation';
@@ -23,24 +23,31 @@ export const validate = (
2323
context?: ICommandContext,
2424
callbacks?: ICommandCallbacks
2525
): ESQLMessage[] => {
26+
const forkCommand = command as ESQLAstForkCommand;
2627
const messages: ESQLMessage[] = [];
2728

28-
if (command.args.length < MIN_BRANCHES) {
29-
messages.push(errors.forkTooFewBranches(command));
29+
if (forkCommand.args.length < MIN_BRANCHES) {
30+
messages.push(errors.forkTooFewBranches(forkCommand));
3031
}
3132

32-
if (command.args.length > MAX_BRANCHES) {
33-
messages.push(errors.forkTooManyBranches(command));
33+
if (forkCommand.args.length > MAX_BRANCHES) {
34+
messages.push(errors.forkTooManyBranches(forkCommand));
3435
}
3536

36-
messages.push(...validateCommandArguments(command, ast, context, callbacks));
37+
messages.push(...validateCommandArguments(forkCommand, ast, context, callbacks));
3738

38-
for (const arg of command.args.flat()) {
39-
if (!Array.isArray(arg) && arg.type === 'query') {
39+
for (const arg of forkCommand.args) {
40+
const query = arg.child;
41+
42+
if (!Array.isArray(query) && query.type === 'query') {
4043
// all the args should be commands
41-
arg.commands.forEach((subCommand) => {
44+
query.commands.forEach((subCommand) => {
4245
const subCommandMethods = esqlCommandRegistry.getCommandMethods(subCommand.name);
43-
const validationMessages = subCommandMethods?.validate?.(subCommand, arg.commands, context);
46+
const validationMessages = subCommandMethods?.validate?.(
47+
subCommand,
48+
query.commands,
49+
context
50+
);
4451
messages.push(...(validationMessages || []));
4552
});
4653
}
@@ -58,7 +65,7 @@ export const validate = (
5865
const hasSubqueries = fromCommands.some((cmd) => cmd.args.some((arg) => isSubQuery(arg)));
5966

6067
if (hasSubqueries) {
61-
messages.push(errors.forkNotAllowedWithSubqueries(command));
68+
messages.push(errors.forkNotAllowedWithSubqueries(forkCommand));
6269
}
6370

6471
return messages;

src/platform/packages/shared/kbn-esql-ast/src/mutate/generic/commands/options/index.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@
88
*/
99

1010
import { Builder } from '../../../../builder';
11-
import type { ESQLAstQueryExpression, ESQLCommand, ESQLCommandOption } from '../../../../types';
11+
import type {
12+
ESQLAstItem,
13+
ESQLAstQueryExpression,
14+
ESQLCommand,
15+
ESQLCommandOption,
16+
} from '../../../../types';
1217
import { Visitor } from '../../../../visitor';
1318
import type { Predicate } from '../../../types';
1419
import * as commands from '..';
@@ -107,7 +112,7 @@ export const remove = (ast: ESQLAstQueryExpression, option: ESQLCommandOption):
107112
return false;
108113
}
109114

110-
const index = ctx.node.args.indexOf(target);
115+
const index = (ctx.node.args as ESQLAstItem[]).indexOf(target);
111116

112117
if (index === -1) {
113118
return false;

src/platform/packages/shared/kbn-esql-ast/src/parser/__tests__/fork.test.ts

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
import { parse } from '..';
1111
import { EsqlQuery } from '../../query';
12-
import type { ESQLAstQueryExpression } from '../../types';
12+
import type { ESQLForkParens } from '../../types';
1313
import { Walker } from '../../walker';
1414

1515
describe('FORK', () => {
@@ -24,9 +24,9 @@ describe('FORK', () => {
2424

2525
expect(ast[1].args).toHaveLength(3);
2626
expect(ast[1].args).toMatchObject([
27-
{ type: 'query', commands: [{ name: 'where' }] },
28-
{ type: 'query', commands: [{ name: 'sort' }] },
29-
{ type: 'query', commands: [{ name: 'limit' }] },
27+
{ type: 'parens', child: { type: 'query', commands: [{ name: 'where' }] } },
28+
{ type: 'parens', child: { type: 'query', commands: [{ name: 'sort' }] } },
29+
{ type: 'parens', child: { type: 'query', commands: [{ name: 'limit' }] } },
3030
]);
3131
});
3232

@@ -36,12 +36,12 @@ describe('FORK', () => {
3636
| FORK (WHERE bytes > 1 | LIMIT 10) (SORT bytes ASC) (LIMIT 100)`;
3737
const { ast } = EsqlQuery.fromSrc(text);
3838
const fork = Walker.match(ast, { type: 'command', name: 'fork' })!;
39-
const firstQuery = Walker.match(fork, { type: 'query' })!;
39+
const firstParens = Walker.match(fork, { type: 'parens' })!;
4040

4141
expect(text.slice(fork.location.min, fork.location.max + 1)).toBe(
4242
'FORK (WHERE bytes > 1 | LIMIT 10) (SORT bytes ASC) (LIMIT 100)'
4343
);
44-
expect(text.slice(firstQuery.location.min, firstQuery.location.max + 1)).toBe(
44+
expect(text.slice(firstParens.location.min, firstParens.location.max + 1)).toBe(
4545
'(WHERE bytes > 1 | LIMIT 10)'
4646
);
4747
});
@@ -55,8 +55,17 @@ describe('FORK', () => {
5555

5656
expect(ast[1].args).toHaveLength(2);
5757
expect(ast[1].args).toMatchObject([
58-
{ type: 'query', commands: [{ name: 'where' }, { name: 'sort' }, { name: 'limit' }] },
59-
{ type: 'query', commands: [{ name: 'where' }, { name: 'limit' }] },
58+
{
59+
type: 'parens',
60+
child: {
61+
type: 'query',
62+
commands: [{ name: 'where' }, { name: 'sort' }, { name: 'limit' }],
63+
},
64+
},
65+
{
66+
type: 'parens',
67+
child: { type: 'query', commands: [{ name: 'where' }, { name: 'limit' }] },
68+
},
6069
]);
6170
});
6271
});
@@ -78,7 +87,11 @@ describe('FORK', () => {
7887
const { root } = parse(text);
7988

8089
expect(root.commands[1].args).toHaveLength(1);
81-
expect((root.commands[1].args[0] as ESQLAstQueryExpression).commands[0]).toMatchObject({
90+
const parens = root.commands[1].args[0] as ESQLForkParens;
91+
92+
expect(parens.type).toBe('parens');
93+
expect(parens.child.type).toBe('query');
94+
expect(parens.child.commands[0]).toMatchObject({
8295
name: type,
8396
});
8497
});

src/platform/packages/shared/kbn-esql-ast/src/parser/cst_to_ast_converter.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1724,7 +1724,7 @@ export class CstToAstConverter {
17241724
return command;
17251725
}
17261726

1727-
private fromForkSubQuery(ctx: cst.ForkSubQueryContext): ast.ESQLAstQueryExpression {
1727+
private fromForkSubQuery(ctx: cst.ForkSubQueryContext): ast.ESQLParens {
17281728
const commands: ast.ESQLCommand[] = [];
17291729
const collectCommand = (cmdCtx: cst.ForkSubQueryProcessingCommandContext) => {
17301730
const processingCommandCtx = cmdCtx.processingCommand();
@@ -1752,10 +1752,25 @@ export class CstToAstConverter {
17521752

17531753
commands.reverse();
17541754

1755-
const parserFields = this.getParserFields(ctx);
1756-
const query = Builder.expression.query(commands, parserFields);
1755+
const openParen = ctx.LP();
1756+
const closeParen = ctx.RP();
17571757

1758-
return query;
1758+
const closeParenText = closeParen?.getText() ?? '';
1759+
const hasCloseParen = closeParen && !/<missing /.test(closeParenText);
1760+
const incomplete = Boolean(ctx.exception) || !hasCloseParen;
1761+
1762+
const query = Builder.expression.query(commands, {
1763+
...this.getParserFields(ctx),
1764+
incomplete,
1765+
});
1766+
1767+
return Builder.expression.parens(query, {
1768+
incomplete: incomplete || query.incomplete,
1769+
location: getPosition(
1770+
openParen?.symbol ?? ctx.start,
1771+
hasCloseParen ? closeParen.symbol : ctx.stop
1772+
),
1773+
});
17591774
}
17601775

17611776
// -------------------------------------------------------------- expressions

src/platform/packages/shared/kbn-esql-ast/src/pretty_print/__tests__/wrapping_pretty_printer.comments.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -970,4 +970,26 @@ FROM index`;
970970
assertReprint(query, expected);
971971
});
972972
});
973+
974+
describe('FORK', () => {
975+
test('preserves comments in various positions', () => {
976+
const query = `FROM index
977+
/* before FORK */
978+
| FORK
979+
/* first branch */
980+
(
981+
KEEP field1, field2, field3 /* important fields */
982+
| WHERE x > 100 /* filter */
983+
)
984+
/* second branch */
985+
(
986+
DROP field4, field5 /* not needed */
987+
| LIMIT 50
988+
)`;
989+
990+
const text = reprint(query, { multiline: true }).text;
991+
992+
expect(text).toBe(query);
993+
});
994+
});
973995
});

src/platform/packages/shared/kbn-esql-ast/src/pretty_print/basic_pretty_printer.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -501,11 +501,15 @@ export class BasicPrettyPrinter {
501501
if (cmd === 'FORK') {
502502
const branches = node.args
503503
.map((branch) => {
504-
if (Array.isArray(branch) || branch.type !== 'query') {
504+
if (Array.isArray(branch)) {
505505
return undefined;
506506
}
507507

508-
return ctx.visitSubQuery(branch);
508+
if (branch.type === 'parens' && branch.child.type === 'query') {
509+
return ctx.visitSubQuery(branch.child);
510+
}
511+
512+
return undefined;
509513
})
510514
.filter(Boolean) as string[];
511515

src/platform/packages/shared/kbn-esql-ast/src/pretty_print/wrapping_pretty_printer.ts

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -404,10 +404,7 @@ export class WrappingPrettyPrinter {
404404
suffix: isLastArg ? '' : needsComma ? ',' : '',
405405
});
406406
const indentation = arg.indented ? '' : indent;
407-
let formattedArg = arg.txt;
408-
if (args[i].type === 'query') {
409-
formattedArg = `(\n${this.opts.tab.repeat(2)}${formattedArg}\n${indentation})`;
410-
}
407+
const formattedArg = arg.txt;
411408
const separator = isFirstArg ? '' : '\n';
412409
txt += separator + indentation + formattedArg;
413410
lines++;
@@ -680,8 +677,37 @@ export class WrappingPrettyPrinter {
680677
})
681678

682679
.on('visitParensExpression', (ctx, inp: Input): Output => {
683-
const child = ctx.visitChild(inp);
684-
const formatted = `(${child.txt.trimStart()})`;
680+
// Check if parent is FORK command
681+
const parent = ctx.parent?.node;
682+
const isForkBranch =
683+
!Array.isArray(parent) &&
684+
parent?.type === 'command' &&
685+
parent.name === 'fork' &&
686+
ctx.node.child?.type === 'query';
687+
688+
let formatted: string;
689+
if (isForkBranch) {
690+
const baseIndent = inp.indent + this.opts.tab;
691+
const childText = this.visitor.visitQuery(ctx.node.child as ESQLAstQueryExpression, {
692+
indent: baseIndent,
693+
remaining: this.opts.wrap - baseIndent.length,
694+
});
695+
696+
const lines = childText.txt.split('\n');
697+
698+
for (let i = 0; i < lines.length; i++) {
699+
if (i === 0) {
700+
lines[i] = ' ' + lines[i];
701+
} else if (lines[i].startsWith(' ')) {
702+
lines[i] = lines[i].slice(2);
703+
}
704+
}
705+
706+
formatted = `(\n${lines.join('\n')}\n${inp.indent})`;
707+
} else {
708+
const child = ctx.visitChild(inp);
709+
formatted = `(${child.txt.trimStart()})`;
710+
}
685711

686712
return this.decorateWithComments(inp, ctx.node, formatted);
687713
})

0 commit comments

Comments
 (0)