al-code-outline icon indicating copy to clipboard operation
al-code-outline copied to clipboard

Remove With Usage Bug when procedure Name is identical

Open pri-kise opened this issue 2 years ago • 3 comments

I have a table where a procedure is defined.

procedure DoMagic(var PTETAble: Record "PTE Table")
begin
   //Magic Happens
end;

Then I have the following Codenit

codeunit 50000 "PTE Magic"
{
    Access = Internal;
    InherentPermissions = X;
    TableNo = "PTE Table";

    trigger OnRun()
    begin
        DoMagic(Rec); // Gets replaced with Rec.DoMagic(Rec); --> This shouldn't happen
    end;    
    procedure DoMagic(var PTETAble: Record "PTE Table")
    begin
        DoMagic(Rec);
    end;
}

When the CleanUp Action resolves the with Usage, then it should respect Overloads on the current object and don't change this code there.

Currently DoMagic(Rec); inside the RunTrigger of the Codeunit is replaced with the implementation on the Table object which results in the following code Rec.DoMagic(Rec);.

pri-kise avatar Oct 02 '23 08:10 pri-kise

The same did happen for me on a NavigatePage where I had a SourceTable defined. I had a global variable with the Name Step and the Rec had Field Name with the Name Step. Every instance of Step was replaced by Rec.Step.

pri-kise avatar Oct 02 '23 09:10 pri-kise

This is very interesting case, I need to think how to fix it. The problem is caused by different behaviour of the compiler when app.json contains "NoImplicitWith" in "features" property.

With this value being there, the priority of methods is correct and select the closes available one, which is the method in the codeunit. If "NoImplicitWith" is not in the app.json, then the priority is different and compiler selects method from the record. Unfortunately when "NoImplicitWith" is specified, then compiler does not understand field names entered without "Rec." in page fields or OnRun trigger. That's why "Remove WITH" command compiles code without this setting as it needs to find information about data types of syntax nodes and decide if prefix needs to be added.

It looks like I need to add additional checks and try to find if there is any local variable or method that should have a priority before "Rec." is added.

anzwdev avatar Oct 08 '23 16:10 anzwdev

I've spend some time thinking about it and I am not sure how to solve the problem as both current command behaviour and your request are correct in different cases.

If you right click on "DoMagic" inside your OnRun trigger and select "Go to definition", you will be moved to the DoMagic procedure below. But if you change code to look like below, then "Go to definition" will take you to the table:

 trigger OnRun()
    begin
        with (Rec) do
            DoMagic(Rec);
    end;    

When you don't have "NoImplicitWith" in the "features" property in the app.json, compiler adds "invisible" "with Rec" to the OnRun trigger, which results in using procedure from the table. During the command run, I am compiling code without this property, so Rec is added to all page fields and inside triggers even if you have it specified in the app.json. It has been done to support scenario when developers are converting old code (in which DoMagic would call procedure from the table), as that was the reason why the function had been created.

I can remove this behaviour, use "features" property from the app.json and if "NoImplicitWith" is enabled, then show message that ImplicitWith won't be fixed. But I am guessing that the main reason why you are running it is to fix page fields.

It seems that there is no good solution if you already started modifying old code and added methods with the same names to pages or codeunits, because now you might have a mix of old code which expects to call tables procedures and new one that wants to call closer ones declared in the same page or codeunit.

anzwdev avatar Oct 14 '23 18:10 anzwdev