From a6f487ec2eb188f2a7f6c10820da852f082ec25d Mon Sep 17 00:00:00 2001 From: bqxbqx Date: Fri, 28 Mar 2025 18:56:56 +0800 Subject: [PATCH] fix: implement short-circuit evaluation for logical operators in interpreter --- src/interpreter.ts | 22 ++++++-- tests/interpreter.test.ts | 111 ++++++++++++++++++++------------------ 2 files changed, 77 insertions(+), 56 deletions(-) diff --git a/src/interpreter.ts b/src/interpreter.ts index 9917624..55ec108 100644 --- a/src/interpreter.ts +++ b/src/interpreter.ts @@ -136,6 +136,24 @@ export const evaluateAst = ( * @example x > y → context.x > context.y */ const evaluateBinaryExpression = (node: BinaryExpression): unknown => { + // Implement short-circuit evaluation for logical operators + if (node.operator === "&&") { + const left = evaluateNode(node.left); + // If left side is falsy, return it immediately without evaluating right side + if (!left) return left; + // Otherwise evaluate and return right side + return evaluateNode(node.right); + } + + if (node.operator === "||") { + const left = evaluateNode(node.left); + // If left side is truthy, return it immediately without evaluating right side + if (left) return left; + // Otherwise evaluate and return right side + return evaluateNode(node.right); + } + + // For other operators, evaluate both sides normally const left = evaluateNode(node.left); const right = evaluateNode(node.right); @@ -164,10 +182,6 @@ export const evaluateAst = ( return (left as number) < (right as number); case "<=": return (left as number) <= (right as number); - case "&&": - return (left as boolean) && (right as boolean); - case "||": - return (left as boolean) || (right as boolean); default: throw new ExpressionError(`Unknown operator: ${node.operator}`); } diff --git a/tests/interpreter.test.ts b/tests/interpreter.test.ts index dc8afd0..29486ec 100644 --- a/tests/interpreter.test.ts +++ b/tests/interpreter.test.ts @@ -4,11 +4,7 @@ import { parse } from "../src/parser"; import { tokenize } from "../src/tokenizer"; describe("Interpreter", () => { - async function evaluateExpression( - input: string, - context = {}, - functions = {}, - ) { + function evaluateExpression(input: string, context = {}, functions = {}) { const tokens = tokenize(input); const ast = parse(tokens); const interpreterState = createInterpreterState({}, functions); @@ -16,21 +12,21 @@ describe("Interpreter", () => { } describe("Literals", () => { - it("should evaluate number literals", async () => { - expect(await evaluateExpression("42")).toBe(42); + it("should evaluate number literals", () => { + expect(evaluateExpression("42")).toBe(42); }); - it("should evaluate string literals", async () => { - expect(await evaluateExpression('"hello"')).toBe("hello"); + it("should evaluate string literals", () => { + expect(evaluateExpression('"hello"')).toBe("hello"); }); - it("should evaluate boolean literals", async () => { - expect(await evaluateExpression("true")).toBe(true); - expect(await evaluateExpression("false")).toBe(false); + it("should evaluate boolean literals", () => { + expect(evaluateExpression("true")).toBe(true); + expect(evaluateExpression("false")).toBe(false); }); - it("should evaluate null", async () => { - expect(await evaluateExpression("null")).toBe(null); + it("should evaluate null", () => { + expect(evaluateExpression("null")).toBe(null); }); }); @@ -44,16 +40,16 @@ describe("Interpreter", () => { }, }; - it("should evaluate dot notation", async () => { - expect(await evaluateExpression("data.value", context)).toBe(42); + it("should evaluate dot notation", () => { + expect(evaluateExpression("data.value", context)).toBe(42); }); - it("should evaluate bracket notation", async () => { - expect(await evaluateExpression('data["value"]', context)).toBe(42); + it("should evaluate bracket notation", () => { + expect(evaluateExpression('data["value"]', context)).toBe(42); }); - it("should evaluate nested access", async () => { - expect(await evaluateExpression("data.nested.array[1]", context)).toBe(2); + it("should evaluate nested access", () => { + expect(evaluateExpression("data.nested.array[1]", context)).toBe(2); }); }); @@ -63,48 +59,46 @@ describe("Interpreter", () => { max: Math.max, }; - it("should evaluate function calls", async () => { - expect(await evaluateExpression("@sum(1, 2, 3)", {}, functions)).toBe(6); + it("should evaluate function calls", () => { + expect(evaluateExpression("@sum(1, 2, 3)", {}, functions)).toBe(6); }); - it("should evaluate nested expressions in arguments", async () => { + it("should evaluate nested expressions in arguments", () => { const context = { x: 1, y: 2 }; - expect( - await evaluateExpression("@max(x, y, 3)", context, functions), - ).toBe(3); + expect(evaluateExpression("@max(x, y, 3)", context, functions)).toBe(3); }); }); describe("Binary Expressions", () => { const context = { a: 5, b: 3 }; - it("should evaluate arithmetic operators", async () => { - expect(await evaluateExpression("a + b", context)).toBe(8); - expect(await evaluateExpression("a - b", context)).toBe(2); - expect(await evaluateExpression("a * b", context)).toBe(15); - expect(await evaluateExpression("a / b", context)).toBe(5 / 3); + it("should evaluate arithmetic operators", () => { + expect(evaluateExpression("a + b", context)).toBe(8); + expect(evaluateExpression("a - b", context)).toBe(2); + expect(evaluateExpression("a * b", context)).toBe(15); + expect(evaluateExpression("a / b", context)).toBe(5 / 3); }); - it("should evaluate comparison operators", async () => { - expect(await evaluateExpression("a > b", context)).toBe(true); - expect(await evaluateExpression("a === b", context)).toBe(false); + it("should evaluate comparison operators", () => { + expect(evaluateExpression("a > b", context)).toBe(true); + expect(evaluateExpression("a === b", context)).toBe(false); }); - it("should evaluate logical operators", async () => { - expect(await evaluateExpression("true && false")).toBe(false); - expect(await evaluateExpression("true || false")).toBe(true); + it("should evaluate logical operators", () => { + expect(evaluateExpression("true && false")).toBe(false); + expect(evaluateExpression("true || false")).toBe(true); }); }); describe("Conditional Expressions", () => { - it("should evaluate simple conditionals", async () => { - expect(await evaluateExpression("true ? 1 : 2")).toBe(1); - expect(await evaluateExpression("false ? 1 : 2")).toBe(2); + it("should evaluate simple conditionals", () => { + expect(evaluateExpression("true ? 1 : 2")).toBe(1); + expect(evaluateExpression("false ? 1 : 2")).toBe(2); }); - it("should evaluate nested conditionals", async () => { + it("should evaluate nested conditionals", () => { const input = "true ? false ? 1 : 2 : 3"; - expect(await evaluateExpression(input)).toBe(2); + expect(evaluateExpression(input)).toBe(2); }); }); @@ -120,32 +114,45 @@ describe("Interpreter", () => { sum: (arr: number[]) => arr.reduce((a, b) => a + b, 0), }; - it("should evaluate complex expressions", async () => { + it("should evaluate complex expressions", () => { const input = '@sum(data.values) > 5 ? data["status"] : "inactive"'; - expect(await evaluateExpression(input, context, functions)).toBe( - "active", - ); + expect(evaluateExpression(input, context, functions)).toBe("active"); }); }); describe("Error Handling", () => { - it("should throw for undefined variables", async () => { - await expect(evaluateExpression("unknownVar")).rejects.toThrow( + it("should throw for undefined variables", () => { + expect(() => evaluateExpression("unknownVar")).toThrow( "Undefined variable", ); }); - it("should throw for undefined functions", async () => { - await expect(evaluateExpression("@unknown()")).rejects.toThrow( + it("should throw for undefined functions", () => { + expect(() => evaluateExpression("@unknown()")).toThrow( "Undefined function", ); }); - it("should throw for null property access", async () => { + it("should throw for null property access", () => { const context = { data: null }; - await expect(evaluateExpression("data.value", context)).rejects.toThrow( + expect(() => evaluateExpression("data.value", context)).toThrow( "Cannot access property of null", ); }); }); + + describe("Logical Operators", () => { + it("should implement short-circuit evaluation for &&", () => { + expect(evaluateExpression("false && true")).toBe(false); + expect(evaluateExpression("true && true")).toBe(true); + expect(evaluateExpression("data && data.value", { data: null })).toBe( + null, + ); + }); + + it("should implement short-circuit evaluation for ||", () => { + expect(evaluateExpression("true || true")).toBe(true); + expect(evaluateExpression("false || true")).toBe(true); + }); + }); });