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

AA0136 Unreachable code detected - Expected for code after if-else that both result in exit #6800

Closed rvanbekkum closed 2 years ago

rvanbekkum commented 3 years ago

1. Describe the bug If there are statements after an if-else-statement where both paths result in an exit-statement, then I would expect rule AA0136 to report a warning on any statement after the if-else-statement.

2. To Reproduce Steps to reproduce the behavior:

  1. Set up an AL project with the following procedure in a codeunit:
    procedure MyProcedure(Bool: Boolean)
    begin
        if Bool then
            exit
        else
            exit;

        Message('Hello World!');
        exit;
    end;

3. Expected behavior Get a warning from AA0136 that unreachable code was detected, similar to how Visual Studio does this for C# (rule: CS0162) for the same example, e.g.:

        public void MyProcedure(bool Bool)
        {
            if (Bool)
            {
                return;
            }
            else
            {
                return;
            }

            Console.WriteLine('Hello World!');
            return;
        }

For the given example, I would expect a warning on the Message-statement.

4. Actual behavior No warning from AA0136 about unreachable code.

5. Versions:

Final Checklist

Please remember to do the following:

MODUSCarstenScholling commented 3 years ago

I would add a new warning, because an if with an exit should never be followed by an else. Case is a bit different, not sure if it works correct with this warning.

dzzzb commented 3 years ago

an if with an exit should never be followed by an else

I understand the reasoning for this, i.e. that the exit means the else can never be hit, but I still think that in some situations it looks nicer to include the redundant else, e.g. for symmetry. So sue me ;-)

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.