Al: AA0214 - The record XXX should be modified before saving to the database.

[fvet]

Not sure about what warning AA0214 means. I've this part of code, resulting in error AA0214. What's wrong with this function?

 local procedure ProcessMessage(precMessage: Record "NVT b-Alert Read Message")
    var
        lrecObcIntegration: Record "NVT OBC Integration";
        ldatPrevMessage: DateTime;
    begin
        //     Processes a b-Alert message and imports it into the OBC integration table

        ldatPrevMessage := 0DT;

        lrecObcIntegration.Reset;
        lrecObcIntegration.SetCurrentkey(Trailer, "From DateTime");
        lrecObcIntegration.SetRange(Trailer, precMessage.Trailer);
        lrecObcIntegration.SetRange(Vehicle, '');
        if lrecObcIntegration.FindLast then begin
            if lrecObcIntegration."From DateTime" < precMessage."Time Stamp" then begin
                ldatPrevMessage := lrecObcIntegration."From DateTime";
                lrecObcIntegration.Validate("To DateTime", precMessage."Time Stamp");
                lrecObcIntegration.Modify;
            end
        end;

[nicolassaleron]

I also have this warning in procedures without Insert/Modify/Delete/Rename: these procedures are just reading the database.
Quite strange...

[DBiernat]

Same for me, even in the newest AL Language extension (5.0.232930) this warning occurs. Sometimes twice for the same variable.

[AskeHolst]

I have disabled AA0214 for now. I like the idea, but the current implementation is broken.
I checked about 100 AA0214 warnings a not a single one of them was anything to warn about.

[atoader]

@MarcHansenMicrosoft

[stefanoPrimo]

same here:

The record Customer should be modified before saving to the database.AL(AA0214)
But I'm only reading it, not modifying.
P.S. sorry the useless code

[Random-PasserBy]

Also merely using a function with a var parameter triggers a warning. Independent of its contents.

If this can't be fixed it would be practical to be able to individually ignore the "var" check, not lose to the other (then fixed) functionality as well while ignoring AA0214.

[TheDenSter]

Not just reading, even when the local variable is modified, it still tells me to modify it (before you hiss at me I am not actually inserting Location records :), I've modified the code here to prevent publishing client sensitive information)

I even get the error twice:

I don't get the message to begin with, what are you trying to catch here? I have like 50 instances of this error in a very small app and I don't see anything wrong with any of the code.

[rdebath]

Nice variant here: Four copies (On the two variable definitions) of:

The record Field should be modified before saving to the database. 
The record TempField should be modified before saving to the database. 

A Filtered list of the 'Field' table is copied to 'TempField' and then "processed".

[stijn9830]

You can add line in settings.json
"al.ruleSetPath": "_BC.ruleset.json"

An put this _BC.ruleset.json in root directory to ignore this warnings.
This will not help solving the bug of course, and the risk is you can forget them once bug is solved

[JSebastianN]

The record Rec2 should be modified before saving to the database.AL(AA0214)

Here is another one.

My I suggest running these rules on the base code before releasing them.

        trigger OnValidate()
        var
            Rec2: Record "ITF Template Setup";
        begin
            Rec2.SetRange(Usage, Rec.Usage);
            if Rec2.FindFirst() then
                Error(OnlyOneTxt, Rec2.Name);
        end;

[mjmatthiesen]

Rule should be junked.

My ruleset.json has been growing exponentially with every update

[MarcHansenMicrosoft]

The fix in latest master branch version.

[AndreasGloeckner]

The fix in latest master branch version.

What exactly did you change? Just deleted the rule?
Btw, is the master branch available for public? This repo's master branch was updated in february ;)

[NKarolak]

@AndreasGloeckner You cannot use the published repo code for testing, but you must pull the latest bc insider docker container (bcsandbox-master). Only this docker container contains the latest VSIX version.

[AndreasGloeckner]

@AndreasGloeckner You cannot use the published repo code for testing, but you must pull the latest bc insider docker container (bcsandbox-master). Only this docker container contains the latest VSIX version.

Didn't want to use it for testing, just wanted to have a look in the source ;) But thanks for your feedback.

[MarcHansenMicrosoft]

We did not removed the rule.
The rules had issues detecting the proper use of a variable this has been improved.

[ElmarFV]

The reports of this issue being fixed have been greatly exaggerated. This is still an issue.

This code…
“ procedure ShowAssemblyRecords(OrderRequisition: Record "WC GAP07 Order Requisition")
var
AssemblyHeader: Record "Assembly Header";
PageManagement: Codeunit "Page Management";
RecordRef: RecordRef;
begin
with OrderRequisition do begin
Clear(AssemblyHeader);
AssemblyHeader.SetRange("WC GAP07 Ord. Req. No.", "No.");
if AssemblyHeader.IsEmpty then exit;
AssemblyHeader.FindFirst();
RecordRef.GetTable(AssemblyHeader);
”
Resulted in this error message…
The record AssemblyHeader should be modified before saving to the database.
{
"resource": "/c:/Repo/Bioworld/GAP07-Contract Print and Kit Types/src/codeunit/WCGAP07CreateAOs.Codeunit.al",
"owner": "_generated_diagnostic_collection_name_#0",
"code": "AA0214",
"severity": 4,
"message": "The record AssemblyHeader should be modified before saving to the database.",
"source": "AL",
"startLineNumber": 99,
"startColumn": 9,
"endLineNumber": 99,
"endColumn": 23
}

Only when I comment out the AssemblyHeader.SetRange("WC GAP07 Ord. Req. No.", "No."); does the error go away.

[mjmatthiesen]

@ElmarFV I would recommend disabling that warning. It's not actually useful to begin with imo. Also, you are identifying your client with the resource folder.

if AssemblyHeader.IsEmpty then exit;
AssemblyHeader.FindFirst();

Personally I would simplify this to
if not AssemblyHeader.FindFirst() then exit;
Though I don't know what optimizations BC does under the hood for each statement, or what caching, so realistically, it may not matter.

[dtkb]

If you have a more specific bug that hasn't been fixed, open a new issue for it, or it won't be.

Agreed that if not FindFirst() makes more sense. Also, the Clear() is totally redundant, since being a new local variable that hasn't been used yet, it's already clear by definition. We only 'needed' that kind of paranoid clear/reset rain-dance when all variables were global as folk were too lazy to make them local, but there's no excuse for that now IMO.