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

LC0050 triggers when trying to add a case insensitive filter, LC0051 triggers when the proposed solution is implemented. #544

Closed rknabben closed 7 months ago

rknabben commented 8 months ago

When I want to set a filter on Table A (Text; Length:100), based on the field in Table B (Text; Length 100), this works without any problems. There can't be an overflow, as both texts are 100 in length.

However, if you want the filter to be case insensitive

Customer.SetFilter(Address,'@%1',MyTable.Address);

LC0050 will trigger and suggest this code:

Customer.SetFilter(Address,'%1',StrSubstNo('@%1',MyTable.Address));

After doing this, LC0051 triggers (correctly) as StrSubstNo returns a text without a length. so lets use a better solution;

Customer.SetFilter(Address,'%1','@' + MyTable.Address);

Combining the Suggestions of LC0050 and LC0051, The only solution would be :

Customer.SetFilter(Address,'%1',copyStr(StrSubstNo('@%1',MyTable.Address),1,100));

However, as this would potentially strip the last character of the actual filter, in favor of the control character @, this would make the filter potentially incorrect as only the first 99 characters of the MyTable.Address are considered in this case. (when using '%1' it would even strip 2 characters, including the last wildcard-character!)

So there is no correct syntax to correctly set this filter.

I believe LC0051 should not trigger for the control / joker characters in a filter, as these won't ever cause overflows?

Or maybe not at all when setting a filter, because even if the filter would be longer than the field length, this is still not an overflow, it just won't give any results, as the record simply can never match. (so that would be an expectable result)

Arthurvdv commented 8 months ago

You're right, the Customer.SetFilter(Address,'%1',StrSubstNo('@%1',MyTable.Address)); won't cause a overflow (the @-sign does not take up a place as a character of the filter string). I'll look into this if I have a way to accurate handle these specific characters.

Know that it's possible to write this SetFilter easier with leaving out the first '%1' part, like this:

Customer.SetFilter(Address, StrSubstNo('@%1', MyTable.Address));
Arthurvdv commented 7 months ago
pageextension 50100 "My Customer List" extends "Customer List"
{
    trigger OnOpenPage()
    var
        Dimension: Record Dimension;
        RecCode: Code[20];
        RecDescription: Text[100];
    begin
        RecCode := 'ABCDEFGHIJKLMNOPQRST'; // Fill variable with 20 characters
        RecDescription := 'ABCDEFGHIJKLMNOPQRSTUVWXZYABCDEFGHIJKLMNOPQRSTUVWXZYABCDEFGHIJKLMNOPQRSTUVWXZYABCDEFGHIJKLMNOPQRSTUV'; // Fill variable with 100 characters

        Dimension.SetFilter(Code, '%1', StrSubstNo('@%1', RecCode)); // raise diagnostic
        Dimension.SetFilter(Description, '%1', StrSubstNo('@%1', RecDescription)) // do not raise diagnostic
    end;
}

This is getting more complex than I expected. The @ operator behaves differently on de Code field then an Text field. This makes sense, know that a Code field case-insensitive, where a Text field is case-sensitive.

In the example above the rule then should raise an diagnostic on one of the two lines. This means I have to expand to rule to validate against the target type to implement a different way of validating.

rknabben commented 7 months ago

Hmm. Just learned something today, I did not know the code validation actually differs from the text validation. (Although your explanation makes perfect sense)

Arthurvdv commented 7 months ago

Apparently on a Text field you can't have a overflow it seems.

pageextension 50100 "My Customer List" extends "Customer List"
{
    trigger OnOpenPage()
    var
        Dimension: Record Dimension;
        RecCode: Code[20];
        RecDescription: Text[100];
    begin
        RecDescription := 'ABCDEFGHIJKLMNOPQRSTUVWXZYABCDEFGHIJKLMNOPQRSTUVWXZYABCDEFGHIJKLMNOPQRSTUVWXZYABCDEFGHIJKLMNOPQRSTUV'; // Fill variable with 100 characters
        Dimension.SetFilter(Description, '%1', 'SomeValueHere' + StrSubstNo('%1&%1', RecDescription))
    end;
}

This example works without a runtime error, so I'm going to completely ignore Text fields on the LC0051 rule.

Arthurvdv commented 7 months ago

The v0.30.14 version of the LinterCop is now released, where I believe this issue is now resolved. If you still encounter issues, feel free to reopen this issue (or create new one).