Skip to content

fix(compiler): preserve parentheses on both binary operands #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
awli merged 2 commits into from
Oct 15, 2026
Merged

fix(compiler): preserve parentheses on both binary operands #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
32f949d
fix(compiler): preserve parentheses on both binary operands
BichengWang Oct 14, 2026
5e407c1
fix(compiler): preserve unary and comparison operand grouping
BichengWang Oct 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 42 additions & 33 deletions packages/compiler/src/expressions/compile-expression.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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}`;
}

Expand All @@ -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}`;
}
Expand All @@ -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}`;
}
Expand Down
69 changes: 69 additions & 0 deletions packages/compiler/test/expressions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand All @@ -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');
Expand All @@ -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(
Expand Down
Loading