From 32f949d5cda4738dcde6837a6e4c7eed1dadcb47 Mon Sep 17 00:00:00 2001 From: BichengWang Date: Tue, 13 Oct 2026 23:36:08 -0700 Subject: [PATCH 1/2] fix(compiler): preserve parentheses on both binary operands --- .../src/expressions/compile-expression.ts | 12 +++------- packages/compiler/test/expressions.test.ts | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/packages/compiler/src/expressions/compile-expression.ts b/packages/compiler/src/expressions/compile-expression.ts index 1189cf1a..95d02cf3 100644 --- a/packages/compiler/src/expressions/compile-expression.ts +++ b/packages/compiler/src/expressions/compile-expression.ts @@ -474,21 +474,15 @@ function compileBinaryExpression( 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}`; - } - - return `${left} ${expr.operator} ${right}`; + const compiledLeft = leftParenthesized ? `(${left})` : left; + const compiledRight = rightParenthesized ? `(${right})` : right; + return `${compiledLeft} ${expr.operator} ${compiledRight}`; } function compileUnaryExpression( diff --git a/packages/compiler/test/expressions.test.ts b/packages/compiler/test/expressions.test.ts index ed134877..d12dc8a2 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), From 5e407c1501c782436bc1e6d362cfe0bdd04c1792 Mon Sep 17 00:00:00 2001 From: BichengWang Date: Wed, 14 Oct 2026 11:30:56 -0700 Subject: [PATCH 2/2] fix(compiler): preserve unary and comparison operand grouping --- .../src/expressions/compile-expression.ts | 71 +++++++++++-------- packages/compiler/test/expressions.test.ts | 45 ++++++++++++ 2 files changed, 88 insertions(+), 28 deletions(-) diff --git a/packages/compiler/src/expressions/compile-expression.ts b/packages/compiler/src/expressions/compile-expression.ts index 95d02cf3..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,31 +458,43 @@ 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'; - - // 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'; - - const compiledLeft = leftParenthesized ? `(${left})` : left; - const compiledRight = rightParenthesized ? `(${right})` : right; - return `${compiledLeft} ${expr.operator} ${compiledRight}`; + const left = compileOperand(expr.left, ctx, opts); + const right = compileOperand(expr.right, ctx, opts); + return `${left} ${expr.operator} ${right}`; } function compileUnaryExpression( @@ -490,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}`; } @@ -502,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 d12dc8a2..19ac8213 100644 --- a/packages/compiler/test/expressions.test.ts +++ b/packages/compiler/test/expressions.test.ts @@ -412,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'); @@ -424,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(