New rule to check IsHandled set to false

BusinessCentral.LinterCop.Test/Rule0071.cs

@@ -0,0 +1,39 @@
+namespace BusinessCentral.LinterCop.Test;
+
+public class Rule0071
+{
+    private string _testCaseDir = "";
+
+    [SetUp]
+    public void Setup()
+    {
+        _testCaseDir = Path.Combine(Directory.GetParent(Environment.CurrentDirectory)!.Parent!.Parent!.FullName,
+            "TestCases", "Rule0071");
+    }
+
+    [Test]
+    [TestCase("Assignment")]
+    [TestCase("Invocation")]
+    [TestCase("Handled")]
+    public async Task HasDiagnostic(string testCase)
+    {
+        var code = await File.ReadAllTextAsync(Path.Combine(_testCaseDir, "HasDiagnostic", $"{testCase}.al"))
+            .ConfigureAwait(false);
+
+        var fixture = RoslynFixtureFactory.Create<Rule0071DoNotSetIsHandledToFalse>();
+        fixture.HasDiagnostic(code, DiagnosticDescriptors.Rule0071DoNotSetIsHandledToFalse.Id);
+    }
+
+    [Test]
+    [TestCase("Assignment")]
+    [TestCase("Invocation")]
+    [TestCase("NoEventSubscriberParameterReference")]
+    public async Task NoDiagnostic(string testCase)
+    {
+        var code = await File.ReadAllTextAsync(Path.Combine(_testCaseDir, "NoDiagnostic", $"{testCase}.al"))
+            .ConfigureAwait(false);
+
+        var fixture = RoslynFixtureFactory.Create<Rule0071DoNotSetIsHandledToFalse>();
+        fixture.NoDiagnosticAtMarker(code, DiagnosticDescriptors.Rule0071DoNotSetIsHandledToFalse.Id);
+    }
+}

BusinessCentral.LinterCop.Test/TestCases/Rule0071/HasDiagnostic/Assignment.al

@@ -0,0 +1,31 @@
+codeunit 50100 MyCodeunit
+{
+    trigger OnRun()
+    var
+        IsHandled: Boolean;
+    begin
+        OnCodeunitRun(IsHandled);
+    end;
+
+    [IntegrationEvent(false, false)]
+    local procedure OnCodeunitRun(var IsHandled: Boolean)
+    begin
+    end;
+}
+
+codeunit 50101 MySubscriber
+{
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::MyCodeunit, OnCodeunitRun, '', false, false)]
+    local procedure OnCodeunitRun_MyCodeunit(var IsHandled: Boolean)
+    var
+        MyCondition: Boolean;
+    begin
+        MyCondition := Random(2) = 1;
+
+        [|IsHandled := false;|]
+        [|IsHandled := MyCondition;|]
+
+        if MyCondition then
+            [|IsHandled := false;|]
+    end;
+}

BusinessCentral.LinterCop.Test/TestCases/Rule0071/HasDiagnostic/Handled.al

@@ -0,0 +1,24 @@
+// tests that also the less used name "handled" is reported, see https://github.com/search?q=repo%3AStefanMaron%2FMSDyn365BC.Code.History+%22var+Handled%3A+Boolean%22&type=code
+codeunit 50100 MyCodeunit
+{
+    trigger OnRun()
+    var
+        Handled: Boolean;
+    begin
+        OnCodeunitRun(Handled);
+    end;
+
+    [IntegrationEvent(false, false)]
+    local procedure OnCodeunitRun(var Handled: Boolean)
+    begin
+    end;
+}
+
+codeunit 50101 MySubscriber
+{
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::MyCodeunit, OnCodeunitRun, '', false, false)]
+    local procedure OnCodeunitRun_MyCodeunit(var Handled: Boolean)
+    begin
+        [|Handled := false;|]
+    end;
+}

BusinessCentral.LinterCop.Test/TestCases/Rule0071/HasDiagnostic/Invocation.al

@@ -0,0 +1,28 @@
+codeunit 50100 MyCodeunit
+{
+    trigger OnRun()
+    var
+        IsHandled: Boolean;
+    begin
+        OnCodeunitRun(IsHandled);
+    end;
+
+    [IntegrationEvent(false, false)]
+    local procedure OnCodeunitRun(var IsHandled: Boolean)
+    begin
+    end;
+}
+
+codeunit 50101 MySubscriber
+{
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::MyCodeunit, OnCodeunitRun, '', false, false)]
+    local procedure OnCodeunitRun_MyCodeunit(var IsHandled: Boolean)
+    begin
+        MyVarProcedure([|IsHandled|]); // not allowed because MyProcedure -> Param is var
+    end;
+
+    local procedure MyVarProcedure(var Param: Boolean)
+    begin
+        Param := false;
+    end;
+}

BusinessCentral.LinterCop.Test/TestCases/Rule0071/NoDiagnostic/Assignment.al

@@ -0,0 +1,30 @@
+codeunit 50100 MyCodeunit
+{
+    trigger OnRun()
+    var
+        IsHandled: Boolean;
+    begin
+        OnCodeunitRun(IsHandled);
+    end;
+
+    [IntegrationEvent(false, false)]
+    local procedure OnCodeunitRun(var IsHandled: Boolean)
+    begin
+    end;
+}
+
+codeunit 50101 MySubscriber
+{
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::MyCodeunit, OnCodeunitRun, '', false, false)]
+    local procedure OnCodeunitRun_MyCodeunit(var IsHandled: Boolean)
+    var
+        MyCondition: Boolean;
+    begin
+        MyCondition := Random(2) = 1;
+
+        [|IsHandled := true;|]
+
+        if MyCondition then
+            [|IsHandled := true;|]
+    end;
+}

BusinessCentral.LinterCop.Test/TestCases/Rule0071/NoDiagnostic/Invocation.al

@@ -0,0 +1,28 @@
+codeunit 50100 MyCodeunit
+{
+    trigger OnRun()
+    var
+        IsHandled: Boolean;
+    begin
+        OnCodeunitRun(IsHandled);
+    end;
+
+    [IntegrationEvent(false, false)]
+    local procedure OnCodeunitRun(var IsHandled: Boolean)
+    begin
+    end;
+}
+
+codeunit 50101 MySubscriber
+{
+    [EventSubscriber(ObjectType::Codeunit, Codeunit::MyCodeunit, OnCodeunitRun, '', false, false)]
+    local procedure OnCodeunitRun_MyCodeunit(var IsHandled: Boolean)
+    begin
+        [|MyProcedure(IsHandled);|] // allowed because MyProcedure -> Param is not var
+    end;
+
+    local procedure MyProcedure(Param: Boolean)
+    begin
+        Param := false;
+    end;
+}

BusinessCentral.LinterCop.Test/TestCases/Rule0071/NoDiagnostic/NoEventSubscriberParameterReference.al

@@ -0,0 +1,15 @@
+codeunit 50100 MyCodeunit
+{
+    trigger OnRun()
+    var
+        IsHandled: Boolean;
+    begin
+        [|IsHandled := false;|]
+        OnCodeunitRun(IsHandled);
+    end;
+
+    [IntegrationEvent(false, false)]
+    local procedure OnCodeunitRun(var IsHandled: Boolean)
+    begin
+    end;
+}

BusinessCentral.LinterCop/Design/Rule0071DoNotSetIsHandledToFalse.cs

@@ -0,0 +1,79 @@
+using BusinessCentral.LinterCop.AnalysisContextExtension;
+using Microsoft.Dynamics.Nav.CodeAnalysis;
+using Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics;
+using Microsoft.Dynamics.Nav.CodeAnalysis.Semantics;
+using System.Collections.Immutable;
+
+namespace BusinessCentral.LinterCop.Design
+{
+    [DiagnosticAnalyzer]
+    public class Rule0071DoNotSetIsHandledToFalse : DiagnosticAnalyzer
+    {
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create<DiagnosticDescriptor>(DiagnosticDescriptors.Rule0071DoNotSetIsHandledToFalse);
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.RegisterOperationAction(new Action<OperationAnalysisContext>(this.CheckIsHandledAssignments), OperationKind.AssignmentStatement);
+            context.RegisterOperationAction(new Action<OperationAnalysisContext>(this.CheckIsHandledInvocations), OperationKind.InvocationExpression);
+        }
+
+        private void CheckIsHandledInvocations(OperationAnalysisContext ctx)
+        {
+            if (ctx.IsObsoletePendingOrRemoved()) return;
+
+            var invocation = (IInvocationExpression)ctx.Operation;
+            for (int i = 0; i < invocation.Arguments.Length; i++)
+            {
+                // check if IsHandled is passed to another method with var
+                var invocationValue = invocation.Arguments[i].Value;
+                if (invocationValue.Kind == OperationKind.ParameterReferenceExpression)
+                    if (IsIsHandledEventSubscriberParameter(((IParameterReferenceExpression)invocationValue).Parameter))
+                        if (invocation.TargetMethod.Parameters[i].IsVar)
+                            ctx.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.Rule0071DoNotSetIsHandledToFalse, invocationValue.Syntax.GetLocation()));
+            }
+        }
+
+        private void CheckIsHandledAssignments(OperationAnalysisContext ctx)
+        {
+            if (ctx.IsObsoletePendingOrRemoved()) return;
+
+            var assignment = (IAssignmentStatement)ctx.Operation;
+            if (assignment.Target.Kind != OperationKind.ParameterReferenceExpression)
+                return; // check the parameter is assigned a value
+
+            if (!IsIsHandledEventSubscriberParameter(((IParameterReferenceExpression)assignment.Target).Parameter))
+                return;
+
+            if (assignment.Value.ConstantValue.HasValue)
+                if ((bool)assignment.Value.ConstantValue.Value)
+                    return; // check for true assignment
+
+            // any other not true assignment should not be done
+            ctx.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.Rule0071DoNotSetIsHandledToFalse, assignment.Syntax.GetLocation()));
+        }
+
+        private bool IsIsHandledEventSubscriberParameter(IParameterSymbol parameter)
+        {
+            if (parameter.ContainingSymbol is null)
+                return false;
+            // check for event subscriber method
+            if (parameter.ContainingSymbol.Kind != SymbolKind.Method)
+                return false;
+            if (!((IMethodSymbol)parameter.ContainingSymbol).IsEventSubscriber())
+                return false;
+
+            // check for "var IsHandled: Boolean" parameter
+            if (!CheckIsHandledName(parameter.Name) || (parameter.ParameterType.NavTypeKind != NavTypeKind.Boolean) || !parameter.IsVar)
+                return false;
+
+            return true;
+        }
+
+        private bool CheckIsHandledName(string name)
+        {
+            // checks for name(s) used with the "IsHandled Pattern"
+            // "Handled" is also used in the Base / System App, see: https://github.com/search?q=repo%3AStefanMaron%2FMSDyn365BC.Code.History+%22var+Handled%3A+Boolean%22&type=code
+            return (name.ToLower() == "ishandled") || (name.ToLower() == "handled");
+        }
+    }
+}

BusinessCentral.LinterCop/LinterCop.ruleset.json

@@ -351,6 +351,11 @@
       "id": "LC0070",
       "action": "Warning",
       "justification": "Zero index access on 1-based List objects."
+    },
+    {
+      "id": "LC0071",
+      "action": "Info",
+      "justification": "The IsHandled event subscriber parameter reference value should never be set to a value that may be false."
     }
   ]
 }

BusinessCentral.LinterCop/LinterCopAnalyzers.Generated.cs

@@ -67,5 +67,6 @@ public static class DiagnosticDescriptors
         public static readonly DiagnosticDescriptor Rule0061SetODataKeyFieldsWithSystemIdField = new DiagnosticDescriptor(LinterCopAnalyzers.AnalyzerPrefix + "0061", (LocalizableString)new LocalizableResourceString("Rule0061SetODataKeyFieldsWithSystemIdFieldTitle", LinterCopAnalyzers.ResourceManager, typeof(LinterCopAnalyzers)), (LocalizableString)new LocalizableResourceString("Rule0061SetODataKeyFieldsWithSystemIdFieldFormat", LinterCopAnalyzers.ResourceManager, typeof(LinterCopAnalyzers)), "Design", DiagnosticSeverity.Info, true, (LocalizableString)new LocalizableResourceString("Rule0061SetODataKeyFieldsWithSystemIdFieldDescription", LinterCopAnalyzers.ResourceManager, typeof(LinterCopAnalyzers)), "https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0061");
         public static readonly DiagnosticDescriptor Rule0062MandatoryFieldMissingOnApiPage = new DiagnosticDescriptor(LinterCopAnalyzers.AnalyzerPrefix + "0062", (LocalizableString)new LocalizableResourceString("Rule0062MandatoryFieldMissingOnApiPageTitle", LinterCopAnalyzers.ResourceManager, typeof(LinterCopAnalyzers)), (LocalizableString)new LocalizableResourceString("Rule0062MandatoryFieldMissingOnApiPageFormat", LinterCopAnalyzers.ResourceManager, typeof(LinterCopAnalyzers)), "Design", DiagnosticSeverity.Info, true, (LocalizableString)new LocalizableResourceString("Rule0062MandatoryFieldMissingOnApiPageDescription", LinterCopAnalyzers.ResourceManager, typeof(LinterCopAnalyzers)), "https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0062");
         public static readonly DiagnosticDescriptor Rule0069EmptyStatements = new DiagnosticDescriptor(LinterCopAnalyzers.AnalyzerPrefix + "0069", (LocalizableString)new LocalizableResourceString("Rule0069EmptyStatementsTitle", LinterCopAnalyzers.ResourceManager, typeof(LinterCopAnalyzers)), (LocalizableString)new LocalizableResourceString("Rule0069EmptyStatementsFormat", LinterCopAnalyzers.ResourceManager, typeof(LinterCopAnalyzers)), "Design", DiagnosticSeverity.Info, true, (LocalizableString)new LocalizableResourceString("Rule0069EmptyStatementsDescription", LinterCopAnalyzers.ResourceManager, typeof(LinterCopAnalyzers)), "https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0069");
+        public static readonly DiagnosticDescriptor Rule0071DoNotSetIsHandledToFalse = new DiagnosticDescriptor(LinterCopAnalyzers.AnalyzerPrefix + "0071", (LocalizableString)new LocalizableResourceString("Rule0071DoNotSetIsHandledToFalseTitle", LinterCopAnalyzers.ResourceManager, typeof(LinterCopAnalyzers)), (LocalizableString)new LocalizableResourceString("Rule0071DoNotSetIsHandledToFalseFormat", LinterCopAnalyzers.ResourceManager, typeof(LinterCopAnalyzers)), "Design", DiagnosticSeverity.Info, true, (LocalizableString)new LocalizableResourceString("Rule0071DoNotSetIsHandledToFalseDescription", LinterCopAnalyzers.ResourceManager, typeof(LinterCopAnalyzers)), "https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0071");    
     }
 }

BusinessCentral.LinterCop/LinterCopAnalyzers.resx

@@ -750,4 +750,13 @@
   <data name="Rule0070ListObjectsAreOneBasedDescription" xml:space="preserve">
     <value>List objects are 1-based, meaning indexing starts from 1 instead of the conventional 0.</value>
   </data>
+  <data name="Rule0071DoNotSetIsHandledToFalseTitle" xml:space="preserve">
+    <value>Incorrect 'IsHandled' parameter assignment.</value>
+  </data>
+  <data name="Rule0071DoNotSetIsHandledToFalseFormat" xml:space="preserve">
+    <value>Do not set the 'IsHandled' parameter to 'false' or any value that may be evaluated as false.</value>
+  </data>
+  <data name="Rule0071DoNotSetIsHandledToFalseDescription" xml:space="preserve">
+    <value>The 'IsHandled' parameter must always be set to 'true' when handled. Avoid assigning 'false' or any value that may be evaluated as false.</value>
+  </data>
 </root>

README.md

@@ -223,4 +223,5 @@ For an example and the default values see: [LinterCop.ruleset.json](LinterCop.ru
 |[LC0067](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0067)|Set `NotBlank` property to `false` when 'No. Series' TableRelation exists.|Warning|
 |[LC0068](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0068)|Informs the user that there are missing permission to access tabledata.|Info|
 |[LC0069](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0069)|Empty statements should be avoided or should have a leading or trailing comment explaining their use.|Info|
-|[LC0070](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0070)|Zero index access on 1-based List objects.|Warning|
+|[LC0070](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0070)|Zero index access on 1-based List objects.|Warning|
+|[LC0071](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0071)|Incorrect 'IsHandled' parameter assignment.|Info|