diff --git a/BusinessCentral.LinterCop.Test/Rule0051.cs b/BusinessCentral.LinterCop.Test/Rule0051.cs new file mode 100644 index 0000000..f5f99a2 --- /dev/null +++ b/BusinessCentral.LinterCop.Test/Rule0051.cs @@ -0,0 +1,37 @@ +#if !LessThenFall2023RV1 +namespace BusinessCentral.LinterCop.Test; + +public class Rule0051 +{ + private string _testCaseDir = ""; + + [SetUp] + public void Setup() + { + _testCaseDir = Path.Combine(Directory.GetParent(Environment.CurrentDirectory)!.Parent!.Parent!.FullName, + "TestCases", "Rule0051"); + } + + [Test] + [TestCase("SetFilterFieldCode")] + public async Task HasDiagnostic(string testCase) + { + var code = await File.ReadAllTextAsync(Path.Combine(_testCaseDir, "HasDiagnostic", $"{testCase}.al")) + .ConfigureAwait(false); + + var fixture = RoslynFixtureFactory.Create(); + fixture.HasDiagnostic(code, DiagnosticDescriptors.Rule0051SetFilterPossibleOverflow.Id); + } + + [Test] + [TestCase("SetFilterFieldRef")] + public async Task NoDiagnostic(string testCase) + { + var code = await File.ReadAllTextAsync(Path.Combine(_testCaseDir, "NoDiagnostic", $"{testCase}.al")) + .ConfigureAwait(false); + + var fixture = RoslynFixtureFactory.Create(); + fixture.NoDiagnosticAtMarker(code, DiagnosticDescriptors.Rule0051SetFilterPossibleOverflow.Id); + } +} +#endif \ No newline at end of file diff --git a/BusinessCentral.LinterCop.Test/TestCases/Rule0051/HasDiagnostic/SetFilterFieldCode.al b/BusinessCentral.LinterCop.Test/TestCases/Rule0051/HasDiagnostic/SetFilterFieldCode.al new file mode 100644 index 0000000..5370467 --- /dev/null +++ b/BusinessCentral.LinterCop.Test/TestCases/Rule0051/HasDiagnostic/SetFilterFieldCode.al @@ -0,0 +1,18 @@ +codeunit 50100 MyCodeunit +{ + procedure MyProcedure() + var + MyTable: Record MyTable; + MyFilterValue: Code[50]; + begin + [|MyTable.SetFilter(MyField, '<>%1', MyFilterValue)|]; + end; +} + +table 50100 MyTable +{ + fields + { + field(1; MyField; Code[20]) { } + } +} \ No newline at end of file diff --git a/BusinessCentral.LinterCop.Test/TestCases/Rule0051/NoDiagnostic/SetFilterFieldRef.al b/BusinessCentral.LinterCop.Test/TestCases/Rule0051/NoDiagnostic/SetFilterFieldRef.al new file mode 100644 index 0000000..a866732 --- /dev/null +++ b/BusinessCentral.LinterCop.Test/TestCases/Rule0051/NoDiagnostic/SetFilterFieldRef.al @@ -0,0 +1,18 @@ +codeunit 50100 MyCodeunit +{ + procedure MyProcedure() + var + MyTable: Record MyTable; + SourceDocRef: RecordRef; + begin + [|MyTable.SetFilter(MyField, '<>%1', SourceDocRef.Field(MyTable.FieldNo(MyField)).Value)|]; + end; +} + +table 50100 MyTable +{ + fields + { + field(1; MyField; Code[20]) { } + } +} \ No newline at end of file diff --git a/BusinessCentral.LinterCop/Design/Rule0051SetFilterPossibleOverflow.cs b/BusinessCentral.LinterCop/Design/Rule0051SetFilterPossibleOverflow.cs index 92f5e0d..e2e4d30 100644 --- a/BusinessCentral.LinterCop/Design/Rule0051SetFilterPossibleOverflow.cs +++ b/BusinessCentral.LinterCop/Design/Rule0051SetFilterPossibleOverflow.cs @@ -1,4 +1,3 @@ -#nullable disable // TODO: Enable nullable and review rule #if !LessThenFall2023RV1 using BusinessCentral.LinterCop.AnalysisContextExtension; using Microsoft.Dynamics.Nav.CodeAnalysis; @@ -22,40 +21,39 @@ public class Rule0051SetFilterPossibleOverflow : DiagnosticAnalyzer private void AnalyzeInvocation(OperationAnalysisContext ctx) { - // Investigate https://github.com/StefanMaron/BusinessCentral.LinterCop/issues/822 - try - { - if (ctx.IsObsoletePendingOrRemoved()) return; - - IInvocationExpression operation = (IInvocationExpression)ctx.Operation; - if (operation.TargetMethod.MethodKind != MethodKind.BuiltInMethod) return; + if (ctx.IsObsoletePendingOrRemoved()) + return; - if (operation.TargetMethod == null || !SemanticFacts.IsSameName(operation.TargetMethod.Name, "SetFilter") || operation.Arguments.Count() < 3) - return; + if ((ctx.Operation is not IInvocationExpression operation) + || operation.TargetMethod.MethodKind != MethodKind.BuiltInMethod + || operation.TargetMethod == null + || !SemanticFacts.IsSameName(operation.TargetMethod.Name, "SetFilter") + || operation.Arguments.Count() < 3 + || operation.Arguments[0].Value.Kind != OperationKind.ConversionExpression) + return; - if (operation.Arguments[0].Value.Kind != OperationKind.ConversionExpression) return; - IOperation fieldOperand = ((IConversionExpression)operation.Arguments[0].Value).Operand; - ITypeSymbol fieldType = fieldOperand.Type; - if (fieldType.GetNavTypeKindSafe() == NavTypeKind.Text) return; + var fieldOperand = ((IConversionExpression)operation.Arguments[0].Value).Operand; + if (fieldOperand.Type is not ITypeSymbol fieldType) + return; - bool isError = false; - int typeLength = GetTypeLength(fieldType, ref isError); - if (isError || typeLength == int.MaxValue) - return; + if (fieldType.GetNavTypeKindSafe() == NavTypeKind.Text) return; - foreach (int argIndex in GetArgumentIndexes(operation.Arguments[1].Value)) - { - int index = argIndex + 1; // The placeholders are defines as %1, %2, %3, where in case of %1 we need the second (zero based) index of the arguments of the SetFilter method - if (index < 2 || index >= operation.Arguments.Count()) continue; + bool isError = false; + int typeLength = GetTypeLength(fieldType, ref isError); + if (isError || typeLength == int.MaxValue) + return; - int expressionLength = this.CalculateMaxExpressionLength(((IConversionExpression)operation.Arguments[index].Value).Operand, ref isError); - if (!isError && expressionLength > typeLength) - ctx.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.Rule0051SetFilterPossibleOverflow, operation.Syntax.GetLocation(), GetDisplayString(operation.Arguments[index], operation), GetDisplayString(operation.Arguments[0], operation))); - } - } - catch (InvalidCastException) + foreach (int argIndex in GetArgumentIndexes(operation.Arguments[1].Value)) { - ctx.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.Rule0000ErrorInRule, ctx.Operation.Syntax.GetLocation(), new Object[] { "Rule0051", "InvalidCastException", "" })); + int index = argIndex + 1; // The placeholders are defines as %1, %2, %3, where in case of %1 we need the second (zero based) index of the arguments of the SetFilter method + if ((index < 2) + || (index >= operation.Arguments.Count()) + || (operation.Arguments[index].Value.Kind != OperationKind.ConversionExpression)) + continue; + + int expressionLength = this.CalculateMaxExpressionLength(((IConversionExpression)operation.Arguments[index].Value).Operand, ref isError); + if (!isError && expressionLength > typeLength) + ctx.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.Rule0051SetFilterPossibleOverflow, operation.Syntax.GetLocation(), GetDisplayString(operation.Arguments[index], operation), GetDisplayString(operation.Arguments[0], operation))); } } @@ -132,7 +130,7 @@ private int CalculateMaxExpressionLength(IOperation expression, ref bool isError break; case "tolower": case "toupper": - if (invocation.Instance.IsBoundExpression()) + if (invocation.Instance != null && invocation.Instance.IsBoundExpression()) return GetTypeLength(invocation.Instance.Type, ref isError); break; } diff --git a/BusinessCentral.LinterCop/Design/Rule0059SingleQuoteEscaping.cs b/BusinessCentral.LinterCop/Design/Rule0059SingleQuoteEscaping.cs index 008d1b3..0e93e0a 100644 --- a/BusinessCentral.LinterCop/Design/Rule0059SingleQuoteEscaping.cs +++ b/BusinessCentral.LinterCop/Design/Rule0059SingleQuoteEscaping.cs @@ -18,47 +18,39 @@ public override void Initialize(AnalysisContext context) => private void AnalyzeCalcFormula(SymbolAnalysisContext ctx) { - // Investigate https://github.com/StefanMaron/BusinessCentral.LinterCop/issues/822 - try - { - if (ctx.IsObsoletePendingOrRemoved()) - return; + if (ctx.IsObsoletePendingOrRemoved()) + return; - SyntaxNode? syntaxNode = ctx.Symbol.DeclaringSyntaxReference?.GetSyntax(ctx.CancellationToken); - if (syntaxNode == null) - return; + SyntaxNode? syntaxNode = ctx.Symbol.DeclaringSyntaxReference?.GetSyntax(ctx.CancellationToken); + if (syntaxNode == null) + return; - if (syntaxNode is not FieldSyntax fieldSyntax) - return; + if (syntaxNode is not FieldSyntax fieldSyntax) + return; - // Retrieve the 'CalcFormula' property from the field's property list - var calcFormulaPropertySyntax = fieldSyntax.PropertyList?.Properties - .OfType() - .Select(p => p.Value) - .OfType() - .FirstOrDefault(); + // Retrieve the 'CalcFormula' property from the field's property list + var calcFormulaPropertySyntax = fieldSyntax.PropertyList?.Properties + .OfType() + .Select(p => p.Value) + .OfType() + .FirstOrDefault(); - if (calcFormulaPropertySyntax is null) - return; + if (calcFormulaPropertySyntax is null) + return; - // Retrieve the filter expression from the 'Where' expression of the CalcFormula - var filterExpressions = calcFormulaPropertySyntax.WhereExpression.Filter.Conditions - .OfType() - .Where(c => c.Filter.Kind == SyntaxKind.UnaryEqualsFilterExpression) - .Select(c => c.Filter); + // Retrieve the filter expression from the 'Where' expression of the CalcFormula + var filterExpressions = calcFormulaPropertySyntax.WhereExpression?.Filter.Conditions + .OfType() + .Where(c => c.Filter.Kind == SyntaxKind.UnaryEqualsFilterExpression) + .Select(c => c.Filter); - if (filterExpressions is null) - return; + if (filterExpressions is null) + return; - foreach (var filter in filterExpressions) - { - if (filter.ToString().Equals(InvalidUnaryEqualsFilter)) - ctx.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.Rule0059SingleQuoteEscapingIssueDetected, filter.GetLocation())); - } - } - catch (NullReferenceException) + foreach (var filter in filterExpressions) { - ctx.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.Rule0000ErrorInRule, ctx.Symbol.GetLocation(), new Object[] { "Rule0029", "NullReferenceException", "" })); + if (filter.ToString().Equals(InvalidUnaryEqualsFilter)) + ctx.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.Rule0059SingleQuoteEscapingIssueDetected, filter.GetLocation())); } } }