mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 06:36:55 +01:00
knowledge(events): scope public-publisher detection to same-app raisers
The detection rule flagged every public event publisher, including ones deliberately public so a sibling app can raise them - a contract `internal` cannot express across app boundaries. Scope the finding to publishers that are public although only their own app raises them, and record the cross-app case as a valid Best Practice option. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0b67b90d-e4b4-4b92-9684-726c72c43b3f
This commit is contained in:
parent
f87bc9c993
commit
d6c45678ce
1 changed files with 3 additions and 2 deletions
|
|
@ -25,8 +25,9 @@ Give an event publisher the narrowest access modifier that still lets the code o
|
|||
|
||||
- `local` when only the declaring object raises the event. This is the common case and the default choice.
|
||||
- `internal` when another object in the same app raises it — typically an internal implementation codeunit raising an event declared on a public facade codeunit. The facade object stays public so dependent extensions can name it in `[EventSubscriber(...)]`; the publisher stays `internal` so only the implementation decides when the event fires.
|
||||
- `public` only when a *different app* must raise the event — a hub or event-bus codeunit in a foundation app that sibling apps signal through, where `internal` cannot reach across the app boundary. This is a deliberate caller contract, not a concession to subscribers, and it is maintained like any other public API.
|
||||
|
||||
Subscribers are unaffected by either choice. A non-public publisher also keeps the freedom to add a parameter later, which a public publisher gives up — see `add-new-event-parameters-at-the-end`.
|
||||
Subscribers are unaffected by any of these choices. A non-public publisher also keeps the freedom to add a parameter later, which a public publisher gives up — see `add-new-event-parameters-at-the-end`.
|
||||
|
||||
See sample: `declare-event-publishers-local-or-internal.good.al`.
|
||||
|
||||
|
|
@ -34,7 +35,7 @@ See sample: `declare-event-publishers-local-or-internal.good.al`.
|
|||
|
||||
An event publisher declared with no access modifier — or widened to public — in the belief that dependent extensions need that to subscribe. They do not. Two consequences follow. Any dependent extension can now call the publisher directly, firing every subscriber outside the owning routine's control flow, on state the publisher never prepared and with an `IsHandled` answer nobody reads. And because the publisher is a public procedure, it is a caller contract: narrowing it back to `local` or `internal` after release is itself a breaking change, so the mistake is not cheaply reversible.
|
||||
|
||||
Detection: an `[IntegrationEvent]` or `[BusinessEvent]` procedure whose declaration carries no `local` or `internal` modifier.
|
||||
Detection: an `[IntegrationEvent]` or `[BusinessEvent]` publisher that is public although every raiser is in its own app — typically raised only from its declaring object. A publisher deliberately made public so another app can raise it is not this anti-pattern; do not report it. When the surrounding repository or API context does not reveal whether an external raiser is intended, treat the public modifier as intentional rather than reporting it.
|
||||
|
||||
The mirror-image anti-pattern belongs to the reviewer, human or agent: recommending that a publisher be made public so extensions can subscribe, or reporting a `local`/`internal` publisher as unreachable dead code. Both readings mistake raising for subscribing. Neither should be raised as a finding.
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue