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

If local procedure is not used, then no code analysis is performed on its further implementation #6472

Closed rvanbekkum closed 2 years ago

rvanbekkum commented 3 years ago

Edited Description (23-03-2022):

If a local procedure is not used, then no code analysis is applied to its implementation.

Original description:

Title A new code analysis rule that warns developers if arguments for values for label placeholders are missing where they would be expected.

Description If there is a label with placeholders then when it is used in procedures/methods like StrSubstNo, Message and Error, a code analysis rule would warn developers when there are insufficient arguments as values for the placeholders in the label.

Reason for the rule It is hard to spot this error, you typically only find out about it later at runtime.

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

local procedure MyProcedure()
var
    MyText: Text;
    MyLabelMsg: Label 'This is my message with some placeholders: %1 %2', Comment = '%1 = Placeholder A; %2 = Placeholder B';
    MyLabelErr: Label 'This is my error with some placeholders: %1 %2', Comment = '%1 = Placeholder A; %2 = Placeholder B';
begin
    MyText := StrSubstNo(MyLabelMsg); // Missing values for %1 and %2
    Message(MyLabelMsg, 1); // Missing values for %1 and %2 for Message
    Error(MyLabelErr, 1, 2, 3); // Too many values, i.e., no %3 in the label but a third argument is passed
end;

image

Good code sample Example of what code should look like:

local procedure MyProcedure()
var
    MyText: Text;
    MyLabelMsg: Label 'This is my message with some placeholders: %1 %2', Comment = '%1 = Placeholder A; %2 = Placeholder B';
    MyLabelErr: Label 'This is my error with some placeholders: %1 %2', Comment = '%1 = Placeholder A; %2 = Placeholder B';
begin
    MyText := StrSubstNo(MyLabelMsg, 1, 2);
    Message(MyLabelMsg, 1, 2);
    Error(MyLabelErr, 1, 2);
end;
rvanbekkum commented 3 years ago

Related to #6456

rvanbekkum commented 3 years ago

I think this should already work through AA0131, but it does not with the latest AL Language Extension (6.5.413786).

rvanbekkum commented 3 years ago

I see: if the procedure is not used yet, then no further static code analysis is applied to its implementation. Maybe this is by design though. If that is the case, then we can simply close this issue.

rvanbekkum commented 2 years ago

Updated the title accordingly.

mazhelez commented 2 years ago

Thanks for reporting this issue. Sorry we haven’t completed it yet, but we’ve had to prioritize elsewhere. We’re planning to give the CodeCop engine and its rules an overhaul in a future major release. Thanks for your patience.