From f3659731c4abc9039fb9166b725f4b26e1e2c282 Mon Sep 17 00:00:00 2001 From: Thomas Lamant Date: Sat, 24 Oct 2026 22:57:15 +0200 Subject: [PATCH] feat(parse)!: throw only on fatal errors BREAKING CHANGE: errors are now reported without exiting the parse() function if they don't compromise the rest of the parsing. See list of error codes for details. This apply to all parse() function i.e. Recipe, Pantry, ProductCatalog and CategoryConfig --- docs/e/index.md | 11 ++++--- docs/guide-diagnostics.md | 35 +++++++++++---------- src/classes/category_config.ts | 4 +-- src/classes/pantry.ts | 4 +-- src/classes/product_catalog.ts | 4 +-- src/classes/recipe.ts | 4 +-- test/aisle_config.test.ts | 9 ------ test/config_diagnostics.test.ts | 42 +++++++++++++++++++------ test/diagnostics.test.ts | 45 ++++++++++++++------------- test/pantry.test.ts | 54 ++++++++++++++------------------- 10 files changed, 113 insertions(+), 99 deletions(-) diff --git a/docs/e/index.md b/docs/e/index.md index bc41ca58..5eb481a5 100644 --- a/docs/e/index.md +++ b/docs/e/index.md @@ -9,11 +9,14 @@ import { withBase } from "vitepress"; # Diagnostic codes -Every error thrown by, and every diagnostic returned by, `@tmlmt/cooklang-parser` has a stable code of the form `COOKLANG_`: +Every error thrown by, and every diagnostic returned by, `@tmlmt/cooklang-parser` has a stable code of the form `COOKLANG_`: -- `S` is the severity: `E` for an error, `W` for a warning -- `D` is the domain: `1` recipe syntax, `2` references, `3` metadata, `4` quantities and units, `5` shopping list, `6` shopping cart and product catalog, `7` pantry and category configuration, `9` fatal errors -- `NNN` is a sequence number +- `L` is the letter: `E` for an error, `W` for a warning, or `I` for an internal/library bug. +- `T` is a tier digit used only for `E` codes: `0` for recoverable errors, which are collected while parsing continues and do not make the call throw, so the partial result stays available. `9` for fatal errors, where it is not safe to continue and the call throws. It is always `0` for `W` and `I` codes. +- `D` is the domain: `0` generic/internal, `1` recipe syntax, `2` references, `3` metadata, `4` quantities and units, `5` shopping list, `6` shopping cart and product catalog, or `7` pantry and category configuration. +- `NN` is a two-digit sequence number. + +Warnings never make a call fail. Internal errors indicate a library bug and behave like fatal errors. Fatal errors stop the affected work when detected and make the call throw a `DiagnosticError`, which carries the diagnostics collected up to that point. See the [parse diagnostics guide](/guide-diagnostics) to learn how to consume them. diff --git a/docs/guide-diagnostics.md b/docs/guide-diagnostics.md index a3b68f8f..e4541996 100644 --- a/docs/guide-diagnostics.md +++ b/docs/guide-diagnostics.md @@ -1,12 +1,12 @@ # Parse diagnostics `Recipe.parse()` (and the `Recipe` constructor) collects known recipe problems -as diagnostics. Warnings are available on `recipe.diagnostics`. Ordinarily, if -at least one diagnostic has error severity, a single `DiagnosticError` is -thrown once the whole source has been processed; the recipe instance keeps the -partial result. A zero-denominator fraction instead stops parsing immediately -and throws `ParseStoppedError`, a `DiagnosticError` subclass that identifies -the premature stop. +as diagnostics. All of them, including non-fatal errors, are available on +`recipe.diagnostics`, and the recipe instance holds the partial result. Only +fatal errors (`COOKLANG_E9…`) make `parse()` throw a `DiagnosticError`. A +zero-denominator fraction stops parsing immediately and throws +`ParseStoppedError`, a `DiagnosticError` subclass that identifies the +premature stop. ```typescript import { @@ -19,6 +19,8 @@ import { const recipe = new Recipe(); try { recipe.parse(source); + // Reaching this point means no fatal error occurred. Non-fatal errors and + // warnings may still be present in recipe.diagnostics. } catch (error) { if (error instanceof DiagnosticError) { if (error instanceof ParseStoppedError) { @@ -30,7 +32,7 @@ try { } } -// Non-fatal diagnostics (warnings) are always available +// Non-fatal errors and warnings are always available console.log(recipe.diagnostics); ``` @@ -40,17 +42,17 @@ columns are 1-based. Offsets are 0-based UTF-16 code-unit indices into `DiagnosticError.source`, which is exactly the input passed to `parse()`. `CategoryConfig`, `Pantry` and `ProductCatalog` behave identically: `parse()` -and the constructors throw `DiagnosticError`, and expose `diagnostics` and -`source` on the instance. +and the constructors throw `DiagnosticError` only for fatal errors, and expose +all diagnostics and the `source` on the instance. ## Diagnostic codes Codes follow the pattern `COOKLANG_`: `L` is the letter (`E` for error, `W` for warning, `I` for internal/library bug); `T` is a tier digit -used only for `E` codes (`0` = non-fatal — caught and collected alongside -other diagnostics while parsing continues, so you get a complete picture of -every problem found; `9` = fatal — the result cannot be considered usable -once this is found). `T` is always `0` for `W` and `I` codes: warnings never +used only for `E` codes (`0` = recoverable — collected alongside other +diagnostics while parsing continues, and the call returns the partial result +instead of throwing; `9` = fatal — it is not safe to continue, so the call +throws `DiagnosticError`). `T` is always `0` for `W` and `I` codes: warnings never make the call fail, and internal errors always behave like fatal ones but indicate a library bug rather than a problem with your input. `D` is the domain digit (`0` generic/internal, `1` syntax, `2` references, `3` metadata, @@ -77,9 +79,10 @@ Parse-time diagnostics, reported by `Recipe.parse()`: | [`COOKLANG_E9402`](/e/cooklang_e9402) | error (fatal) | A fraction has a zero denominator; parsing stops immediately. | | [`COOKLANG_W0401`](/e/cooklang_w0401) | warning | A text quantity could not be added to a numeric quantity and was kept separate. | -Warnings never make `parse()` throw. Non-fatal errors (`E0…`) are collected -alongside everything else so that the whole recipe gets reported in one -pass; fatal errors (`E9…`) abort the call immediately once encountered. The +Warnings and non-fatal errors (`E0…`) never make `parse()` throw: they are +collected so that the whole recipe gets reported in one pass, and you can +display them next to the partially parsed result. Fatal errors (`E9…`) make +the call throw. The catalog and the severity of a code are available programmatically through `diagnosticCatalog` and `severityOfCode()`. diff --git a/src/classes/category_config.ts b/src/classes/category_config.ts index 670840c8..1f8bca25 100644 --- a/src/classes/category_config.ts +++ b/src/classes/category_config.ts @@ -68,12 +68,12 @@ export class CategoryConfig { * All problems are collected in {@link CategoryConfig.diagnostics}. * * @param config - The category configuration to parse. - * @throws {@link DiagnosticError} if error-severity diagnostics exist. + * @throws {@link DiagnosticError} if a fatal (`COOKLANG_E9…`) diagnostic exists. */ parse(config: string): void { this.source = config; this.diagnostics = this._parseAndCollect(config).map((e) => e.diagnostic); - if (this.diagnostics.some((d) => d.severity === "error")) { + if (this.diagnostics.some((d) => d.code.startsWith("COOKLANG_E9"))) { throw new DiagnosticError(this.diagnostics, config); } } diff --git a/src/classes/pantry.ts b/src/classes/pantry.ts index 32cd7a7b..52290c77 100644 --- a/src/classes/pantry.ts +++ b/src/classes/pantry.ts @@ -100,14 +100,14 @@ export class Pantry { * * @param tomlContent - The TOML string to parse. * @returns The parsed pantry items. - * @throws {@link DiagnosticError} if error-severity diagnostics exist. + * @throws {@link DiagnosticError} if a fatal (`COOKLANG_E9…`) diagnostic exists. */ parse(tomlContent: string): PantryItem[] { this.source = tomlContent; this.diagnostics = this._parseAndCollect(tomlContent).map( (e) => e.diagnostic, ); - if (this.diagnostics.some((d) => d.severity === "error")) { + if (this.diagnostics.some((d) => d.code.startsWith("COOKLANG_E9"))) { throw new DiagnosticError(this.diagnostics, tomlContent); } return this.items; diff --git a/src/classes/product_catalog.ts b/src/classes/product_catalog.ts index 9f89f374..e42fa081 100644 --- a/src/classes/product_catalog.ts +++ b/src/classes/product_catalog.ts @@ -79,13 +79,13 @@ export class ProductCatalog { * * @param tomlContent - The TOML string to parse. * @returns The parsed products. - * @throws {@link DiagnosticError} if error-severity diagnostics exist. + * @throws {@link DiagnosticError} if a fatal (`COOKLANG_E9…`) diagnostic exists. */ public parse(tomlContent: string): ProductOption[] { const error = this.validate(tomlContent); this.source = tomlContent; this.diagnostics = error ? [error.diagnostic] : []; - if (this.diagnostics.some((d) => d.severity === "error")) { + if (this.diagnostics.some((d) => d.code.startsWith("COOKLANG_E9"))) { throw new DiagnosticError(this.diagnostics, tomlContent); } return this.products; diff --git a/src/classes/recipe.ts b/src/classes/recipe.ts index f3d06c2d..0a1c04c5 100644 --- a/src/classes/recipe.ts +++ b/src/classes/recipe.ts @@ -1616,7 +1616,7 @@ export class Recipe { * Known problems are collected in {@link Recipe.diagnostics}. * * @param content - The recipe content to parse. - * @throws {@link DiagnosticError} if error-severity diagnostics exist. + * @throws {@link DiagnosticError} if a fatal (`COOKLANG_E9…`) diagnostic exists. * @throws {@link ParseStoppedError} if parsing must stop immediately. */ parse(content: string): void { @@ -2080,7 +2080,7 @@ export class Recipe { (a, b) => (a.span?.start.offset ?? 0) - (b.span?.start.offset ?? 0), ); - if (this.diagnostics.some((d) => d.severity === "error")) { + if (this.diagnostics.some((d) => d.code.startsWith("COOKLANG_E9"))) { throw new DiagnosticError(this.diagnostics, this._source); } } diff --git a/test/aisle_config.test.ts b/test/aisle_config.test.ts index f8a751e4..6103a039 100644 --- a/test/aisle_config.test.ts +++ b/test/aisle_config.test.ts @@ -1,6 +1,5 @@ import { describe, it, expect } from "vitest"; import { CategoryConfig } from "../src/classes/category_config"; -import { DiagnosticError } from "../src/errors"; describe("parse_category_config", () => { it("parses a simple config", () => { @@ -108,14 +107,6 @@ describe("parse_category_config", () => { }); }); - it("throws when duplicate categories are found", () => { - const config = ` - [produce] - [produce] - `; - expect(() => new CategoryConfig(config)).toThrow(DiagnosticError); - }); - it("warns (without throwing) when duplicate ingredients are found", () => { const config = ` [produce] diff --git a/test/config_diagnostics.test.ts b/test/config_diagnostics.test.ts index ced4480c..ef8bb282 100644 --- a/test/config_diagnostics.test.ts +++ b/test/config_diagnostics.test.ts @@ -5,6 +5,7 @@ import { ProductCatalog, DiagnosticError, } from "../src/index"; +import { createDiagnostic } from "../src/diagnostics"; import { parseCollect } from "./mocks/parse_collect"; describe("CategoryConfig.parse()", () => { @@ -32,14 +33,32 @@ describe("CategoryConfig.parse()", () => { expect(r.output.categories.map((c) => c.name)).toEqual(["A", "B"]); }); - it("constructor throws like parse() for fatal diagnostics", () => { - expect(() => new CategoryConfig("[A]\n[A]")).toThrow(DiagnosticError); + it("constructor does not throw for non-fatal errors", () => { + const config = "[A]\n[A]"; + let categoryConfig: CategoryConfig | undefined; + expect(() => (categoryConfig = new CategoryConfig(config))).not.toThrow(); + expect(categoryConfig!.source).toBe(config); + expect(categoryConfig!.diagnostics).toHaveDiagnostic("COOKLANG_E0702"); }); it("constructor does not throw for warning-only diagnostics", () => { expect(() => new CategoryConfig("a")).not.toThrow(); expect(() => new CategoryConfig("[A]\nx\ny|x")).not.toThrow(); }); + + it("throws if a fatal diagnostic is collected", () => { + const config = new CategoryConfig(); + Reflect.set(config, "_parseAndCollect", () => [ + { + diagnostic: createDiagnostic("COOKLANG_E9701", { + detail: "synthetic fatal diagnostic", + }), + }, + ]); + + expect(() => config.parse("input")).toThrow(DiagnosticError); + expect(config.diagnostics).toHaveDiagnostic("COOKLANG_E9701"); + }); }); describe("Pantry.parse()", () => { @@ -56,13 +75,18 @@ describe("Pantry.parse()", () => { }); it("omits items with invalid dates and keeps the others", () => { - const toml = `[fridge]\nmilk = { quantity = "1%l", expire = "not-a-date" }\neggs = "6"`; - const r = parseCollect( - new Pantry(undefined, { dateFormat: "DD.MM.YYYY" }), - toml, - ); - expect(r.diagnostics.map((d) => d.code)).toEqual(["COOKLANG_E0701"]); - expect(r.output.items.map((i) => i.name)).toEqual(["eggs"]); + const toml = + `[fridge]\nmilk = { quantity = "1%l", expire = "not-a-date" }\n` + + `cream = { bought = "also-not-a-date" }\neggs = "6"`; + const pantry = new Pantry(undefined, { dateFormat: "DD.MM.YYYY" }); + + expect(() => pantry.parse(toml)).not.toThrow(); + expect(pantry.source).toBe(toml); + expect(pantry.diagnostics.map((d) => d.code)).toEqual([ + "COOKLANG_E0701", + "COOKLANG_E0701", + ]); + expect(pantry.items.map((i) => i.name)).toEqual(["eggs"]); }); it("constructor throws like parse()", () => { diff --git a/test/diagnostics.test.ts b/test/diagnostics.test.ts index 94e9b4ed..776f4010 100644 --- a/test/diagnostics.test.ts +++ b/test/diagnostics.test.ts @@ -303,37 +303,40 @@ describe("Recipe.parse() diagnostics", () => { }); describe("Recipe.parse() throwing", () => { - it("throws DiagnosticError when there are error-severity diagnostics", () => { - expect(() => new Recipe().parse("Add @flour{%two}")).toThrow( - DiagnosticError, - ); - }); - - it("DiagnosticError carries diagnostics and source", () => { + it("throws DiagnosticError when there is a fatal diagnostic", () => { let thrown: DiagnosticError | undefined; try { - new Recipe().parse("Add @flour{%two}"); + new Recipe().parse("Add @flour{1/0}"); } catch (e) { if (e instanceof DiagnosticError) thrown = e; } expect(thrown).toBeDefined(); - expect(thrown!.diagnostics).toHaveLength(1); - expect(thrown!.diagnostics[0]!.code).toBe("COOKLANG_E0101"); - expect(thrown!.source).toBe("Add @flour{%two}"); + expect(thrown!.diagnostics).toHaveDiagnostic("COOKLANG_E9402"); + expect(thrown!.source).toBe("Add @flour{1/0}"); }); - it("does not throw for a clean recipe", () => { - expect(() => new Recipe().parse("Add @flour{100%g}")).not.toThrow(); + it("does not throw for non-fatal errors and keeps the partial result", () => { + const recipe = new Recipe(); + expect(() => recipe.parse("Add @flour{%two} and @salt")).not.toThrow(); + expect(recipe.diagnostics).toHaveDiagnostic("COOKLANG_E0101"); + expect(recipe.ingredients.map((i) => i.name)).toEqual(["flour", "salt"]); }); - it("error message summarizes error count", () => { - let thrown: DiagnosticError | undefined; - try { - new Recipe().parse("@&a and @&b"); - } catch (e) { - if (e instanceof DiagnosticError) thrown = e; - } - expect(thrown!.message).toContain("2 errors"); + it("DiagnosticError carries diagnostics and source", () => {}); + + it("throws at the end of parsing when a fatal diagnostic was collected", () => { + const recipe = new Recipe(); + Reflect.set(recipe, "_populateIngredientQuantities", () => { + recipe.diagnostics.push(createDiagnostic("COOKLANG_E9402", {})); + }); + + expect(() => recipe.parse("Add @flour{%two}")).toThrow(DiagnosticError); + expect(recipe.diagnostics).toHaveDiagnostic("COOKLANG_E0101"); + expect(recipe.diagnostics).toHaveDiagnostic("COOKLANG_E9402"); + }); + + it("does not throw for a clean recipe", () => { + expect(() => new Recipe().parse("Add @flour{100%g}")).not.toThrow(); }); it("DiagnosticError snapshots all diagnostics but counts only errors", () => { diff --git a/test/pantry.test.ts b/test/pantry.test.ts index 7af8b818..3f2bedf1 100644 --- a/test/pantry.test.ts +++ b/test/pantry.test.ts @@ -1,7 +1,6 @@ import { describe, it, expect } from "vitest"; import { Pantry } from "../src/classes/pantry"; import { CategoryConfig } from "../src/classes/category_config"; -import { DiagnosticError } from "../src/errors"; import type { PantryItem, FixedValue } from "../src/types"; const simplePantryToml = ` @@ -177,37 +176,28 @@ describe("Pantry", () => { expect(pantry.items[0]!.expire).toEqual(new Date(2025, 5, 15)); }); - it("should throw on invalid date input with explicit format", () => { - expect( - () => - new Pantry(`[fridge]\nmilk = { expire = "2025-13-01" }`, { - dateFormat: "YYYY-MM-DD", - }), - ).toThrow(DiagnosticError); - try { - new Pantry(`[fridge]\nmilk = { expire = "2025-13-01" }`, { - dateFormat: "YYYY-MM-DD", - }); - } catch (error) { - expect(error).toBeInstanceOf(DiagnosticError); - expect((error as DiagnosticError).diagnostics[0]!.message).toContain( - "Invalid date", - ); - } - }); - - it("should throw on non-numeric date parts", () => { - expect( - () => new Pantry(`[fridge]\nmilk = { expire = "abc.06.2025" }`), - ).toThrow(DiagnosticError); - try { - new Pantry(`[fridge]\nmilk = { expire = "abc.06.2025" }`); - } catch (error) { - expect(error).toBeInstanceOf(DiagnosticError); - expect((error as DiagnosticError).diagnostics[0]!.message).toContain( - "non-numeric", - ); - } + it("should report invalid date input with explicit format", () => { + const pantry = new Pantry(undefined, { dateFormat: "YYYY-MM-DD" }); + expect(() => + pantry.parse(`[fridge]\nmilk = { expire = "2025-13-01" }`), + ).not.toThrow(); + expect(pantry.diagnostics.map((diagnostic) => diagnostic.code)).toEqual([ + "COOKLANG_E0701", + ]); + expect(pantry.diagnostics[0]!.message).toContain("Invalid date"); + expect(pantry.items).toEqual([]); + }); + + it("should report non-numeric date parts", () => { + const pantry = new Pantry(); + expect(() => + pantry.parse(`[fridge]\nmilk = { expire = "abc.06.2025" }`), + ).not.toThrow(); + expect(pantry.diagnostics.map((diagnostic) => diagnostic.code)).toEqual([ + "COOKLANG_E0701", + ]); + expect(pantry.diagnostics[0]!.message).toContain("non-numeric"); + expect(pantry.items).toEqual([]); }); });