diff --git a/CHANGELOG.md b/CHANGELOG.md index 620344f..0dbd9e2 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/deno.json b/deno.json index 64add93..ef93413 100644 --- a/deno.json +++ b/deno.json @@ -1,6 +1,6 @@ { "name": "@casys/constraint-solver", - "version": "0.1.0", + "version": "0.1.1", "exports": { ".": "./mod.ts" }, diff --git a/src/format.ts b/src/format.ts index 6383107..aad6077 100644 --- a/src/format.ts +++ b/src/format.ts @@ -29,6 +29,9 @@ const PRECEDENCE: Partial> = { "/": 5, }; +/** Operators whose right operand cannot shed parentheses at the same level. */ +const NON_ASSOCIATIVE = new Set(["-", "/"]); + /** * Render an expression as a readable string. * @@ -36,6 +39,7 @@ const PRECEDENCE: Partial> = { * - `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; } diff --git a/tests/format_test.ts b/tests/format_test.ts new file mode 100644 index 0000000..0033fee --- /dev/null +++ b/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)", + ); +});