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
728 stars 241 forks source link

Diagnostic AL0780 not working as expected #7793

Open LucaRoesnick opened 1 month ago

LucaRoesnick commented 1 month ago

1. Describe the bug

The diagnostic AL0780 is currently not resulting in a warning or an error. Even though it is supposed to become an error in Business Central 2024 release wave 2.

https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/diagnostics/diagnostic-al780

The diagnostic should result in a warning before runtime 14.0, and should result in an error with runtime 14.0.

2. To Reproduce

table 1000000 TESTAL0780
{
    fields
    {
        field(1; PK; Code[20])
        {
            DataClassification = CustomerContent;
            ToolTip = ' ';
        }
        field(2; ItemNo; Code[20])
        {
            FieldClass = FlowField;
            ToolTip = ' ';
            CalcFormula = lookup(Item."No." where("No." = const('1000')));
        }
        field(3; TestFlowFilter; Code[20])
        {
            FieldClass = FlowFilter;
            ToolTip = ' ';
        }
    }

    trigger OnModify()
    begin
        ItemNo := '2000';
        TestFlowFilter := 'TEST';
    end;
}

3. Expected behavior

The mentioned Code snippet should result in an error or in a warning.

4. Actual behavior

The mentioned Code snippet is not resulting in an error or in a warning.

5. Versions:

NKarolak commented 1 month ago

What happens if you explicitly add AL0780 to a ruleset file (as Warning)? I'm asking due to this: #7787

LucaRoesnick commented 1 month ago

No difference at all. Tried Error as well, and it didn't change anything as well.

someC0d3r commented 5 days ago

(Bump)

Are there any news regarding this issue as BC25 comes closer (preview available expected in 2 weeks)?

thloke commented 4 days ago

Sorry for the time taken to get to this. I finally looked into the history of this error and it's very misleading.

Initially this was introduced to catch the exact case that you refer to where we wanted to prevent assignment to fields that were flowfields. However this was walked back after feedback from our internal app developers (the dev who did the rollback is currently away, so I can't say specifically why. All I have is a statement that "there are legitimate use cases").

However this error code was left in, and it's used in one specific case which the message doesn't even have anything to do with. This case is when you invoke ModifyAll on a record and target a field of FieldClass = FlowField. So in the example given, if there was:

procedure MyProc()
var
    r: Record TESTAL0780;
begin
    r.ModifyAll(r.ItemNo, '2000'); // should invoke AL0780
end;

So in short, the message is very misleading and not at all accurate with regards to how the actual error is being used. If you're not seeing it as a warning now, you won't be getting this as an error in BC25. I will however accept this with the intent to actually make the message accurate.