microsoft / AL

Home of the Dynamics 365 Business Central AL Language extension for Visual Studio Code. Used to track issues regarding the latest version of the AL compiler and developer tools available in the Visual Studio Code Marketplace or as part of the AL Developer Preview builds for Dynamics 365 Business Central.
MIT License
744 stars 245 forks source link

Misleading error message - CodeCop #4492

Open ghost opened 5 years ago

ghost commented 5 years ago

Dear Development Team,

It took as a long time what this error message means - it's very misleading.

image

The issue here actually is that the variable name should be TempAggregatedAssistedSetup and not TempAggregatedAssistSetup

To reproduce create the following code in a new codeunit

codeunit 50101 CompanyInfoAssistedSetup
{
    [EventSubscriber(ObjectType::Table, Database::"Aggregated Assisted Setup", 'OnRegisterAssistedSetup', '', true, true)]
    local procedure AggregatedAssistedSetup_OnRegisterAssistedSetup(var TempAggregatedAssistSetup: Record "Aggregated Assisted Setup" temporary)
    Var
        CompanyInformation: Record "Company Information";
    begin
        TempAggregatedAssistSetup.AddExtensionAssistedSetup(
            page::CompanyInfoWi, 'Setup Company Information', true, CompanyInformation.RecordId(),
            GetCompanyInformationSetStatus(TempAggregatedAssistSetup), ''
        );
    end;

    local procedure GetCompanyInformationSetStatus(AggregatedAssistSetup: Record "Aggregated Assisted Setup"): Integer

    var
        CompanyInformation: Record "Company Information";
    begin
        with AggregatedAssistSetup do begin
            if CompanyInformation.Get() then
                if (CompanyInformation.Name = '') or (CompanyInformation."E-Mail" = '') then
                    Status := Status::"Not Completed"
                else
                    Status := Status::Completed
            else
                Status := Status::"Not Completed";
            exit(Status);

        end;
    end;

}

The error message should tell us that the issue is the naming of the variable and not the missing member

AL Extension version: 2.0.56865

Business Central version: image

JohanStenberg100 commented 5 years ago

Hi @martonnagy, the issue is that your parameter inside the local procedureis of the wrong name, that's why it's stating it can't find what to bind it to. Let me if we can adjust the error message somehow.

ghost commented 5 years ago

Hi @JohanStenberg100, but why the variable name is wrong? It's a variable so I can name it...but the message is saying that it can't find it. As soon as name it "properly" the error message goes away...

JohanStenberg100 commented 5 years ago

@martonnagy they actually have to match, the procedure publishing the event's arguments must match the subscriber's arguments, that's how it's designed. Now when you are aware of this - do you still think that the error message needs to be improved?

ghost commented 5 years ago

Hi @JohanStenberg100, got it - sorry, you're right! In this case the error message should tell us what are the correct parameters for the Event or maybe the IntelliSense ToolTip could give us the same information:

image

JohanStenberg100 commented 5 years ago

Hi again @martonnagy I agree, that would be a nice improvement. Let's keep this issue open and we'll consider this.

ghost commented 5 years ago

Cool stuff! Really appreciated 👍