Warning on using fields not set by SetLoadFields/ AddLoadFields

[fvet]

Title Please provide a CodeCop warning if record fields are used after a record is fetched - where the record fetch is preceded by SetLoadFields/ AddLoadFields. Developers will definitly forget to add the new fields to the SetLoadFields/ AddLoadFields. (especially in parts of code where the function spans multiple pages) and it seems more logic including these already in the SetLoadFields/ AddLoadFields then to force a JIT loading.

Description Following the sample code on https://docs.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-partial-records, I decided to add one more field to the function ComputeArithmeticMean.

procedure ComputeArithmeticMean(): Decimal;
    var
        Item: Record Item;
        SumTotal: Decimal;
        Counter: Integer;
    begin
        Item.SetLoadFields(Item."Standard Cost");
        if Item.FindSet() then begin
            repeat
                SumTotal += Item."Standard Cost";
                SumTotal += Item."Unit Cost"; ///// ADDED FIELD
                Counter += 1;
            until Item.Next() = 0;
            exit(SumTotal / Counter);
        end;
    end;

If I understand the documentation correctly, only the 'Standard Cost' (added via SetLoadFields) will be fetched and any other fields (Unit Cost) will only be loading JIT. I would expect the compiler to warn me in case of a record.field usage in combination with a record.SetLoadFields/ record.AddLoadFields where the field has not been included, so we can avoid the JIT loading.

Reason for the rule If fields are used after data is fetched - where the data fetch is conditioned by SetLoadFields/ AddLoadFields - developers will definitly forget to add the new fields to the SetLoadFields/ AddLoadFields (especially in parts of code where the function spans multiple pages), resulting in additional JIT loading.

Bad code sample Example of what bad code the rule should catch:

procedure ComputeArithmeticMean(): Decimal;
    var
        Item: Record Item;
        SumTotal: Decimal;
        Counter: Integer;
    begin
        Item.SetLoadFields(Item."Standard Cost");
        if Item.FindSet() then begin
            repeat
                SumTotal += Item."Standard Cost";
                SumTotal += Item."Unit Cost"; ///// ADDED FIELD
                Counter += 1;
            until Item.Next() = 0;
            exit(SumTotal / Counter);
        end;
    end;

Good code sample Example of what code should look like:

procedure ComputeArithmeticMean(): Decimal;
    var
        Item: Record Item;
        SumTotal: Decimal;
        Counter: Integer;
    begin
        Item.SetLoadFields(Item."Standard Cost", Item."Unit Cost");
        if Item.FindSet() then begin
            repeat
                SumTotal += Item."Standard Cost";
                SumTotal += Item."Unit Cost"; ///// ADDED FIELD
                Counter += 1;
            until Item.Next() = 0;
            exit(SumTotal / Counter);
        end;
    end;

[kalberes]

I think it makes sense, at least to me.

[PhDuck]

@fvet this sounds like a good suggestion, will talk with someone who has more experience writing CodeCop, they could help estimate how hard it would be.

Regarding this particular example I would like to point out a smart optimization that I wrote while implementing Partial Records. When you are looping over rows, like in the above example, an incorrect estimate will only cause a single JIT load, when you call NEXT we will correct the SQL query to also fetch the field that caused a JIT load, so after first iteration the "Unit Cost" field will also by eager loaded, not triggering JIT loads.

[PhDuck]

I'm working on this and have a prototype that we will try to merge into a future version of the AL extension.

[PhDuck]

@qutreson we can close this issue since this was implemented ages ago in rule 0242.