feat: add LC0098 - Event subscriber name does not match the configured template#408
Conversation
…d template
Adds LinterCop rule LC0098 EventSubscriberNamingPattern with CodeFix
(BatchFixer FixAll). Reports when an event subscriber procedure name does
not match the identifier derived from its [EventSubscriber] attribute via
a single configurable template.
Default template: {Event Source}_{EventName}[_{Element Name}]
This produces exactly the identifier the AL Language extension's "Find
Event" feature generates verbatim (raw source and element names, quoted
when they contain characters that require AL identifier quoting). Teams
that already accept the tooling default satisfy the rule with zero
configuration. Severity Info, category Naming.
Template syntax
---------------
* Token placeholders address three tokens (source object name, event
name, element name) and five output styles:
Raw {Event Source} / {Event Name} / {Element Name}
PascalCase {EventSource} / {EventName} / {ElementName}
camelCase {eventSource} / {eventName} / {elementName}
snake_case {event_source} / {event_name} / {element_name}
kebab-case {event-source} / {event-name} / {element-name}
Raw placeholders emit the token verbatim (no word splitting, no case
transform). Non-raw placeholders route through the shared word
splitter and case-style renderer in ALCops.Common.
* Optional groups [...] are emitted only when every token placeholder
inside resolves to a non-empty value. Handles the common case where
the element name is absent without leaving orphan separators.
* Unknown {...} sequences are passed through verbatim; adding new tokens
(e.g. {ObjectType}, {ObjectId}) is a forward-compatible dictionary
entry in TemplateParser.KnownPlaceholders — no grammar change.
Acronym-aware canonical rendering
---------------------------------
For non-raw styles, each word is rendered to exactly one canonical
form. The renderer follows an "original casing wins" rule: as long as
the source word carries any uppercase character, its casing is
preserved and the acronym registry is not consulted. Only all-lowercase
source words fall back to the registry to recover a canonical casing.
* ID (any casing) -> Id
* Two-letter all-uppercase word -> kept uppercase
(IOLog, not IoLog)
* Word contains any uppercase character -> EnsureUpperFirst
(VAT, Sales, Http, XYZ) (registry NOT consulted;
original casing wins)
* All-lowercase word, registered acronym -> canonical casing from
(vat, odata, acme) registry (VAT, OData, Acme)
* All-lowercase word, not registered -> first-upper, remainder
(amount, header) as-is (Amount, Header)
The comparison is byte-for-byte ordinal, so exactly one spelling per
subscriber is accepted; the CodeFix always suggests that canonical name.
Configuration
-------------
alcops.json:
"SubscriberNameTemplate": "<template>" default: null (built-in)
"KnownAcronyms": ["Acme", "MyCo", ...] merged into built-in list
User acronym entries are merged with the built-in defaults and take
precedence on identical case-insensitive keys. Entries only affect
all-lowercase source words: KnownAcronyms cannot re-cast Microsoft-
or partner-owned identifiers that already carry an uppercase signal
(field "VAT Amount" stays VATAmount regardless of a "Vat" entry;
field "vat amount" becomes VATAmount when VAT is registered and
VatAmount when it is not).
Safety guards
-------------
LC0098 never emits a diagnostic that would move the violation from
itself to another rule or to a compile error:
* AL304 length guard: the canonical name is capped at 120 characters
(MaxAlIdentifierLength). Longer suggestions are suppressed. A W1
codebase survey found only two derived names above the limit.
* Duplicate-name collision guard: suppressed when a sibling in the
same containing type already carries the canonical name, or when
another event subscriber in that type would compute to the same
canonical name. Self-identity is checked via ISymbol.Equals (AL
allows method overloading; name-based self-filtering is unsafe).
The guard is deliberately conservative — any sibling name clash
suppresses, even when signatures technically differ.
* Unresolvable source objects and obsolete methods are skipped.
Shared infrastructure in ALCops.Common
--------------------------------------
* Helpers/AcronymRegistry.cs — built-in defaults + user merge
* Helpers/IdentifierCaseStyle.cs — case-style enum
* Helpers/IdentifierNameRenderer.cs — word splitter + per-word renderer
Placed in Common so future identifier-generating rules can consume the
same registry and renderer. Full unit-test coverage for both new helpers
under ALCops.Common.Test/Helpers/.
Interaction with LC0092
-----------------------
LC0092 (NamingPattern) also targets EventSubscriber methods via its
regex-based EventSubscriber sub-target. The two rules are orthogonal:
structural template (LC0098) vs. character-class pattern (LC0092), and
their settings are decoupled. When the default LC0098 template produces
a quoted identifier because the source object name begins with a
non-letter, LC0092's default ^[A-Z] sees the opening quote — teams that
hit this should adjust their EventSubscriber pattern accordingly. Both
instruction files carry the cross-reference.
CodeFix
-------
Renames only the subscriber's declaration identifier via
SyntaxNode.ReplaceToken; call sites are not touched because subscribers
are wired by the [EventSubscriber] attribute, not by name. The preferred
name is passed via diagnostic.Properties from analyzer to CodeFix so the
CodeFix does not need to reload settings or re-resolve the referenced
object. SupportsFixAll = true; FixAll routes through
WellKnownFixAllProviders.BatchFixer.
Compatibility
-------------
Full netstandard2.1 support — no net8.0-exclusive SDK APIs are used.
Tests
-----
LC0098 project contributes 15 fixtures; ALCops.Common adds unit tests for
the "original casing wins" renderer semantics.
Closes ALCops#406
…asing as preferred Add accepted-name set handling for uppercase acronym words (for example LCY and Lcy), keep CodeFix suggestion on original casing, and extend renderer/analyzer tests plus schema/instruction docs.
Arthurvdv
left a comment
Arthurvdv
left a comment
There was a problem hiding this comment.
PR #408 Review — LC0098 EventSubscriberNamingPattern
Summary: Solid implementation overall. Built on net10.0 and netstandard2.1, all 33 LC0098 analyzer/CodeFix tests pass, but 2 of the PR's own new unit tests fail, and the collision guard is case-sensitive where the AL compiler is case-insensitive.
🔴 Critical — PR's own AcronymRegistry tests fail: duplicate case-insensitive keys in DefaultAcronyms
src/ALCops.Common/Helpers/AcronymRegistry.cs:35,40
DefaultAcronyms contains both "Bom"/"BoM" and "Uom"/"UoM". The registry keys on ToUpperInvariant() with last-entry-wins, so "Bom"/"Uom" are dead entries. dotnet test on the PR head: Failed: 2 (Default_ExposesEveryEntryFromDefaultAcronyms, Create_WithNullUserList_ReturnsDefaultsOnly — Expected: "Bom" But was: "BoM").
Fix: pick one canonical casing per key, or adjust the tests, but as written the duplicates are pointless since only one survives Create().
🟠 Major — Collision guard is case-sensitive; AL duplicate-method detection is case-insensitive
src/ALCops.LinterCop/Analyzers/EventSubscriberNamingPattern.cs:176,187
WouldCollideInContainingType compares sibling names with StringComparison.Ordinal. If a sibling is named mypublisher_onsomething() and the preferred name is MyPublisher_OnSomething, the guard misses it and applying the CodeFix produces a duplicate-method compile error. SDK's MethodSymbol.HasSameSignature uses SemanticFacts.IsSameName (case-insensitive), matching repo convention. (The ordinal check at line 74 is correct — that one intentionally enforces casing.)
Fix: use SemanticFacts.IsSameName() for both collision checks.
🔵 Minor — Resx description documents a wrong default template
src/ALCops.LinterCop/ALCops.LinterCopAnalyzers.resx:430
Says '{Event Source}_{EventName}[{Element Name}]'; actual default is '{Event Source}_{Event Name}[_{Element Name}]'. Placeholder spelling is semantically significant (PascalCase vs raw), and the _ in the optional group matters. Schema description is correct.
✅ Conventions verified OK
Analyzer structure (matches LC0092/LC0097), EnumProvider usage, IsObsolete, attribute argument extraction (indices match SDK EventSubscriber order, quoted names covered), per-compilation caching/performance, template parser robustness (unclosed braces/brackets degrade gracefully, bounded cross-product), netstandard2.1 (builds clean, correct #if pattern), CodeFix structure (BatchFixer, properties handoff, trivia preserved), descriptor/resx/schema/settings patterns, all 33 LC0098 tests pass with valid markers.
Generated with Claude Fable 5 via GitHub Copilot CLI
- AcronymRegistry: store ordered variants per upper-invariant key, add
TryGetVariants; user entries displace defaults per key; reorder
DefaultAcronyms so canonical (BoM, UoM) precedes additional variants
- IdentifierNameRenderer: HasAnyUpper branch accepts every registered
variant alongside the source's original casing
- EventSubscriberNamingPattern: WouldCollideInContainingType now uses
SemanticFacts.IsSameName; AL treats duplicate methods case-insensitively
- Resx: correct default template placeholders to
{Event Source}_{Event Name}[_{Element Name}]
- Tests: +4 multi-variant registry tests, +1 case-insensitive collision
regression fixture
|
Addresses three review findings on LC0098 (EventSubscriberNamingPattern) and its shared 1.
|
|
Thans for looking into the feedback. Let's get this merged :-) |
2 participants
feat: add LC0098 - Event subscriber name does not match the configured template
Adds LinterCop rule LC0098 EventSubscriberNamingPattern with CodeFix (BatchFixer FixAll).
The rule reports when an event subscriber procedure name does not match the identifier derived from its [EventSubscriber] attribute using one configurable template.
Default template:
This default matches the AL Language extension Find Event output verbatim (raw source and element names, quoted when required). Teams already using the tooling default get zero-config alignment. Severity is Info, category Naming.
Template syntax
Acronym-aware rendering
For non-raw styles, rendering now produces an accepted-name set:
Per-word rules:
Configuration
alcops.json properties:
KnownAcronyms semantics:
Example:
If source contains LCY and KnownAcronyms includes Lcy:
Safety guards
LC0098 suppresses diagnostics when rename would be unsafe or invalid:
Shared infrastructure in ALCops.Common
Interaction with LC0092
LC0098 and LC0092 are orthogonal: structural template validation vs regex naming validation.
Both can report independently.
CodeFix
Renames only subscriber declaration identifier via syntax token replacement.
PreferredName is passed from analyzer to CodeFix through diagnostic properties, so CodeFix does not recompute naming input.
Supports FixAll via BatchFixer.
Compatibility
Full netstandard2.1 support; no net8-only SDK dependency required.
Tests (updated)
Implements #406