Add analyzer to detect commented code

[ernestasjuska]

Hi,

Please add analyzer that can detect commented code. At least in our company, we have many programmers that are very used to old Navision practices and would rather comment out code than just delete it and use Git functionality.

Detecting code in comments could be troublesome because sometimes comments could contain examples and more importantly it would involve parsing.
I would suggest the opposite. Require the comments to be either documentation comments (starting with ///) or "todo" style comments with format // TODO: My message here (where TODO could be TODO, FIXME, NOTE, etc.).

Multiline "todo" comments can be either (examples from Todo Tree):

    // TODO: item 1 line 1
    //  item 1 line 2

or

    /* TODO: item 1 line 1
    item 1 line 2 */

or

    // TODO: item 1 line 1
    // TODO: item 2 line 1

Also allow all comments following directly after the "todo" comment:

    // TODO: Fix this later.
    // NOTE: Reasons ....
    /*
    ThisDoesNotWorkForReasons();
    */

Thanks.

[Arthurvdv]

I like this rule!
During Code Reviews commented code without an explanation is a red flag for me, would be better is the LinterCop would point these out for me with a info/warning.

I'm doubting a bit on the verbs that this rule should check on, is this list below complete?

• BUG – a known bug that should be corrected.
• FIXME – should be corrected.
• HACK – a workaround.
• TODO – something to be done.
• UNDONE – a reversal or "roll back" of previous code.
• XXX – warn other programmers of problematic or misguiding code, "dirty code" that needs improving

[Arthurvdv]

@ernestasjuska I've been thinking about the viability of this rule, and not sure howto resolve the license part on the examples below. This would be a false positive and not sure on howto handle this at this moment.

// ------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.
// ------------------------------------------------------------------------------------------------

namespace System.Environment.Configuration;

/// <summary>Advanced settings exposes integration events raised at Advance Settings Page open.</summary>
codeunit 9202 "Advanced Settings"
{
    Access = Public;
    InherentEntitlements = X;
    InherentPermissions = X;

    /// <summary>Notifies that the Open General Setup Experience has been invoked.</summary>
    /// <param name="PageID">The Page ID of the page been invoked.</param>
    /// <param name="Handled">The flag which if set, would stop executing the OpenGeneralSetupExperience of the assisted setup guide.</param> 
    [IntegrationEvent(false, false)]
    internal procedure OnBeforeOpenGeneralSetupExperience(var PageID: Integer; var Handled: Boolean)
    begin
    end;
}

https://github.com/microsoft/BCApps/blob/main/src/System%20Application/App/Advanced%20Settings/src/AdvancedSettings.Codeunit.al

// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License. See LICENSE in the project root for license information.
codeunit 82567 ADLSE
{
    /// <summary>
    /// This is the main facade of Azure Data Lake Storage Export (ADLSE).
    /// </summary>

    Access = Public;

    /// <summary>
    /// This is the event which represents a successful export of a batch of 
    /// records for a single table to the data lake.
    /// </summary>
    /// <param name="TableID">
    /// The table number whose data was exported.
    /// </param>
    /// <param name="LastTimeStampExported">
    /// The value for the TimeStamp field for the last record that was synced.
    /// </param>
    [IntegrationEvent(false, false)]
    internal procedure OnTableExported(TableID: Integer; LastTimeStampExported: BigInteger)
    begin
    end;
}

https://github.com/Bertverbeek4PS/bc2adls/blob/main/businessCentral/app/src/ADLSE.Codeunit.al