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

Rule137DoNotDeclareVariablesThatAreUnused ArgumentNullException with empty options #5076

Closed MODUSCarstenScholling closed 4 years ago

MODUSCarstenScholling commented 5 years ago

Describe the bug A combination of an Option parameter with empty string and local variable declaration causing an ArgumentNullException in CodeCop.

To Reproduce

procedure AnalyzerException(someOptions: Option FirstOption,,LastOption): Boolean
var
    removeMeToRemoveExceptions: Boolean;
begin
    exit(removeMeToRemoveExceptions);
end;

The above procedure causes the following exception: Analyzer 'Microsoft.Dynamics.Nav.CodeCop.Design.Rule137DoNotDeclareVariablesThatAreUnused' threw an exception of type 'System.ArgumentNullException' with message 'System.ArgumentNullException: Value cannot be null.

If you either remove the local variable OR give a proper name to the empty option, the exception disappears.

Exception

procedure AnalyzerException(someOptions: Option FirstOption,,LastOption): Boolean
var
    removeMeToRemoveExceptions: Boolean;
begin
    exit(removeMeToRemoveExceptions);
end;

NO Exception

procedure AnalyzerException(someOptions: Option FirstOption,SecondOption,LastOption): Boolean
var
    removeMeToRemoveExceptions: Boolean;
begin
    exit(removeMeToRemoveExceptions);
end;

NO Exception

procedure AnalyzerException(someOptions: Option FirstOption,,LastOption): Boolean
begin
    exit(removeMeToRemoveExceptions);
end;

Expected behavior No exception in first example procedure.

Versions:

Example app: AnalyzerException.zip

tinfister commented 5 years ago

Try to use someOptions: Option FirstOption,"",LastOption;

This warning was weird to us to because it didn't highlight the option with wrong options, instead reported problem in app.json file.

MODUSCarstenScholling commented 5 years ago

Well, it's not the option or the option values that are the problem, but rather the fact that something like that, which is valid code, must not lead to an exception without getting information about the code position. Fixing is our smallest problem.

MarcHansenMicrosoft commented 4 years ago

Fixed

MODUSCarstenScholling commented 4 years ago

Thank you!