Skip to content

feat(LC0098): support configurable event subscriber type templates - #572

Open
MODUSCarstenScholling wants to merge 2 commits into
ALCops:mainfrom
MODUSCarstenScholling:dev-lc0098-event-subscriber-objecttype
Open

MODUSCarstenScholling wants to merge 2 commits into
ALCops:mainfrom
MODUSCarstenScholling:dev-lc0098-event-subscriber-objecttype

Conversation

@MODUSCarstenScholling

@MODUSCarstenScholling MODUSCarstenScholling commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor
  • Add source and subscriber type templates to the LC0098 settings schema
  • Validate event subscriber names against the configured type templates
  • Cover supported source and subscriber type scenarios with regression fixtures

Implements #529

- Add source and subscriber type templates to the LC0098 settings schema
- Validate event subscriber names against the configured type templates
- Cover supported source and subscriber type scenarios with regression fixtures
@Arthurvdv

Copy link
Copy Markdown
Member

Investigation results

Thanks @MODUSCarstenScholling, this reads well and the {EventSourceType} / {EventSourceType|Text} split is a clean way to keep the self-reference opt-in. Moving SameApplicationObject into Common as a shared helper is a nice bonus. I went through the diff together with the SDK source; here is what I found.

Should fix

1. The new paths only have NoDiagnostic coverage

  • No HasDiagnostic fixture for the type token or the self-reference literal (e.g. a self-subscriber named (Codeunit) under [({EventSourceType|This})] should be reported with (This) as the expected name). NoDiagnostic tests pass even when the template is never evaluated.
  • The extension branch of IsSelfReference (containingObject is IApplicationObjectExtensionTypeSymbol extension → extension.Target) has no fixture at all; the four new fixtures only contain codeunits. A tableextension subscribing to its target table's event would be the natural case.
  • No HasFix fixture is strictly needed since the CodeFix only consumes PreferredName, but one self-reference HasFix case would pin the end-to-end behaviour.

Question

2. What should {EventSourceType|This} produce next to {EventSource}?

The fixtures only use [({EventSourceType|This})], without the object name. With a realistic template from #529 such as ({EventSourceType|This} {EventSource}) {EventName}, a self-subscriber is expected to be (This Self Publisher) OnGetSelf, because {EventSource} still renders. The outcomes listed as acceptable in the issue were (This) OnGetSelf or (Codeunit Self Publisher) OnGetSelf. Is (This Self Publisher) the intended result, or should the self-reference literal replace the whole type + name pair? Either answer is fine with me, but the docs and a fixture should show it explicitly.

Nits

  • ExtendAccepted checks token.SelfReferenceText is not null && isSelfReference and then calls the NamingTokenValue(TokenSegment, …) overload that repeats the same check. Appending token.SelfReferenceText directly removes the overload and the extra isSelfReference plumbing in that branch.
  • The resx, schema and rule doc say the literal is emitted "only when the subscribed object contains the subscriber" but not what is emitted otherwise. One clause ("otherwise the object type is rendered as usual") avoids the reading that nothing is emitted.
  • The header comments of the five rewritten fixtures (WithElementName, RawEventSourceWithSpace, AcronymFromKnownListPreserved, IdAbbreviationNormalized, ElementNameWithPercent) still describe the old default-template names, e.g. WithElementName.al says MyTable_OnAfterValidateEvent_MyField while the marker is now "Ontable_my-table_on-after-validate-event_my-field".
  • TransferFieldsSchemaCompatibility.cs keeps a blank line before the closing class brace after the helper was removed.

Checked and fine

  • SymbolKind names (Table, Codeunit, Page, Report, Query, XmlPort) match the AL ObjectType spellings, so Kind.ToString() is safe for every subscribable object type.
  • IApplicationObjectExtensionTypeSymbol derives from IApplicationObjectTypeSymbol and Target is nullable; the shared helper takes ISymbol?, so the ext.Target.IsSameApplicationObject(...) call sites in PlatformCop are null-safe.
  • CodeFix needs no change (reads PreferredName from the diagnostic properties).
  • ALCopsSettings.cs is unchanged and only the schema description changed, so the parity test is unaffected.
  • Rule doc updated, companion docs PR docs(LC0098): support configurable event subscriber type templates alcops.dev#196 is open, no PR numbers in code comments.

This review was created by Claude (Claude Code) and checked by the maintainer before posting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants