Merge 'prerelease' into master (#885)

* Add support for Primary Key field other then of type Code (#875) * Allow Implicit conversion from Integer to Decimal (#876) * Resolve bug in Rule0027 (#877) * Resolve false positive on Rule0008 (#878) * Resolve InvalidCastException on Rule0053/0053 (#879) * New Rule0087: Use IsNullGuid (#882) * Change Guid to GUID on Documentation for Rule0087
StefanMaron · Jan 20, 2025 · 2c1bd57 · 2c1bd57
1 parent be296ba
commit 2c1bd57
Show file tree

Hide file tree

Showing 20 changed files with 255 additions and 56 deletions.
diff --git a/BusinessCentral.LinterCop.Test/Rule0008.cs b/BusinessCentral.LinterCop.Test/Rule0008.cs
@@ -24,6 +24,7 @@ public async Task HasDiagnostic(string testCase)
 
     [Test]
     [TestCase("1")]
+    [TestCase("2")]
     public async Task NoDiagnostic(string testCase)
     {
         var code = await File.ReadAllTextAsync(Path.Combine(_testCaseDir, "NoDiagnostic", $"{testCase}.al"))

diff --git a/BusinessCentral.LinterCop.Test/Rule0027.cs b/BusinessCentral.LinterCop.Test/Rule0027.cs
@@ -11,16 +11,17 @@ public void Setup()
             "TestCases", "Rule0027");
     }
 
-    // [Test]
-    // [TestCase("1")]
-    // public async Task HasDiagnostic(string testCase)
-    // {
-    //     var code = await File.ReadAllTextAsync(Path.Combine(_testCaseDir, "HasDiagnostic", $"{testCase}.al"))
-    //         .ConfigureAwait(false);
-    //
-    //     var fixture = RoslynFixtureFactory.Create<Rule0027RunPageImplementPageManagement>();
-    //     fixture.HasDiagnostic(code, DiagnosticDescriptors.Rule0027RunPageImplementPageManagement.Id);
-    // }
+    [Test]
+    [TestCase("1")]
+    [TestCase("2")]
+    public async Task HasDiagnostic(string testCase)
+    {
+        var code = await File.ReadAllTextAsync(Path.Combine(_testCaseDir, "HasDiagnostic", $"{testCase}.al"))
+            .ConfigureAwait(false);
+
+        var fixture = RoslynFixtureFactory.Create<Rule0027RunPageImplementPageManagement>();
+        fixture.HasDiagnostic(code, DiagnosticDescriptors.Rule0027RunPageImplementPageManagement.Id);
+    }
 
     [Test]
     [TestCase("1")]

diff --git a/BusinessCentral.LinterCop.Test/Rule0075.cs b/BusinessCentral.LinterCop.Test/Rule0075.cs
@@ -37,8 +37,10 @@ public async Task HasDiagnostic(string testCase)
     [Test]
     [TestCase("ImplicitConversiontIntegerToEnum")]
     [TestCase("ImplicitConversiontLabelToCode")]
+    [TestCase("PrimaryKeyAsInteger")]
     [TestCase("RecordGetBuiltInMethodRecordId")]
     [TestCase("RecordGetCode10ToCode20")]
+    [TestCase("RecordGetDecimalToInteger")]
     [TestCase("RecordGetFieldRecordId")]
     [TestCase("RecordGetGlobalVariable")]
     [TestCase("RecordGetLocalVariable")]

diff --git a/BusinessCentral.LinterCop.Test/Rule0087.cs b/BusinessCentral.LinterCop.Test/Rule0087.cs
@@ -0,0 +1,35 @@
+namespace BusinessCentral.LinterCop.Test;
+
+public class Rule0087
+{
+    private string _testCaseDir = "";
+
+    [SetUp]
+    public void Setup()
+    {
+        _testCaseDir = Path.Combine(Directory.GetParent(Environment.CurrentDirectory)!.Parent!.Parent!.FullName,
+            "TestCases", "Rule0087");
+    }
+
+    [Test]
+    [TestCase("EmptyStringLiteral")]
+    public async Task HasDiagnostic(string testCase)
+    {
+        var code = await File.ReadAllTextAsync(Path.Combine(_testCaseDir, "HasDiagnostic", $"{testCase}.al"))
+            .ConfigureAwait(false);
+
+        var fixture = RoslynFixtureFactory.Create<Rule0087UseIsNullGuid>();
+        fixture.HasDiagnostic(code, DiagnosticDescriptors.Rule0087UseIsNullGuid.Id);
+    }
+
+    [Test]
+    [TestCase("EmptyStringLiteral")]
+    public async Task NoDiagnostic(string testCase)
+    {
+        var code = await File.ReadAllTextAsync(Path.Combine(_testCaseDir, "NoDiagnostic", $"{testCase}.al"))
+            .ConfigureAwait(false);
+
+        var fixture = RoslynFixtureFactory.Create<Rule0087UseIsNullGuid>();
+        fixture.NoDiagnosticAtMarker(code, DiagnosticDescriptors.Rule0087UseIsNullGuid.Id);
+    }
+}
diff --git a/BusinessCentral.LinterCop.Test/TestCases/Rule0008/NoDiagnostic/2.al b/BusinessCentral.LinterCop.Test/TestCases/Rule0008/NoDiagnostic/2.al
@@ -0,0 +1,20 @@
+codeunit 50100 MyCodeunit
+{
+    procedure MyProcedure()
+    var
+        Field: Record Field;
+    begin
+        [|Field.SetRange(FieldName, 'Document No.')|];
+    end;
+}
+
+table 50100 Field
+{
+    fields
+    {
+        field(1; TableNo; Integer) { }
+        field(2; "No."; Integer) { }
+        field(3; TableName; Text[30]) { }
+        field(4; FieldName; Text[30]) { }
+    }
+}
diff --git a/BusinessCentral.LinterCop.Test/TestCases/Rule0027/HasDiagnostic/1.al b/BusinessCentral.LinterCop.Test/TestCases/Rule0027/HasDiagnostic/1.al
@@ -1,45 +1,14 @@
-table 50100 MyTable
-{
-    LookupPageId = MyPage;
-    DrillDownPageId = MyPage;
-
-    fields
-    {
-        field(1; MyField; Integer)
-        {
-
-        }
-    }
-
-    keys
-    {
-        key(Key1; MyField) { }
-    }
-}
-
-page 50100 MyPage
-{
-    PageType = List;
-    SourceTable = MyTable;
-
-    layout
-    {
-        area(Content)
-        {
-            field(MyField; MyField)
-            {
-
-            }
-        }
-    }
-}
-
 codeunit 50100 MyCodeunit
 {
-    local procedure MyProcedure()
+    procedure MyProcedure()
     var
         MyTable: Record MyTable;
     begin
-        [|Page.Run(Page::MyPage, MyTable);|]
+        [|Page.Run(0, MyTable)|];
     end;
+}
+
+table 50100 MyTable
+{
+    fields { field(1; MyField; Integer) { } }
 }
diff --git a/BusinessCentral.LinterCop.Test/TestCases/Rule0027/HasDiagnostic/2.al b/BusinessCentral.LinterCop.Test/TestCases/Rule0027/HasDiagnostic/2.al
@@ -0,0 +1,15 @@
+codeunit 50100 MyCodeunit
+{
+    procedure MyProcedure()
+    var
+        SalesHeader: Record "Sales Header";
+    begin
+        [|Page.Run(Page::MyPage, SalesHeader)|];
+    end;
+}
+
+page 50100 MyPage { }
+table 36 "Sales Header"
+{
+    fields { field(1; MyField; Integer) { } }
+}
diff --git a/BusinessCentral.LinterCop.Test/TestCases/Rule0075/NoDiagnostic/PrimaryKeyAsInteger.al b/BusinessCentral.LinterCop.Test/TestCases/Rule0075/NoDiagnostic/PrimaryKeyAsInteger.al
@@ -0,0 +1,17 @@
+codeunit 50100 MyCodeunit
+{
+    procedure MyProcedure()
+    var
+        MyTable: Record MyTable;
+    begin
+        [|MyTable.Get()|];
+    end;
+}
+
+table 50100 MyTable
+{
+    fields
+    {
+        field(1; "Primary Key"; Integer) { }
+    }
+}
diff --git a/BusinessCentral.LinterCop.Test/TestCases/Rule0075/NoDiagnostic/RecordGetDecimalToInteger.al b/BusinessCentral.LinterCop.Test/TestCases/Rule0075/NoDiagnostic/RecordGetDecimalToInteger.al
@@ -0,0 +1,17 @@
+codeunit 50100 MyCodeunit
+{
+    procedure MyProcedure(MyInteger: Integer)
+    var
+        MyTabe: Record MyTabe;
+    begin
+        [|MyTabe.Get(MyInteger)|];
+    end;
+}
+
+table 50100 MyTabe
+{
+    fields
+    {
+        field(1; MyField; Decimal) { }
+    }
+}
diff --git a/BusinessCentral.LinterCop.Test/TestCases/Rule0087/HasDiagnostic/EmptyStringLiteral.al b/BusinessCentral.LinterCop.Test/TestCases/Rule0087/HasDiagnostic/EmptyStringLiteral.al
@@ -0,0 +1,9 @@
+codeunit 50100 MyCodeunit
+{
+    procedure MyProcedure()
+    var
+        MyGuid: Guid;
+    begin
+        if MyGuid = [|''|] then;
+    end;
+}
diff --git a/BusinessCentral.LinterCop.Test/TestCases/Rule0087/NoDiagnostic/EmptyStringLiteral.al b/BusinessCentral.LinterCop.Test/TestCases/Rule0087/NoDiagnostic/EmptyStringLiteral.al
@@ -0,0 +1,9 @@
+codeunit 50100 MyCodeunit
+{
+    procedure MyProcedure()
+    var
+        MyGuid: Guid;
+    begin
+        MyGuid := [|''|];
+    end;
+}
diff --git a/BusinessCentral.LinterCop/Design/Rule0008NoFilterOperatorsInSetRange.cs b/BusinessCentral.LinterCop/Design/Rule0008NoFilterOperatorsInSetRange.cs
@@ -67,5 +67,5 @@ private void CheckParameter(IOperation operand, ref IInvocationExpression operat
     }
 
     private static bool ContainsFilterOperators(string parameterString) =>
-        parameterString.IndexOfAny(new[] { '<', '>', '.', '*', '&', '|' }) >= 0 || parameterString.Contains("..");
+        parameterString.IndexOfAny(['<', '>', '*', '&', '|']) >= 0 || parameterString.Contains("..");
 }
diff --git a/BusinessCentral.LinterCop/Design/Rule0027RunPageImplementPageManagement.cs b/BusinessCentral.LinterCop/Design/Rule0027RunPageImplementPageManagement.cs
@@ -51,7 +51,7 @@ private void CheckRunPageImplementPageManagement(OperationAnalysisContext ctx)
             return;
 
         if (operation.TargetMethod.MethodKind != MethodKind.BuiltInMethod ||
-            operation.TargetMethod.Name != "EnqueueBackgroundTask" ||                                           // do not execute on CurrPage.EnqueueBackgroundTask
+            operation.TargetMethod.Name == "EnqueueBackgroundTask" ||                                           // do not execute on CurrPage.EnqueueBackgroundTask
             operation.TargetMethod.ContainingType?.GetTypeSymbol().GetNavTypeKindSafe() != NavTypeKind.Page ||
             operation.TargetMethod.ReturnValueSymbol.ReturnType.GetNavTypeKindSafe() == NavTypeKind.Action ||   // Page Management Codeunit doesn't support returntype Action
             operation.Arguments.Length < 2)

diff --git a/BusinessCentral.LinterCop/Design/Rule0052and0053InternalProceduresNotReferenced.cs b/BusinessCentral.LinterCop/Design/Rule0052and0053InternalProceduresNotReferenced.cs
@@ -144,8 +144,11 @@ public void AnalyzeObjectSyntax(CompilationAnalysisContext compilationAnalysisCo
                     {
                         return;
                     }
-                    IdentifierNameSyntax identifierNameSyntax = (IdentifierNameSyntax)syntaxNode;
-                    if (methodSymbols.ContainsValue(identifierNameSyntax.Identifier.ValueText.ToLowerInvariant()) && TryGetSymbolFromIdentifier(semanticModel, (IdentifierNameSyntax)syntaxNode, SymbolKind.Method, out var methodSymbol))
+                    if (syntaxNode is not IdentifierNameSyntax identifierNameSyntax)
+                    {
+                        return;
+                    }
+                    if (methodSymbols.ContainsValue(identifierNameSyntax.Identifier.ValueText.ToLowerInvariant()) && TryGetSymbolFromIdentifier(semanticModel, identifierNameSyntax, SymbolKind.Method, out var methodSymbol))
                     {
                         if (methodSymbol.IsInternal)
                         {

diff --git a/BusinessCentral.LinterCop/Design/Rule0075RecordGetProcedureArguments.cs b/BusinessCentral.LinterCop/Design/Rule0075RecordGetProcedureArguments.cs
@@ -16,8 +16,8 @@ public class Rule0075RecordGetProcedureArguments : DiagnosticAnalyzer
 
     private static readonly Dictionary<NavTypeKind, HashSet<NavTypeKind>> ImplicitConversions = new()
     {
-        // Integer can be converted to Option and/or BigInteger
-        { NavTypeKind.Integer, new HashSet<NavTypeKind> { NavTypeKind.Option, NavTypeKind.BigInteger } },
+        // Integer can be converted to Decimal, Option and/or BigInteger
+        { NavTypeKind.Integer, new HashSet<NavTypeKind> { NavTypeKind.Decimal, NavTypeKind.Option, NavTypeKind.BigInteger } },
 
         // BigInteger can be converted to Duration
         { NavTypeKind.BigInteger, new HashSet<NavTypeKind> { NavTypeKind.Duration } },
@@ -137,7 +137,7 @@ private static bool IsSingletonTable(ITableTypeSymbol table)
     {
         return table.PrimaryKey.Fields.Length == 1 &&
             table.PrimaryKey.Fields[0].OriginalDefinition.GetTypeSymbol() is { } typeSymbol &&
-            typeSymbol.GetNavTypeKindSafe() == NavTypeKind.Code;
+            (typeSymbol.GetNavTypeKindSafe() == NavTypeKind.Code || SemanticFacts.IsSameName(table.PrimaryKey.Fields[0].Name, "Primary Key"));
     }
 }
 #endif
diff --git a/BusinessCentral.LinterCop/Design/Rule0087UseIsNullGuid.cs b/BusinessCentral.LinterCop/Design/Rule0087UseIsNullGuid.cs
@@ -0,0 +1,76 @@
+using BusinessCentral.LinterCop.Helpers;
+using Microsoft.Dynamics.Nav.CodeAnalysis;
+using Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics;
+using Microsoft.Dynamics.Nav.CodeAnalysis.Symbols;
+using Microsoft.Dynamics.Nav.CodeAnalysis.Utilities;
+using System.Collections.Immutable;
+
+namespace BusinessCentral.LinterCop.Design;
+
+[DiagnosticAnalyzer]
+public class Rule0087UseIsNullGuid : DiagnosticAnalyzer
+{
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
+        ImmutableArray.Create(DiagnosticDescriptors.Rule0087UseIsNullGuid);
+
+    public override void Initialize(AnalysisContext context) =>
+        context.RegisterOperationAction(new Action<OperationAnalysisContext>(this.AnalyzeEqualsStatement), OperationKind.BinaryOperatorExpression);
+
+    private void AnalyzeEqualsStatement(OperationAnalysisContext ctx)
+    {
+        if (ctx.IsObsoletePendingOrRemoved() || ctx.Operation is not IBinaryOperatorExpression operation)
+            return;
+
+        // Validate the left operand is a Guid type
+        if (!IsNavTypeKindGuid(operation.LeftOperand))
+            return;
+
+        // Validate the right operand is an empty string literal
+        if (IsEmptyStringLiteral(operation.RightOperand))
+        {
+            ctx.ReportDiagnostic(Diagnostic.Create(
+                DiagnosticDescriptors.Rule0087UseIsNullGuid,
+                operation.RightOperand.Syntax.GetLocation(),
+                GetVariableName(operation.LeftOperand),
+                "''"));
+        }
+    }
+
+    private static bool IsNavTypeKindGuid(IOperation operand)
+    {
+        // Check for a conversion expression with a valid Guid type
+        if (operand.Kind == OperationKind.ConversionExpression &&
+            operand is IConversionExpression conversion &&
+            conversion.Operand is IOperation innerOperation)
+        {
+            return innerOperation.Type.GetNavTypeKindSafe() == NavTypeKind.Guid;
+        }
+
+        return false;
+    }
+
+    private static bool IsEmptyStringLiteral(IOperation operand)
+    {
+        // Ensure the operand is a literal expression of a string type
+        if (operand.Kind == OperationKind.LiteralExpression && operand.Type.IsTextType())
+        {
+            var constantValue = operand.ConstantValue.Value?.ToString();
+            return string.IsNullOrEmpty(constantValue);
+        }
+
+        return false;
+    }
+
+    private static string GetVariableName(IOperation operand)
+    {
+        // Extract the name of the Guid variable, or return an empty string if unavailable
+        if (operand.Kind == OperationKind.ConversionExpression &&
+            operand is IConversionExpression conversion &&
+            conversion.Operand is IOperation innerOperation)
+        {
+            return innerOperation.GetSymbol()?.Name.QuoteIdentifierIfNeeded() ?? string.Empty;
+        }
+
+        return string.Empty;
+    }
+}
diff --git a/BusinessCentral.LinterCop/LinterCop.ruleset.json b/BusinessCentral.LinterCop/LinterCop.ruleset.json
@@ -431,6 +431,11 @@
       "id": "LC0086",
       "action": "Info",
       "justification": "Use the new PageStyle datatype instead string literals."
+    },
+    {
+      "id": "LC0087",
+      "action": "Warning",
+      "justification": "Use IsNullGuid() to check for empty GUID values."
     }
   ]
 }
diff --git a/BusinessCentral.LinterCop/LinterCopAnalyzers.Generated.cs b/BusinessCentral.LinterCop/LinterCopAnalyzers.Generated.cs
@@ -884,4 +884,14 @@ public static class DiagnosticDescriptors
         isEnabledByDefault: true,
         description: LinterCopAnalyzers.GetLocalizableString("Rule0086PageStyleDataTypeDescription"),
         helpLinkUri: "https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0086");
+
+    public static readonly DiagnosticDescriptor Rule0087UseIsNullGuid = new(
+        id: LinterCopAnalyzers.AnalyzerPrefix + "0087",
+        title: LinterCopAnalyzers.GetLocalizableString("Rule0087UseIsNullGuidTitle"),
+        messageFormat: LinterCopAnalyzers.GetLocalizableString("Rule0087UseIsNullGuidFormat"),
+        category: "Design",
+        defaultSeverity: DiagnosticSeverity.Warning,
+        isEnabledByDefault: true,
+        description: LinterCopAnalyzers.GetLocalizableString("Rule0087UseIsNullGuidDescription"),
+        helpLinkUri: "https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0087");
 }