diff --git a/docs/e/codes/COOKLANG_W0201.md b/docs/e/codes/COOKLANG_W0201.md new file mode 100644 index 00000000..82f8b06e --- /dev/null +++ b/docs/e/codes/COOKLANG_W0201.md @@ -0,0 +1,25 @@ +--- +code: COOKLANG_W0201 +level: warning +title: Duplicate ingredient without reference +--- + +## What this error means + +An ingredient with the same name, flags and preparation was already defined earlier in the recipe, but it was added again without the reference symbol `&`. A new, separate entry was added instead of updating the existing one. + +## Common causes + +- The same ingredient is listed twice with identical flags and preparation, without noticing an earlier definition +- The `&` was forgotten when intending to add more of an existing ingredient + +## How to fix it + +```text +-- Wrong +Add @?salt{1%tsp}(ground), then add @?salt{2%tsp}(ground). +-- Correct +Add @?salt{1%tsp}(ground), then add @&?salt{2%tsp}(ground). +``` + +Or keep the new entry if two separate occurrences are intentional. diff --git a/docs/guide-diagnostics.md b/docs/guide-diagnostics.md index e4541996..f17382bf 100644 --- a/docs/guide-diagnostics.md +++ b/docs/guide-diagnostics.md @@ -54,7 +54,10 @@ 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 +indicate a library bug rather than a problem with your input. The tier +describes whether the call succeeds, not necessarily when parsing stops: the +parser stops immediately when it cannot safely continue, and may otherwise +finish collecting diagnostics before throwing. `D` is the domain digit (`0` generic/internal, `1` syntax, `2` references, `3` metadata, `4` quantities and units, `5` shopping list, `6` cart and product catalog, `7` pantry and category configuration) and `NN` a 2-digit sequence number. @@ -72,6 +75,7 @@ Parse-time diagnostics, reported by `Recipe.parse()`: | [`COOKLANG_E0201`](/e/cooklang_e0201) | error | An ingredient reference has no earlier definition. | | [`COOKLANG_E0202`](/e/cooklang_e0202) | error | A cookware reference has no earlier definition. | | [`COOKLANG_E0203`](/e/cooklang_e0203) | error | A reference attempts to change a flag from its definition. | +| [`COOKLANG_W0201`](/e/cooklang_w0201) | warning | An unreferenced ingredient duplicates an earlier ingredient with the same flags and preparation. | | [`COOKLANG_E0301`](/e/cooklang_e0301) | error | Metadata indentation contains a tab. | | [`COOKLANG_E0302`](/e/cooklang_e0302) | error | Metadata indentation levels are inconsistent. | | [`COOKLANG_W0301`](/e/cooklang_w0301) | warning | `servings` or `serves` is not a number. | diff --git a/src/classes/recipe.ts b/src/classes/recipe.ts index 0a1c04c5..ec6e9be5 100644 --- a/src/classes/recipe.ts +++ b/src/classes/recipe.ts @@ -577,6 +577,14 @@ export class Recipe { this.ingredients, newIngredient, reference, + this.diagnostics, + makeSpan( + this._lineOffsets, + lineIdx, + matchStart, + ingredientMatchString.length, + ), + linkedVariants, ); } catch (e) { this._collectParseError( @@ -748,6 +756,14 @@ export class Recipe { this.ingredients, newIngredient, reference, + this.diagnostics, + makeSpan( + this._lineOffsets, + lineIdx, + matchStart, + ingredientMatchString.length, + ), + linkedVariants, ); } catch (e) { this._collectParseError( @@ -1988,6 +2004,13 @@ export class Recipe { this.cookware, newCookware, reference, + this.diagnostics, + makeSpan( + this._lineOffsets, + lineIdx, + sourceColumn, + sourceLength, + ), ); // Adding the item itself in the preparation const newItem: CookwareItem = { diff --git a/src/diagnostics.ts b/src/diagnostics.ts index 5cc3b82a..8e7cd418 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -70,6 +70,11 @@ export const diagnosticCatalog = defineDiagnostics({ fix: `Remove the reference ('&') to create a new entry, or add the flag to the original definition.`, }, // -- Domain 3: metadata -------------------------------------------------- + COOKLANG_W0201: { + why: (p: { name: string }) => + `Ingredient "${p.name}" with the same flags and preparation is already present, but without the reference symbol "&" a new ingredient entry was added.`, + fix: `Add "&" before the ingredient name to reference the existing entry, or keep the new entry if it is intentional.`, + }, COOKLANG_E0301: { why: `Tabs are not allowed for indentation in metadata blocks.`, fix: `Replace tab characters with spaces.`, diff --git a/src/utils/parser_helpers.ts b/src/utils/parser_helpers.ts index 3cc4d4e2..138ec279 100644 --- a/src/utils/parser_helpers.ts +++ b/src/utils/parser_helpers.ts @@ -17,6 +17,8 @@ import type { ArbitraryScalable, FixedNumericValue, Yield, + Diagnostic, + DiagnosticSourceSpan, } from "../types"; import { metadataRegex, @@ -46,6 +48,7 @@ import { } from "../errors"; import { parseTimeToMinutes } from "./time"; import { normalizeInputString, normalizeKey } from "./normalization"; +import { createDiagnostic } from "../diagnostics"; /** * Pushes a pending note to the section content if it has items. @@ -96,12 +99,18 @@ export function flushPendingItems( return false; } +/** Variants under which each ingredient was declared (absent = all variants). */ +const declaredVariants = new WeakMap(); + /** * Finds an ingredient in the list (case-insensitively) and updates it, or adds it if not present. * This function mutates the `ingredients` array. * @param ingredients - The list of ingredients. * @param newIngredient - The ingredient to find or add. * @param isReference - Whether this is a reference ingredient (`&` modifier). + * @param diagnostics - The diagnostics list to push warnings onto. + * @param span - The source span of the new ingredient, used for diagnostics. + * @param linkedVariants - Variants the new ingredient is declared under; declarations in disjoint variants are not duplicates. * @returns The index of the ingredient in the list. * @returns An object containing the index of the ingredient and its quantity part in the list. */ @@ -109,6 +118,9 @@ export function findAndUpsertIngredient( ingredients: Ingredient[], newIngredient: Ingredient, isReference: boolean, + diagnostics: Diagnostic[], + span?: DiagnosticSourceSpan, + linkedVariants?: string[], ): number { const { name } = newIngredient; @@ -156,14 +168,47 @@ export function findAndUpsertIngredient( return indexFind; } + // Checking for an existing ingredient with the same flags and preparation + const indexFind = ingredients.findIndex((existing) => { + const existingFlags = existing.flags ?? []; + const ingredientFlags = newIngredient.flags ?? []; + + // Declarations restricted to disjoint variants are not duplicates + const existingVariants = declaredVariants.get(existing); + if ( + existingVariants && + linkedVariants && + !existingVariants.some((v) => linkedVariants.includes(v)) + ) { + return false; + } + + return ( + normalizeKey(existing.name) === normalizeKey(newIngredient.name) && + existing.preparation === newIngredient.preparation && + existingFlags.length === ingredientFlags.length && + ingredientFlags.every((flag) => existingFlags.includes(flag)) + ); + }); + + if (indexFind !== -1) { + diagnostics.push( + createDiagnostic("COOKLANG_W0201", { name: newIngredient.name }, span), + ); + } + // Not a reference, so add as a new ingredient. - return ingredients.push(newIngredient) - 1; + const newIndex = ingredients.push(newIngredient) - 1; + if (linkedVariants) declaredVariants.set(newIngredient, linkedVariants); + return newIndex; } export function findAndUpsertCookware( cookware: Cookware[], newCookware: Cookware, isReference: boolean, + diagnostics: Diagnostic[], + span?: DiagnosticSourceSpan, ): number { const { name, quantity } = newCookware; @@ -219,6 +264,7 @@ export function findAndUpsertCookware( } catch (e) { /* v8 ignore else -- expliciting error type -- @preserve */ if (e instanceof CannotAddTextValueError) { + diagnostics.push(createDiagnostic("COOKLANG_W0401", {}, span)); return cookware.push(newCookware) - 1; } } diff --git a/test/diagnostics.test.ts b/test/diagnostics.test.ts index 776f4010..f5dc1618 100644 --- a/test/diagnostics.test.ts +++ b/test/diagnostics.test.ts @@ -359,4 +359,44 @@ describe("Recipe.parse() throwing", () => { expect(error.message).toContain("1 error"); expect(error.diagnostics).toHaveLength(2); }); + + it("warns when an unreferenced ingredient repeats with the same flags and preparation", () => { + const source = "Add @?salt{1%tsp}(ground), then add @?salt{2%tsp}(ground)."; + const result = parseCollect(new Recipe(), source); + + expect(result.diagnostics).toHaveDiagnostic("COOKLANG_W0201", { + times: 1, + match: { + severity: "warning", + span: { + start: { + offset: source.lastIndexOf("@?salt"), + line: 1, + }, + }, + }, + }); + expect(result.output.ingredients).toHaveLength(2); + }); + + it("does not warn when preparation or flags differ, or when the ingredient is referenced", () => { + const result = parseCollect( + new Recipe(), + "Add @?salt{1%tsp}(ground), @salt{1%tsp}(ground), " + + "@?salt{1%tsp}(crushed), then @&?salt{1%tsp}(ground).", + ); + + expect(result.diagnostics).not.toHaveDiagnostic("COOKLANG_W0201"); + }); + + it("warns for duplicate ingredients added through group-key syntax", () => { + const result = parseCollect( + new Recipe(), + "Add @|spices|salt{1%tsp}(ground) and @|spices|salt{2%tsp}(ground).", + ); + + expect(result.diagnostics).toHaveDiagnostic("COOKLANG_W0201", { + times: 1, + }); + }); }); diff --git a/test/parser_helpers.test.ts b/test/parser_helpers.test.ts index ab44394d..9d57ef30 100644 --- a/test/parser_helpers.test.ts +++ b/test/parser_helpers.test.ts @@ -8,6 +8,7 @@ import type { NoteItem, TextItem, Yield, + Diagnostic, } from "../src/types"; import { flushPendingNote, @@ -864,7 +865,7 @@ describe("findAndUpsertCookware", () => { it("should correctly add a non-referenced cookware", () => { const cookware: Cookware[] = [{ name: "oven", flags: [] }]; const newCookware: Cookware = { name: "pan", flags: [] }; - expect(findAndUpsertCookware(cookware, newCookware, false)).toBe(1); + expect(findAndUpsertCookware(cookware, newCookware, false, [])).toBe(1); expect(cookware.length).toEqual(2); }); @@ -875,7 +876,7 @@ describe("findAndUpsertCookware", () => { quantity: { type: "fixed", value: { type: "decimal", decimal: 1 } }, flags: [], }; - expect(findAndUpsertCookware(cookware, newCookware, true)).toBe(0); + expect(findAndUpsertCookware(cookware, newCookware, true, [])).toBe(0); expect(cookware.length).toBe(1); expect(cookware[0]!.quantity).toEqual({ type: "fixed", @@ -896,7 +897,7 @@ describe("findAndUpsertCookware", () => { quantity: { type: "fixed", value: { type: "decimal", decimal: 2 } }, flags: [], }; - findAndUpsertCookware(cookware, newCookware, true); + findAndUpsertCookware(cookware, newCookware, true, []); expect(cookware[0]!.quantity).toEqual({ type: "fixed", value: { type: "decimal", decimal: 3 }, @@ -915,13 +916,15 @@ describe("findAndUpsertCookware", () => { name: "oven", quantity: { type: "fixed", value: { type: "decimal", decimal: 1 } }, }; - findAndUpsertCookware(cookware, newCookware, true); + const diagnostics: Diagnostic[] = []; + findAndUpsertCookware(cookware, newCookware, true, diagnostics); expect(cookware.length).toEqual(2); + expect(diagnostics).toHaveDiagnostic("COOKLANG_W0401"); }); it("should throw a ReferencedCookwareNotFoundError if a reference cookware does not exist", () => { const newCookware: Cookware = { name: "unreferenced-cookware", flags: [] }; - expect(() => findAndUpsertCookware([], newCookware, true)).toThrow( + expect(() => findAndUpsertCookware([], newCookware, true, [])).toThrow( ReferencedCookwareNotFoundError, ); }); @@ -939,19 +942,20 @@ describe("findAndUpsertCookware", () => { quantity: { type: "fixed", value: { type: "decimal", decimal: 1 } }, }; - expect(() => findAndUpsertCookware(cookware, newCookware, true)).toThrow( - ReferencedItemCannotBeRedefinedError, - ); + expect(() => + findAndUpsertCookware(cookware, newCookware, true, []), + ).toThrow(ReferencedItemCannotBeRedefinedError); newCookware.flags = ["optional"]; - expect(() => findAndUpsertCookware(cookware, newCookware, true)).toThrow( - ReferencedItemCannotBeRedefinedError, - ); + expect(() => + findAndUpsertCookware(cookware, newCookware, true, []), + ).toThrow(ReferencedItemCannotBeRedefinedError); }); }); describe("findAndUpsertIngredient", () => { it("should correctly add a non-referenced ingredient", () => { const ingredients: Ingredient[] = []; + const diagnostics: Diagnostic[] = []; const newIngredient: Ingredient = { name: "eggs", quantities: [ @@ -960,9 +964,9 @@ describe("findAndUpsertIngredient", () => { }, ], }; - expect(findAndUpsertIngredient(ingredients, newIngredient, false)).toEqual( - 0, - ); + expect( + findAndUpsertIngredient(ingredients, newIngredient, false, diagnostics), + ).toEqual(0); expect(ingredients).toEqual([newIngredient]); }); @@ -980,6 +984,7 @@ describe("findAndUpsertIngredient", () => { ], }, ]; + const diagnostics: Diagnostic[] = []; const newIngredient: Ingredient = { name: "eggs", quantities: [ @@ -988,9 +993,9 @@ describe("findAndUpsertIngredient", () => { }, ], }; - expect(findAndUpsertIngredient(ingredients, newIngredient, true)).toEqual( - 0, - ); + expect( + findAndUpsertIngredient(ingredients, newIngredient, true, diagnostics), + ).toEqual(0); expect(ingredients[0]!.quantities).toEqual([ { quantity: { type: "fixed", value: { type: "decimal", decimal: 1 } }, @@ -1000,7 +1005,12 @@ describe("findAndUpsertIngredient", () => { const ingredients_noqtt: Ingredient[] = [{ name: "salt" }]; const newIngredient_noqtt: Ingredient = { name: "salt" }; expect( - findAndUpsertIngredient(ingredients_noqtt, newIngredient_noqtt, true), + findAndUpsertIngredient( + ingredients_noqtt, + newIngredient_noqtt, + true, + diagnostics, + ), ).toEqual(0); expect(ingredients_noqtt[0]!.quantities).toBe(undefined); }); @@ -1016,6 +1026,7 @@ describe("findAndUpsertIngredient", () => { ], }, ]; + const diagnostics: Diagnostic[] = []; const newIngredient: Ingredient = { name: "eggs", quantities: [ @@ -1024,9 +1035,9 @@ describe("findAndUpsertIngredient", () => { }, ], }; - expect(findAndUpsertIngredient(ingredients, newIngredient, true)).toEqual( - 0, - ); + expect( + findAndUpsertIngredient(ingredients, newIngredient, true, diagnostics), + ).toEqual(0); expect(ingredients).toHaveLength(1); }); @@ -1044,6 +1055,7 @@ describe("findAndUpsertIngredient", () => { ], }, ]; + const diagnostics: Diagnostic[] = []; const newIngredient: Ingredient = { name: "unreferenced-ingredient", quantities: [ @@ -1058,7 +1070,7 @@ describe("findAndUpsertIngredient", () => { flags: [], }; expect(() => - findAndUpsertIngredient(ingredients, newIngredient, true), + findAndUpsertIngredient(ingredients, newIngredient, true, diagnostics), ).toThrow(ReferencedIngredientNotFoundError); }); @@ -1077,6 +1089,7 @@ describe("findAndUpsertIngredient", () => { flags: ["hidden"], }, ]; + const diagnostics: Diagnostic[] = []; const newIngredient: Ingredient = { name: "eggs", quantities: [ @@ -1086,11 +1099,11 @@ describe("findAndUpsertIngredient", () => { ], }; expect(() => - findAndUpsertIngredient(ingredients, newIngredient, true), + findAndUpsertIngredient(ingredients, newIngredient, true, diagnostics), ).toThrow(ReferencedItemCannotBeRedefinedError); newIngredient.flags = ["optional"]; expect(() => - findAndUpsertIngredient(ingredients, newIngredient, true), + findAndUpsertIngredient(ingredients, newIngredient, true, diagnostics), ).toThrow(ReferencedItemCannotBeRedefinedError); }); }); diff --git a/test/recipe_parsing.test.ts b/test/recipe_parsing.test.ts index f6332a96..8d8f285c 100644 --- a/test/recipe_parsing.test.ts +++ b/test/recipe_parsing.test.ts @@ -850,6 +850,25 @@ describe("parse function", () => { }); }); + it("collects a warning when text and numeric cookware quantities stay separate", () => { + const source = "Use #bowl{one}, then #&bowl{2}"; + const result = parseCollect(new Recipe(), source); + + expect(result.diagnostics).toHaveDiagnostic("COOKLANG_W0401", { + times: 1, + match: { + severity: "warning", + span: { + start: { + offset: source.indexOf("#&bowl"), + line: 1, + }, + }, + }, + }); + expect(result.output.cookware).toHaveLength(2); + }); + it("should correctly handle modifiers for cookware", () => { const recipe = ` Use an #oven or a #?pan to cook. Clean the #&oven after use. Don't bother using a #-stove diff --git a/test/recipe_variants.test.ts b/test/recipe_variants.test.ts index 853c35c6..468cccd1 100644 --- a/test/recipe_variants.test.ts +++ b/test/recipe_variants.test.ts @@ -1091,3 +1091,22 @@ Mix flour. }); }); }); + +describe("duplicate ingredient detection across variants", () => { + it("does not warn when the same ingredient is declared in disjoint variants", () => { + const recipe = new Recipe( + "[vegan] Add @oil{1%tbsp}.\n\n[keto] Add @oil{1%tbsp}.", + ); + expect(recipe.diagnostics).not.toHaveDiagnostic("COOKLANG_W0201"); + expect(recipe.ingredients).toHaveLength(2); + }); + + it("still warns when variants overlap or are untagged", () => { + const recipe = new Recipe( + "[vegan] Add @oil{1%tbsp}.\n\n[vegan,keto] Add @oil{1%tbsp}.", + ); + expect(recipe.diagnostics).toHaveDiagnostic("COOKLANG_W0201", { + times: 1, + }); + }); +});