fix: implement short-circuit evaluation for logical operators in interpreter

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}`);
     }

tests/interpreter.test.ts

@@ -4,33 +4,29 @@ 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);
     return evaluateAst(ast, interpreterState, context);
   }
 
   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);
+    });
+  });
 });