Skip to content

Commit

Permalink
New Rule0081: Use Rec.IsEmpty() for checking record existence (#833)
Browse files Browse the repository at this point in the history
  • Loading branch information
Arthurvdv authored Dec 14, 2024
1 parent 9650306 commit f0a7234
Show file tree
Hide file tree
Showing 15 changed files with 312 additions and 1 deletion.
43 changes: 43 additions & 0 deletions BusinessCentral.LinterCop.Test/Rule0081.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
namespace BusinessCentral.LinterCop.Test;

public class Rule0081
{
private string _testCaseDir = "";

[SetUp]
public void Setup()
{
_testCaseDir = Path.Combine(Directory.GetParent(Environment.CurrentDirectory)!.Parent!.Parent!.FullName,
"TestCases", "Rule0081");
}

[Test]
[TestCase("OneGreaterThanRecordCount")]
[TestCase("RecordCountEqualsZero")]
[TestCase("RecordCountGreaterThanOrEqualZero")]
[TestCase("RecordCountGreaterThanZero")]
[TestCase("RecordCountLessThanOne")]
[TestCase("RecordCountLessThanOrEqualZero")]
[TestCase("RecordCountLessThanZero")]
[TestCase("RecordCountNotEqualsZero")]
public async Task HasDiagnostic(string testCase)
{
var code = await File.ReadAllTextAsync(Path.Combine(_testCaseDir, "HasDiagnostic", $"{testCase}.al"))
.ConfigureAwait(false);

var fixture = RoslynFixtureFactory.Create<Rule0081UseIsEmptyMethod>();
fixture.HasDiagnostic(code, Rule0081UseIsEmptyMethod.DiagnosticDescriptors.Rule0081UseIsEmptyMethod.Id);
}

[Test]
[TestCase("RecordCountEqualsOne")]
[TestCase("RecordTemporaryCountEqualsZero")]
public async Task NoDiagnostic(string testCase)
{
var code = await File.ReadAllTextAsync(Path.Combine(_testCaseDir, "NoDiagnostic", $"{testCase}.al"))
.ConfigureAwait(false);

var fixture = RoslynFixtureFactory.Create<Rule0081UseIsEmptyMethod>();
fixture.NoDiagnosticAtMarker(code, Rule0081UseIsEmptyMethod.DiagnosticDescriptors.Rule0081UseIsEmptyMethod.Id);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyTable: Record MyTable;
begin
if [|1 > MyTable.Count()|] then;
end;
}

table 50100 MyTable
{
fields
{
field(1; MyField; Integer) { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyTable: Record MyTable;
begin
if [|MyTable.Count() = 0|] then;
end;
}

table 50100 MyTable
{
fields
{
field(1; MyField; Integer) { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyTable: Record MyTable;
begin
if [|MyTable.Count() >= 0|] then;
end;
}

table 50100 MyTable
{
fields
{
field(1; MyField; Integer) { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyTable: Record MyTable;
begin
if [|MyTable.Count() > 0|] then;
end;
}

table 50100 MyTable
{
fields
{
field(1; MyField; Integer) { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyTable: Record MyTable;
begin
if [|MyTable.Count() < 1|] then;
end;
}

table 50100 MyTable
{
fields
{
field(1; MyField; Integer) { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyTable: Record MyTable;
begin
if [|MyTable.Count() <= 0|] then;
end;
}

table 50100 MyTable
{
fields
{
field(1; MyField; Integer) { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyTable: Record MyTable;
begin
if [|MyTable.Count() < 0|] then;
end;
}

table 50100 MyTable
{
fields
{
field(1; MyField; Integer) { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyTable: Record MyTable;
begin
if [|MyTable.Count() <> 0|] then;
end;
}

table 50100 MyTable
{
fields
{
field(1; MyField; Integer) { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyTable: Record MyTable;
begin
if [|MyTable.Count() = 1|] then;
end;
}

table 50100 MyTable
{
fields
{
field(1; MyField; Integer) { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
TempMyTable: Record MyTable temporary;
begin
if [|TempMyTable.Count() = 0|] then;
end;
}

table 50100 MyTable
{
fields
{
field(1; MyField; Integer) { }
}
}
83 changes: 83 additions & 0 deletions BusinessCentral.LinterCop/Design/Rule0081UseIsEmptyMethod.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
using BusinessCentral.LinterCop.AnalysisContextExtension;
using Microsoft.Dynamics.Nav.CodeAnalysis;
using Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics;
using Microsoft.Dynamics.Nav.CodeAnalysis.Syntax;
using Microsoft.Dynamics.Nav.CodeAnalysis.Utilities;
using System.Collections.Immutable;

namespace BusinessCentral.LinterCop.Design
{
[DiagnosticAnalyzer]
public class Rule0081UseIsEmptyMethod : DiagnosticAnalyzer
{
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
ImmutableArray.Create(DiagnosticDescriptors.Rule0081UseIsEmptyMethod);

public override void Initialize(AnalysisContext context) =>
context.RegisterOperationAction(new Action<OperationAnalysisContext>(this.AnalyzeCountMethod), OperationKind.InvocationExpression);

private void AnalyzeCountMethod(OperationAnalysisContext ctx)
{
if (ctx.IsObsoletePendingOrRemoved())
return;

if (ctx.Operation is not IInvocationExpression operation)
return;

if (operation.TargetMethod.MethodKind != MethodKind.BuiltInMethod ||
operation.TargetMethod.Name != "Count" ||
operation.TargetMethod.ContainingSymbol?.Name != "Table")
return;

if (operation.Instance?.GetSymbol() is not IVariableSymbol { Type: IRecordTypeSymbol recordTypeSymbol } || recordTypeSymbol.Temporary)
return;

if (operation.Syntax.Parent is not BinaryExpressionSyntax binaryExpression)
return;

if (IsLiteralExpressionValue(binaryExpression.Left, 0) ||
IsLiteralExpressionValue(binaryExpression.Right, 0))
{
ReportDiagnostic(ctx, operation);
return;
}

if (binaryExpression.OperatorToken.Kind == SyntaxKind.LessThanToken &&
IsLiteralExpressionValue(binaryExpression.Right, 1))
{
ReportDiagnostic(ctx, operation);
return;
}

if (binaryExpression.OperatorToken.Kind == SyntaxKind.GreaterThanToken &&
IsLiteralExpressionValue(binaryExpression.Left, 1))
{
ReportDiagnostic(ctx, operation);
}
}

private static bool IsLiteralExpressionValue(CodeExpressionSyntax codeExpression, int value) =>
codeExpression is LiteralExpressionSyntax { Literal: { Kind: SyntaxKind.Int32SignedLiteralValue } literal }
&& literal.GetLiteralValue() is int literalvalue && literalvalue == value;

private static void ReportDiagnostic(OperationAnalysisContext ctx, IInvocationExpression operation)
{
ctx.ReportDiagnostic(Diagnostic.Create(
DiagnosticDescriptors.Rule0081UseIsEmptyMethod,
operation.Syntax.Parent.GetLocation(),
new object[] { operation.Instance?.GetSymbol()?.Name.QuoteIdentifierIfNeeded() ?? string.Empty }));
}

public static class DiagnosticDescriptors
{
public static readonly DiagnosticDescriptor Rule0081UseIsEmptyMethod = new(
id: LinterCopAnalyzers.AnalyzerPrefix + "0081",
title: LinterCopAnalyzers.GetLocalizableString("Rule0081UseIsEmptyMethodTitle"),
messageFormat: LinterCopAnalyzers.GetLocalizableString("Rule0081UseIsEmptyMethodFormat"),
category: "Design",
defaultSeverity: DiagnosticSeverity.Info, isEnabledByDefault: true,
description: LinterCopAnalyzers.GetLocalizableString("Rule0081UseIsEmptyMethodDescription"),
helpLinkUri: "https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0081");
}
}
}
5 changes: 5 additions & 0 deletions BusinessCentral.LinterCop/LinterCop.ruleset.json
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,11 @@
"id": "LC0080",
"action": "Warning",
"justification": "Replace double quotes in JPath expressions with two single quotes."
},
{
"id": "LC0081",
"action": "Info",
"justification": "Use Rec.IsEmpty() for checking record existence."
}
]
}
9 changes: 9 additions & 0 deletions BusinessCentral.LinterCop/LinterCopAnalyzers.resx
Original file line number Diff line number Diff line change
Expand Up @@ -840,4 +840,13 @@
<data name="Rule0080AnalyzeJsonTokenJPathDescription" xml:space="preserve">
<value>Detects and warns against the use of double-quote (") characters in JPath expressions.</value>
</data>
<data name="Rule0081UseIsEmptyMethodTitle" xml:space="preserve">
<value>Use Rec.IsEmpty() for checking record existence.</value>
</data>
<data name="Rule0081UseIsEmptyMethodFormat" xml:space="preserve">
<value>Avoid using {0}.Count() for checking record existence. Use {0}.IsEmpty() instead for better performance.</value>
</data>
<data name="Rule0081UseIsEmptyMethodDescription" xml:space="preserve">
<value>To check for the existence of records, use the more efficient Rec.IsEmpty() function instead of Rec.Count().</value>
</data>
</root>
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,4 +233,5 @@ For an example and the default values see: [LinterCop.ruleset.json](./BusinessCe
|[LC0077](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0077)|Methods should always be called with parenthesis.|Info|
|[LC0078](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0078)|Temporary records should not trigger table triggers.|Info|
|[LC0079](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0079)|Event publishers should not be public.|Info|
|[LC0080](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0080)|Replace double quotes in JPath expressions with two single quotes.|Warning|
|[LC0080](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0080)|Replace double quotes in JPath expressions with two single quotes.|Warning|
|[LC0081](https://github.com/StefanMaron/BusinessCentral.LinterCop/wiki/LC0081)|Use `Rec.IsEmpty()` for checking record existence.|Info|

0 comments on commit f0a7234

Please sign in to comment.