New Rule0075: Record Get Procedure Arguments

[Arthurvdv]

Discussion: #85

Thank you @rvanbekkum for providing the test cases!

I could use some feedback on this new rule, as my intention is to have set the default severity as a warning for this new rule, where I want to prevent possible false positives.

Some remarks on this PR:

Implicit conversation

procedure GetSalesOrder(DocumentNo: Code[20])
var
    SalesOrder: Record "Sales Header";
begin
    SalesOrder.Get(1, DocumentNo);
end;

Not the best example, but I want to allow this in this rule (and maybe have LC0003 report on this).

For now I have these implicit conversation in place, where I don't have a lot of examples where the parameter type is different then the field type on the table. Could use some input on this if I need to add or remove some conversions here.

Parameter in .Get()	Table field type
Integer	Option
Integer	BigInteger
Integer	Enum
BigInteger	Duration
Code	Text
Text	Code
String(literal)	Text
String(literal)	Code

String(literal) can be a hard coded value directly in code or for example a Label value.

Enums
Currently I expect the provided Enum to be exactly the same as the Enum declaration of the field. Are there exceptions on this I need to take into account?

Setup Table
For a setup table, where I expect the Singleton Pattern, a .Get() call without any values should be sufficiënt. I've now created the check that when a table has one a single primary key of type Code, the rule will handle this as a setup table. Potentially this could raise a false positive on a table with a single primary key of another type, like for example Text?

Length of Code/Text field
Currently the rule will not raise on a Code[20] argument on a Code[10] field, which can possibly create a overflow. I'm looking into handling this with the LC0051 rule.

[tinestaric]

Thank you very much for doing this. I'm sorry, I wasn't able to bring our changes over in a timely manner, but I'm still trying my best to resolve the internal issues, so I can get back to contributing.

As for this rule, here's a few of my views.
Enum:
I like requiring that it's the same enum. I would even avoid the implicit Integer -> Enum conversion. If someone wants to execute a Get(Int), where PK is an Enum, they can do the Get(Enum.FromInt(Int)). I also don't like the Int -> Option, but okay, there I don't really see a choice if you got the argument passed as an integer parameter...

Setup tables:
Our approach was to simply exit out of the rule if the Get invocation has 0 arguments. Realistically, the chance of accidentally creating Get(), when the table is not a setup table, is pretty low.

Length
We do check that the argument length needs to be shorter than the key field length. I think it's important. I'm not against LC0051 covering it, but the benefit of this rule covering it is that the error message would be more "in line" with what I'd expect.

I couldn't contribute this one directly, but while it's public, you can still have a look at our implementation of this rule here:
https://github.com/Companial/CompanialCop/blob/main/CompanialCopAnalyzer/Design/Rule0028AnalyzeGetInvocations.cs

[Arthurvdv]

@tinestaric, Thank you for taking the time to review this rule, very much appreciated!

I somehow didn't realize that this rule was already created in the CompanialCop 😳 where I now created this rule from scratch (sorry).

You're remark on the Enum is valid. I forgot you can easily cast an Int to the proper Enum value with the FromInt(), so I'm going to take out allowing this impliciet conversation directly.

I'll leave the check for the setup table in place for now. If this raises false positives, or any other unwanted side-effects, I'll probably take the same approach as you suggested to skip analyzing the .Get() method when zero arguments are provided.

On the length, I think I can cover more exotic scenario with the logic in the LC0051 rule. For those creative minds out there, like the example below. I don't think this code below is a good idea, but I'm also withhold on raise an warning on this.

procedure MyProcedure()
var
    SalesHeader: Record "Sales Header";
    Prefix: Code[5];
    DocumentNo: Code[15];
begin
    SalesHeader.Get("Sales Document Type"::Order, StrSubstNo(Prefix, DocumentNo));
end;