From 5016962b40cb7d9a7e4bad30a42e8e4d18f4b5b0 Mon Sep 17 00:00:00 2001 From: wenjiefan Date: Wed, 2 Sep 2026 11:08:16 +0200 Subject: [PATCH] Align the IsHandled article slug, keywords and good sample with its narrowed scope The article was rewritten to say a reset is required only when the value can carry over, and its H1 was updated to match, but three artefacts still carried the old "always initialize to false" premise: - The slug still read `initialize-ishandled-to-false-before-publishing`, which contradicts the body. The slug is not cosmetic: Build-KnowledgeIndex.ps1 ranks candidates on keywords, frontmatter dimensions, domain, path and title, so a stale path pushes selection back toward the behaviour this change narrows. Renamed to `reset-ishandled-only-when-the-value-can-carry-over`, following the existing precedent for conditional slugs such as `unreleased-symbol-change-is-not-a-breaking-change`. - Keywords still listed `initialization` and `deterministic` and omitted `false-positive`, the tag this repository uses for suppression articles. Replaced with `carry-over` and `loop-iteration` and added `false-positive`. - The good sample demonstrated only the "prefer separate fresh locals" clause and contained no reset at all, so the article's headline case had no positive example. It was also asymmetric with the bad sample, which gained a loop procedure showing a local that carries `true` into the next iteration. Added the matching loop procedure to the good sample: a local declared outside the loop is reset at the top of each iteration. That case cannot be solved by introducing another local, because AL has no block scope, so it is the only shape that demonstrates the reset the article still requires. It also gives the engine the correct `suggested-code` shape for the loop finding; without it the one-click fix adapted from the good sample would propose splitting the variable rather than adding one line. Also renamed the sample codeunits from "IsHandled Init ..." to "IsHandled Carry Over ...", and updated the two references to the old slug: the events leaf skill cue and the events pin in evaluation/review-fixtures.json. validate_frontmatter.py reports 0 errors; Test-ReviewFixtures.ps1 passes with 32 cases across 16 leaf domains and resolves the events fixture to the renamed article. --- evaluation/review-fixtures.json | 2 +- ...only-when-the-value-can-carry-over.bad.al} | 2 +- ...nly-when-the-value-can-carry-over.good.al} | 22 ++++++++++++++++++- ...led-only-when-the-value-can-carry-over.md} | 6 ++--- microsoft/skills/review/al-events-review.md | 2 +- 5 files changed, 27 insertions(+), 7 deletions(-) rename microsoft/knowledge/events/{initialize-ishandled-to-false-before-publishing.bad.al => reset-ishandled-only-when-the-value-can-carry-over.bad.al} (97%) rename microsoft/knowledge/events/{initialize-ishandled-to-false-before-publishing.good.al => reset-ishandled-only-when-the-value-can-carry-over.good.al} (58%) rename microsoft/knowledge/events/{initialize-ishandled-to-false-before-publishing.md => reset-ishandled-only-when-the-value-can-carry-over.md} (89%) diff --git a/evaluation/review-fixtures.json b/evaluation/review-fixtures.json index 2f85d5c..68a646d 100644 --- a/evaluation/review-fixtures.json +++ b/evaluation/review-fixtures.json @@ -11,7 +11,7 @@ "article": "do-not-expose-sensitive-data-through-public-api" }, "events": { - "article": "initialize-ishandled-to-false-before-publishing" + "article": "reset-ishandled-only-when-the-value-can-carry-over" }, "interfaces": { "article": "set-defaultimplementation-on-enum" diff --git a/microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.bad.al b/microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.bad.al similarity index 97% rename from microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.bad.al rename to microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.bad.al index 7192bc1..a7d7709 100644 --- a/microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.bad.al +++ b/microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.bad.al @@ -1,5 +1,5 @@ // Demonstration-only AL. Not compiled by CI; illustrates the article. -codeunit 50241 "IsHandled Init Bad Sample" +codeunit 50241 "IsHandled Carry Over Bad Sample" { procedure ApplyDiscounts(var SalesHeader: Record "Sales Header") var diff --git a/microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.good.al b/microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.good.al similarity index 58% rename from microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.good.al rename to microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.good.al index f3e57a3..84d547e 100644 --- a/microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.good.al +++ b/microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.good.al @@ -1,5 +1,5 @@ // Demonstration-only AL. Not compiled by CI; illustrates the article. -codeunit 50240 "IsHandled Init Good Sample" +codeunit 50240 "IsHandled Carry Over Good Sample" { procedure ApplyDiscounts(var SalesHeader: Record "Sales Header") var @@ -18,6 +18,21 @@ codeunit 50240 "IsHandled Init Good Sample" DiscountPct += 2; end; + procedure ApplyLineDiscounts(var SalesLine: Record "Sales Line") + var + LineIsHandled: Boolean; + begin + if SalesLine.FindSet() then + repeat + // The local initializes once, so reset it per iteration; a + // subscriber that handles one line must not skip the rest. + LineIsHandled := false; + OnBeforeApplyLineDiscount(SalesLine, LineIsHandled); + if not LineIsHandled then + SalesLine.Validate("Line Discount %", 5); + until SalesLine.Next() = 0; + end; + [IntegrationEvent(false, false)] local procedure OnBeforeApplyHeaderDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean) begin @@ -27,4 +42,9 @@ codeunit 50240 "IsHandled Init Good Sample" local procedure OnBeforeApplyPaymentDiscount(var SalesHeader: Record "Sales Header"; var DiscountPct: Decimal; var IsHandled: Boolean) begin end; + + [IntegrationEvent(false, false)] + local procedure OnBeforeApplyLineDiscount(var SalesLine: Record "Sales Line"; var IsHandled: Boolean) + begin + end; } diff --git a/microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.md b/microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.md similarity index 89% rename from microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.md rename to microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.md index 12eb39c..fba378b 100644 --- a/microsoft/knowledge/events/initialize-ishandled-to-false-before-publishing.md +++ b/microsoft/knowledge/events/reset-ishandled-only-when-the-value-can-carry-over.md @@ -1,7 +1,7 @@ --- bc-version: [all] domain: events -keywords: [ishandled, initialization, deterministic, onbefore, reset, integration-event, control-flow] +keywords: [ishandled, carry-over, loop-iteration, onbefore, reset, integration-event, control-flow, false-positive] technologies: [al] countries: [w1] application-area: [all] @@ -17,10 +17,10 @@ A routine that raises an `OnBefore…` integration event with a `var IsHandled: Reset `IsHandled := false;` before a raise only when the value might otherwise carry over as `true`: the same variable is reused after an earlier raise without a control-flow proof that it is false, a raise is re-entered by a loop, the value comes from an input parameter, field, or global, or earlier code seeds it. Prefer separate fresh locals when independent event seams need independent handled state. A reset on a guaranteed-false fresh local used by one non-looping raise, or before a later raise reached only after a semantically valid `if IsHandled then exit;`, can be retained for readability, but its absence is not a correctness finding. -See sample: `initialize-ishandled-to-false-before-publishing.good.al`. +See sample: `reset-ishandled-only-when-the-value-can-carry-over.good.al`. ## Anti Pattern Raising `OnBeforeX(…, IsHandled)` when the variable can still be `true` from an earlier raise, an earlier loop iteration, or another source, so the publisher call starts with stale state. Do not match a single non-looping raise using a fresh local Boolean, or a later raise reached only after a semantically valid `if IsHandled then exit;` proves the value is false. -See sample: `initialize-ishandled-to-false-before-publishing.bad.al`. +See sample: `reset-ishandled-only-when-the-value-can-carry-over.bad.al`. diff --git a/microsoft/skills/review/al-events-review.md b/microsoft/skills/review/al-events-review.md index ebd2976..c854f1c 100644 --- a/microsoft/skills/review/al-events-review.md +++ b/microsoft/skills/review/al-events-review.md @@ -51,7 +51,7 @@ When the post-conflict worklist is empty because no applicable events knowledge The following targeted checks map diff signals to specific `events` articles. Treat each as a candidate-selection cue: when the signal appears in the changed code, add the named article to the worklist and evaluate it in Action. -- An `IsHandled` value that can carry over as `true` (reused after an earlier raise, re-entered on a later loop iteration, input/global/field, or otherwise seeded) is passed to a publisher without a reset — `initialize-ishandled-to-false-before-publishing`. Do not match one non-looping raise using a fresh local Boolean, or a later raise reached only after a semantically valid `if IsHandled then exit;` proves the value is false. +- An `IsHandled` value that can carry over as `true` (reused after an earlier raise, re-entered on a later loop iteration, input/global/field, or otherwise seeded) is passed to a publisher without a reset — `reset-ishandled-only-when-the-value-can-carry-over`. Do not match one non-looping raise using a fresh local Boolean, or a later raise reached only after a semantically valid `if IsHandled then exit;` proves the value is false. - `if IsHandled then exit;` in a routine that also raises a paired `OnAfter…` event later, so the after-event is skipped whenever the call is handled — `preserve-onafter-execution-when-ishandled-skips-the-body`. - Any parameter added to a public Business/Integration event procedure, regardless of position; do not flag additions or reordering on `local`/`internal` publishers merely because a new parameter was not appended — `add-new-event-parameters-at-the-end`. - A shipped Business/Integration event renamed or removed, or an existing parameter renamed, removed, retyped, or changed to/from `var`, based on the mistaken assumption that `local` or `internal` prevents dependent subscription; parameter order alone is not a subscriber-contract violation — `treat-local-and-internal-events-as-subscriber-contracts`.