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`.