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

spurious AA0181 (FindSet must only be used with Next) on helper function named FindSet() #6546

Closed dzzzb closed 2 years ago

dzzzb commented 3 years ago

1. Describe the bug I made a helper function named FindSet(), which compiles fine, but it seems the rule AA0181 sees the word FindSet and falsely concludes I'm breaking it.

I'm not! The function takes a var record on which I will iterate with Next() later.

Renaming my function silences this spurious warning.

2. To Reproduce This code gives the bug:

local procedure FindSet(var InstallmentPlanTBSL: Record "Installment Plan TBSL"; RecordId: RecordId): Boolean
begin
    InstallmentPlanTBSL.SetRange("Record ID", RecordId);
    exit(InstallmentPlanTBSL.FindSet(false));
end;

local procedure FindSetAfter(var InstallmentPlanTBSL: Record "Installment Plan TBSL"; RecordId: RecordId; FirstDueDate: Date): Boolean
begin
    InstallmentPlanTBSL.SetFilter("Due Date", '>%1', FirstDueDate);
    exit(FindSet(InstallmentPlanTBSL, RecordId));
end;

image

Renaming the helper function avoids it:

local procedure Dennis(var InstallmentPlanTBSL: Record "Installment Plan TBSL"; RecordId: RecordId): Boolean
begin
    InstallmentPlanTBSL.SetRange("Record ID", RecordId);
    exit(InstallmentPlanTBSL.FindSet(false));
end;

local procedure FindSetAfter(var InstallmentPlanTBSL: Record "Installment Plan TBSL"; RecordId: RecordId; FirstDueDate: Date): Boolean
begin
    InstallmentPlanTBSL.SetFilter("Due Date", '>%1', FirstDueDate);
    exit(Dennis(InstallmentPlanTBSL, RecordId));
end;

image

3. Expected behavior No warning, because I'm not doing anything wrong here, and the analyser should only analyse calls to Record.FindFirst(), not users' functions with the same name.

4. Actual behavior See above.

5. Versions:

dzzzb commented 2 years ago

To confirm, I can still replicate this, and have now isolated it from the original project/context:

table 50100 Test
{
    fields
    {
        field(20; "Record ID"; RecordId) { }
        field(30; "Due Date"; Date) { }
    }
}

codeunit 50100 Test
{
    trigger OnRun()
    var
        Test: Record Test;
        NullRecordId: RecordId;
    begin
        FindSetAfter(Test, NullRecordId, WorkDate());
    end;

    local procedure FindSet(var Test: Record Test; RecordId: RecordId): Boolean
    begin
        Test.SetRange("Record ID", RecordId);
        exit(Test.FindSet(false));
    end;

    local procedure FindSetAfter(var Test: Record Test; RecordId: RecordId; FirstDueDate: Date): Boolean
    begin
        Test.SetFilter("Due Date", '>%1', FirstDueDate);
        exit(FindSet(Test, RecordId));
    end;
}

image

Name: AL Language
Id: ms-dynamics-smb.al
Description: AL development tools for Dynamics 365 Business Central
Version: 8.2.550913
Publisher: Microsoft
VS Marketplace Link: https://marketplace.visualstudio.com/items?itemName=ms-dynamics-smb.al
dzzzb commented 2 years ago

still an issue with AL 8.4.586670

perhaps related: https://github.com/microsoft/AL/issues/6962 for unrelated procs named SetLoadFields()

mazhelez commented 2 years ago

Thanks for reporting this issue. Sorry we haven’t completed it yet, but we’ve had to prioritize elsewhere. We’re planning to give the CodeCop engine and its rules an overhaul in a future major release. Thanks for your patience.