microsoft / AL

Home of the Dynamics 365 Business Central AL Language extension for Visual Studio Code. Used to track issues regarding the latest version of the AL compiler and developer tools available in the Visual Studio Code Marketplace or as part of the AL Developer Preview builds for Dynamics 365 Business Central.
MIT License
744 stars 245 forks source link

AA0198 should also observe procedure parameters #6488

Closed ChrisBlankDe closed 2 years ago

ChrisBlankDe commented 3 years ago

Title AA0198 should also observe procedure parameters

Description CodeCop Rule AA0198 helps us to not use identical names for local and global variables. But currently it only observe global and local variables and not the paramters of an procedure.

Reason for the rule If parameters have the same name as global variables, this can lead to the same problems as if local and global variables have the same name.

Bad code sample Example of what bad code the rule should catch:

codeunit 50100 MyCodeunit
{
    procedure MyProcedure(VarName: Boolean)
    begin
    end;

    var
        VarName: Boolean;
}

Good code sample Example of what code should look like:

codeunit 50100 MyCodeunit
{
    procedure MyProcedure(VarName: Boolean)
    begin
    end;

    var
        AnotherVarName: Boolean;
}
jwikman commented 3 years ago

@qutreson any updates on this one? It's quite annoying when it happens...

jwikman commented 2 years ago

@qutreson what happened with this one? 🤔

fvet commented 2 years ago

Seems to be fixed in runtime 9.2 by the new AA0244 rule

dzzzb commented 2 years ago

AA0244 also catches event publisher arguments though, which IMO aren't helpful to diagnose, as since there cannot be a body of the publisher, we can't possibly get confused and use the wrong local vs global...!

dsaveyn commented 2 years ago

AA0244 also catches event publisher arguments though, which IMO aren't helpful to diagnose, as since there cannot be a body of the publisher, we can't possibly get confused and use the wrong local vs global...!

We are also running into this. For example the following event in a page: image

The only way to solve this is obsolete the event and create a new one...

dzzzb commented 2 years ago

Can't you just #pragma away the warning? Hopefully it will be fixed, so no need to disrupt your solution in the meantime.

dzzzb commented 2 years ago

=> https://github.com/microsoft/AL/issues/7081

JesperSchulz commented 2 years ago

The fix for this issue has been checked in to the master branch. It will be available in the bcinsider.azurecr.io/bcsandbox-master Docker image starting from platform build number 42332 and VS Code Extension Version 10.0.640052.

If you don’t have access to these images you need to become part of the Ready2Go program: aka.ms/readytogo

For more details on code branches and docker images please read: https://blogs.msdn.microsoft.com/nav/2018/05/03/al-developer-previews-multiple-releases-and-github/ https://freddysblog.com/2020/06/25/working-with-artifacts/

NKarolak commented 2 years ago

@JesperSchulz I hope that this will be downgraded into AL Language 9.2.x as well ...?

JesperSchulz commented 2 years ago

Yes, it releases with 9.2.639414 as well.