Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyTable: Record MyTable;
MyGuid: Guid;
begin
MyTable.[|GetBySystemId|](MyGuid);
end;
}

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

table 50100 MyTable
{
fields
{
field(1; MyField; Code[20]) { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
codeunit 50100 MyCodeunit
{
procedure MyProcedure()
var
MyTable: Record MyTable;
MyGuid: Guid;
begin
if MyTable.[|GetBySystemId|](MyGuid) then;
end;
}

table 50100 MyTable
{
fields
{
field(1; MyField; Code[20]) { }
}
}
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.[|Get|]() then;
end;
}

table 50100 MyTable
{
fields
{
field(1; MyField; Code[20]) { }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
using RoslynTestKit;

namespace ALCops.ApplicationCop.Test
{
public class UseReturnValueForDatabaseReadMethods : NavCodeAnalysisBase
{
private AnalyzerTestFixture _fixture;
private string _testCasePath;

[SetUp]
public void Setup()
{
_fixture = RoslynFixtureFactory.Create<Analyzers.UseReturnValueForDatabaseReadMethods>();

_testCasePath = Path.Combine(
Directory.GetParent(
Environment.CurrentDirectory)!.Parent!.Parent!.FullName,
Path.Combine("Rules", nameof(UseReturnValueForDatabaseReadMethods)));
}

[Test]
[TestCase("GetMethod")]
[TestCase("GetBySystemIdMethod")]
public async Task HasDiagnostic(string testCase)
{
var code = await File.ReadAllTextAsync(Path.Combine(_testCasePath, nameof(HasDiagnostic), $"{testCase}.al"))
.ConfigureAwait(false);

_fixture.HasDiagnosticAtAllMarkers(code, DiagnosticIds.UseReturnValueForDatabaseReadMethods);
}

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

_fixture.NoDiagnosticAtAllMarkers(code, DiagnosticIds.UseReturnValueForDatabaseReadMethods);
}
}
}
27 changes: 27 additions & 0 deletions src/ALCops.ApplicationCop/ALCops.ApplicationCopAnalyzers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,33 @@ internal static string ToolTipShouldStartWithSpecifiesTitle {
}
}

/// <summary>
/// Looks up a localized string similar to Database read methods, like Record.Get(), returns a boolean indicating whether the record was successfully retrieved. Failing to use this return value can lead to uncaught errors, poor error handling, and a lack of actionable feedback for users when something goes wrong..
/// </summary>
internal static string UseReturnValueForDatabaseReadMethodsDescription {
get {
return ResourceManager.GetString("UseReturnValueForDatabaseReadMethodsDescription", resourceCulture);
}
}

/// <summary>
/// Looks up a localized string similar to The return value of the &apos;{0}&apos; method must be used to improve error handling or provide meaningful feedback to the user..
/// </summary>
internal static string UseReturnValueForDatabaseReadMethodsMessageFormat {
get {
return ResourceManager.GetString("UseReturnValueForDatabaseReadMethodsMessageFormat", resourceCulture);
}
}

/// <summary>
/// Looks up a localized string similar to Use return value for better error handling.
/// </summary>
internal static string UseReturnValueForDatabaseReadMethodsTitle {
get {
return ResourceManager.GetString("UseReturnValueForDatabaseReadMethodsTitle", resourceCulture);
}
}

/// <summary>
/// Looks up a localized string similar to Zero (0) should be reserved as the empty Enum value. Business Central stores Enums as integers and does not support null; new records (and existing records after adding a field via table extension) default to 0, which makes non-empty meaning at 0 ambiguous..
/// </summary>
Expand Down
9 changes: 9 additions & 0 deletions src/ALCops.ApplicationCop/ALCops.ApplicationCopAnalyzers.resx
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,15 @@
<data name="ToolTipShouldStartWithSpecifiesDescription" xml:space="preserve">
<value>For field ToolTips, the Business Central user assistance model expects the text to start with the verb 'Specifies' to describe what the field represents.</value>
</data>
<data name="UseReturnValueForDatabaseReadMethodsTitle" xml:space="preserve">
<value>Use return value for better error handling</value>
</data>
<data name="UseReturnValueForDatabaseReadMethodsMessageFormat" xml:space="preserve">
<value>The return value of the '{0}' method must be used to improve error handling or provide meaningful feedback to the user.</value>
</data>
<data name="UseReturnValueForDatabaseReadMethodsDescription" xml:space="preserve">
<value>Database read methods, like Record.Get(), returns a boolean indicating whether the record was successfully retrieved. Failing to use this return value can lead to uncaught errors, poor error handling, and a lack of actionable feedback for users when something goes wrong.</value>
</data>
<data name="ZeroEnumValueReservedForEmptyTitle" xml:space="preserve">
<value>Reserve Enum value zero (0) for empty value</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
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;

namespace ALCops.ApplicationCop.Analyzers;

[DiagnosticAnalyzer]
public class UseReturnValueForDatabaseReadMethods : DiagnosticAnalyzer
{
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
ImmutableArray.Create(
DiagnosticDescriptors.UseReturnValueForDatabaseReadMethods);

private static readonly ImmutableHashSet<string> DatabaseReadMethods =
ImmutableHashSet.Create(
StringComparer.OrdinalIgnoreCase,
"Find",
"FindFirst",
"FindLast",
"Get",
"GetBySystemId");

public override void Initialize(AnalysisContext context) =>
context.RegisterOperationAction(
AnalyzeInvocation,
EnumProvider.OperationKind.InvocationExpression);

private void AnalyzeInvocation(OperationAnalysisContext ctx)
{
if (ctx.IsObsolete() || ctx.Operation is not IInvocationExpression invocation)
return;

if (invocation.TargetMethod.MethodKind != EnumProvider.MethodKind.BuiltInMethod ||
invocation.TargetMethod.ContainingSymbol?.Name != "Table" ||
!DatabaseReadMethods.Contains(invocation.TargetMethod.Name))
return;

if (ctx.Operation.Syntax.Parent.Kind == EnumProvider.SyntaxKind.ExpressionStatement)
{
var methodName = invocation.TargetMethod.Name;
if (invocation.Syntax is not InvocationExpressionSyntax invocationSyntax)
return;

var location = invocationSyntax.Expression.GetLocation();

ctx.ReportDiagnostic(Diagnostic.Create(
DiagnosticDescriptors.UseReturnValueForDatabaseReadMethods,
location,
methodName));
}
}
}
10 changes: 10 additions & 0 deletions src/ALCops.ApplicationCop/DiagnosticDescriptors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,16 @@ public static class DiagnosticDescriptors
description: ApplicationCopAnalyzers.ToolTipShouldStartWithSpecifiesDescription,
helpLinkUri: GetHelpUri(DiagnosticIds.ToolTipShouldStartWithSpecifies));

public static readonly DiagnosticDescriptor UseReturnValueForDatabaseReadMethods = new(
id: DiagnosticIds.UseReturnValueForDatabaseReadMethods,
title: ApplicationCopAnalyzers.UseReturnValueForDatabaseReadMethodsTitle,
messageFormat: ApplicationCopAnalyzers.UseReturnValueForDatabaseReadMethodsMessageFormat,
category: Category.Design,
defaultSeverity: DiagnosticSeverity.Info,
isEnabledByDefault: true,
description: ApplicationCopAnalyzers.UseReturnValueForDatabaseReadMethodsDescription,
helpLinkUri: GetHelpUri(DiagnosticIds.UseReturnValueForDatabaseReadMethods));

public static readonly DiagnosticDescriptor ZeroEnumValueReservedForEmpty = new(
id: DiagnosticIds.ZeroEnumValueReservedForEmpty,
title: ApplicationCopAnalyzers.ZeroEnumValueReservedForEmptyTitle,
Expand Down
1 change: 1 addition & 0 deletions src/ALCops.ApplicationCop/DiagnosticIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@ public static class DiagnosticIds
public static readonly string LabelTokenRequiresTokSuffix = "AC0027";
public static readonly string TableFieldToolTipShouldBeDefined = "AC0028";
public static readonly string DuplicateToolTipBetweenPageAndTable = "AC0029";
public static readonly string UseReturnValueForDatabaseReadMethods = "AC0030";
}
5 changes: 4 additions & 1 deletion src/ALCops.Common/Reflection/EnumProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1375,7 +1375,7 @@ public static class SyntaxKind
private static readonly Lazy<NavCodeAnalysis.SyntaxKind> _lessThanToken =
new(() => ParseEnum<NavCodeAnalysis.SyntaxKind>(nameof(NavCodeAnalysis.SyntaxKind.LessThanToken)));
private static readonly Lazy<NavCodeAnalysis.SyntaxKind> _endOfLineTrivia =
new(() => ParseEnum<NavCodeAnalysis.SyntaxKind>(nameof(NavCodeAnalysis.SyntaxKind.EndOfLineTrivia)));
new(() => ParseEnum<NavCodeAnalysis.SyntaxKind>(nameof(NavCodeAnalysis.SyntaxKind.EndOfLineTrivia)));
private static readonly Lazy<NavCodeAnalysis.SyntaxKind> _enumExtensionType =
new(() => ParseEnum<NavCodeAnalysis.SyntaxKind>(nameof(NavCodeAnalysis.SyntaxKind.EnumExtensionType)));
private static readonly Lazy<NavCodeAnalysis.SyntaxKind> _enumDataType =
Expand All @@ -1388,6 +1388,8 @@ public static class SyntaxKind
new(() => ParseEnum<NavCodeAnalysis.SyntaxKind>(nameof(NavCodeAnalysis.SyntaxKind.Entitlement)));
private static readonly Lazy<NavCodeAnalysis.SyntaxKind> _exitStatement =
new(() => ParseEnum<NavCodeAnalysis.SyntaxKind>(nameof(NavCodeAnalysis.SyntaxKind.ExitStatement)));
private static readonly Lazy<NavCodeAnalysis.SyntaxKind> _expressionStatement =
new(() => ParseEnum<NavCodeAnalysis.SyntaxKind>(nameof(NavCodeAnalysis.SyntaxKind.ExpressionStatement)));
private static readonly Lazy<NavCodeAnalysis.SyntaxKind> _falseKeyword =
new(() => ParseEnum<NavCodeAnalysis.SyntaxKind>(nameof(NavCodeAnalysis.SyntaxKind.FalseKeyword)));
private static readonly Lazy<NavCodeAnalysis.SyntaxKind> _fieldKeyword =
Expand Down Expand Up @@ -1640,6 +1642,7 @@ public static class SyntaxKind
public static NavCodeAnalysis.SyntaxKind EnumValue => _enumValue.Value;
public static NavCodeAnalysis.SyntaxKind Entitlement => _entitlement.Value;
public static NavCodeAnalysis.SyntaxKind ExitStatement => _exitStatement.Value;
public static NavCodeAnalysis.SyntaxKind ExpressionStatement => _expressionStatement.Value;
public static NavCodeAnalysis.SyntaxKind FalseKeyword => _falseKeyword.Value;
public static NavCodeAnalysis.SyntaxKind FieldKeyword => _fieldKeyword.Value;
public static NavCodeAnalysis.SyntaxKind ForKeyword => _forKeyword.Value;
Expand Down
2 changes: 1 addition & 1 deletion src/ALCops.LinterCop/ALCops.LinterCopAnalyzers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ internal static string AppManifestRuntimeBehindTitle {
}

/// <summary>
/// Looks up a localized string similar to APClops: Use modern Date/Time/DateTime method.
/// Looks up a localized string similar to ALCops: Use modern Date/Time/DateTime method.
/// </summary>
internal static string BuiltInDateTimeMethodCodeAction {
get {
Expand Down
Loading