diff --git a/.changeset/ast-unboxing-major.md b/.changeset/ast-unboxing-major.md new file mode 100644 index 00000000..012f854f --- /dev/null +++ b/.changeset/ast-unboxing-major.md @@ -0,0 +1,27 @@ +--- +'@elastic/esql-types': major +'@elastic/esql-traversal': major +'@elastic/esql-parser': minor +'@elastic/esql': minor +--- + +Remove array-boxed nodes (`[node]`) from the ES|QL AST — in parser output and in the type system. + +**Breaking changes in `@elastic/esql-types`:** + +- `ESQLAstItem` no longer has an array arm: it is now a deprecated alias of `ESQLSingleAstItem`. Code that hand-builds boxed args (`args: [left, [right]]`) no longer compiles — write `args: [left, right]` instead. +- `args` is narrowed from `ESQLAstItem[]` to `ESQLAstExpression[]` on `ESQLCommand`, `ESQLCommandOption`, `ESQLFunction`, and `ESQLFunctionCallExpression`, and the `ESQLUnaryExpression`, `ESQLPostfixUnaryExpression`, `ESQLOrderExpression`, and `ESQLBinaryExpression` tuples are narrowed accordingly. +- `ESQLProperNode` is now a deprecated alias of `ESQLAstNode` — all nodes are *proper* nodes now. + +**Breaking changes in `@elastic/esql-traversal`:** + +- `Walker.walkExpression()` is typed `ESQLAstExpression | ESQLAstExpression[]` (previously relied on the `ESQLAstItem` array arm). +- `VisitorContext.args()` yields `ESQLAstExpression` and no longer yields raw arrays. +- `firstItem`, `lastItem`, `resolveItem`, and `singleItems` are deprecated: the AST no longer contains array-boxed nodes, so access args directly (`args[0]`, `args.at(-1)`, plain iteration). They remain exported and runtime-tolerant of legacy boxed input for one major cycle. + +**Parser output changes (`@elastic/esql-parser`):** + +- No AST node is ever wrapped in an array anymore. Notable shapes that changed: `LIMIT ?` / `SAMPLE ?` (`args: [[param]]` to `args: [param]`), `DISSECT ... append_separator=?`, `RERANK ?` (`query` was an array in violation of its declared type), `WHERE x : ?`, and field assignments (`args: [column, [expression]]` to `args: [column, expression]`). +- A missing assignment right-hand side (`SET x =`, `ENRICH p WITH x =`) is now an explicit `{ type: 'unknown', incomplete: true }` placeholder node instead of an empty array. + +**Migration:** replace `[node]` boxing with `node` when building ASTs; replace `firstItem(args)`/`resolveItem(arg)`/`lastItem(args)`/`singleItems(args)` with `args[0]`/`arg`/`args.at(-1)`/`args`. diff --git a/packages/esql-ast/src/esql/builder/builder.ts b/packages/esql-ast/src/esql/builder/builder.ts index 511a966d..5e63db9c 100644 --- a/packages/esql-ast/src/esql/builder/builder.ts +++ b/packages/esql-ast/src/esql/builder/builder.ts @@ -34,7 +34,6 @@ import type { ESQLSource, ESQLParamLiteral, ESQLFunction, - ESQLAstItem, ESQLStringLiteral, ESQLBinaryExpression, ESQLUnaryExpression, @@ -349,7 +348,7 @@ export namespace Builder { export const call = ( nameOrOperator: string | ESQLIdentifier | ESQLParamLiteral, - args: ESQLAstItem[], + args: ESQLAstExpression[], template?: Omit, 'subtype' | 'name' | 'operator' | 'args'>, fromParser?: Partial ): ESQLFunction => { @@ -370,7 +369,7 @@ export namespace Builder { export const unary = ( name: string, - arg: ESQLAstItem, + arg: ESQLAstExpression, template?: Omit, 'subtype' | 'name' | 'operator' | 'args'>, fromParser?: Partial ): ESQLUnaryExpression => { @@ -383,7 +382,7 @@ export namespace Builder { export const postfix = ( name: string, - arg: ESQLAstItem, + arg: ESQLAstExpression, template?: Omit, 'subtype' | 'name' | 'operator' | 'args'>, fromParser?: Partial ): ESQLUnaryExpression => { @@ -396,7 +395,7 @@ export namespace Builder { export const binary = ( name: Name, - args: [left: ESQLAstItem, right: ESQLAstItem], + args: [left: ESQLAstExpression, right: ESQLAstExpression], template?: Omit>, 'subtype' | 'name' | 'args'>, fromParser?: Partial ): ESQLBinaryExpression => { @@ -409,7 +408,7 @@ export namespace Builder { } export const where = ( - args: [left: ESQLAstItem, right: ESQLAstItem], + args: [left: ESQLAstExpression, right: ESQLAstExpression], template?: Omit, 'subtype' | 'name' | 'operator' | 'args'>, fromParser?: Partial ) => Builder.expression.func.binary('where', args, template, fromParser); diff --git a/packages/esql-ast/src/esql/is.ts b/packages/esql-ast/src/esql/is.ts index aa8b3dee..a5ae3ce0 100644 --- a/packages/esql-ast/src/esql/is.ts +++ b/packages/esql-ast/src/esql/is.ts @@ -132,7 +132,7 @@ export const isESQLFunction = (node: unknown): node is types.ESQLFunction => (node as types.ESQLFunction).type === 'function'; export const isESQLNamedParamLiteral = ( - node: types.ESQLAstItem + node: types.ESQLAstExpression ): node is types.ESQLNamedParamLiteral => isESQLAstBaseItem(node) && (node as types.ESQLNamedParamLiteral).literalType === 'param' && diff --git a/packages/esql-parser/src/esql/__tests__/binary_expression_grouping.test.ts b/packages/esql-parser/src/esql/__tests__/binary_expression_grouping.test.ts index 30adabfe..96324580 100644 --- a/packages/esql-parser/src/esql/__tests__/binary_expression_grouping.test.ts +++ b/packages/esql-parser/src/esql/__tests__/binary_expression_grouping.test.ts @@ -6,8 +6,8 @@ */ import { EsqlQuery } from './query'; -import type { ESQLAstItem, ESQLAstQueryExpression, ESQLProperNode } from '@elastic/esql-types'; -import { singleItems, Walker } from '@elastic/esql-traversal'; +import type { ESQLAstQueryExpression, ESQLProperNode } from '@elastic/esql-types'; +import { Walker } from '@elastic/esql-traversal'; const removeParserFields = (tree: ESQLAstQueryExpression): void => { Walker.walk(tree, { @@ -15,10 +15,6 @@ const removeParserFields = (tree: ESQLAstQueryExpression): void => { delete node.text; delete node.location; delete node.incomplete; - const args = (node as { args?: ESQLAstItem[] }).args; - if (Array.isArray(args)) { - (node as { args?: ESQLAstItem[] }).args = [...singleItems(args)]; - } }, }); }; diff --git a/packages/esql-parser/src/esql/__tests__/completion.test.ts b/packages/esql-parser/src/esql/__tests__/completion.test.ts index 5406d812..d61511b6 100644 --- a/packages/esql-parser/src/esql/__tests__/completion.test.ts +++ b/packages/esql-parser/src/esql/__tests__/completion.test.ts @@ -6,7 +6,7 @@ */ import { EsqlQuery } from './query'; -import type { ESQLAstCompletionCommand, ESQLAstItem, ESQLFunction } from '@elastic/esql-types'; +import type { ESQLAstCompletionCommand, ESQLFunction } from '@elastic/esql-types'; describe('COMPLETION command', () => { describe('correctly formatted', () => { @@ -35,7 +35,7 @@ describe('COMPLETION command', () => { it('parses prompt when it is a param', () => { const text = `FROM index | COMPLETION ? WITH { "inference_id": "my_inference_endpoint" }`; const query = EsqlQuery.fromSrc(text); - const promptArg = query.ast.commands[1].args[0] as ESQLAstItem[]; + const promptArg = query.ast.commands[1].args[0]; expect(promptArg).toMatchObject({ type: 'literal', diff --git a/packages/esql-parser/src/esql/__tests__/header.set.test.ts b/packages/esql-parser/src/esql/__tests__/header.set.test.ts index ee8e5292..6280d87c 100644 --- a/packages/esql-parser/src/esql/__tests__/header.set.test.ts +++ b/packages/esql-parser/src/esql/__tests__/header.set.test.ts @@ -470,7 +470,10 @@ describe('SET instruction parsing', () => { type: 'function', subtype: 'binary-expression', name: '=', - args: [{ type: 'identifier', name: 'timezone' }, []], + args: [ + { type: 'identifier', name: 'timezone' }, + { type: 'unknown', incomplete: true }, + ], incomplete: true, }, ], diff --git a/packages/esql-parser/src/esql/__tests__/limit.test.ts b/packages/esql-parser/src/esql/__tests__/limit.test.ts index 5498641f..db4dfa9c 100644 --- a/packages/esql-parser/src/esql/__tests__/limit.test.ts +++ b/packages/esql-parser/src/esql/__tests__/limit.test.ts @@ -27,22 +27,20 @@ describe('LIMIT', () => { type: 'command', name: 'limit', args: [ - [ - { - incomplete: false, - name: '', - paramKind: '?', - paramType: 'named', - text: '?param', - type: 'literal', - literalType: 'param', - value: 'param', - location: { - max: 24, - min: 19, - }, + { + incomplete: false, + name: '', + paramKind: '?', + paramType: 'named', + text: '?param', + type: 'literal', + literalType: 'param', + value: 'param', + location: { + max: 24, + min: 19, }, - ], + }, ], }, ]); diff --git a/packages/esql-parser/src/esql/__tests__/rerank.test.ts b/packages/esql-parser/src/esql/__tests__/rerank.test.ts index adf9936c..b88ee2c7 100644 --- a/packages/esql-parser/src/esql/__tests__/rerank.test.ts +++ b/packages/esql-parser/src/esql/__tests__/rerank.test.ts @@ -136,12 +136,10 @@ describe('RERANK', () => { name: '=', args: [ {}, - [ - { - type: 'function', - name: 'substring', - }, - ], + { + type: 'function', + name: 'substring', + }, ], }, ], diff --git a/packages/esql-parser/src/esql/__tests__/stats.test.ts b/packages/esql-parser/src/esql/__tests__/stats.test.ts index b44ae954..01e119c9 100644 --- a/packages/esql-parser/src/esql/__tests__/stats.test.ts +++ b/packages/esql-parser/src/esql/__tests__/stats.test.ts @@ -102,18 +102,16 @@ describe('STATS', () => { }, ], }, - [ - { - type: 'function', - name: 'agg', - args: [ - { - type: 'literal', - valueUnquoted: 'salary', - }, - ], - }, - ], + { + type: 'function', + name: 'agg', + args: [ + { + type: 'literal', + valueUnquoted: 'salary', + }, + ], + }, ], }, ], @@ -293,18 +291,16 @@ describe('STATS', () => { }, ], }, - [ - { - type: 'function', - name: 'agg', - args: [ - { - type: 'literal', - valueUnquoted: 'salary', - }, - ], - }, - ], + { + type: 'function', + name: 'agg', + args: [ + { + type: 'literal', + valueUnquoted: 'salary', + }, + ], + }, ], }, { diff --git a/packages/esql-parser/src/esql/cst_to_ast_converter.ts b/packages/esql-parser/src/esql/cst_to_ast_converter.ts index 47194fa3..cc945e66 100644 --- a/packages/esql-parser/src/esql/cst_to_ast_converter.ts +++ b/packages/esql-parser/src/esql/cst_to_ast_converter.ts @@ -19,7 +19,6 @@ import { import { getPosition } from '../tokens'; import { PromQLParser } from '../promql'; import { nonNullable, unescapeColumn } from './helpers'; -import { firstItem, lastItem, resolveItem, singleItems } from '@elastic/esql-traversal'; import { type ArithmeticUnaryContext } from '@elastic/esql-grammar'; import type { Parser } from './parser'; import type { PromQLAstQueryExpression } from '@elastic/esql-types'; @@ -96,6 +95,16 @@ export class CstToAstConverter { }; } + private toUnknownMissingNode(anchor: antlr.Token): ast.ESQLUnknownItem { + return { + type: 'unknown', + name: 'unknown', + text: '', + location: { min: anchor.stop, max: anchor.stop }, + incomplete: true, + }; + } + /** * Extends `fn.location` to cover all its arguments. * @@ -114,13 +123,6 @@ export class CstToAstConverter { 'max', (args) => args.length - 1 ); - // in case of empty array as last arg, bump the max location by 3 chars (empty brackets) - if ( - Array.isArray(fn.args[fn.args.length - 1]) && - !(fn.args[fn.args.length - 1] as ast.ESQLAstItem[]).length - ) { - location.max += 3; - } } return location; } @@ -130,24 +132,17 @@ export class CstToAstConverter { * `extendLocationToArgs` is removed. */ private walkFunctionStructure( - args: ast.ESQLAstItem[], + args: ast.ESQLAstExpression[], initialLocation: ast.ESQLLocation, prop: 'min' | 'max', - getNextItemIndex: (arg: ast.ESQLAstItem[]) => number + getNextItemIndex: (arg: ast.ESQLAstExpression[]) => number ) { - let nextArg: ast.ESQLAstItem | undefined = args[getNextItemIndex(args)]; + let nextArg: ast.ESQLAstExpression | undefined = args[getNextItemIndex(args)]; const location = { ...initialLocation }; - while (Array.isArray(nextArg) || nextArg) { - if (Array.isArray(nextArg)) { - nextArg = nextArg[getNextItemIndex(nextArg)]; - } else { - location[prop] = Math[prop](location[prop], nextArg.location[prop]); - if (nextArg.type === 'function') { - nextArg = nextArg.args[getNextItemIndex(nextArg.args)]; - } else { - nextArg = undefined; - } - } + while (nextArg) { + location[prop] = Math[prop](location[prop], nextArg.location[prop]); + nextArg = + nextArg.type === 'function' ? nextArg.args[getNextItemIndex(nextArg.args)] : undefined; } return location[prop]; } @@ -303,7 +298,7 @@ export class CstToAstConverter { // Handle constant value if (constantCtx) { - const right = this.fromConstantToArray(constantCtx) as ast.ESQLLiteral; + const right = this.fromConstantStrict(constantCtx); const expression = this.toBinaryExpression('=', ctx, [left, right]); if (left.incomplete || right.incomplete) { @@ -326,7 +321,8 @@ export class CstToAstConverter { } // Handle missing value (incomplete assignment) if (assignToken) { - const expression = this.toBinaryExpression('=', ctx, [left, []]); + const right = this.toUnknownMissingNode(assignToken.symbol); + const expression = this.toBinaryExpression('=', ctx, [left, right]); expression.incomplete = true; expression.location = { min: left.location.min, @@ -592,7 +588,7 @@ export class CstToAstConverter { private toOption( name: string, ctx: antlr.ParserRuleContext, - args: ast.ESQLAstItem[] = [], + args: ast.ESQLAstExpression[] = [], incomplete?: boolean ): ast.ESQLCommandOption { return { @@ -601,9 +597,7 @@ export class CstToAstConverter { text: ctx.getText(), location: getPosition(ctx.start, ctx.stop), args, - incomplete: - incomplete ?? - (Boolean(ctx.exception) || [...singleItems(args)].some((arg) => arg.incomplete)), + incomplete: incomplete ?? (Boolean(ctx.exception) || args.some((arg) => arg.incomplete)), }; } @@ -771,7 +765,7 @@ export class CstToAstConverter { private fromLimitCommand(ctx: cst.LimitCommandContext): ast.ESQLCommand<'limit'> { const command = this.createCommand('limit', ctx); if (ctx.constant()) { - const limitValue = this.fromConstantToArray(ctx.constant()); + const limitValue = this.fromConstant(ctx.constant()); if (limitValue != null) { command.args.push(limitValue); } @@ -890,8 +884,8 @@ export class CstToAstConverter { {}, { location: { - min: firstItem([resolveItem(field)])?.location?.min ?? 0, - max: firstItem([resolveItem(condition)])?.location?.max ?? 0, + min: field.location.min, + max: condition.location.max, }, } ); @@ -901,7 +895,7 @@ export class CstToAstConverter { private toByOption( ctx: antlr.ParserRuleContext & Pick, - args: ast.ESQLAstItem[] + args: ast.ESQLAstExpression[] ): ast.ESQLCommandOption | undefined { const byCtx = ctx.BY(); @@ -912,7 +906,7 @@ export class CstToAstConverter { const option = this.toOption(byCtx.getText().toLowerCase(), ctx, args, !args.length); option.location.min = byCtx.symbol.start; - const lastArg = lastItem(option.args); + const lastArg = option.args.at(-1); option.location.max = lastArg?.location.max ?? byCtx.symbol.stop; return option; @@ -928,10 +922,8 @@ export class CstToAstConverter { return command; } - private fromOrderExpressions( - ctx: cst.OrderExpressionContext[] - ): Array { - const expressions: Array = []; + private fromOrderExpressions(ctx: cst.OrderExpressionContext[]): ast.ESQLAstExpression[] { + const expressions: ast.ESQLAstExpression[] = []; for (const orderCtx of ctx) { expressions.push(this.fromOrderExpression(orderCtx)); @@ -940,9 +932,7 @@ export class CstToAstConverter { return expressions; } - private fromOrderExpression( - ctx: cst.OrderExpressionContext - ): ast.ESQLOrderExpression | ast.ESQLAstItem { + private fromOrderExpression(ctx: cst.OrderExpressionContext): ast.ESQLAstExpression { const arg = this.fromBooleanExpressionToExpressionOrUnknown(ctx.booleanExpression()); let order: ast.ESQLOrderExpression['order'] = ''; @@ -994,7 +984,7 @@ export class CstToAstConverter { return command; } - private fromRenameClauses(clausesCtx: cst.RenameClauseContext[]): ast.ESQLAstItem[] { + private fromRenameClauses(clausesCtx: cst.RenameClauseContext[]): ast.ESQLAstExpression[] { return clausesCtx .map((clause) => { const asToken = clause.getToken(cst.EsqlParser.AS, 0); @@ -1019,8 +1009,8 @@ export class CstToAstConverter { renameFunction.args.push(this.toColumn(arg)); } } - const firstArg = firstItem(renameFunction.args); - const lastArg = lastItem(renameFunction.args); + const firstArg = renameFunction.args.at(0); + const lastArg = renameFunction.args.at(-1); const location = renameFunction.location; if (firstArg) location.min = firstArg.location.min; if (lastArg) location.max = lastArg.location.max; @@ -1069,7 +1059,7 @@ export class CstToAstConverter { options.push(option); // it can throw while accessing constant for incomplete commands, so try catch it try { - const optionValue = this.fromConstantToArray(optionCtx.constant()); + const optionValue = this.fromConstant(optionCtx.constant()); if (optionValue != null) { option.args.push(optionValue); } @@ -1265,8 +1255,9 @@ export class CstToAstConverter { for (const clause of clauses) { if (clause._enrichField) { const args: ast.ESQLColumn[] = []; + const assignCtx = clause.ASSIGN(); - if (clause.ASSIGN()) { + if (assignCtx) { args.push(this.toColumn(clause._newName)); if (textExistsAndIsValid(clause._enrichField?.getText())) { args.push(this.toColumn(clause._enrichField)); @@ -1280,13 +1271,20 @@ export class CstToAstConverter { } if (args.length) { const fn = this.toFunction('=', clause, undefined, 'binary-expression'); - fn.args.push(args[0], args[1] ? [args[1]] : []); + let right: ast.ESQLAstExpression | undefined = args[1]; + + if (!right) { + right = this.toUnknownMissingNode(assignCtx!.symbol); + fn.incomplete = true; + } + + fn.args.push(args[0], right); option.args.push(fn); } } const location = option.location; - const lastArg = lastItem(option.args); + const lastArg = option.args.at(-1); location.min = withCtx.symbol.start; location.max = lastArg?.location?.max ?? withCtx.symbol.stop; @@ -1322,17 +1320,15 @@ export class CstToAstConverter { const joinTarget = this.fromJoinTarget(ctx.joinTarget()); const joinCondition = ctx.joinCondition(); const onOption = this.toOption('on', joinCondition); - const joinPredicates: ast.ESQLAstItem[] = onOption.args; + const joinPredicates: ast.ESQLAstExpression[] = onOption.args; for (const joinPredicateCtx of joinCondition.booleanExpression_list()) { const expression = this.fromBooleanExpressionToExpressionOrUnknown(joinPredicateCtx); - if (expression) { - joinPredicates.push(expression); + joinPredicates.push(expression); - if (resolveItem(expression).incomplete) { - onOption.incomplete = true; - } + if (expression.incomplete) { + onOption.incomplete = true; } } @@ -1538,7 +1534,7 @@ export class CstToAstConverter { const command = this.createCommand('sample', ctx); if (ctx.constant()) { - const probability = this.fromConstantToArray(ctx.constant()); + const probability = this.fromConstant(ctx.constant()); if (probability != null) { command.args.push(probability); } @@ -1581,7 +1577,7 @@ export class CstToAstConverter { return; } - const queryText = this.fromConstantToArray(ctx._queryText); + const queryText = this.fromConstant(ctx._queryText); if (!queryText) { return; } @@ -1629,7 +1625,7 @@ export class CstToAstConverter { onOption.args.push(...fields); onOption.location.min = onToken.symbol.start; - const lastArg = lastItem(onOption.args); + const lastArg = onOption.args.at(-1); if (lastArg) { onOption.location.max = lastArg.location.max; } @@ -1690,7 +1686,7 @@ export class CstToAstConverter { private fromFuseCommand(ctx: cst.FuseCommandContext): ast.ESQLAstFuseCommand { const fuseTypeCtx = ctx.identifier(); - const args: ast.ESQLAstItem[] = []; + const args: ast.ESQLAstExpression[] = []; let incomplete = false; const fuseType = fuseTypeCtx ? this.fromIdentifier(fuseTypeCtx) : undefined; @@ -1729,7 +1725,7 @@ export class CstToAstConverter { // SCORE BY const scoreCtx = configCtx.SCORE(); if (scoreCtx && byContext) { - const args: ast.ESQLAstItem[] = []; + const args: ast.ESQLAstExpression[] = []; const scoreColumnCtx = configCtx.qualifiedName(); if (textExistsAndIsValid(scoreColumnCtx.getText())) { @@ -1751,7 +1747,7 @@ export class CstToAstConverter { // GROUP BY const groupCtx = configCtx.GROUP(); if (groupCtx && byContext) { - const args: ast.ESQLAstItem[] = []; + const args: ast.ESQLAstExpression[] = []; const groupColumnCtx = configCtx.qualifiedName(); if (textExistsAndIsValid(groupColumnCtx.getText())) { @@ -1765,7 +1761,7 @@ export class CstToAstConverter { // WITH const withCtx = configCtx.WITH(); if (withCtx) { - const args: ast.ESQLAstItem[] = []; + const args: ast.ESQLAstExpression[] = []; const mapExpressionCtx = configCtx.mapExpression(); const map = this.fromMapExpression(mapExpressionCtx); @@ -2239,7 +2235,7 @@ export class CstToAstConverter { // ---------------------------------------------------------------------- MMR private fromMmrCommand(ctx: cst.MmrCommandContext): ast.ESQLCommand<'mmr'> { - const args: ast.ESQLAstItem[] = []; + const args: ast.ESQLAstExpression[] = []; const queryVector = this.fromMmrQueryVectorParam(ctx.mmrQueryVectorParams()); if (queryVector) args.push(queryVector); @@ -2324,7 +2320,7 @@ export class CstToAstConverter { const limitOption = this.toOption(limitToken.getText().toLowerCase(), limitValueCtx); - limitOption.args.push(this.fromConstantToArray(limitValueCtx)); + limitOption.args.push(this.fromConstantStrict(limitValueCtx)); limitOption.location.min = limitToken.symbol.start; limitOption.location.max = limitValueCtx.stop?.stop ?? limitToken.symbol.stop; @@ -2629,14 +2625,7 @@ export class CstToAstConverter { ); } - /** - * @todo Make it return a single value, not an array. - */ - private visitValueExpression(ctx: cst.ValueExpressionContext) { - if (!ctx.getText()) { - return []; - } - + private visitValueExpression(ctx: cst.ValueExpressionContext): ast.ESQLAstExpression | undefined { if (ctx instanceof cst.ValueExpressionDefaultContext) { return this.fromOperatorExpression(ctx.operatorExpression()); } @@ -2832,8 +2821,8 @@ export class CstToAstConverter { private fromBooleanExpressions( ctx: cst.BooleanExpressionContext[] | undefined - ): ast.ESQLAstItem[] { - const list: ast.ESQLAstItem[] = []; + ): ast.ESQLAstExpression[] { + const list: ast.ESQLAstExpression[] = []; if (!ctx) { return list; @@ -2889,13 +2878,7 @@ export class CstToAstConverter { } if (ctx instanceof cst.BooleanDefaultContext) { - const node = this.fromBooleanDefault(ctx); - - if (Array.isArray(node)) { - return resolveItem(node); - } - - return node; + return this.fromBooleanDefault(ctx); } return undefined; @@ -2964,9 +2947,7 @@ export class CstToAstConverter { } private fromLogicalInLeft(leftCtx: cst.ValueExpressionContext): ast.ESQLAstExpression { - return resolveItem( - this.visitValueExpression(leftCtx) ?? this.fromParserRuleToUnknown(leftCtx) - ) as ast.ESQLAstExpression; + return this.visitValueExpression(leftCtx) ?? this.fromParserRuleToUnknown(leftCtx); } private toLogicalInFunction( @@ -3012,9 +2993,7 @@ export class CstToAstConverter { private toRegexBinaryExpression( ctx: cst.LikeExpressionContext | cst.RlikeExpressionContext ): ast.ESQLBinaryExpression | undefined { - const left = resolveItem(this.visitValueExpression(ctx.valueExpression()) ?? []) as - | ast.ESQLAstExpression - | undefined; + const left = this.visitValueExpression(ctx.valueExpression()); if (!left) { return undefined; @@ -3044,9 +3023,7 @@ export class CstToAstConverter { private toRegexListExpression( ctx: cst.LikeListExpressionContext | cst.RlikeListExpressionContext ): ast.ESQLBinaryExpression | undefined { - const left = resolveItem(this.visitValueExpression(ctx.valueExpression()) ?? []) as - | ast.ESQLAstExpression - | undefined; + const left = this.visitValueExpression(ctx.valueExpression()); if (!left) { return undefined; @@ -3101,7 +3078,7 @@ export class CstToAstConverter { const arg = this.visitValueExpression(ctx.valueExpression()); if (arg) { - fn.args.push(Array.isArray(arg) ? resolveItem(arg) : arg); + fn.args.push(arg); } return fn; @@ -3122,7 +3099,7 @@ export class CstToAstConverter { const constantCtx = ctx.constant(); if (constantCtx) { - const constantExpression = this.fromConstantToArray(constantCtx); + const constantExpression = this.fromConstantStrict(constantCtx); return this.toBinaryExpression(':', ctx, [expression, constantExpression]); } @@ -3474,12 +3451,7 @@ export class CstToAstConverter { if (qualifiedNameCtx && ctx.ASSIGN()) { const left = this.fromQualifiedName(qualifiedNameCtx); const right = this.fromBooleanExpressionToExpressionOrUnknown(ctx.booleanExpression()); - const args = [ - left, - // TODO: Remove array boxing here. This fails many autocomplete tests, - // should be probably fixed in a standalone PR. - [right], - ] as ast.ESQLBinaryExpression['args']; + const args = [left, right] as ast.ESQLBinaryExpression['args']; const assignment = this.toFunction( '=', @@ -3559,7 +3531,7 @@ export class CstToAstConverter { ctx: antlr.ParserRuleContext, customPosition?: ast.ESQLLocation, subtype?: Subtype, - args: ast.ESQLAstItem[] = [], + args: ast.ESQLAstExpression[] = [], incomplete?: boolean ): ast.ESQLFunction { const node: ast.ESQLFunction = { @@ -3642,7 +3614,7 @@ export class CstToAstConverter { const constantCtx = valueCtx.constant(); if (constantCtx) { - value = this.fromConstantToArray(constantCtx) as ast.ESQLAstExpression; + value = this.fromConstant(constantCtx); } const mapExpressionCtx = valueCtx.mapExpression(); @@ -3672,25 +3644,13 @@ export class CstToAstConverter { // ----------------------------------------------------- constant expressions - private fromConstant(ctx: cst.ConstantContext): ast.ESQLAstExpression | undefined { - const node = this.fromConstantToArray(ctx); - - if (Array.isArray(node)) { - return resolveItem(node); - } - - return node; - } - private fromConstantStrict(ctx: cst.ConstantContext): ast.ESQLAstExpression { return this.fromConstant(ctx) ?? this.fromParserRuleToUnknown(ctx); } - /** - * @todo Make return type more specific. - * @todo Make it not return arrays. - */ - private fromConstantToArray(ctx: cst.ConstantContext): ast.ESQLAstItem { + private fromConstant( + ctx: cst.ConstantContext + ): ast.ESQLLiteral | ast.ESQLList | ast.ESQLUnknownItem | undefined { if (ctx instanceof cst.NullLiteralContext) { return this.toLiteral('null', ctx.NULL()); } else if (ctx instanceof cst.QualifiedIntegerLiteralContext) { @@ -3847,19 +3807,12 @@ export class CstToAstConverter { return this.toParam(ctx); } - private fromInputParameter(ctx: cst.InputParameterContext): ast.ESQLLiteral[] { - const values: ast.ESQLLiteral[] = []; - const children = ctx.children; + private fromInputParameter(ctx: cst.InputParameterContext): ast.ESQLParam | undefined { + for (const child of ctx.children ?? []) { + const param = this.toParam(child); - if (children) { - for (const child of children) { - const param = this.toParam(child); - - if (param) values.push(param); - } + if (param) return param; } - - return values; } private toParam(ctx: antlr.ParseTree): ast.ESQLParam | undefined { @@ -3939,15 +3892,9 @@ export class CstToAstConverter { continue; } - const resolved = resolveItem(element) as ast.ESQLAstExpression; - - if (!resolved) { - continue; - } - - values.push(resolved); + values.push(element); - if (resolved.incomplete) { + if (element.incomplete) { incomplete = true; } } diff --git a/packages/esql-parser/src/esql/parser.ts b/packages/esql-parser/src/esql/parser.ts index 20e632b9..de13184a 100644 --- a/packages/esql-parser/src/esql/parser.ts +++ b/packages/esql-parser/src/esql/parser.ts @@ -29,7 +29,6 @@ import type { ESQLProperNode, EditorError, } from '@elastic/esql-types'; -import { singleItems } from '@elastic/esql-traversal'; import { DEFAULT_CHANNEL, SOURCE_COMMANDS } from '../constants'; import type { EsqlParsingTarget } from './types'; import { assertQueryNesting, QUERY_NESTING_ERROR_CODE, QueryNestingError } from './query_nesting'; @@ -268,7 +267,7 @@ export class Parser { } const { root, ast, errors, ...result } = Parser.parseCommand('EVAL ' + src, options); - const expressions = [...singleItems(root.args)]; + const expressions = root.args; if (expressions.length !== 1) { throw new Error( @@ -294,7 +293,7 @@ export class Parser { public static readonly parseMap = (src: string, options?: ParseOptions): ParseResult => { const { root, ast, errors, ...result } = Parser.parseCommand('ROW f(1,' + src + ')', options); - const expressions = [...singleItems(root.args)]; + const expressions = root.args; if (expressions.length !== 1) { throw new Error( diff --git a/packages/esql-traversal/src/debug/__tests__/fixtures.ts b/packages/esql-traversal/src/debug/__tests__/fixtures.ts index 1ead4c89..547b1059 100644 --- a/packages/esql-traversal/src/debug/__tests__/fixtures.ts +++ b/packages/esql-traversal/src/debug/__tests__/fixtures.ts @@ -111,78 +111,76 @@ export const fromStatsByLimit = (): ESQLAstQueryExpression => undefined, { location: { min: 15, max: 16 }, text: 'fn' } ), - [ - expr.func.node( - { - name: 'count', - subtype: 'variadic-call', - operator: Builder.identifier( - { name: 'count' }, - { location: { min: 20, max: 24 }, text: 'count' } + expr.func.node( + { + name: 'count', + subtype: 'variadic-call', + operator: Builder.identifier( + { name: 'count' }, + { location: { min: 20, max: 24 }, text: 'count' } + ), + args: [ + expr.func.node( + { + name: '*', + subtype: 'binary-expression', + args: [ + expr.column( + { + args: [ + Builder.identifier( + { name: 'a' }, + { location: { min: 26, max: 26 }, text: 'a' } + ), + ], + }, + undefined, + { location: { min: 26, max: 26 }, text: 'a' } + ), + expr.func.node( + { + name: '+', + subtype: 'binary-expression', + args: [ + expr.literal.integer(1, undefined, { + location: { min: 31, max: 31 }, + text: '1', + }), + expr.literal.integer(3, undefined, { + location: { min: 35, max: 35 }, + text: '3', + }), + ], + }, + { location: { min: 31, max: 35 }, text: '1+3' } + ), + ], + }, + { location: { min: 26, max: 36 }, text: 'a*(1+3)' } ), - args: [ - expr.func.node( - { - name: '*', - subtype: 'binary-expression', - args: [ - expr.column( - { - args: [ - Builder.identifier( - { name: 'a' }, - { location: { min: 26, max: 26 }, text: 'a' } - ), - ], - }, - undefined, - { location: { min: 26, max: 26 }, text: 'a' } - ), - expr.func.node( - { - name: '+', - subtype: 'binary-expression', - args: [ - expr.literal.integer(1, undefined, { - location: { min: 31, max: 31 }, - text: '1', - }), - expr.literal.integer(3, undefined, { - location: { min: 35, max: 35 }, - text: '3', - }), - ], - }, - { location: { min: 31, max: 35 }, text: '1+3' } - ), - ], - }, - { location: { min: 26, max: 36 }, text: 'a*(1+3)' } - ), - expr.map( - { - entries: [ - expr.entry( - expr.literal.string( - 'adf', - { name: '"adf"' }, - { location: { min: 40, max: 44 }, text: '"adf"' } - ), - expr.literal.integer(123, undefined, { - location: { min: 47, max: 49 }, - text: '123', - }), - { location: { min: 40, max: 49 }, text: '"adf": 123' } + expr.map( + { + entries: [ + expr.entry( + expr.literal.string( + 'adf', + { name: '"adf"' }, + { location: { min: 40, max: 44 }, text: '"adf"' } ), - ], - }, - { location: { min: 39, max: 50 }, text: '{"adf": 123}' } - ), - ], - }, - { location: { min: 20, max: 51 }, text: 'count(a*(1+3),{"adf":123})' } - ), - ], + expr.literal.integer(123, undefined, { + location: { min: 47, max: 49 }, + text: '123', + }), + { location: { min: 40, max: 49 }, text: '"adf": 123' } + ), + ], + }, + { location: { min: 39, max: 50 }, text: '{"adf": 123}' } + ), + ], + }, + { location: { min: 20, max: 51 }, text: 'count(a*(1+3),{"adf":123})' } + ), ], }, { location: { min: 15, max: 51 }, text: 'fn=count(a*(1+3),{"adf":123})' } diff --git a/packages/esql-traversal/src/esql/children.ts b/packages/esql-traversal/src/esql/children.ts index 4f35678b..c47730ce 100644 --- a/packages/esql-traversal/src/esql/children.ts +++ b/packages/esql-traversal/src/esql/children.ts @@ -13,7 +13,6 @@ import type { ESQLCommand, ESQLProperNode, } from '@elastic/esql-types'; -import { singleItems } from './utils'; import { childrenOfPromqlNode } from '../promql/children'; export function* children( @@ -25,11 +24,11 @@ export function* children( case 'header-command': case 'order': case 'option': { - yield* singleItems(node.args); + yield* node.args; break; } case 'list': { - yield* singleItems(node.values); + yield* node.values; break; } case 'map': { @@ -42,11 +41,7 @@ export function* children( break; } case 'inlineCast': { - if (Array.isArray(node.value)) { - yield* singleItems(node.value); - } else { - yield node.value; - } + yield node.value; break; } case 'parens': { @@ -65,7 +60,7 @@ export function* childrenOfAnyNode( } if ('args' in node && Array.isArray(node.args)) { - yield* singleItems(node.args); + yield* node.args; return; } diff --git a/packages/esql-traversal/src/esql/utils.ts b/packages/esql-traversal/src/esql/utils.ts index 98f04d98..8651e82e 100644 --- a/packages/esql-traversal/src/esql/utils.ts +++ b/packages/esql-traversal/src/esql/utils.ts @@ -10,6 +10,9 @@ import type { ESQLAstExpression, ESQLAstItem, ESQLSingleAstItem } from '@elastic /** * Normalizes AST "item" list to only contain *single* items. * + * @deprecated The AST no longer contains array-boxed nodes — iterate the list + * directly. Kept (runtime-tolerant of legacy boxed input) for one major + * release cycle. * @param items A list of single or nested items. */ export function* singleItems( @@ -27,6 +30,9 @@ export function* singleItems( /** * Returns the first normalized "single item" from the "item" list. * + * @deprecated The AST no longer contains array-boxed nodes — use `items[0]` + * directly. Kept (runtime-tolerant of legacy boxed input) for one major + * release cycle. * @param items Returns the first "single item" from the "item" list. * @returns A "single item", if any. */ @@ -36,6 +42,11 @@ export const firstItem = (items: ESQLAstItem[]): ESQLAstExpression | undefined = } }; +/** + * @deprecated The AST no longer contains array-boxed nodes — use the item + * directly. Kept (runtime-tolerant of legacy boxed input) for one major + * release cycle. + */ export const resolveItem = (items: ESQLAstItem | ESQLAstItem[]): ESQLSingleAstItem => { return Array.isArray(items) ? resolveItem(items[0]) : items; }; @@ -43,6 +54,9 @@ export const resolveItem = (items: ESQLAstItem | ESQLAstItem[]): ESQLSingleAstIt /** * Returns the last normalized "single item" from the "item" list. * + * @deprecated The AST no longer contains array-boxed nodes — use + * `items.at(-1)` directly. Kept (runtime-tolerant of legacy boxed input) + * for one major release cycle. * @param items Returns the last "single item" from the "item" list. * @returns A "single item", if any. */ diff --git a/packages/esql-traversal/src/esql/visitor/__tests__/fixtures_commands.ts b/packages/esql-traversal/src/esql/visitor/__tests__/fixtures_commands.ts index ac565a93..1a07f310 100644 --- a/packages/esql-traversal/src/esql/visitor/__tests__/fixtures_commands.ts +++ b/packages/esql-traversal/src/esql/visitor/__tests__/fixtures_commands.ts @@ -51,7 +51,7 @@ const statsCommand = () => expr.literal.integer(1), expr.literal.string('str'), expr.list.literal({ values: [expr.literal.boolean(true)] }), - assign(expr.column('a'), [expr.column('b')]), + assign(expr.column('a'), expr.column('b')), Builder.option({ name: 'by', args: [expr.column('field')] }), ], }); @@ -84,16 +84,17 @@ export const fromChangePoint = (): ESQLAstQueryExpression => { const stats = Builder.command({ name: 'stats', args: [ - assign(expr.column('count'), [call('count', 'COUNT', [])]), + assign(expr.column('count'), call('count', 'COUNT', [])), Builder.option({ name: 'by', args: [ - assign(expr.column('@timestamp'), [ + assign( + expr.column('@timestamp'), call('bucket', 'BUCKET', [ expr.column('@timestamp'), expr.literal.timespan(1, 'MINUTE'), - ]), - ]), + ]) + ), ], }), ], @@ -175,10 +176,11 @@ const missingInferenceId = () => export const fromRerankLimit = (): ESQLAstQueryExpression => { const query = expr.literal.string('star wars'); const fields = [ - assign(expr.column('title'), [call('x', 'X', [expr.column('title'), expr.literal.integer(2)])]), - assign(expr.column('description'), [ - call('x', 'X', [expr.column('description'), expr.literal.decimal(1.5)]), - ]), + assign(expr.column('title'), call('x', 'X', [expr.column('title'), expr.literal.integer(2)])), + assign( + expr.column('description'), + call('x', 'X', [expr.column('description'), expr.literal.decimal(1.5)]) + ), ]; const rerank: ESQLAstRerankCommand = { @@ -282,14 +284,12 @@ export const fromSubqueries = (): ESQLAstQueryExpression => { }), Builder.command({ name: 'eval', - args: [ - assign(expr.column('b'), [binary('*', [expr.column('a'), expr.literal.integer(2)])]), - ], + args: [assign(expr.column('b'), binary('*', [expr.column('a'), expr.literal.integer(2)]))], }), Builder.command({ name: 'stats', args: [ - assign(expr.column('cnt'), [call('count', 'COUNT', [expr.column('*')])]), + assign(expr.column('cnt'), call('count', 'COUNT', [expr.column('*')])), Builder.option({ name: 'by', args: [expr.column('c')] }), ], }), @@ -323,7 +323,7 @@ export const fromSubqueries = (): ESQLAstQueryExpression => { Builder.command({ name: 'stats', args: [ - assign(expr.column('max'), [call('max', 'max', [expr.column('*')])]), + assign(expr.column('max'), call('max', 'max', [expr.column('*')])), Builder.option({ name: 'by', args: [expr.column('e')] }), ], }), diff --git a/packages/esql-traversal/src/esql/visitor/__tests__/fixtures_expressions.ts b/packages/esql-traversal/src/esql/visitor/__tests__/fixtures_expressions.ts index aabc253d..4625b1a9 100644 --- a/packages/esql-traversal/src/esql/visitor/__tests__/fixtures_expressions.ts +++ b/packages/esql-traversal/src/esql/visitor/__tests__/fixtures_expressions.ts @@ -20,7 +20,7 @@ const binary = (name: string, args: [left: ESQLAstItem, right: ESQLAstItem]): ES expr.func.node({ name, subtype: 'binary-expression', args }); const assign = (target: string, value: ESQLAstItem): ESQLFunction => - binary('=', [expr.column(target), [value]]); + binary('=', [expr.column(target), value]); const from = (...indices: string[]) => Builder.command({ name: 'from', args: indices.map((index) => expr.source.index(index)) }); diff --git a/packages/esql-traversal/src/esql/visitor/contexts.ts b/packages/esql-traversal/src/esql/visitor/contexts.ts index 445b40cc..fab93708 100644 --- a/packages/esql-traversal/src/esql/visitor/contexts.ts +++ b/packages/esql-traversal/src/esql/visitor/contexts.ts @@ -11,13 +11,11 @@ import type { SharedData } from './global_visitor_context'; import { type GlobalVisitorContext } from './global_visitor_context'; import { children } from '../children'; -import { firstItem, singleItems } from '../utils'; import type { ESQLAstChangePointCommand, ESQLAstCommand, ESQLAstExpression, ESQLAstHeaderCommand, - ESQLAstItem, ESQLAstJoinCommand, ESQLAstMetricsInfoCommand, ESQLAstQueryExpression, @@ -216,9 +214,6 @@ export class CommandVisitorContext< public *options(): Iterable { for (const arg of this.node.args) { - if (!arg || Array.isArray(arg)) { - continue; - } if (arg.type === 'option') { yield arg; } @@ -238,7 +233,7 @@ export class CommandVisitorContext< } } - public *args(option: '' | string = ''): Iterable { + public *args(option: '' | string = ''): Iterable { option = option.toLowerCase(); if (!option) { @@ -246,10 +241,6 @@ export class CommandVisitorContext< if (!arg) { continue; } - if (Array.isArray(arg)) { - yield arg; - continue; - } if (arg.type !== 'option') { yield arg; } @@ -257,7 +248,7 @@ export class CommandVisitorContext< } const optionNode = this.node.args.find( - (arg) => !Array.isArray(arg) && arg && arg.type === 'option' && arg.name === option + (arg) => arg && arg.type === 'option' && arg.name === option ); if (optionNode) { @@ -273,7 +264,7 @@ export class CommandVisitorContext< ): Iterable> { this.ctx.assertMethodExists('visitExpression'); - for (const arg of singleItems(this.args(option))) { + for (const arg of this.args(option)) { yield this.visitExpression( arg, typeof input === 'function' @@ -288,7 +279,7 @@ export class CommandVisitorContext< ): Iterable>> { this.ctx.assertMethodExists('visitSourceExpression'); - for (const arg of singleItems(this.node.args)) { + for (const arg of this.node.args) { if (arg.type === 'source') { const sourceContext = new SourceExpressionVisitorContext(this.ctx, arg, this); const result = this.ctx.methods.visitSourceExpression!(sourceContext, input); @@ -306,10 +297,6 @@ export class CommandVisitorContext< public *visitSubQueries() { this.ctx.assertMethodExists('visitQuery'); for (const arg of this.node.args) { - if (!arg || Array.isArray(arg)) { - continue; - } - if (arg.type === 'query' && 'commands' in arg) { const result = this.visitSubQuery(arg); yield result; @@ -374,7 +361,7 @@ export class FromCommandVisitorContext< let metadataOption: ESQLCommandOption | undefined; - for (const arg of singleItems(this.node.args)) { + for (const arg of this.node.args) { if (arg.type === 'option' && arg.name === 'metadata') { metadataOption = arg; break; @@ -385,7 +372,7 @@ export class FromCommandVisitorContext< return; } - for (const arg of singleItems(metadataOption.args)) { + for (const arg of metadataOption.args) { if (arg.type === 'column') { const columnContext = new ColumnExpressionVisitorContext(this.ctx, arg, this); const result = this.ctx.methods.visitColumnExpression!(columnContext, input); @@ -405,7 +392,7 @@ export class LimitCommandVisitorContext< * @returns The first numeric literal argument of the command. */ public numericLiteral(): ESQLIntegerLiteral | ESQLDecimalLiteral | undefined { - const arg = firstItem(this.node.args); + const arg = this.node.args[0]; if ( arg && @@ -427,7 +414,7 @@ export class LimitCommandVisitorContext< public setLimit(value: number): void { const literalNode = Builder.expression.literal.numeric({ value, literalType: 'integer' }); - const options = this.node.args.filter((arg) => !Array.isArray(arg) && arg.type === 'option'); + const options = this.node.args.filter((arg) => arg.type === 'option'); this.node.args = [literalNode, ...options]; } @@ -718,9 +705,7 @@ export class InlineCastExpressionVisitorContext< public value(): ESQLAstExpression { this.ctx.assertMethodExists('visitExpression'); - const value = firstItem([this.node.value])!; - - return value; + return this.node.value; } public visitValue( diff --git a/packages/esql-traversal/src/esql/visitor/global_visitor_context.ts b/packages/esql-traversal/src/esql/visitor/global_visitor_context.ts index 8d0ae793..f14013a5 100644 --- a/packages/esql-traversal/src/esql/visitor/global_visitor_context.ts +++ b/packages/esql-traversal/src/esql/visitor/global_visitor_context.ts @@ -778,10 +778,6 @@ export class GlobalVisitorContext< expressionNode: types.ESQLAstExpressionNode, input: types.ExpressionVisitorInput ): types.ExpressionVisitorOutput { - if (Array.isArray(expressionNode)) { - throw new Error('should not happen'); - } - switch (expressionNode.type) { case 'column': { if (!this.methods.visitColumnExpression) break; diff --git a/packages/esql-traversal/src/esql/visitor/types.ts b/packages/esql-traversal/src/esql/visitor/types.ts index 6c772ad1..ccedbf6e 100644 --- a/packages/esql-traversal/src/esql/visitor/types.ts +++ b/packages/esql-traversal/src/esql/visitor/types.ts @@ -17,7 +17,6 @@ export type ESQLAstQueryNode = ast.ESQLAstQueryExpression; /** * Represents an "expression" node in the AST. */ -// export type ESQLAstExpressionNode = ESQLAstItem; export type ESQLAstExpressionNode = ast.ESQLAstExpression; /** diff --git a/packages/esql-traversal/src/esql/walker/__tests__/walker.test.ts b/packages/esql-traversal/src/esql/walker/__tests__/walker.test.ts index 3268cf53..1c5ed827 100644 --- a/packages/esql-traversal/src/esql/walker/__tests__/walker.test.ts +++ b/packages/esql-traversal/src/esql/walker/__tests__/walker.test.ts @@ -475,15 +475,16 @@ const fromWhereDeepInlineCast = () => binary('+', [ expr.literal.integer(1), expr.func.call('fn', [ - unary('not', [ + unary( + 'not', binary('*', [ expr.literal.integer(-1), expr.inlineCast({ castType: 'integer', value: expr.column(['a', 'b', 'c']), }), - ]), - ]), + ]) + ), ]), ]), ]), diff --git a/packages/esql-traversal/src/esql/walker/walker.ts b/packages/esql-traversal/src/esql/walker/walker.ts index 9cf89f65..dd698eb2 100644 --- a/packages/esql-traversal/src/esql/walker/walker.ts +++ b/packages/esql-traversal/src/esql/walker/walker.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { resolveItem } from '../utils'; import { isPromqlNode, replaceProperties, templateToPredicate } from './helpers'; import { PromqlWalker, type PromqlWalkerOptions } from '../../promql/walker'; import type * as types from '@elastic/esql-types'; @@ -754,15 +753,13 @@ export class Walker { } public walkExpression( - node: types.ESQLAstItem | types.ESQLAstExpression, + node: types.ESQLAstExpression | types.ESQLAstExpression[], parent: types.ESQLProperNode | undefined = undefined ): void { if (Array.isArray(node)) { - const list = node as types.ESQLAstItem[]; - this.walkList(list, parent); + this.walkList(node, parent); } else { - const item = node as types.ESQLSingleAstItem; - this.walkSingleAstItem(item, parent); + this.walkSingleAstItem(node, parent); } } @@ -858,11 +855,11 @@ export class Walker { if (this.readAndResetSkippedChildren()) return; if (options.order === 'backward') { - this.walkSingleAstItem(resolveItem(node.value), node); - this.walkSingleAstItem(resolveItem(node.key), node); + this.walkSingleAstItem(node.value, node); + this.walkSingleAstItem(node.key, node); } else { - this.walkSingleAstItem(resolveItem(node.key), node); - this.walkSingleAstItem(resolveItem(node.value), node); + this.walkSingleAstItem(node.key, node); + this.walkSingleAstItem(node.value, node); } } diff --git a/packages/esql-types/src/types.ts b/packages/esql-types/src/types.ts index b7b41fd6..572674e1 100644 --- a/packages/esql-types/src/types.ts +++ b/packages/esql-types/src/types.ts @@ -31,7 +31,7 @@ export type ESQLAstCommand = export type ESQLAstAllCommands = ESQLAstCommand | ESQLAstHeaderCommand; -export type ESQLAstNode = ESQLAstCommand | ESQLAstHeaderCommand | ESQLAstExpression | ESQLAstItem; +export type ESQLAstNode = ESQLAstCommand | ESQLAstHeaderCommand | ESQLAstExpression; /** * Represents an *expression* in the AST. @@ -73,20 +73,20 @@ export type ESQLSingleAstItem = export type ESQLAstField = ESQLColumn | ESQLBinaryExpression | ESQLAstExpression | ESQLParam; /** - * An array of AST nodes represents different things in different contexts. - * For example, in command top level arguments it is treated as an "assignment expression". + * @deprecated The AST no longer contains array-boxed nodes, use + * {@link ESQLAstExpression} (or {@link ESQLSingleAstItem}) directly. */ -export type ESQLAstItem = ESQLSingleAstItem | ESQLAstItem[]; +export type ESQLAstItem = ESQLSingleAstItem; export type ESQLAstNodeWithArgs = ESQLCommand | ESQLCommandOption | ESQLFunction; export type ESQLAstNodeWithChildren = ESQLAstNodeWithArgs | ESQLList; /** - * *Proper* are nodes which are objects with `type` property, once we get rid - * of the nodes which are plain arrays, all nodes will be *proper* and we can - * remove this type. + * @deprecated Historically, *proper* nodes were nodes with a `type` property, + * as opposed to nodes represented by plain arrays. Plain-array nodes no + * longer exist, hence all nodes are now *proper* — use {@link ESQLAstNode} instead. */ -export type ESQLProperNode = ESQLAstExpression | ESQLAstCommand | ESQLAstHeaderCommand; +export type ESQLProperNode = ESQLAstNode; export interface ESQLLocation { min: number; @@ -128,7 +128,7 @@ export interface ESQLCommand extends ESQLAstBaseItem { */ commandType?: string; - args: ESQLAstItem[]; + args: ESQLAstExpression[]; } export interface ESQLAstJoinCommand extends ESQLCommand<'join'> { @@ -305,7 +305,7 @@ export type ESQLIdentifierOrParam = ESQLIdentifier | ESQLParamLiteral; export interface ESQLCommandOption extends ESQLAstBaseItem { type: 'option'; - args: ESQLAstItem[]; + args: ESQLAstExpression[]; } export interface ESQLAstQueryExpression extends ESQLAstBaseItem<''> { @@ -345,12 +345,12 @@ export interface ESQLFunction< */ operator?: ESQLIdentifier | ESQLParamLiteral; - args: ESQLAstItem[]; + args: ESQLAstExpression[]; } export interface ESQLFunctionCallExpression extends ESQLFunction<'variadic-call'> { subtype: 'variadic-call'; - args: ESQLAstItem[]; + args: ESQLAstExpression[]; } export interface ESQLUnaryExpression extends ESQLFunction< @@ -358,7 +358,7 @@ export interface ESQLUnaryExpression extends ESQLF Name > { subtype: 'unary-expression'; - args: [ESQLAstItem]; + args: [ESQLAstExpression]; } export interface ESQLPostfixUnaryExpression extends ESQLFunction< @@ -366,7 +366,7 @@ export interface ESQLPostfixUnaryExpression extend Name > { subtype: 'postfix-unary-expression'; - args: [ESQLAstItem]; + args: [ESQLAstExpression]; } /** @@ -380,14 +380,14 @@ export interface ESQLOrderExpression extends ESQLAstBaseItem { type: 'order'; order: '' | 'ASC' | 'DESC'; nulls: '' | 'NULLS FIRST' | 'NULLS LAST'; - args: [field: ESQLAstItem]; + args: [field: ESQLAstExpression]; } export interface ESQLBinaryExpression< Name extends BinaryExpressionOperator = BinaryExpressionOperator, > extends ESQLFunction<'binary-expression', Name> { subtype: 'binary-expression'; - args: [ESQLAstItem, ESQLAstItem]; + args: [ESQLAstExpression, ESQLAstExpression]; } export type BinaryExpressionOperator = @@ -411,7 +411,7 @@ export type BinaryExpressionMatchOperator = ':'; export type BinaryExpressionIn = 'in' | 'not in'; export type BinaryExpressionLogical = 'and' | 'or'; -export interface ESQLInlineCast extends ESQLAstBaseItem { +export interface ESQLInlineCast extends ESQLAstBaseItem { type: 'inlineCast'; value: ValueType; castType: string; diff --git a/packages/esql/src/ast/mutate/commands/limit/index.ts b/packages/esql/src/ast/mutate/commands/limit/index.ts index fde3d7fb..c868d515 100644 --- a/packages/esql/src/ast/mutate/commands/limit/index.ts +++ b/packages/esql/src/ast/mutate/commands/limit/index.ts @@ -93,7 +93,7 @@ export const set = ( } const literal = Builder.expression.literal.numeric({ literalType: 'integer', value }); - const options = node.args.filter((arg) => !Array.isArray(arg) && arg.type === 'option'); + const options = node.args.filter((arg) => arg.type === 'option'); node.args = [literal, ...options]; diff --git a/packages/esql/src/ast/mutate/commands/rerank/index.ts b/packages/esql/src/ast/mutate/commands/rerank/index.ts index 25d3de87..a939c4b8 100644 --- a/packages/esql/src/ast/mutate/commands/rerank/index.ts +++ b/packages/esql/src/ast/mutate/commands/rerank/index.ts @@ -13,7 +13,7 @@ import type { ESQLStringLiteral, ESQLParamLiteral, ESQLMap, - ESQLAstItem, + ESQLAstExpression, ESQLMapEntry, ESQLLiteral, } from '../../../../types'; @@ -44,12 +44,7 @@ export const setQuery = (cmd: ESQLAstRerankCommand, query: string | ESQLStringLi cmd.query = queryLiteral; - if ( - firstArg && - !Array.isArray(firstArg) && - firstArg.type === 'function' && - firstArg.name === '=' - ) { + if (firstArg && firstArg.type === 'function' && firstArg.name === '=') { // It's an assignment, update the right side of the expression firstArg.args[1] = queryLiteral; } else { @@ -68,8 +63,7 @@ export const setQuery = (cmd: ESQLAstRerankCommand, query: string | ESQLStringLi */ export const setTargetField = (cmd: ESQLAstRerankCommand, target: string | null) => { const firstArg = cmd.args[0]; - const isAssignment = - firstArg && !Array.isArray(firstArg) && firstArg.type === 'function' && firstArg.name === '='; + const isAssignment = firstArg && firstArg.type === 'function' && firstArg.name === '='; // Case 1: Set a new target field if (target !== null) { @@ -117,8 +111,8 @@ export const setFields = ( }); } - const isOnOption = (arg: ESQLAstItem): arg is ESQLCommandOption => - !!arg && !Array.isArray(arg) && arg.type === 'option' && arg.name === 'on'; + const isOnOption = (arg: ESQLAstExpression): arg is ESQLCommandOption => + !!arg && arg.type === 'option' && arg.name === 'on'; const onOption = cmd.args.find(isOnOption); @@ -156,14 +150,14 @@ export const setWithParameter = ( return val; }; - const isWithOption = (arg: ESQLAstItem): arg is ESQLCommandOption => - !!arg && !Array.isArray(arg) && arg.type === 'option' && arg.name === 'with'; + const isWithOption = (arg: ESQLAstExpression): arg is ESQLCommandOption => + !!arg && arg.type === 'option' && arg.name === 'with'; // Validates and retrieves the map from a WITH option const getWithOptionMap = (withOption: ESQLCommandOption): ESQLMap => { const mapArg = withOption.args[0]; - if (!mapArg || typeof mapArg === 'string' || Array.isArray(mapArg) || mapArg.type !== 'map') { + if (!mapArg || mapArg.type !== 'map') { throw new Error('WITH option must contain a map'); } diff --git a/packages/esql/src/ast/mutate/generic/commands/args/index.ts b/packages/esql/src/ast/mutate/generic/commands/args/index.ts index 095e498a..34eedf4e 100644 --- a/packages/esql/src/ast/mutate/generic/commands/args/index.ts +++ b/packages/esql/src/ast/mutate/generic/commands/args/index.ts @@ -71,7 +71,7 @@ export const remove = ( return ctx.node; } - if (!Array.isArray(arg) && isSubQuery(arg)) { + if (isSubQuery(arg)) { const found = remove(arg.child, node); if (found) { diff --git a/packages/esql/src/ast/mutate/generic/commands/options/index.ts b/packages/esql/src/ast/mutate/generic/commands/options/index.ts index b4eee4f9..44b886d0 100644 --- a/packages/esql/src/ast/mutate/generic/commands/options/index.ts +++ b/packages/esql/src/ast/mutate/generic/commands/options/index.ts @@ -7,7 +7,7 @@ import { Builder } from '@elastic/esql-ast'; import type { - ESQLAstItem, + ESQLAstExpression, ESQLAstQueryExpression, ESQLCommand, ESQLCommandOption, @@ -110,7 +110,7 @@ export const remove = (ast: ESQLAstQueryExpression, option: ESQLCommandOption): return false; } - const index = (ctx.node.args as ESQLAstItem[]).indexOf(target); + const index = (ctx.node.args as ESQLAstExpression[]).indexOf(target); if (index === -1) { return false; diff --git a/packages/esql/src/composer/composer_query.ts b/packages/esql/src/composer/composer_query.ts index 81cc5bfa..03a67002 100644 --- a/packages/esql/src/composer/composer_query.ts +++ b/packages/esql/src/composer/composer_query.ts @@ -42,7 +42,7 @@ import type { QueryCommandTag, QueryCommandTagParametrized, } from './types'; -import { Walker, replaceProperties, resolveItem } from '@elastic/esql-traversal'; +import { Walker, replaceProperties } from '@elastic/esql-traversal'; import { printAst } from '../debug'; export class ComposerQuery { @@ -1182,8 +1182,8 @@ export class ComposerQuery { const left = arg.args[0]; const right = arg.args[1]; - if (isIdentifier(left) && (isProperNode(right) || Array.isArray(right))) { - sets.push([left.name as string, resolveItem(right)]); + if (isIdentifier(left) && isProperNode(right)) { + sets.push([left.name as string, right]); } } } diff --git a/packages/esql/src/pretty_print/basic_pretty_printer.ts b/packages/esql/src/pretty_print/basic_pretty_printer.ts index e82afba4..1bfcaa1d 100644 --- a/packages/esql/src/pretty_print/basic_pretty_printer.ts +++ b/packages/esql/src/pretty_print/basic_pretty_printer.ts @@ -23,7 +23,7 @@ import { binaryExpressionGroup, unaryExpressionGroup, } from '../ast/grouping'; -import { Visitor, resolveItem, type ESQLAstExpressionNode } from '@elastic/esql-traversal'; +import { Visitor, type ESQLAstExpressionNode } from '@elastic/esql-traversal'; import { commandOptionsWithEqualsSeparator, commandsWithNoCommaArgSeparator, @@ -32,7 +32,7 @@ import { import type { ESQLAstBaseItem, ESQLAstCommand, - ESQLAstItem, + ESQLAstExpression, ESQLAstQueryExpression, ESQLMap, ESQLProperNode, @@ -211,9 +211,7 @@ export class BasicPrettyPrinter { minusCount: number = 0 ): string | undefined { if (isBinaryExpression(node) && node.name === '*') { - let [left, right] = node.args; - left = resolveItem(left); - right = resolveItem(right); + const [left, right] = node.args; if (isProperNode(left) && isProperNode(right)) { if (!!left.formatting || !!right.formatting) { @@ -449,12 +447,13 @@ export class BasicPrettyPrinter { // For assignments (=), left is the target name, right is the expression to assign. const [left, right] = ctx.arguments(); - const formatOperand = (operand: ESQLAstItem, index: number): string => { + const formatOperand = (operand: ESQLAstExpression, index: number): string => { const operandGroup = binaryExpressionGroup(operand); let formatted = ctx.visitArgument(index); const shouldGroup = operandGroup && + group !== BinaryExpressionGroup.assignment && (operandGroup === BinaryExpressionGroup.unknown || operandGroup < group || // Right operand at same precedence needs parens for /, -, % @@ -555,10 +554,6 @@ export class BasicPrettyPrinter { if (cmd === 'FORK') { const branches = node.args .map((branch) => { - if (Array.isArray(branch)) { - return undefined; - } - // Check for ESQLAstQueryExpression specifically (has 'commands' property) if ( branch.type === 'parens' && @@ -614,13 +609,8 @@ export class BasicPrettyPrinter { .on('visitQuery', (ctx) => { const opts = this.opts; - let parentNode; - if (ctx.parent?.node && !Array.isArray(ctx.parent.node)) { - parentNode = ctx.parent.node; - } - - const useMultiLine = - opts.multiline && !Array.isArray(parentNode) && parentNode?.name !== 'fork'; + const parentNode = ctx.parent?.node; + const useMultiLine = opts.multiline && parentNode?.name !== 'fork'; const cmdSeparator = useMultiLine ? `\n${opts.pipeTab ?? ' '}| ` : ' | '; let text = ''; diff --git a/packages/esql/src/pretty_print/wrapping_pretty_printer/printer.ts b/packages/esql/src/pretty_print/wrapping_pretty_printer/printer.ts index d8032918..8225c1c4 100644 --- a/packages/esql/src/pretty_print/wrapping_pretty_printer/printer.ts +++ b/packages/esql/src/pretty_print/wrapping_pretty_printer/printer.ts @@ -53,7 +53,6 @@ import type { ESQLOrderExpression, ESQLParens, ESQLSource, - ESQLAstItem, ESQLAstExpression, } from '@elastic/esql-types'; import type { BasicPrettyPrinterOptions } from '../basic_pretty_printer'; @@ -65,7 +64,6 @@ import { import { getPrettyPrintStats } from '../helpers'; import { PromQLWrappingPrettyPrinter } from '../../embedded_languages/promql/pretty_print'; import type { PromQLAstQueryExpression } from '@elastic/esql-types'; -import { singleItems, resolveItem } from '@elastic/esql-traversal'; import { BinaryExpressionGroup, binaryExpressionGroup, @@ -261,7 +259,7 @@ export class WrappingPrettyPrinter { protected docHeaderCommand(cmd: ESQLAstHeaderCommand): Doc { const lower = this.opts.lowercaseCommands; const name = lower ? cmd.name.toLowerCase() : cmd.name.toUpperCase(); - const argItems = [...singleItems(cmd.args)]; + const argItems = cmd.args; const argDocs = argItems.map((a) => this.docExpression(a)); const hasLBD = argItems.some((a) => getPrettyPrintStats(a).hasLineBreakingDecorations); const argsDoc: Doc = argDocs.length @@ -294,7 +292,7 @@ export class WrappingPrettyPrinter { const cmdName = this.formatCmdName(cmd); // Separate args from options - const allArgs = [...singleItems(cmd.args)]; + const allArgs = cmd.args; const args = allArgs.filter((a) => a.type !== 'option'); const opts = allArgs.filter((a) => a.type === 'option') as ESQLCommandOption[]; @@ -486,7 +484,7 @@ export class WrappingPrettyPrinter { protected docCommandOption(opt: ESQLCommandOption): Doc { const lower = this.opts.lowercaseOptions; const name = lower ? opt.name.toLowerCase() : opt.name.toUpperCase(); - const args = [...singleItems(opt.args)]; + const args = opt.args; const argDocs = args.map((a) => this.docExpression(a)); if (argDocs.length === 0) return name; @@ -574,13 +572,7 @@ export class WrappingPrettyPrinter { // -------------------------------------------------------------- Expressions - protected docExpression(node: ESQLAstItem): Doc { - if (Array.isArray(node)) { - const items = [...singleItems([node])]; - if (items.length === 1) return this.docExpression(items[0]); - return items.map((item) => this.docExpression(item)); - } - + protected docExpression(node: ESQLAstExpression): Doc { if (isPromqlNode(node)) { const promqlDoc = this.docPromql(node as PromQLAstQueryExpression); @@ -683,12 +675,15 @@ export class WrappingPrettyPrinter { } const operator = this.opts.lowercaseKeywords ? op.toLowerCase() : op.toUpperCase(); - const [leftItem, rightItem] = node.args as [ESQLAstItem, ESQLAstItem]; + const [leftItem, rightItem] = node.args; const group_of = binaryExpressionGroup(node); const leftGroup = binaryExpressionGroup(leftItem); const rightGroup = binaryExpressionGroup(rightItem); - const wrapLeft = leftGroup !== BinaryExpressionGroup.none && leftGroup < group_of; + const isAssignment = group_of === BinaryExpressionGroup.assignment; + const wrapLeft = + !isAssignment && leftGroup !== BinaryExpressionGroup.none && leftGroup < group_of; const wrapRight = + !isAssignment && rightGroup !== BinaryExpressionGroup.none && (rightGroup < group_of || (rightGroup === group_of && (node.name === '/' || node.name === '-' || node.name === '%'))); @@ -708,8 +703,7 @@ export class WrappingPrettyPrinter { */ private docSimplifyMultiplicationByOne(node: ESQLAstExpression, minusCount = 0): Doc | undefined { if (isBinaryExpression(node) && node.name === '*') { - const left = resolveItem(node.args[0]); - const right = resolveItem(node.args[1]); + const [left, right] = node.args; if (isProperNode(left) && isProperNode(right)) { if (!!left.formatting || !!right.formatting) return undefined; @@ -752,7 +746,7 @@ export class WrappingPrettyPrinter { : this.opts.lowercaseFunctions ? op.toLowerCase() : op.toUpperCase(); - const args = [...singleItems(node.args)]; + const args = node.args; if (args.length === 0) return `${name}()`; @@ -884,7 +878,7 @@ export class WrappingPrettyPrinter { // --------------------------------------------------- Inline cast expression protected docInlineCast(node: ESQLInlineCast): Doc { - const value = node.value as ESQLAstExpression; + const value = node.value; const wrapInBrackets = value.type !== 'literal' && value.type !== 'column' &&