fix(format): keep the parentheses an expression's reading depends on; release 0.1.1

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 All notable changes to `@casys/constraint-solver` will be documented in this file.
 
+## [0.1.1] - 2026-10-25
+
+### Fixed
+
+- **`exprToString` keeps the parentheses the reading depends on.** A right operand at the same level as `-` or `/` lost them, so `thrust / (totalMass * 9.81) ≥ 2` was shown as `thrust / totalMass * 9.81 ≥ 2`, another expression than the one evaluated. The same held for `a - (b - c)`, a comparison inside a comparison, a unary minus over a compound operand (`-(a + b)` was shown as `-a + b`), and an operator outside the precedence table (`a ^ (b + c)`). A quantity right of `*` or `/` is now grouped, since mathjs reads `5 / 2 kg` as `(5 / 2) kg`: `5 / (2 kg)`. A negative right of `-` is grouped (`a - (-5)`), and so is a negative under a unary minus (`-(-5)`, formerly `--5`). Parentheses that change no value are still left out: `a - b - c`, `a * b * c`, `t ≥ -10`.
+
 ## [0.1.0] - 2026-07-30
 
 Initial release. Extracted from the constraint evaluator in `lib/sim`, with the evaluation core rewritten over [mathjs](https://mathjs.org) so that units participate in the comparison.

deno.json

@@ -1,6 +1,6 @@
 {
   "name": "@casys/constraint-solver",
-  "version": "0.1.0",
+  "version": "0.1.1",
   "exports": {
     ".": "./mod.ts"
   },

src/format.ts

@@ -29,13 +29,17 @@ const PRECEDENCE: Partial<Record<string, number>> = {
   "/": 5,
 };
 
+/** Operators whose right operand cannot shed parentheses at the same level. */
+const NON_ASSOCIATIVE = new Set<string>(["-", "/"]);
+
 /**
  * Render an expression as a readable string.
  *
  * Examples:
  * - `totalMass ≤ 5 kg`
  * - `operatingTemp ≥ -10 and operatingTemp ≤ 80`
  * - `(a + b) * 2`
+ * - `thrust / (totalMass * 9.81) ≥ 2`
  */
 export function exprToString(expr: ConstraintExpr): string {
   switch (expr.kind) {
@@ -45,26 +49,64 @@ export function exprToString(expr: ConstraintExpr): string {
       return expr.featurePath.join(".");
     case "binary": {
       const symbol = OP_SYMBOLS[expr.op] ?? expr.op;
-      const precedence = PRECEDENCE[expr.op] ?? 0;
-      return `${maybeParen(expr.left, precedence)} ${symbol} ${
-        maybeParen(expr.right, precedence)
+      return `${operand(expr.left, expr.op, "left")} ${symbol} ${
+        operand(expr.right, expr.op, "right")
       }`;
     }
-    case "unary":
-      return expr.op === "not"
-        ? `not(${exprToString(expr.operand)})`
-        : `-${exprToString(expr.operand)}`;
+    case "unary": {
+      if (expr.op === "not") return `not(${exprToString(expr.operand)})`;
+      // A compound or negative operand keeps its parentheses: -(a + b),
+      // -(-a), -(-5).
+      const rendered = exprToString(expr.operand);
+      return expr.operand.kind === "binary" || expr.operand.kind === "unary" ||
+          isNegative(expr.operand)
+        ? `-(${rendered})`
+        : `-${rendered}`;
+    }
     case "call":
       return `${expr.name}(${expr.args.map(exprToString).join(", ")})`;
   }
 }
 
-/** Parenthesise a sub-expression that binds more loosely than its parent. */
-function maybeParen(expr: ConstraintExpr, parentPrecedence: number): string {
+/** A negation, or a negative number. */
+function isNegative(expr: ConstraintExpr): boolean {
+  return (expr.kind === "unary" && expr.op === "-") ||
+    (expr.kind === "literal" && expr.value < 0);
+}
+
+/**
+ * Render an operand of `parentOp`, parenthesised wherever the bare text would
+ * read as another expression or another value:
+ * - it binds more loosely than its parent;
+ * - it sits right of `-` or `/` at the same level (`a - (b - c)`,
+ *   `a / (b * c)`);
+ * - it is a comparison inside a comparison;
+ * - it is a negative right of `-` (`a - (-b)`, `a - (-5)`);
+ * - it is a quantity right of `*` or `/` (`5 / (2 kg)` is not `(5 / 2) kg`);
+ * - its parent is an operator outside the precedence table.
+ */
+function operand(
+  expr: ConstraintExpr,
+  parentOp: BinaryOp,
+  side: "left" | "right",
+): string {
   const rendered = exprToString(expr);
-  if (expr.kind === "binary") {
-    const own = PRECEDENCE[expr.op] ?? 0;
-    if (own < parentPrecedence) return `(${rendered})`;
+  const grouped = `(${rendered})`;
+  const parent = PRECEDENCE[parentOp];
+  if (parent === undefined) {
+    return expr.kind === "binary" ? grouped : rendered;
+  }
+  if (
+    expr.kind === "literal" && expr.unit && side === "right" &&
+    (parentOp === "*" || parentOp === "/")
+  ) return grouped;
+  if (isNegative(expr)) {
+    return side === "right" && parentOp === "-" ? grouped : rendered;
   }
-  return rendered;
+  if (expr.kind !== "binary") return rendered;
+  const own = PRECEDENCE[expr.op] ?? 0;
+  const sameLevelNeedsParens = own === parent &&
+    (parent === PRECEDENCE["<"] ||
+      (side === "right" && NON_ASSOCIATIVE.has(parentOp)));
+  return own < parent || sameLevelNeedsParens ? grouped : rendered;
 }

tests/format_test.ts

@@ -0,0 +1,164 @@
+/**
+ * Formatting tests
+ *
+ * The rendered expression is what an engineer reads next to the computed
+ * value: it must denote the same expression, with no parentheses dropped.
+ */
+
+import { assertEquals } from "@std/assert";
+import { exprToString } from "../src/format.ts";
+import type { BinaryOp, ConstraintExpr } from "../src/types.ts";
+
+const ref = (name: string): ConstraintExpr => ({
+  kind: "ref",
+  featurePath: name.split("."),
+});
+const num = (value: number, unit?: string): ConstraintExpr => ({
+  kind: "literal",
+  value,
+  unit,
+});
+const bin = (
+  left: ConstraintExpr,
+  op: BinaryOp,
+  right: ConstraintExpr,
+): ConstraintExpr => ({ kind: "binary", op, left, right });
+const neg = (operand: ConstraintExpr): ConstraintExpr => ({
+  kind: "unary",
+  op: "-",
+  operand,
+});
+
+Deno.test("format — a right operand under / or - keeps its parentheses", () => {
+  // thrust / (totalMass * 9.81) >= 2 is not (thrust / totalMass) * 9.81 >= 2.
+  assertEquals(
+    exprToString(
+      bin(
+        bin(ref("thrust"), "/", bin(ref("totalMass"), "*", num(9.81))),
+        ">=",
+        num(2),
+      ),
+    ),
+    "thrust / (totalMass * 9.81) ≥ 2",
+  );
+  assertEquals(
+    exprToString(bin(ref("a"), "-", bin(ref("b"), "-", ref("c")))),
+    "a - (b - c)",
+  );
+  assertEquals(
+    exprToString(bin(ref("a"), "-", bin(ref("b"), "+", ref("c")))),
+    "a - (b + c)",
+  );
+  assertEquals(
+    exprToString(bin(ref("a"), "/", bin(ref("b"), "/", ref("c")))),
+    "a / (b / c)",
+  );
+});
+
+Deno.test("format — leaves out parentheses that change no value", () => {
+  assertEquals(
+    exprToString(bin(bin(ref("a"), "-", ref("b")), "-", ref("c"))),
+    "a - b - c",
+  );
+  assertEquals(
+    exprToString(bin(ref("a"), "*", bin(ref("b"), "*", ref("c")))),
+    "a * b * c",
+  );
+  assertEquals(
+    exprToString(bin(ref("a"), "+", bin(ref("b"), "-", ref("c")))),
+    "a + b - c",
+  );
+  assertEquals(
+    exprToString(bin(bin(ref("a"), "+", ref("b")), "*", num(2))),
+    "(a + b) * 2",
+  );
+  assertEquals(
+    exprToString(
+      bin(bin(ref("t"), ">=", num(-10)), "and", bin(ref("t"), "<=", num(80))),
+    ),
+    "t ≥ -10 and t ≤ 80",
+  );
+  assertEquals(
+    exprToString(bin(ref("totalMass"), "<=", num(5, "kg"))),
+    "totalMass ≤ 5 kg",
+  );
+});
+
+Deno.test("format — comparisons nested in comparisons are parenthesised", () => {
+  assertEquals(
+    exprToString(
+      bin(bin(ref("a"), "<", ref("b")), "==", bin(ref("c"), "<", ref("d"))),
+    ),
+    "(a < b) == (c < d)",
+  );
+});
+
+Deno.test("format — unary minus keeps the parentheses of a compound operand", () => {
+  assertEquals(exprToString(neg(bin(ref("a"), "+", ref("b")))), "-(a + b)");
+  assertEquals(exprToString(neg(ref("a"))), "-a");
+  assertEquals(exprToString(neg(neg(ref("a")))), "-(-a)");
+  assertEquals(
+    exprToString(bin(ref("a"), "-", neg(ref("b")))),
+    "a - (-b)",
+  );
+});
+
+Deno.test("format — a negative is grouped only right of a minus", () => {
+  assertEquals(exprToString(bin(ref("t"), ">=", neg(num(10)))), "t ≥ -10");
+  assertEquals(exprToString(bin(ref("a"), "+", neg(ref("b")))), "a + -b");
+  assertEquals(exprToString(bin(ref("a"), "*", neg(ref("b")))), "a * -b");
+  assertEquals(exprToString(bin(neg(ref("a")), "*", ref("b"))), "-a * b");
+  assertEquals(exprToString(bin(ref("a"), "-", num(-5))), "a - (-5)");
+  assertEquals(exprToString(neg(num(-5))), "-(-5)");
+  assertEquals(exprToString(neg(num(-5, "kg"))), "-(-5 kg)");
+});
+
+Deno.test("format — a quantity right of * or / is grouped", () => {
+  // `5 / 2 kg` reads (5 / 2) kg; `d / 5 km/h` reads (d / 5 km) / h.
+  assertEquals(exprToString(bin(num(5), "/", num(2, "kg"))), "5 / (2 kg)");
+  assertEquals(
+    exprToString(bin(ref("d"), "/", num(5, "km/h"))),
+    "d / (5 km/h)",
+  );
+  assertEquals(exprToString(bin(num(2), "*", num(5, "kg"))), "2 * (5 kg)");
+  assertEquals(exprToString(bin(num(5, "kg"), "/", num(2))), "5 kg / 2");
+  assertEquals(exprToString(bin(ref("m"), "<=", num(5, "kg"))), "m ≤ 5 kg");
+});
+
+Deno.test("format — logic, calls and operators outside the table", () => {
+  assertEquals(
+    exprToString(bin(bin(ref("a"), "or", ref("b")), "and", ref("c"))),
+    "(a or b) and c",
+  );
+  assertEquals(
+    exprToString(bin(ref("a"), "or", bin(ref("b"), "and", ref("c")))),
+    "a or b and c",
+  );
+  assertEquals(
+    exprToString({
+      kind: "unary",
+      op: "not",
+      operand: bin(ref("a"), "<", ref("b")),
+    }),
+    "not(a < b)",
+  );
+  assertEquals(
+    exprToString(
+      bin(
+        {
+          kind: "call",
+          name: "max",
+          args: [bin(ref("a"), "+", ref("b")), num(2)],
+        },
+        "*",
+        num(3),
+      ),
+    ),
+    "max(a + b, 2) * 3",
+  );
+  // An operator from untyped input has no known precedence: group operands.
+  assertEquals(
+    exprToString(bin(ref("a"), "^" as BinaryOp, bin(ref("b"), "+", ref("c")))),
+    "a ^ (b + c)",
+  );
+});