Skip to content

Commit

Permalink
New Rule0087: Use IsNullGuid (#882)
Browse files Browse the repository at this point in the history
  • Loading branch information
Arthurvdv authored Jan 18, 2025
1 parent 98ede68 commit 3e6b7d7
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 1 deletion.
35 changes: 35 additions & 0 deletions BusinessCentral.LinterCop.Test/Rule0087.cs
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyGuid: Guid;
begin
if MyGuid = [|''|] then;
end;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyGuid: Guid;
begin
MyGuid := [|''|];
end;
}
76 changes: 76 additions & 0 deletions BusinessCentral.LinterCop/Design/Rule0087UseIsNullGuid.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
5 changes: 5 additions & 0 deletions BusinessCentral.LinterCop/LinterCop.ruleset.json
Original file line number Diff line number Diff line change
Expand Up @@ -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."
}
]
}
10 changes: 10 additions & 0 deletions BusinessCentral.LinterCop/LinterCopAnalyzers.Generated.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
9 changes: 9 additions & 0 deletions BusinessCentral.LinterCop/LinterCopAnalyzers.resx
Original file line number Diff line number Diff line change
Expand Up @@ -894,4 +894,13 @@
<data name="Rule0086PageStyleDataTypeDescription" xml:space="preserve">
<value>Adopting the use of the new PageStyle datatype allows to more easily get the supported pagestyles via IntelliSense and avoids incorrect behaviour when a typo is made in hardcoded strings or label variables.</value>
</data>
<data name="Rule0087UseIsNullGuidTitle" xml:space="preserve">
<value>Use IsNullGuid() to check for empty Guid values.</value>
</data>
<data name="Rule0087UseIsNullGuidFormat" xml:space="preserve">
<value>Use 'IsNullGuid({0})' method instead of comparing the Guid '{0}' with {1}, as this will result in a runtime error.</value>
</data>
<data name="Rule0087UseIsNullGuidDescription" xml:space="preserve">
<value>When checking if a Guid is empty, avoid using direct comparisons such as MyGuid = '', as this will result in a runtime error due to the invalid Guid format. Instead, use the IsNullGuid() method to perform the check safely and correctly.</value>
</data>
</root>
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -239,4 +239,5 @@ For an example and the default values see: [LinterCop.ruleset.json](./BusinessCe
|[LC0083](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0083)|Use new Date/Time/DateTime methods for extracting parts.|Info|
|[LC0084](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0084)|Use return value for better error handling.|Info|
|[LC0085](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0085)|Use the (CR)LFSeparator from the "Type Helper" codeunit.|Info|
|[LC0086](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0086)|Use the new `PageStyle` datatype instead string literals.|Info|14.0|
|[LC0086](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0086)|Use the new `PageStyle` datatype instead string literals.|Info|14.0|
|[LC0087](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0087)|Use `IsNullGuid()` to check for empty Guid values.|Warning|

0 comments on commit 3e6b7d7

Please sign in to comment.