Validate EventSubscriber parameter signatures against publisher overloads

[frottke]

Suggestion: Validate EventSubscriber parameter signatures against publisher overloads

Description

This suggestion is related to #145, but covers a different case.

Issue #145 asks for a warning for unused parameters in regular procedures. For EventSubscriber methods, unused parameters need to be handled more carefully. In an event subscriber, a parameter can be unused in the procedure body but still be required to identify the intended event publisher signature.

Therefore, this suggestion is not to warn about unused EventSubscriber parameters. Instead, it suggests a rule that validates whether the EventSubscriber parameter signature matches an existing event publisher signature exactly.

Problem

In AL, multiple event publisher procedures can have the same name but different parameter signatures. This effectively means that the event publisher name is overloaded.

If an EventSubscriber does not declare all parameters of the intended publisher because some of them are not used, the subscriber can accidentally match another publisher overload with the same event name. This can lead to the subscriber being attached to a different publisher location than intended.

Example

codeunit 50000 "Sales Price Update"
{
    procedure UpdateDocumentPrices(var SalesHeader: Record "Sales Header")
    var
        SalesLine: Record "Sales Line";
    begin
        OnBeforeUpdateSalesPrices(SalesHeader);

        SalesLine.SetRange("Document Type", SalesHeader."Document Type");
        SalesLine.SetRange("Document No.", SalesHeader."No.");

        if SalesLine.FindSet() then
            repeat
                RecalculateLinePrice(SalesLine);
            until SalesLine.Next() = 0;
    end;

    procedure UpdateCampaignPrices(var SalesHeader: Record "Sales Header"; CampaignNo: Code[20])
    var
        IsHandled: Boolean;
    begin
        OnBeforeUpdateSalesPrices(SalesHeader, CampaignNo, IsHandled);

        if IsHandled then
            exit;

        ApplyCampaignPrices(SalesHeader, CampaignNo);
    end;

    local procedure RecalculateLinePrice(var SalesLine: Record "Sales Line")
    begin
    end;

    local procedure ApplyCampaignPrices(var SalesHeader: Record "Sales Header"; CampaignNo: Code[20])
    begin
    end;

    [IntegrationEvent(false, false)]
    local procedure OnBeforeUpdateSalesPrices(
        var SalesHeader: Record "Sales Header";
        CampaignNo: Code[20];
        var IsHandled: Boolean)
    begin
    end;

    [IntegrationEvent(false, false)]
    local procedure OnBeforeUpdateSalesPrices(var SalesHeader: Record "Sales Header")
    begin
    end;
}

Problematic subscriber:

codeunit 50031 "Sales Price Update Sub"
{
    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Sales Price Update", 'OnBeforeUpdateSalesPrices', '', false, false)]
    local procedure BlockCampaignPriceUpdateForReleasedOrder(var SalesHeader: Record "Sales Header")
    begin
        if SalesHeader.Status = SalesHeader.Status::Released then
            Error('Campaign price update is not allowed for released sales documents.');
    end;
}

The subscriber is intended to run before ApplyCampaignPrices, but it only declares SalesHeader.

Because another publisher overload exists with exactly this signature, the subscriber matches the one-parameter publisher instead of the intended campaign-specific publisher.

Expected subscriber:

codeunit 50032 "Sales Price Update Sub Fixed"
{
    [EventSubscriber(ObjectType::Codeunit, Codeunit::"Sales Price Update", 'OnBeforeUpdateSalesPrices', '', false, false)]
    local procedure BlockCampaignPriceUpdateForReleasedOrder(
        var SalesHeader: Record "Sales Header";
        CampaignNo: Code[20];
        var IsHandled: Boolean)
    begin
        if SalesHeader.Status = SalesHeader.Status::Released then
            Error('Campaign price update is not allowed for released sales documents.');
    end;
}

Even if CampaignNo and IsHandled are not used in the subscriber body, they are part of the intended publisher signature. Declaring them makes the subscription unambiguous.

Suggested rule

For every method marked with [EventSubscriber], resolve all matching event publishers by:

• object type
• object id or object name
• event name
• element name, if specified

Then compare the EventSubscriber method parameters with the available publisher signatures.

The EventSubscriber should use a parameter signature that exists as a publisher signature.

Diagnostic

Report a diagnostic when an EventSubscriber uses a parameter combination that cannot be matched clearly to exactly one existing publisher signature.

Suggested message:

The parameter signature of this event subscriber does not clearly match a single event publisher signature. Declare the full signature of the intended publisher to avoid subscribing to another overload with the same event name.

Relation to unused parameter warnings

This rule should be separated from a general unused-parameter warning.

For regular procedures, unused parameters may be a code smell.

For EventSubscriber methods, unused parameters can be intentional and necessary because they define which overloaded event publisher signature is subscribed to.

Therefore, a general unused-parameter warning should either ignore EventSubscriber methods or treat them differently.

Note: AI only helped me phrase this text. The thoughts and ideas behind it are my own.

[Arthurvdv]

Hi @frottke, thanks for ceating an discussion here and it's looks at first site a great suggestion. Before we dig into the rule itself, I think something might not be quite right with the example, and I wanted to check with you rather than assume.

When I take the Sales Price Update codeunit from your example and try to compile it, it doesn't compile.

The two OnBeforeUpdateSalesPrices procedures both carry the [IntegrationEvent] decorator and that combination produces compiler error AL0155:

A member of type Method with name 'OnBeforeUpdateSalesPrices' is already defined in Codeunit 'Sales Price Update' by ....

To be clear, AL does support procedure overloading in general, so two regular procedures with the same name and different signatures are fine. But that doesn't seem to extend to event publishers: as soon as both same-named procedures are marked [IntegrationEvent], the compiler rejects them with AL0155. If that's correct, then the "two overloaded event publishers with the same name in one object" scenario the rule is built around wouldn't actually be possible to create, which makes me unsure how we'd reproduce, validate, or test the rule.

So a couple of questions, and it's entirely possible I'm missing something:

• Could it be that the example just needs a small adjustment, and the real scenario you ran into was shaped a bit differently?
• Was the real-world case perhaps more complex, for example involving publishers spread across different objects, base/extension scenarios, or [BusinessEvent] / [InternalEvent] variants?

If you could share a minimal example that actually compiles and reproduces the ambiguity you're describing, that would help us a lot in understanding the exact situation and figuring out whether (and how) an analyzer rule could catch it.

Thanks again for the raising an discussion here and looking forward to hearing more 🙋

[frottke]

Hi @Arthurvdv,

You are right. I had only checked the example in VSC and did not see an inline error at that point. After running package, I now get the compiler error as well.

For context, we got to this topic while discussing whether it actually makes sense for LC0095 to check event subscribers as well. During that discussion, we considered whether overloaded event publishers could become a problem when a subscriber signature does not include all parameters from the publisher.

I thought I had successfully reproduced that case with the example above. Apparently, however, the example is not actually compilable.

Given that event publisher overloads are not allowed, there is no real scenario left for the issue I described. Without overloads, the subscriber cannot accidentally bind to another publisher with the same name but a different signature.

Thanks for pointing it out, I totally missed it.

[Arthurvdv]

Thanks for the response @frottke!

Agreed on the conclusion: since AL rejects same-named [IntegrationEvent] publishers within the same object (AL0155), the overload-ambiguity scenario can't actually occur, so there's nothing for a rule to catch here. I'll go ahead and close this.

For the record, the underlying question that sparked this, whether LC0095 should also inspect event subscribers, is still a valid and useful thread and worth keeping as an archive here.

I appreciated you taking the time to raise an discussion here. That's exactly the kind of input that makes the project better and more often than not these explorations lead somewhere useful even when a specific case maybe doesn't pan out.

So please keep them coming. If you spot something else, have an idea, or run into a case that feels off, feel free to open a new discussion any time. Always happy to take a look 🙋