mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 17:36:53 +01:00
Add P0 extensibility compatibility knowledge
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 645349fd-1892-48f3-8a84-db77d6abd1c3
This commit is contained in:
parent
9214f73819
commit
23af51e02d
18 changed files with 307 additions and 15 deletions
|
|
@ -6,13 +6,13 @@ codeunit 50250 "Param Append Good Sample"
|
||||||
IsHandled: Boolean;
|
IsHandled: Boolean;
|
||||||
begin
|
begin
|
||||||
IsHandled := false;
|
IsHandled := false;
|
||||||
// The new 'CalledFromBatch' parameter was appended at the end of the
|
// This local event can gain an optional trailing subscriber parameter.
|
||||||
// existing signature, so existing subscribers needed no re-mapping.
|
|
||||||
OnBeforePostDocument(SalesHeader, IsHandled, CalledFromBatch);
|
OnBeforePostDocument(SalesHeader, IsHandled, CalledFromBatch);
|
||||||
if IsHandled then
|
if IsHandled then
|
||||||
exit;
|
exit;
|
||||||
end;
|
end;
|
||||||
|
|
||||||
|
// Public events cannot use this evolution: dependent apps may raise them.
|
||||||
[IntegrationEvent(false, false)]
|
[IntegrationEvent(false, false)]
|
||||||
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean; CalledFromBatch: Boolean)
|
local procedure OnBeforePostDocument(var SalesHeader: Record "Sales Header"; var IsHandled: Boolean; CalledFromBatch: Boolean)
|
||||||
begin
|
begin
|
||||||
|
|
|
||||||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
Adding a parameter to an existing event publisher changes its signature. Appending the new parameter at the end of the parameter list keeps the change easy to review and track: existing subscribers still bind to the leading parameters, and the diff is a single clean addition. Inserting a parameter in the middle makes diffs noisy and harder to review, and obscures the history of how the signature evolved. New parameters belong after the existing ones.
|
Adding a parameter to an existing event publisher changes its signature. For a `local` or `internal` Business or Integration event, subscribers may omit parameters, so a new parameter can be compatible when appended at the end. A public event is also a public procedure that dependent extensions can raise; adding a parameter to it is breaking and requires a new event. Existing parameters must never be renamed, removed, reordered, or have their type changed.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
When extending an existing publisher, append the new parameter after all existing ones, including after a trailing `var IsHandled: Boolean` when present. Subscribers that already match keep working against the leading parameters, and the change stays a one-line addition that is trivial to review.
|
When extending an existing `local` or `internal` Business or Integration event, append the new parameter after all existing ones, including after a trailing `var IsHandled: Boolean` when present. Existing subscribers can continue omitting the new trailing parameter. Create a new event instead when the publisher procedure is public.
|
||||||
|
|
||||||
See sample: `add-new-event-parameters-at-the-end.good.al`.
|
See sample: `add-new-event-parameters-at-the-end.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Inserting a new parameter in the middle of an existing event's signature, shifting every subsequent parameter and making the change noisy and harder to review. Detection: a changed event signature where an added parameter appears before existing parameters rather than at the tail of the list.
|
Inserting a new parameter before an existing parameter of a `local` or `internal` event, or adding any parameter to a public event. Detection: a changed event signature where a new parameter is not a compatible trailing addition.
|
||||||
|
|
||||||
See sample: `add-new-event-parameters-at-the-end.bad.al`.
|
See sample: `add-new-event-parameters-at-the-end.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,14 @@
|
||||||
|
// Demonstration-only AL. Version 1 used [IntegrationEvent(true, true, false)].
|
||||||
|
codeunit 50531 "Shipment Events Bad"
|
||||||
|
{
|
||||||
|
procedure NotifyShipment(ShipmentNo: Code[20])
|
||||||
|
begin
|
||||||
|
OnShipmentCreated(ShipmentNo);
|
||||||
|
end;
|
||||||
|
|
||||||
|
// Version 2 mutates all three contract-significant arguments in place.
|
||||||
|
[IntegrationEvent(false, false, true)]
|
||||||
|
local procedure OnShipmentCreated(ShipmentNo: Code[20])
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,21 @@
|
||||||
|
// Demonstration-only AL. The Isolated argument requires runtime 9.0 / BC20.
|
||||||
|
codeunit 50530 "Shipment Events"
|
||||||
|
{
|
||||||
|
procedure NotifyShipment(ShipmentNo: Code[20])
|
||||||
|
begin
|
||||||
|
OnShipmentCreated(ShipmentNo);
|
||||||
|
OnShipmentCreatedIsolated(ShipmentNo);
|
||||||
|
end;
|
||||||
|
|
||||||
|
// Preserve the shipped attribute contract.
|
||||||
|
[IntegrationEvent(true, false, false)]
|
||||||
|
local procedure OnShipmentCreated(ShipmentNo: Code[20])
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
// Publish a new event for different isolation and sender semantics.
|
||||||
|
[IntegrationEvent(false, false, true)]
|
||||||
|
local procedure OnShipmentCreatedIsolated(ShipmentNo: Code[20])
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [20..]
|
||||||
|
domain: events
|
||||||
|
keywords: [event-attribute, includesender, globalvaraccess, isolated-event, compatibility, integration-event, business-event, appsourcecop, as0021, as0101]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Do not change shipped event attribute flags
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
`IncludeSender`, `GlobalVarAccess`, and `Isolated` affect a subscriber contract, not just publisher implementation. Removing sender or global access breaks subscribers; changing `Isolated` changes transaction, error, and rollback behavior. AppSourceCop AS0021 prevents changing exposed sender or globals from `true` to `false`, while AS0101 prevents adding, removing, or changing `Isolated`. The three-argument event form with `Isolated` is available from runtime 9.0 (Business Central 2022 release wave 1, BC20).
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Keep every attribute argument exactly as shipped. If new subscribers need different sender/global exposure or isolation semantics, publish a new event with the desired flags and raise both events while the original contract is supported. Choose preferred flags only when designing a new event.
|
||||||
|
|
||||||
|
See sample: `do-not-change-shipped-event-attribute-flags.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Changing a shipped event's attribute arguments to modernize its design, remove `GlobalVarAccess`, replace `IncludeSender` with an explicit parameter, or make the event isolated. Even a change that leaves old subscribers compiling can alter observable execution or exposure; version the event instead.
|
||||||
|
|
||||||
|
See sample: `do-not-change-shipped-event-attribute-flags.bad.al`.
|
||||||
|
|
@ -7,20 +7,20 @@ countries: [w1]
|
||||||
application-area: [all]
|
application-area: [all]
|
||||||
---
|
---
|
||||||
|
|
||||||
# Prefer this over IncludeSender in codeunit events
|
# Prefer this over IncludeSender in new codeunit events
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
Some publishers set `IncludeSender` to `true` on `[IntegrationEvent]` or `[BusinessEvent]` so subscribers receive the publishing object as an implicit sender parameter. From Business Central 2024 release wave 2, a codeunit can instead pass itself explicitly with the `this` keyword as a normal, strongly-typed `Sender` parameter. Explicit passing is clearer at both the publisher and the subscriber: the sender appears in the signature, it is concretely typed to the publishing codeunit, and it avoids the implicit-parameter mechanics of `IncludeSender`. Reserve `IncludeSender = true` for cases where the sender genuinely cannot be passed explicitly. This guidance applies to code targeting Business Central 2024 release wave 2 or later, where the `this` keyword is available.
|
When designing a new publisher, setting `IncludeSender` to `true` on `[IntegrationEvent]` or `[BusinessEvent]` gives subscribers the publishing object as an implicit sender parameter. From Business Central 2024 release wave 2, a codeunit can instead pass itself explicitly with the `this` keyword as a normal, strongly-typed `Sender` parameter. Explicit passing makes the sender visible and typed in the signature. This is new-event design guidance only: never change `IncludeSender` on an event that has already shipped.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Declare the publisher `[IntegrationEvent(false, false)]` with an explicit `Sender: Codeunit "…"` parameter and raise it with `this`, for example `OnBeforeProcessOrder(OrderNo, this);`. Subscribers then receive a typed sender they can call directly.
|
For a new event, declare the publisher `[IntegrationEvent(false, false)]` with an explicit `Sender: Codeunit "…"` parameter and raise it with `this`, for example `OnBeforeProcessOrder(OrderNo, this);`. Subscribers then receive a typed sender they can call directly.
|
||||||
|
|
||||||
See sample: `prefer-this-over-includesender-in-codeunit-events.good.al`.
|
See sample: `prefer-this-over-includesender-in-codeunit-events.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Relying on `[IntegrationEvent(true, …)]` solely to hand subscribers the publisher instance, where a codeunit could pass `this` explicitly as a typed parameter. Detection: `IncludeSender = true` on a codeunit event whose only purpose is to expose the sender, in code targeting Business Central 2024 release wave 2 or later.
|
Designing a new codeunit event with `[IntegrationEvent(true, …)]` solely to hand subscribers the publisher instance, where `this` could be passed explicitly as a typed parameter. Do not apply this rule by mutating a shipped event's attribute flags.
|
||||||
|
|
||||||
See sample: `prefer-this-over-includesender-in-codeunit-events.bad.al`.
|
See sample: `prefer-this-over-includesender-in-codeunit-events.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,15 @@
|
||||||
|
// Demonstration-only AL. Version 1 exposed Score as an Integer named Score.
|
||||||
|
codeunit 50521 "Customer Scoring Events Bad"
|
||||||
|
{
|
||||||
|
procedure ScoreCustomer(CustomerNo: Code[20]; ScoreText: Text)
|
||||||
|
begin
|
||||||
|
OnCustomerScored(CustomerNo, ScoreText);
|
||||||
|
end;
|
||||||
|
|
||||||
|
// 'local' limits raising, not subscription. This type/name change breaks
|
||||||
|
// subscribers compiled against the shipped event.
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnCustomerScored(CustomerNo: Code[20]; ScoreText: Text)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,19 @@
|
||||||
|
// Demonstration-only AL. Version 2 keeps the shipped local event unchanged.
|
||||||
|
codeunit 50520 "Customer Scoring Events"
|
||||||
|
{
|
||||||
|
procedure ScoreCustomer(CustomerNo: Code[20]; Score: Integer; Reason: Text)
|
||||||
|
begin
|
||||||
|
OnCustomerScored(CustomerNo, Score);
|
||||||
|
OnCustomerScoredV2(CustomerNo, Score, Reason);
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnCustomerScored(CustomerNo: Code[20]; Score: Integer)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
|
||||||
|
[IntegrationEvent(false, false)]
|
||||||
|
local procedure OnCustomerScoredV2(CustomerNo: Code[20]; Score: Integer; Reason: Text)
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: events
|
||||||
|
keywords: [local-event, internal-event, event-subscriber, compatibility, access-modifier, integration-event, business-event, appsourcecop, as0025]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Treat local and internal events as subscriber contracts
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
The `local` and `internal` access modifiers on Business and Integration event publishers restrict who can raise the procedure; they do not prevent dependent extensions from subscribing. Once shipped, the event name and existing parameter names, types, order, and passing modes are compatibility contracts even when the publisher is not public. This differs from `[InternalEvent]`, which is module-only except for modules named by `internalsVisibleTo`.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Preserve a shipped Business or Integration event's identity and existing parameters regardless of its procedure access modifier. A compatible trailing parameter may be added to a `local` or `internal` event as described by `add-new-event-parameters-at-the-end`; for an incompatible signature or a public publisher, add a new event and keep the original.
|
||||||
|
|
||||||
|
See sample: `treat-local-and-internal-events-as-subscriber-contracts.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Renaming, removing, reordering, or changing an existing parameter because the event publisher procedure is `local` or `internal`. AppSourceCop AS0025 checks these subscriber-breaking changes because dependent event subscribers can still bind to the event.
|
||||||
|
|
||||||
|
See sample: `treat-local-and-internal-events-as-subscriber-contracts.bad.al`.
|
||||||
|
|
@ -0,0 +1,16 @@
|
||||||
|
// Demonstration-only AL. Version 1 shipped with only CalculateAmount().
|
||||||
|
interface "I Shipping Quote Bad"
|
||||||
|
{
|
||||||
|
procedure CalculateAmount(): Decimal;
|
||||||
|
|
||||||
|
// Added in version 2: every existing implementer now fails to compile.
|
||||||
|
procedure CalculateDeliveryDate(): Date;
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50511 "Existing Shipping Quote" implements "I Shipping Quote Bad"
|
||||||
|
{
|
||||||
|
procedure CalculateAmount(): Decimal
|
||||||
|
begin
|
||||||
|
exit(10);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,23 @@
|
||||||
|
// Demonstration-only AL. Interface inheritance requires runtime 14.0 / BC25.
|
||||||
|
interface "I Shipping Quote"
|
||||||
|
{
|
||||||
|
procedure CalculateAmount(): Decimal;
|
||||||
|
}
|
||||||
|
|
||||||
|
interface "I Shipping Quote V2" extends "I Shipping Quote"
|
||||||
|
{
|
||||||
|
procedure CalculateDeliveryDate(): Date;
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50510 "Shipping Quote V2" implements "I Shipping Quote V2"
|
||||||
|
{
|
||||||
|
procedure CalculateAmount(): Decimal
|
||||||
|
begin
|
||||||
|
exit(10);
|
||||||
|
end;
|
||||||
|
|
||||||
|
procedure CalculateDeliveryDate(): Date
|
||||||
|
begin
|
||||||
|
exit(Today() + 1);
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [16..]
|
||||||
|
domain: interfaces
|
||||||
|
keywords: [published-interface, interface-method, breaking-change, interface-extends, versioned-interface, appsourcecop, as0066]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Extend published interfaces; do not edit them
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Adding a method to a shipped interface changes the contract every implementing codeunit must satisfy. Implementers can live in dependent extensions, so the addition breaks code the interface publisher cannot update; AppSourceCop reports AS0066. Interface inheritance is available from runtime 14.0 (Business Central 2024 release wave 2, BC25), but the original interface must remain unchanged.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
On BC25 or later, declare a new interface that `extends` the published interface and add the new method there. Existing implementers remain valid for the original contract, while new implementers opt in to the extended contract. For targets BC16 through BC24, where interface inheritance is unavailable, publish a new or versioned sibling interface instead.
|
||||||
|
|
||||||
|
See sample: `extend-published-interfaces-dont-edit-them.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Adding a procedure directly to an interface that has already shipped. Every dependent implementation must immediately add that procedure, so an otherwise compatible app update breaks its implementers.
|
||||||
|
|
||||||
|
See sample: `extend-published-interfaces-dont-edit-them.bad.al`.
|
||||||
|
|
@ -0,0 +1,36 @@
|
||||||
|
// Demonstration-only AL. A removed enum-extension value left ordinal 700 in data.
|
||||||
|
enum 50503 "Delivery Method Bad" implements "I Delivery Method Bad"
|
||||||
|
{
|
||||||
|
Extensible = true;
|
||||||
|
DefaultImplementation = "I Delivery Method Bad" = "Default Delivery Method Bad";
|
||||||
|
|
||||||
|
value(0; Default)
|
||||||
|
{
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
interface "I Delivery Method Bad"
|
||||||
|
{
|
||||||
|
procedure Deliver();
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50504 "Default Delivery Method Bad" implements "I Delivery Method Bad"
|
||||||
|
{
|
||||||
|
procedure Deliver()
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50505 "Delivery Dispatch Bad"
|
||||||
|
{
|
||||||
|
procedure DeliverPersistedValue()
|
||||||
|
var
|
||||||
|
DeliveryMethod: Enum "Delivery Method Bad";
|
||||||
|
Delivery: Interface "I Delivery Method Bad";
|
||||||
|
begin
|
||||||
|
DeliveryMethod := 700;
|
||||||
|
// DefaultImplementation does not handle an ordinal that is not declared.
|
||||||
|
Delivery := DeliveryMethod;
|
||||||
|
Delivery.Deliver();
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,34 @@
|
||||||
|
// Demonstration-only AL. UnknownValueImplementation requires runtime 7.0 / BC18.
|
||||||
|
interface "I Delivery Method"
|
||||||
|
{
|
||||||
|
procedure Deliver();
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50500 "Unknown Delivery Method" implements "I Delivery Method"
|
||||||
|
{
|
||||||
|
procedure Deliver()
|
||||||
|
begin
|
||||||
|
Error(UnknownMethodErr);
|
||||||
|
end;
|
||||||
|
|
||||||
|
var
|
||||||
|
UnknownMethodErr: Label 'The saved delivery method is no longer installed. Select another method.';
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50501 "Default Delivery Method" implements "I Delivery Method"
|
||||||
|
{
|
||||||
|
procedure Deliver()
|
||||||
|
begin
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
||||||
|
enum 50502 "Delivery Method" implements "I Delivery Method"
|
||||||
|
{
|
||||||
|
Extensible = true;
|
||||||
|
DefaultImplementation = "I Delivery Method" = "Default Delivery Method";
|
||||||
|
UnknownValueImplementation = "I Delivery Method" = "Unknown Delivery Method";
|
||||||
|
|
||||||
|
value(0; Default)
|
||||||
|
{
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [18..]
|
||||||
|
domain: interfaces
|
||||||
|
keywords: [unknownvalueimplementation, unknown-enum-value, persisted-ordinal, enum-extension, extension-uninstall, interface-fallback]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Handle unknown enum ordinals with UnknownValueImplementation
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
An enum ordinal can remain in persisted data after the enum extension that declared it is uninstalled. The ordinal is then unknown: it matches no currently declared enum value. `DefaultImplementation` does not cover this case; it covers declared values that have no explicit interface implementation. `UnknownValueImplementation`, available from runtime 7.0 (Business Central 2021 release wave 1, BC18), provides the distinct interface implementation for an unknown ordinal.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
On BC18 or later, set `UnknownValueImplementation = <Interface> = <Codeunit>;` on an enum that implements an interface and can be persisted. Use an implementation that reports a clear domain error or safely contains the unknown state. Keep `DefaultImplementation` separately when declared but unmapped values also need a fallback.
|
||||||
|
|
||||||
|
See sample: `handle-unknown-enum-ordinals-with-unknownvalueimplementation.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Defining only `DefaultImplementation` and assuming it also handles a stored ordinal whose enum value has disappeared. After an enum extension is uninstalled, converting that unknown ordinal to the interface can produce a technical runtime error instead of controlled handling.
|
||||||
|
|
||||||
|
See sample: `handle-unknown-enum-ordinals-with-unknownvalueimplementation.bad.al`.
|
||||||
|
|
@ -11,11 +11,11 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
An `enum` that `implements` an interface maps each value to a codeunit through the `Implementation` property. But an extensible enum can carry values that set no `Implementation` — values added later by an extension, or a value left intentionally blank. Assigning such a value to an interface variable and calling a method on it fails at runtime unless the enum provides a fallback. The enum-level `DefaultImplementation` property names the codeunit used whenever a value has no explicit `Implementation`, so resolution always yields a usable object. LLMs are generally unaware this property exists and leave the gap open.
|
An `enum` that `implements` an interface maps each declared value to a codeunit through the `Implementation` property. A declared value, including one supplied by an enum extension, can omit that mapping. Assigning that value to an interface variable then fails at runtime unless the enum provides `DefaultImplementation`. This property is for declared but unmapped values; an ordinal that is no longer declared is a different case covered by `handle-unknown-enum-ordinals-with-unknownvalueimplementation`.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
On any extensible enum that implements an interface, set `DefaultImplementation = <Interface> = <Codeunit>;` at the enum level, pointing at a safe implementation that does nothing harmful. Values with their own `Implementation` keep using it; declared values without one resolve to the default. For an ordinal that matches no currently declared value — for example persisted data left after an enum extension is uninstalled — runtime 7.0 and later can use `UnknownValueImplementation` as a distinct fallback. Do not recommend that property to apps targeting an earlier runtime.
|
On any extensible enum that implements an interface, set `DefaultImplementation = <Interface> = <Codeunit>;` at the enum level, pointing at a safe implementation. Values with their own `Implementation` keep using it; declared values without one resolve to the default. Do not rely on this property for persisted ordinals that match no declared enum value.
|
||||||
|
|
||||||
See sample: `set-defaultimplementation-on-enum.good.al`.
|
See sample: `set-defaultimplementation-on-enum.good.al`.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -39,7 +39,7 @@ Narrow the relevant files to the subset that applies to the changes under review
|
||||||
|
|
||||||
- The changed AL object names and types — especially codeunits that publish events or host event subscribers, posting/release/validation routines that should expose extension points, and test codeunits that bind subscribers.
|
- The changed AL object names and types — especially codeunits that publish events or host event subscribers, posting/release/validation routines that should expose extension points, and test codeunits that bind subscribers.
|
||||||
- The changed procedures and triggers, weighted toward event publisher methods, methods carrying the `[EventSubscriber(...)]` attribute, routines that raise `OnBefore`/`OnAfter` events, and any procedure that calls `BindSubscription`/`UnbindSubscription`.
|
- The changed procedures and triggers, weighted toward event publisher methods, methods carrying the `[EventSubscriber(...)]` attribute, routines that raise `OnBefore`/`OnAfter` events, and any procedure that calls `BindSubscription`/`UnbindSubscription`.
|
||||||
- Tokens extracted from the diff that relate to events and the publish/subscribe model (`IntegrationEvent`, `BusinessEvent`, `EventSubscriber`, `IsHandled`, `BindSubscription`, `UnbindSubscription`, `EventSubscriberInstance`, `OnBefore`, `OnAfter`, `Manual`, `IncludeSender`, `Sender`, `this`, `RecordRef`, `xRec`, `temporary`, `Temp`, `repeat`).
|
- Tokens extracted from the diff that relate to events and the publish/subscribe model (`IntegrationEvent`, `BusinessEvent`, `InternalEvent`, `EventSubscriber`, `IsHandled`, `BindSubscription`, `UnbindSubscription`, `EventSubscriberInstance`, `OnBefore`, `OnAfter`, `Manual`, `IncludeSender`, `GlobalVarAccess`, `Isolated`, `local`, `internal`, `Sender`, `this`, `RecordRef`, `xRec`, `temporary`, `Temp`, `repeat`).
|
||||||
|
|
||||||
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
|
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
|
||||||
|
|
||||||
|
|
@ -53,13 +53,15 @@ The following targeted checks map diff signals to specific `events` articles. Tr
|
||||||
|
|
||||||
- `IsHandled` raised without an immediately preceding `IsHandled := false;`, or one `IsHandled` variable reused across several raises with no reset between them — `initialize-ishandled-to-false-before-publishing`.
|
- `IsHandled` raised without an immediately preceding `IsHandled := false;`, or one `IsHandled` variable reused across several raises with no reset between them — `initialize-ishandled-to-false-before-publishing`.
|
||||||
- `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`.
|
- `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`.
|
||||||
- A parameter added before existing parameters on a changed event signature instead of appended at the end — `add-new-event-parameters-at-the-end`.
|
- A parameter added before existing parameters on a changed `local` or `internal` Business/Integration event, or any parameter added to a public event — `add-new-event-parameters-at-the-end`.
|
||||||
|
- A shipped Business/Integration event renamed or removed, or one of its existing parameters renamed, removed, reordered, or changed, based on the mistaken assumption that `local` or `internal` prevents dependent subscription — `treat-local-and-internal-events-as-subscriber-contracts`.
|
||||||
|
- Any change to `IncludeSender`, `GlobalVarAccess`, or `Isolated` on a shipped event, including a change intended to modernize the publisher — `do-not-change-shipped-event-attribute-flags`.
|
||||||
- Publisher names that do not encode firing position (`OnBefore`/`OnAfter<Routine>` at the boundaries, `On<Routine>OnBefore`/`OnAfter<Context>` mid-routine) — `name-events-by-publisher-position`.
|
- Publisher names that do not encode firing position (`OnBefore`/`OnAfter<Routine>` at the boundaries, `On<Routine>OnBefore`/`OnAfter<Context>` mid-routine) — `name-events-by-publisher-position`.
|
||||||
- Two consecutive `OnBefore`/`OnAfter` raises with no logic between them, or a near-duplicate event differing only by an extra parameter — `prefer-reusing-or-extending-existing-events`.
|
- Two consecutive `OnBefore`/`OnAfter` raises with no logic between them, or a near-duplicate event differing only by an extra parameter — `prefer-reusing-or-extending-existing-events`.
|
||||||
- An event raised between `repeat` and `until` inside a record loop — `do-not-publish-events-inside-loops`.
|
- An event raised between `repeat` and `until` inside a record loop — `do-not-publish-events-inside-loops`.
|
||||||
- A `temporary` record event parameter whose name does not start with `Temp` — `prefix-temporary-record-event-parameters-with-temp`.
|
- A `temporary` record event parameter whose name does not start with `Temp` — `prefix-temporary-record-event-parameters-with-temp`.
|
||||||
- Abbreviated event parameter names (`SalesHdr`, `DocNo`, `Amt`) instead of full table names and spelled-out values — `name-event-parameters-without-abbreviations`.
|
- Abbreviated event parameter names (`SalesHdr`, `DocNo`, `Amt`) instead of full table names and spelled-out values — `name-event-parameters-without-abbreviations`.
|
||||||
- `[IntegrationEvent(true, …)]` (`IncludeSender`) on a codeunit event used only to expose the publisher, where `this` could be passed as a typed `Sender` parameter (Business Central 2024 release wave 2 and later) — `prefer-this-over-includesender-in-codeunit-events`.
|
- `[IntegrationEvent(true, …)]` (`IncludeSender`) on a newly added codeunit event used only to expose the publisher, where `this` could be passed as a typed `Sender` parameter (Business Central 2024 release wave 2 and later) — `prefer-this-over-includesender-in-codeunit-events`.
|
||||||
- A `RecordRef` event parameter, or a passed-through `xRec`, where a concrete typed record fits — `avoid-loosely-typed-event-parameters`.
|
- A `RecordRef` event parameter, or a passed-through `xRec`, where a concrete typed record fits — `avoid-loosely-typed-event-parameters`.
|
||||||
- A `var IsHandled` added to a pre-existing event rather than introduced through a new `OnBefore` publisher — `do-not-add-ishandled-to-an-existing-event`.
|
- A `var IsHandled` added to a pre-existing event rather than introduced through a new `OnBefore` publisher — `do-not-add-ishandled-to-an-existing-event`.
|
||||||
- An `if IsHandled then exit;` whose skipped body performs posting, ledger-entry creation, number-series consumption, or integrity/permission validation — `do-not-bypass-critical-operations-with-ishandled`.
|
- An `if IsHandled then exit;` whose skipped body performs posting, ledger-entry creation, number-series consumption, or integrity/permission validation — `do-not-bypass-critical-operations-with-ishandled`.
|
||||||
|
|
|
||||||
|
|
@ -39,7 +39,7 @@ Narrow the relevant files to the subset that applies to the changes under review
|
||||||
|
|
||||||
- The changed AL object names and types — especially `interface` objects, codeunits and enums declared with the `implements` keyword, and consumers that declare or assign an `Interface` variable.
|
- The changed AL object names and types — especially `interface` objects, codeunits and enums declared with the `implements` keyword, and consumers that declare or assign an `Interface` variable.
|
||||||
- The changed procedures and triggers, weighted toward factory or dispatch routines that resolve a variant to behaviour, setter-injection procedures that take an `Interface` parameter, and `case`-over-enum blocks that select between strategies.
|
- The changed procedures and triggers, weighted toward factory or dispatch routines that resolve a variant to behaviour, setter-injection procedures that take an `Interface` parameter, and `case`-over-enum blocks that select between strategies.
|
||||||
- Tokens extracted from the diff that relate to interfaces and enum-backed implementation (`interface`, `implements`, `Implementation`, `DefaultImplementation`, `UnknownValueImplementation`, `enum`, `Extensible`, `Interface`, `case`, and the `case <enum> of` anti-pattern signal — a `case` over an enum value whose branches choose between variant computations).
|
- Tokens extracted from the diff that relate to interfaces and enum-backed implementation (`interface`, `extends`, `implements`, `Implementation`, `DefaultImplementation`, `UnknownValueImplementation`, `enum`, `Extensible`, `Interface`, `case`, and the `case <enum> of` anti-pattern signal — a `case` over an enum value whose branches choose between variant computations).
|
||||||
|
|
||||||
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
|
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
|
||||||
|
|
||||||
|
|
@ -47,6 +47,14 @@ Once the candidate worklist is known, resolve layer-precedence conflicts per REA
|
||||||
|
|
||||||
When the post-conflict worklist is empty because no applicable interfaces knowledge exists, or because configuration suppressed every candidate, emit `outcome: "no-knowledge"`. When the worklist is empty because no applicable interfaces knowledge matched the changes, emit `outcome: "completed"` with an empty `findings` array.
|
When the post-conflict worklist is empty because no applicable interfaces knowledge exists, or because configuration suppressed every candidate, emit `outcome: "no-knowledge"`. When the worklist is empty because no applicable interfaces knowledge matched the changes, emit `outcome: "completed"` with an empty `findings` array.
|
||||||
|
|
||||||
|
### Interface-compatibility checks
|
||||||
|
|
||||||
|
The following targeted checks map diff signals to specific `interfaces` 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.
|
||||||
|
|
||||||
|
- `DefaultImplementation` used as the only fallback where a persisted ordinal may no longer match any declared enum value, or a persisted enum lacks `UnknownValueImplementation` on BC18 or later — `handle-unknown-enum-ordinals-with-unknownvalueimplementation`.
|
||||||
|
- A method added directly to an interface that exists in the baseline, instead of adding a BC25+ interface that `extends` it or a versioned sibling for older targets — `extend-published-interfaces-dont-edit-them`.
|
||||||
|
- A declared enum value with no `Implementation` and no enum-level `DefaultImplementation` — `set-defaultimplementation-on-enum`.
|
||||||
|
|
||||||
## Action
|
## Action
|
||||||
|
|
||||||
For each worklist entry, evaluate the diff against the file's `## Best Practice` and `## Anti Pattern` sections. Emit findings as follows:
|
For each worklist entry, evaluate the diff against the file's `## Best Practice` and `## Anti Pattern` sections. Emit findings as follows:
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue