From 37f0bc1d270ae82461ff13932fc76d507e788270 Mon Sep 17 00:00:00 2001 From: Arthur van de Vondervoort Date: Mon, 26 Jan 2026 16:47:35 +0100 Subject: [PATCH] Add "Analyze Count Method" to LinterCop --- src/ALCops.Common/Reflection/EnumProvider.cs | 8 +- .../OneGreaterThanRecordCount.al | 17 ++ .../HasDiagnostic/RecordCountEqualsZero.al | 17 ++ .../RecordCountGreaterThanOrEqualZero.al | 17 ++ .../RecordCountGreaterThanZero.al | 17 ++ .../HasDiagnostic/RecordCountLessThanOne.al | 17 ++ .../RecordCountLessThanOrEqualZero.al | 17 ++ .../HasDiagnostic/RecordCountLessThanZero.al | 17 ++ .../HasDiagnostic/RecordCountNotEqualsZero.al | 17 ++ .../NoDiagnostic/RecordCountEqualsOne.al | 17 ++ .../RecordTemporaryCountEqualsZero.al | 17 ++ .../UseIsEmptyMethodInsteadOfCount.cs | 49 ++++++ .../HasDiagnostic/RecordCountEqualsOne.al | 17 ++ .../RecordCountGreaterThanOne.al | 17 ++ .../RecordCountGreaterThanOrEqualOne.al | 17 ++ .../RecordCountLessThanOrEqualZero.al | 17 ++ .../HasDiagnostic/RecordCountLessThanTwo.al | 17 ++ .../HasDiagnostic/RecordCountNotEqualsOne.al | 17 ++ .../TwoGreaterThanRecordCount.al | 17 ++ .../NoDiagnostic/RecordCountEqualsTwo.al | 17 ++ .../RecordTemporaryCountEqualsOne.al | 17 ++ .../UseQueryOrFindWithNextInsteadOfCount.cs | 48 ++++++ .../ALCops.LinterCopAnalyzers.cs | 54 ++++++ .../ALCops.LinterCopAnalyzers.resx | 18 ++ .../Analyzers/AnalyzeCountMethod.cs | 156 ++++++++++++++++++ src/ALCops.LinterCop/DiagnosticDescriptors.cs | 20 +++ src/ALCops.LinterCop/DiagnosticIds.cs | 2 + 27 files changed, 677 insertions(+), 1 deletion(-) create mode 100644 src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/OneGreaterThanRecordCount.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountEqualsZero.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountGreaterThanOrEqualZero.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountGreaterThanZero.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountLessThanOne.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountLessThanOrEqualZero.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountLessThanZero.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountNotEqualsZero.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/NoDiagnostic/RecordCountEqualsOne.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/NoDiagnostic/RecordTemporaryCountEqualsZero.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/UseIsEmptyMethodInsteadOfCount.cs create mode 100644 src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountEqualsOne.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountGreaterThanOne.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountGreaterThanOrEqualOne.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountLessThanOrEqualZero.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountLessThanTwo.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountNotEqualsOne.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/TwoGreaterThanRecordCount.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/NoDiagnostic/RecordCountEqualsTwo.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/NoDiagnostic/RecordTemporaryCountEqualsOne.al create mode 100644 src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/UseQueryOrFindWithNextInsteadOfCount.cs create mode 100644 src/ALCops.LinterCop/Analyzers/AnalyzeCountMethod.cs diff --git a/src/ALCops.Common/Reflection/EnumProvider.cs b/src/ALCops.Common/Reflection/EnumProvider.cs index a131b79..b3667b6 100644 --- a/src/ALCops.Common/Reflection/EnumProvider.cs +++ b/src/ALCops.Common/Reflection/EnumProvider.cs @@ -1370,8 +1370,12 @@ public static class SyntaxKind new(() => ParseEnum(nameof(NavCodeAnalysis.SyntaxKind.EmptyProperty))); private static readonly Lazy _equalsToken = new(() => ParseEnum(nameof(NavCodeAnalysis.SyntaxKind.EqualsToken))); + private static readonly Lazy _greaterThanToken = + new(() => ParseEnum(nameof(NavCodeAnalysis.SyntaxKind.GreaterThanToken))); + private static readonly Lazy _lessThanToken = + new(() => ParseEnum(nameof(NavCodeAnalysis.SyntaxKind.LessThanToken))); private static readonly Lazy _endOfLineTrivia = - new(() => ParseEnum(nameof(NavCodeAnalysis.SyntaxKind.EndOfLineTrivia))); + new(() => ParseEnum(nameof(NavCodeAnalysis.SyntaxKind.EndOfLineTrivia))); private static readonly Lazy _enumExtensionType = new(() => ParseEnum(nameof(NavCodeAnalysis.SyntaxKind.EnumExtensionType))); private static readonly Lazy _enumDataType = @@ -1627,6 +1631,8 @@ public static class SyntaxKind public static NavCodeAnalysis.SyntaxKind ElifKeyword => _elifKeyword.Value; public static NavCodeAnalysis.SyntaxKind EmptyProperty => _emptyProperty.Value; public static NavCodeAnalysis.SyntaxKind EqualsToken => _equalsToken.Value; + public static NavCodeAnalysis.SyntaxKind GreaterThanToken => _greaterThanToken.Value; + public static NavCodeAnalysis.SyntaxKind LessThanToken => _lessThanToken.Value; public static NavCodeAnalysis.SyntaxKind EndOfLineTrivia => _endOfLineTrivia.Value; public static NavCodeAnalysis.SyntaxKind EnumExtensionType => _enumExtensionType.Value; public static NavCodeAnalysis.SyntaxKind EnumDataType => _enumDataType.Value; diff --git a/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/OneGreaterThanRecordCount.al b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/OneGreaterThanRecordCount.al new file mode 100644 index 0000000..1f3b756 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/OneGreaterThanRecordCount.al @@ -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) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountEqualsZero.al b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountEqualsZero.al new file mode 100644 index 0000000..83ced9a --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountEqualsZero.al @@ -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) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountGreaterThanOrEqualZero.al b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountGreaterThanOrEqualZero.al new file mode 100644 index 0000000..db9eae7 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountGreaterThanOrEqualZero.al @@ -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) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountGreaterThanZero.al b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountGreaterThanZero.al new file mode 100644 index 0000000..0ae86b0 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountGreaterThanZero.al @@ -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) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountLessThanOne.al b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountLessThanOne.al new file mode 100644 index 0000000..7c5f2ca --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountLessThanOne.al @@ -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) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountLessThanOrEqualZero.al b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountLessThanOrEqualZero.al new file mode 100644 index 0000000..0ce2216 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountLessThanOrEqualZero.al @@ -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) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountLessThanZero.al b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountLessThanZero.al new file mode 100644 index 0000000..0526fca --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountLessThanZero.al @@ -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) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountNotEqualsZero.al b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountNotEqualsZero.al new file mode 100644 index 0000000..a3fb3cb --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/HasDiagnostic/RecordCountNotEqualsZero.al @@ -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) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/NoDiagnostic/RecordCountEqualsOne.al b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/NoDiagnostic/RecordCountEqualsOne.al new file mode 100644 index 0000000..b84e45c --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/NoDiagnostic/RecordCountEqualsOne.al @@ -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) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/NoDiagnostic/RecordTemporaryCountEqualsZero.al b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/NoDiagnostic/RecordTemporaryCountEqualsZero.al new file mode 100644 index 0000000..0235084 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/NoDiagnostic/RecordTemporaryCountEqualsZero.al @@ -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) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/UseIsEmptyMethodInsteadOfCount.cs b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/UseIsEmptyMethodInsteadOfCount.cs new file mode 100644 index 0000000..632801e --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseIsEmptyMethodInsteadOfCount/UseIsEmptyMethodInsteadOfCount.cs @@ -0,0 +1,49 @@ +using RoslynTestKit; + +namespace ALCops.LinterCop.Test +{ + public class UseIsEmptyMethodInsteadOfCount : NavCodeAnalysisBase + { + private AnalyzerTestFixture _fixture; + private string _testCasePath; + + [SetUp] + public void Setup() + { + _fixture = RoslynFixtureFactory.Create(); + + _testCasePath = Path.Combine( + Directory.GetParent( + Environment.CurrentDirectory)!.Parent!.Parent!.FullName, + Path.Combine("Rules", nameof(UseIsEmptyMethodInsteadOfCount))); + } + + [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(_testCasePath, nameof(HasDiagnostic), $"{testCase}.al")) + .ConfigureAwait(false); + + _fixture.HasDiagnosticAtAllMarkers(code, DiagnosticIds.UseIsEmptyMethodInsteadOfCount); + } + + [Test] + [TestCase("RecordCountEqualsOne")] + [TestCase("RecordTemporaryCountEqualsZero")] + public async Task NoDiagnostic(string testCase) + { + var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(NoDiagnostic), $"{testCase}.al")) + .ConfigureAwait(false); + + _fixture.NoDiagnosticAtAllMarkers(code, DiagnosticIds.UseIsEmptyMethodInsteadOfCount); + } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountEqualsOne.al b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountEqualsOne.al new file mode 100644 index 0000000..e5564b3 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountEqualsOne.al @@ -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; "Entry No."; Integer) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountGreaterThanOne.al b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountGreaterThanOne.al new file mode 100644 index 0000000..68bcc26 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountGreaterThanOne.al @@ -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; "Entry No."; Integer) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountGreaterThanOrEqualOne.al b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountGreaterThanOrEqualOne.al new file mode 100644 index 0000000..d73c912 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountGreaterThanOrEqualOne.al @@ -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; "Entry No."; Integer) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountLessThanOrEqualZero.al b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountLessThanOrEqualZero.al new file mode 100644 index 0000000..d0ea1fb --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountLessThanOrEqualZero.al @@ -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; "Entry No."; Integer) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountLessThanTwo.al b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountLessThanTwo.al new file mode 100644 index 0000000..3a22c59 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountLessThanTwo.al @@ -0,0 +1,17 @@ +codeunit 50100 MyCodeunit +{ + procedure MyProcedure() + var + MyTable: Record MyTable; + begin + if [|MyTable.Count() < 2|] then; + end; +} + +table 50100 MyTable +{ + fields + { + field(1; "Entry No."; Integer) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountNotEqualsOne.al b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountNotEqualsOne.al new file mode 100644 index 0000000..d8c9617 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/RecordCountNotEqualsOne.al @@ -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; "Entry No."; Integer) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/TwoGreaterThanRecordCount.al b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/TwoGreaterThanRecordCount.al new file mode 100644 index 0000000..6684034 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/HasDiagnostic/TwoGreaterThanRecordCount.al @@ -0,0 +1,17 @@ +codeunit 50100 MyCodeunit +{ + procedure MyProcedure() + var + MyTable: Record MyTable; + begin + if [|2 > MyTable.Count()|] then; + end; +} + +table 50100 MyTable +{ + fields + { + field(1; "Entry No."; Integer) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/NoDiagnostic/RecordCountEqualsTwo.al b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/NoDiagnostic/RecordCountEqualsTwo.al new file mode 100644 index 0000000..800de24 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/NoDiagnostic/RecordCountEqualsTwo.al @@ -0,0 +1,17 @@ +codeunit 50100 MyCodeunit +{ + procedure MyProcedure() + var + MyTable: Record MyTable; + begin + if [|MyTable.Count() = 2|] then; + end; +} + +table 50100 MyTable +{ + fields + { + field(1; "Entry No."; Integer) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/NoDiagnostic/RecordTemporaryCountEqualsOne.al b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/NoDiagnostic/RecordTemporaryCountEqualsOne.al new file mode 100644 index 0000000..0295391 --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/NoDiagnostic/RecordTemporaryCountEqualsOne.al @@ -0,0 +1,17 @@ +codeunit 50100 MyCodeunit +{ + procedure MyProcedure() + var + TempMyTable: Record MyTable temporary; + begin + if [|TempMyTable.Count() = 1|] then; + end; +} + +table 50100 MyTable +{ + fields + { + field(1; "Entry No."; Integer) { } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/UseQueryOrFindWithNextInsteadOfCount.cs b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/UseQueryOrFindWithNextInsteadOfCount.cs new file mode 100644 index 0000000..96294aa --- /dev/null +++ b/src/ALCops.LinterCop.Test/Rules/UseQueryOrFindWithNextInsteadOfCount/UseQueryOrFindWithNextInsteadOfCount.cs @@ -0,0 +1,48 @@ +using RoslynTestKit; + +namespace ALCops.LinterCop.Test +{ + public class UseQueryOrFindWithNextInsteadOfCount : NavCodeAnalysisBase + { + private AnalyzerTestFixture _fixture; + private string _testCasePath; + + [SetUp] + public void Setup() + { + _fixture = RoslynFixtureFactory.Create(); + + _testCasePath = Path.Combine( + Directory.GetParent( + Environment.CurrentDirectory)!.Parent!.Parent!.FullName, + Path.Combine("Rules", nameof(UseQueryOrFindWithNextInsteadOfCount))); + } + + [Test] + [TestCase("RecordCountEqualsOne")] + [TestCase("RecordCountGreaterThanOne")] + [TestCase("RecordCountGreaterThanOrEqualOne")] + [TestCase("RecordCountLessThanOrEqualZero")] + [TestCase("RecordCountLessThanTwo")] + [TestCase("RecordCountNotEqualsOne")] + [TestCase("TwoGreaterThanRecordCount")] + public async Task HasDiagnostic(string testCase) + { + var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(HasDiagnostic), $"{testCase}.al")) + .ConfigureAwait(false); + + _fixture.HasDiagnosticAtAllMarkers(code, DiagnosticIds.UseQueryOrFindWithNextInsteadOfCount); + } + + [Test] + [TestCase("RecordCountEqualsTwo")] + [TestCase("RecordTemporaryCountEqualsOne")] + public async Task NoDiagnostic(string testCase) + { + var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(NoDiagnostic), $"{testCase}.al")) + .ConfigureAwait(false); + + _fixture.NoDiagnosticAtAllMarkers(code, DiagnosticIds.UseQueryOrFindWithNextInsteadOfCount); + } + } +} \ No newline at end of file diff --git a/src/ALCops.LinterCop/ALCops.LinterCopAnalyzers.cs b/src/ALCops.LinterCop/ALCops.LinterCopAnalyzers.cs index 612c503..342208b 100644 --- a/src/ALCops.LinterCop/ALCops.LinterCopAnalyzers.cs +++ b/src/ALCops.LinterCop/ALCops.LinterCopAnalyzers.cs @@ -642,6 +642,60 @@ internal static string RecordInstanceIsolationLevelTitle { } } + /// + /// Looks up a localized string similar to To check for the existence of records, use the more efficient Rec.IsEmpty() function instead of Rec.Count().. + /// + internal static string UseIsEmptyMethodInsteadOfCountDescription { + get { + return ResourceManager.GetString("UseIsEmptyMethodInsteadOfCountDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Avoid using {0}.Count() for checking record existence. Use {0}.IsEmpty() instead for better performance.. + /// + internal static string UseIsEmptyMethodInsteadOfCountMessageFormat { + get { + return ResourceManager.GetString("UseIsEmptyMethodInsteadOfCountMessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Use Rec.IsEmpty() for checking record existence. + /// + internal static string UseIsEmptyMethodInsteadOfCountTitle { + get { + return ResourceManager.GetString("UseIsEmptyMethodInsteadOfCountTitle", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Instead of relying on Rec.Count(), consider using a Query object or a combination of Rec.Find('-') and Rec.Next() for faster and more efficient record checks.. + /// + internal static string UseQueryOrFindWithNextInsteadOfCountDescription { + get { + return ResourceManager.GetString("UseQueryOrFindWithNextInsteadOfCountDescription", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Consider using a Query object or {0}.Find('-') together with {0}.Next() instead of {0}.Count() for faster and more efficient record checks.. + /// + internal static string UseQueryOrFindWithNextInsteadOfCountMessageFormat { + get { + return ResourceManager.GetString("UseQueryOrFindWithNextInsteadOfCountMessageFormat", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Consider using a Query object or Rec.Find('-') with Rec.Next() for checking exactly one record. + /// + internal static string UseQueryOrFindWithNextInsteadOfCountTitle { + get { + return ResourceManager.GetString("UseQueryOrFindWithNextInsteadOfCountTitle", resourceCulture); + } + } + /// /// Looks up a localized string similar to Using SecretText prevents confidential information such as API keys, tokens, passwords, and similar secrets from being exposed through the AL debugger during regular or snapshot debugging.. /// diff --git a/src/ALCops.LinterCop/ALCops.LinterCopAnalyzers.resx b/src/ALCops.LinterCop/ALCops.LinterCopAnalyzers.resx index aefef7a..4a4c547 100644 --- a/src/ALCops.LinterCop/ALCops.LinterCopAnalyzers.resx +++ b/src/ALCops.LinterCop/ALCops.LinterCopAnalyzers.resx @@ -315,6 +315,24 @@ ALCops: Replace LockTable() with ReadIsolation + + Use Rec.IsEmpty() for checking record existence + + + Avoid using {0}.Count() for checking record existence. Use {0}.IsEmpty() instead for better performance. + + + To check for the existence of records, use the more efficient Rec.IsEmpty() function instead of Rec.Count(). + + + Consider using a Query object or Rec.Find('-') with Rec.Next() for checking exactly one record + + + Consider using a Query object or {0}.Find('-') together with {0}.Next() instead of {0}.Count() for faster and more efficient record checks. + + + Instead of relying on Rec.Count(), consider using a Query object or a combination of Rec.Find('-') and Rec.Next() for faster and more efficient record checks. + Use SecretText type to protect credentials and sensitive textual values from being revealed diff --git a/src/ALCops.LinterCop/Analyzers/AnalyzeCountMethod.cs b/src/ALCops.LinterCop/Analyzers/AnalyzeCountMethod.cs new file mode 100644 index 0000000..a72bae6 --- /dev/null +++ b/src/ALCops.LinterCop/Analyzers/AnalyzeCountMethod.cs @@ -0,0 +1,156 @@ +using System.Collections.Immutable; +using ALCops.Common.Extensions; +using ALCops.Common.Reflection; +using Microsoft.Dynamics.Nav.CodeAnalysis; +using Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics; +using Microsoft.Dynamics.Nav.CodeAnalysis.Syntax; +using Microsoft.Dynamics.Nav.CodeAnalysis.Utilities; + +namespace ALCops.LinterCop.Analyzers; + +[DiagnosticAnalyzer] +public class AnalyzeCountMethod : DiagnosticAnalyzer +{ + private const string CountMethodName = "Count"; + private const int Zero = 0; + private const int One = 1; + private const int Two = 2; + private const int MaxRelevantValue = 2; + + // Tables with one of these identifiers in the name could possible have a large amount of records + private static readonly HashSet possibleLargeTableIdentifierKeywords = new HashSet + { + "Ledger", "GL", "G/L", + "Posted", "Pstd", + "Log", + "Entry", + "Archive", + }; + + public override ImmutableArray SupportedDiagnostics { get; } = + ImmutableArray.Create( + DiagnosticDescriptors.UseIsEmptyMethodInsteadOfCount, + DiagnosticDescriptors.UseQueryOrFindWithNextInsteadOfCount); + + public override void Initialize(AnalysisContext context) => + context.RegisterOperationAction( + AnalyzeCountInvocation, + EnumProvider.OperationKind.InvocationExpression); + + private void AnalyzeCountInvocation(OperationAnalysisContext ctx) + { + if (ctx.IsObsolete() || ctx.Operation is not IInvocationExpression invocation) + return; + + if (invocation.TargetMethod.MethodKind != EnumProvider.MethodKind.BuiltInMethod || + !string.Equals(invocation.TargetMethod.Name, CountMethodName, StringComparison.Ordinal) || + invocation.TargetMethod.ContainingSymbol?.Name != "Table") + return; + + if (invocation.Instance?.GetSymbol() is not IVariableSymbol { Type: IRecordTypeSymbol recordTypeSymbol } || recordTypeSymbol.Temporary) + return; + + if (invocation.Syntax.Parent is not BinaryExpressionSyntax binaryExpression) + return; + + int rightValue = GetLiteralExpressionValue(binaryExpression.Right); + if (rightValue > MaxRelevantValue) + return; + + int leftValue = GetLiteralExpressionValue(binaryExpression.Left); + if (leftValue > MaxRelevantValue) + return; + + if (IsZeroComparison(leftValue, rightValue)) + { + ReportUseIsEmptyDiagnostic(ctx, invocation); + return; + } + + if (IsLessThanOneComparison(binaryExpression, rightValue) || IsGreaterThanOneComparison(binaryExpression, leftValue)) + { + ReportUseIsEmptyDiagnostic(ctx, invocation); + return; + } + + if (IsEligibleUseQueryOrFindWithNext(recordTypeSymbol)) + { + if (IsOneComparison(leftValue, rightValue)) + { + ReportUseFindWithNextDiagnostic(ctx, invocation, GetOperatorKind(binaryExpression.OperatorToken.Kind)); + return; + } + + if (IsLessThanTwoComparison(binaryExpression, rightValue) || IsGreaterThanTwoComparison(binaryExpression, leftValue)) + { + ReportUseFindWithNextDiagnostic(ctx, invocation, EnumProvider.SyntaxKind.EqualsToken); + return; + } + } + } + + private static int GetLiteralExpressionValue(CodeExpressionSyntax codeExpression) + { + if (codeExpression is not LiteralExpressionSyntax literal) + return -1; + + if (literal.Literal.Kind != EnumProvider.SyntaxKind.Int32SignedLiteralValue) + return -1; + + return literal.Literal.GetLiteralValue() is int value ? value : -1; + } + + private static SyntaxKind GetOperatorKind(SyntaxKind tokenKind) => + tokenKind == EnumProvider.SyntaxKind.EqualsToken ? EnumProvider.SyntaxKind.EqualsToken : EnumProvider.SyntaxKind.NotEqualsToken; + + private static bool IsZeroComparison(int left, int right) + => left == Zero || right == Zero; + + private static bool IsLessThanOneComparison(BinaryExpressionSyntax expr, int right) => + expr.OperatorToken.Kind == EnumProvider.SyntaxKind.LessThanToken && right == One; + + private static bool IsGreaterThanOneComparison(BinaryExpressionSyntax expr, int left) => + expr.OperatorToken.Kind == EnumProvider.SyntaxKind.GreaterThanToken && left == One; + + private static bool IsOneComparison(int left, int right) => + left == One || right == One; + + private static bool IsLessThanTwoComparison(BinaryExpressionSyntax expr, int right) => + expr.OperatorToken.Kind == EnumProvider.SyntaxKind.LessThanToken && right == Two; + + private static bool IsGreaterThanTwoComparison(BinaryExpressionSyntax expr, int left) => + expr.OperatorToken.Kind == EnumProvider.SyntaxKind.GreaterThanToken && left == Two; + + private static bool IsEligibleUseQueryOrFindWithNext(IRecordTypeSymbol record) + { + if (possibleLargeTableIdentifierKeywords.Any(keyword => record.Name.IndexOf(keyword, StringComparison.OrdinalIgnoreCase) >= 0)) + return true; + + // Tables with a field "Entry No." could possible have a large amount of records + if (record.OriginalDefinition is ITableTypeSymbol table) + return table.PrimaryKey.Fields.Any(field => string.Equals(field.Name, "Entry No.", StringComparison.OrdinalIgnoreCase)); + + return false; + } + + private static void ReportUseIsEmptyDiagnostic(OperationAnalysisContext ctx, IInvocationExpression operation) + { + ctx.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.UseIsEmptyMethodInsteadOfCount, + operation.Syntax.Parent.GetLocation(), + GetSymbolName(operation))); + } + + private static void ReportUseFindWithNextDiagnostic(OperationAnalysisContext ctx, IInvocationExpression operation, SyntaxKind operatorToken) + { + string operatorSign = operatorToken == EnumProvider.SyntaxKind.EqualsToken ? "=" : "<>"; + + ctx.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.UseQueryOrFindWithNextInsteadOfCount, + operation.Syntax.Parent.GetLocation(), + GetSymbolName(operation), operatorSign)); + } + + private static string GetSymbolName(IInvocationExpression operation) => + operation.Instance?.GetSymbol()?.Name.QuoteIdentifierIfNeededWithReflection() ?? string.Empty; +} \ No newline at end of file diff --git a/src/ALCops.LinterCop/DiagnosticDescriptors.cs b/src/ALCops.LinterCop/DiagnosticDescriptors.cs index dc154ca..148c0d6 100644 --- a/src/ALCops.LinterCop/DiagnosticDescriptors.cs +++ b/src/ALCops.LinterCop/DiagnosticDescriptors.cs @@ -214,6 +214,26 @@ public static class DiagnosticDescriptors description: LinterCopAnalyzers.RecordInstanceIsolationLevelDescription, helpLinkUri: GetHelpUri(DiagnosticIds.RecordInstanceIsolationLevel)); + public static readonly DiagnosticDescriptor UseIsEmptyMethodInsteadOfCount = new( + id: DiagnosticIds.UseIsEmptyMethodInsteadOfCount, + title: LinterCopAnalyzers.UseIsEmptyMethodInsteadOfCountTitle, + messageFormat: LinterCopAnalyzers.UseIsEmptyMethodInsteadOfCountMessageFormat, + category: Category.Design, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: LinterCopAnalyzers.UseIsEmptyMethodInsteadOfCountDescription, + helpLinkUri: GetHelpUri(DiagnosticIds.UseIsEmptyMethodInsteadOfCount)); + + public static readonly DiagnosticDescriptor UseQueryOrFindWithNextInsteadOfCount = new( + id: DiagnosticIds.UseQueryOrFindWithNextInsteadOfCount, + title: LinterCopAnalyzers.UseQueryOrFindWithNextInsteadOfCountTitle, + messageFormat: LinterCopAnalyzers.UseQueryOrFindWithNextInsteadOfCountMessageFormat, + category: Category.Design, + defaultSeverity: DiagnosticSeverity.Info, + isEnabledByDefault: true, + description: LinterCopAnalyzers.UseQueryOrFindWithNextInsteadOfCountDescription, + helpLinkUri: GetHelpUri(DiagnosticIds.UseQueryOrFindWithNextInsteadOfCount)); + public static readonly DiagnosticDescriptor UseSecretTextForSensitiveText = new( id: DiagnosticIds.UseSecretTextForSensitiveText, title: LinterCopAnalyzers.UseSecretTextForSensitiveTextTitle, diff --git a/src/ALCops.LinterCop/DiagnosticIds.cs b/src/ALCops.LinterCop/DiagnosticIds.cs index ca28093..58043b5 100644 --- a/src/ALCops.LinterCop/DiagnosticIds.cs +++ b/src/ALCops.LinterCop/DiagnosticIds.cs @@ -19,6 +19,8 @@ public static class DiagnosticIds public static readonly string InternalProcedureOnlyUsedInCurrentObject = "LC0053"; public static readonly string InterfaceObjectNameGuide = "LC0054"; public static readonly string ApiPageCanonicalFieldNameGuide = "LC0063"; + public static readonly string UseIsEmptyMethodInsteadOfCount = "LC0081"; + public static readonly string UseQueryOrFindWithNextInsteadOfCount = "LC0082"; public static readonly string PageStyleStringLiteral = "LC0086"; public static readonly string CognitiveComplexityMetric = "LC0089"; public static readonly string CognitiveComplexityIncrement = "LC0089i";