LC0044 - False Positive?

[pri-kise]

I have a table, that looks identical to a BaseApp, where I use transferfields.

Now Image the following.

table 50002 "PTE First Line"
{
    fields
    {
#pragma warning disable LC0044 // Note: Ingore Transferfields Warning
        field(1; "First Header No."; Code[20])
        {
            Caption = 'First Header No.';
            DataClassification = CustomerContent;
            NotBlank = true;
            TableRelation = "PTE First Header";
        }
#pragma warning restore LC0044
        field(5; "Line No."; Integer)
        {
            Caption = 'Line No.';
            DataClassification = SystemMetadata;
        }

    keys
    {
        key(Key1; "First Header No.", "Line No.")
        {
            Clustered = true;
        }
    }
}


table 50004 "PTE Second Line"
{
    fields
    {
#pragma warning disable LC0044 // Note: Ingore Transferfields Warning
		field(1; "Second Header No."; Code[20])
		{
			Caption = 'Second Header No.';
			DataClassification = CustomerContent;
			NotBlank = true;
			TableRelation = "PTE Second Header";
		}
#pragma warning restore LC0044
		field(5; "Line No."; Integer)
		{
			Caption = 'Line No.';
			DataClassification = SystemMetadata;
		}

    keys
    {
        key(Key1; "Second Header No.", "Line No.")
        {
            Clustered = true;
        }
    }
}

local procedure CopyFirsToSecond(FirstHeaderNo: Code[20]; SecondHeaderNo)
var
    ToSecondLine: Record  "PTE First Line";
    FromFirstLine: Record  "PTE Second Line";
begin

    FromFirstLine.SetRange("First Header No.", FirstHeaderNo);
    if FromFirstLine.FindSet() then
        repeat
            ToSecondLine.Init();
            ToSecondLine.TransferFields(FirstHeaderNo);
            ToSecondLine."Second Header No." := SecondHeaderNo; //I will receive an warning here although I'd like to ignore this warning for Field 1
            ToJobCalcDocumentText.Insert(true);
        until FromFirstLine.Next() = 0;
end;

Disclaimer:
I know that this example is very simple in this example the Field Names could have been identical, but we have tables where the Fields are different on purpose.

[Arthurvdv]

If i understand correctly, if we mark both fields, that we are aware that the Field Names aren't matching, the rule no longer should raise on the TransferFields line where the tables are coupled.

I'm not sure if the LinterCop can use the #pragma syntax. The complexity here is that these are not part of the field declaration and could be anywhere in the source code (where you for example could have two fields between those pragma's).

I think it's a great idea to have this, so when other fields are added the rule will verify these, but at this moment I have no idea how I could resolve this.

@tinestaric: Do you maybe have any insights on this?

[tinestaric]

Hmm, I see the point. However, I don't know how this could be resolved. As far as I know, #pragma silences the already-raised warnings...

I was thinking if we even need to throw a warning on the TransferFields invocation, since the main purpose is to align the fields, but on the other hand, I also kind of like it, as it helps you pinpoint where the TransferFields are, and perhaps implementation is the way one would like to resolve the issue...

Another option is that we find a way to exclude certain table pairs from being processed in this rule. Though I'm not a fan of having to maintain lists like that...

[tinestaric]

An idea just popped into my mind. Maybe we can get the pragmas during analysis (the same way we get leading/trailing comments), and if there's a pragma to silence this rule, it could skip adding the field into the list it's using to do the comparison...

I'll have some time next week, maybe I can play around a bit with this idea.

[pri-kise]

To be honest, I'm in general very happy about the Warning at the TransferFields call.
This helped to Find places where the TransferField is used.

I only wanted to report this problem, because it's not ideal to add a pragma there as well.

One thought I had - it would help if there were two different rules.
One for the TransferFields and one for the Warning inside the table/tableextension object, then I could lower the TransferFields Warning to an Info.

[tinestaric]

That would be an easy change, It's already using a separate DiagnosticsDescriptior, so it would just need to have a separate number assigned for the error raised at Invocation...

[jwikman]

One thought I had - it would help if there were two different rules. One for the TransferFields and one for the Warning inside the table/tableextension object, then I could lower the TransferFields Warning to an Info.

Yes, that would help.

@pri-kise remember that you also can silence this warning by using the SkipFieldsNotMatchingType parameter of TransferFields(), if you are aware of the differences between those two tables. The downside of that approach is that you don't get this check on those two tables going forward. If you later add conflicting fields no them, this rule won't care...

But, if you use this:
ToSecondLine.TransferFields(FirstHeaderNo,true,true);
you do not need those pragmas.

[Arthurvdv]

Maybe we can get the pragmas during analysis (the same way we get leading/trailing comments), and if there's a pragma to silence this rule, it could skip adding the field into the list it's using to do the comparison...

Should we take into account that there's a reportSuppressedDiagnostics flag on the compiler? Not sure how this is handled if we're also applying logic on the pragma's in the LinterCop itself.

Not sure how complicated we should make this, maybe splitting the rule into two separate rules is the most viable option here?

[jwikman]

maybe splitting the rule into two separate rules is the most viable option here?

I think that is a very good start. 👍

[Arthurvdv]

Id	Title	Raises On
LC0044	Conflicting ID, Name or Type with Table '{0}'	Field in Table(Ext)
LC00xx	TransferFields with conflicting ID, Name or Type with Table '{0}'	TransferField method

I given it some more thought, and doubting if splitting the rule into two separate rules indeed resolve this.

I will receive an warning here although I'd like to ignore this warning for Field 1

In this case, setting a #pragma would hide the LC0044 warning, but the (new) LC00xx rule will still be raised on the TransferFields method (for Field 1). When setting the severity of the LC00xx rule to Hidden/Disabled there will be never a warning on the TransferFields method shown.

I'm unsure if this is the right way forward in the case the rule LC00xx isn't set to Hidden/Disabled. Then this would be becoming confusing again, where perhaps it would be better to look at a different solution?

[tinestaric]

@Arthurvdv
Yeah, you're right, splitting the rules doesn't solve much here.

I did some investigation, I think I have a working proof of concept. You can get directives for each field, so I've added a condition to both PopulateFields() functions, that skips fields that have a pragma for rule 0044. As fields are not added to the list of fields, it won't report a collision, meaning it won't report a warning on Rec.TransferFields(OtherRec).

I'll send you the PoC, to get your thoughts on it.

[Arthurvdv]

Thank you @tinestaric for sharing a the proof of concept code, this was very helpfull!

In the (pre)release version of v0.30.12 of the LinterCop the rule will now detect if one of the fields has a #pragma warning disable LC0044 around it set, then the rule exclude that field(s).

[jwikman]

Thanks @tinestaric and @Arthurvdv, I like this approach! 👍

[Arthurvdv]

The version v0.30.12 of the LinterCop is now released.

@pri-kise, can you confirm this is working for you?

[pri-kise]

I can confirm that works when you ingore every field with a single pragma

Is Working

table 50010 "PTE Template Structure"
{

    fields
    {
#pragma warning disable LC0044 // Note: Ingore Transferfields Warning
        field(1; "Template No."; Code[20])
        {
            Caption = 'Job Template No.';
            DataClassification = CustomerContent;
            NotBlank = true;
            TableRelation = "PTE Template";
        }
#pragma warning restore LC0044
#pragma warning disable LC0044 // Note: Ingore Transferfields Warning
        field(2; "Template Structure No."; Code[20])
        {
            Caption = 'Template Structure No.;
            DataClassification = CustomerContent;
        }
#pragma warning restore LC0044
...
}

Is not working:

table 50010 "PTE Template Structure"
{

    fields
    {
#pragma warning disable LC0044 // Note: Ingore Transferfields Warning
        field(1; "Template No."; Code[20])
        {
            Caption = 'Job Template No.';
            DataClassification = CustomerContent;
            NotBlank = true;
            TableRelation = "PTE Template";
        }
        field(2; "Template Structure No."; Code[20])
        {
            Caption = 'Template Structure No.;
            DataClassification = CustomerContent;
        }
#pragma warning restore LC0044
...
}

But this is okay and can deal with it.
I only wanted to inform you that the wiki about pragma for the LC0044 warning can be as detailled as possible.