StefanMaron / BusinessCentral.LinterCop

Community driven code linter for AL (MS Dynamics 365 Business Central)
https://stefanmaron.com
MIT License
77 stars 31 forks source link

LC0044 - False positives for SqlTimestamp #467

Closed jwikman closed 10 months ago

jwikman commented 10 months ago

I get a, at least in my opinion, false positive when everything is identical, except that one field has SqlTimestamp = true set in one of the tables (the source of TransferFields). This is very intentional and should not raise LC0044. I cannot set the RowVersion of a record anyway, so having that on the target table does not make sense...

What properties are being looked at to identify the fields as conflicting?

My scenario is that we've got a setup table, in this sample connection details to an Azure Storage. And everytime time this is being used, we are logging this. And we want to log the setup details as it was when it was used. To achieve this we use the RowVersion of the setup record, but in the old way of specifying a BigInteger field with SqlTimestamp = true (this was created ages ago, before we had RowVersion on all tables). So, every time someone is using the setup, we check if we already have logged the setup or if the setup has been changed since last time and then save a new "snapshot" of the setup.

Source Table:

table 70314096 "QWESR IQCM Azure Storage"
{
    Access = Public;
    Caption = 'Azure Storage Setup';
    DataPerCompany = true;
    fields
    {
        field(1; "Code"; Code[20])
        {
            Caption = 'Code';
            DataClassification = CustomerContent;
            NotBlank = true;
            TableRelation = "QWESR IQ Communication Method";
        }
        field(9; "Record Time Stamp"; BigInteger)
        {
            Caption = 'Record Time Stamp';
            DataClassification = CustomerContent;
            SqlTimestamp = true;
        }
        ...
    }
    keys
    {
        key(Key1; "Code")
        {
        }
    }
}

Target Table:

table 70314097 "QWESR IQCM Az. St. Snapshot"
{
    Access = Public;
    Caption = 'Azure Storage Communication Method Snapshot';
    DataClassification = CustomerContent;
    DataPerCompany = true;

    fields
    {
        field(1; "Code"; Code[20])
        {
            Caption = 'Code';
            NotBlank = true;
            TableRelation = "QWESR IQ Communication Method";
        }
        field(9; "Snapshot Id"; BigInteger)
        {
            Caption = 'Snapshot Id';
        }
        ...
    }
    keys
    {
        key(Key1; "Snapshot Id")
        {
        }
        key(Key2; "Code")
        {
        }
    }
}

The TransferFields() call:

        if not IQCMAzStSnapshot.Get(IQCMAzureStorage."Record Time Stamp") then begin
            IQCMAzStSnapshot.Init();
            IQCMAzStSnapshot.TransferFields(IQCMAzureStorage); // Emits LC0044
            IQCMAzStSnapshot.Insert();
        end;

Bottom line: Yes, I can use pragma or SkipFieldsNotMatchingType parameter of TransferFields, but I still believe that this is a false positive that could be fixed instead. :)

(This might be a bit related to #466)

Arthurvdv commented 10 months ago

You're right, it's indeed related to #466.

The problem here is not the SqlTimestamp = true, but the fields don't have the samen name.

field(9; "Record Time Stamp"; BigInteger)
field(9; "Snapshot Id"; BigInteger)

I'm going to close this one so we can continue the discussion further in #466.

jwikman commented 10 months ago

The problem here is not the SqlTimestamp = true, but the fields don't have the samen name.

Oh, of course. I should've realized that... 😳

Let's continue discussion in other thread then.