diff --git a/packages/compiler/src/expressions/compile-expression.ts b/packages/compiler/src/expressions/compile-expression.ts index 1189cf1a..50674314 100644 --- a/packages/compiler/src/expressions/compile-expression.ts +++ b/packages/compiler/src/expressions/compile-expression.ts @@ -31,7 +31,7 @@ import { decomposeAtMemberExpression, escapeStringValue, } from '@agentscript/language'; -import type { Range } from '@agentscript/types'; +import type { Range, SyntaxNode } from '@agentscript/types'; import type { CompilerContext } from '../compiler-context.js'; import { extractNumberValue } from '../ast-helpers.js'; @@ -458,36 +458,42 @@ function compileSlice( return `${start}:${stop}`; } +function getParenthesizedCst(expr: Expression): SyntaxNode | undefined { + let node = expr.__cst?.node; + let parenthesized: SyntaxNode | undefined; + // The dialect unwraps grouping and expression/atom nodes from the AST. + // Retain the outermost grouping node for comparison source-text boundaries. + while (node) { + if (node.type === 'parenthesized_expression') parenthesized = node; + const parent = node.parent; + if ( + parent?.type !== 'parenthesized_expression' && + parent?.type !== 'expression' && + parent?.type !== 'atom' + ) { + break; + } + node = parent; + } + return parenthesized; +} + +function compileOperand( + expr: Expression, + ctx: CompilerContext, + opts: CompileExpressionOptions +): string { + const compiled = compileExprNode(expr, ctx, opts); + return getParenthesizedCst(expr) ? `(${compiled})` : compiled; +} + function compileBinaryExpression( expr: BinaryExpression, ctx: CompilerContext, opts: CompileExpressionOptions ): string { - const left = compileExprNode(expr.left, ctx, opts); - const right = compileExprNode(expr.right, ctx, opts); - - // Check if the right operand was parenthesized in the source - // CST structure: parenthesized_expression > expression > binary_expression - // So we need to check parent or grandparent for parenthesized_expression - const rightCst = expr.right.__cst?.node; - const rightParenthesized = - rightCst?.parent?.type === 'parenthesized_expression' || - rightCst?.parent?.parent?.type === 'parenthesized_expression'; - - if (rightParenthesized) { - return `${left} ${expr.operator} (${right})`; - } - - // Check if the left operand was parenthesized - const leftCst = expr.left.__cst?.node; - const leftParenthesized = - leftCst?.parent?.type === 'parenthesized_expression' || - leftCst?.parent?.parent?.type === 'parenthesized_expression'; - - if (leftParenthesized) { - return `(${left}) ${expr.operator} ${right}`; - } - + const left = compileOperand(expr.left, ctx, opts); + const right = compileOperand(expr.right, ctx, opts); return `${left} ${expr.operator} ${right}`; } @@ -496,7 +502,7 @@ function compileUnaryExpression( ctx: CompilerContext, opts: CompileExpressionOptions ): string { - const operand = compileExprNode(expr.operand, ctx, opts); + const operand = compileOperand(expr.operand, ctx, opts); if (expr.operator === 'not') { return `not ${operand}`; } @@ -508,19 +514,22 @@ function compileComparisonExpression( ctx: CompilerContext, opts: CompileExpressionOptions ): string { - const left = compileExprNode(expr.left, ctx, opts); - const right = compileExprNode(expr.right, ctx, opts); + const left = compileOperand(expr.left, ctx, opts); + const right = compileOperand(expr.right, ctx, opts); // Preserve original whitespace around the operator from the source text const cstText = expr.__cst?.node?.text; if (cstText) { - const leftCst = expr.left.__cst?.node?.text; - const rightCst = expr.right.__cst?.node?.text; + const leftCst = (getParenthesizedCst(expr.left) ?? expr.left.__cst?.node) + ?.text; + const rightCst = (getParenthesizedCst(expr.right) ?? expr.right.__cst?.node) + ?.text; if (leftCst && rightCst) { // Extract the operator with surrounding whitespace from the CST - const leftEnd = cstText.indexOf(leftCst) + leftCst.length; + const leftStart = cstText.indexOf(leftCst); + const leftEnd = leftStart + leftCst.length; const rightStart = cstText.lastIndexOf(rightCst); - if (leftEnd >= 0 && rightStart > leftEnd) { + if (leftStart >= 0 && rightStart > leftEnd) { const operatorWithSpace = cstText.slice(leftEnd, rightStart); return `${left}${operatorWithSpace}${right}`; } diff --git a/packages/compiler/test/expressions.test.ts b/packages/compiler/test/expressions.test.ts index ed134877..19ac8213 100644 --- a/packages/compiler/test/expressions.test.ts +++ b/packages/compiler/test/expressions.test.ts @@ -7,8 +7,11 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { DiagnosticSeverity } from '@agentscript/types'; +import { parse } from '@agentscript/parser'; import { type Expression, + Dialect, + ExpressionValue, StringLiteral, NumberLiteral, BooleanLiteral, @@ -368,6 +371,27 @@ describe('compileExpression', () => { }); describe('binary expressions', () => { + it.each([ + ['(1 + 2) * (3 + 4)', '(1 + 2) * (3 + 4)'], + ['(1 + 2) * 3', '(1 + 2) * 3'], + ['1 * (2 + 3)', '1 * (2 + 3)'], + ['1 * 2', '1 * 2'], + [ + '(True or False) and (False or False)', + '(True or False) and (False or False)', + ], + ])('preserves operand grouping in %s', (source, expected) => { + const { rootNode } = parse(`value: ${source}\n`); + const mapping = + rootNode.namedChildren.find(node => node.type === 'mapping') ?? + rootNode; + const result = new Dialect().parse(mapping, { value: ExpressionValue }); + + expect(result.diagnostics).toEqual([]); + expect(result.value.value).toBeInstanceOf(BinaryExpression); + expect(compileExpression(result.value.value, ctx)).toBe(expected); + }); + it('should compile binary addition', () => { const expr = new BinaryExpression( new NumberLiteral(1), @@ -388,6 +412,30 @@ describe('compileExpression', () => { }); describe('unary expressions', () => { + it.each([ + ['not (True and False)', 'not (True and False)'], + [ + '@variables.a and not (@variables.b or @variables.c)', + 'state.a and not (state.b or state.c)', + ], + ['-(1 + 2)', '-(1 + 2)'], + ['not True', 'not True'], + ['-1', '-1'], + ])('preserves operand grouping in %s', (source, expected) => { + ctx.mutableVariableNames.add('a'); + ctx.mutableVariableNames.add('b'); + ctx.mutableVariableNames.add('c'); + const { rootNode } = parse(`value: ${source}\n`); + const mapping = + rootNode.namedChildren.find(node => node.type === 'mapping') ?? + rootNode; + const result = new Dialect().parse(mapping, { value: ExpressionValue }); + + expect(result.diagnostics).toEqual([]); + expect(compileExpression(result.value.value, ctx)).toBe(expected); + expect(ctx.diagnostics).toEqual([]); + }); + it('should compile not operator', () => { const expr = new UnaryExpression('not', new BooleanLiteral(true)); expect(compileExpression(expr, ctx)).toBe('not True'); @@ -400,6 +448,27 @@ describe('compileExpression', () => { }); describe('comparison expressions', () => { + it.each([ + ['(True or False) == False', '(True or False) == False'], + ['True == (False or True)', 'True == (False or True)'], + ['(1 + 2) < (3 + 4)', '(1 + 2) < (3 + 4)'], + ['((1 + 2)) == ((3 + 4))', '(1 + 2) == (3 + 4)'], + ['(1 + 2)==(1 + 2)', '(1 + 2)==(1 + 2)'], + ['1 == 1', '1 == 1'], + ['1==1', '1==1'], + ])('preserves operand grouping and spacing in %s', (source, expected) => { + const { rootNode } = parse(`value: ${source}\n`); + const mapping = + rootNode.namedChildren.find(node => node.type === 'mapping') ?? + rootNode; + const result = new Dialect().parse(mapping, { value: ExpressionValue }); + + expect(result.diagnostics).toEqual([]); + expect(result.value.value).toBeInstanceOf(ComparisonExpression); + expect(compileExpression(result.value.value, ctx)).toBe(expected); + expect(ctx.diagnostics).toEqual([]); + }); + it('should compile == comparison', () => { ctx.mutableVariableNames.add('x'); const expr = new ComparisonExpression(