Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude LC0052 and LC0053 on procedure which implements an interface #572

Merged
merged 1 commit into from
Mar 30, 2024
Merged

Exclude LC0052 and LC0053 on procedure which implements an interface #572

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 12 additions & 50 deletions Design/Rule0052and0053InternalProceduresNotReferenced.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,47 +6,31 @@
using Microsoft.Dynamics.Nav.CodeAnalysis.Symbols;
using Microsoft.Dynamics.Nav.CodeAnalysis.Syntax;
using Microsoft.Dynamics.Nav.CodeAnalysis.Packaging;



using System.Collections.Immutable;

namespace BusinessCentral.LinterCop.Design
{
[DiagnosticAnalyzer]
public class Rule0052InternalProceduresNotReferencedAnalyzer : DiagnosticAnalyzer
{

private class MethodSymbolAnalyzer : IDisposable
{
private readonly PooledDictionary<IMethodSymbol, string> methodSymbols = PooledDictionary<IMethodSymbol, string>.GetInstance();

private readonly PooledDictionary<IMethodSymbol, string> internalMethodsUnused = PooledDictionary<IMethodSymbol, string>.GetInstance();
private readonly PooledDictionary<IMethodSymbol, string> internalMethodsUsedInCurrentObject = PooledDictionary<IMethodSymbol, string>.GetInstance();
private readonly PooledDictionary<IMethodSymbol, string> internalMethodsUsedInOtherObjects = PooledDictionary<IMethodSymbol, string>.GetInstance();

private readonly AttributeKind[] attributeKindsOfMethodsToSkip = new AttributeKind[] { AttributeKind.ConfirmHandler, AttributeKind.FilterPageHandler, AttributeKind.HyperlinkHandler, AttributeKind.MessageHandler, AttributeKind.ModalPageHandler, AttributeKind.PageHandler, AttributeKind.RecallNotificationHandler, AttributeKind.ReportHandler, AttributeKind.RequestPageHandler, AttributeKind.SendNotificationHandler, AttributeKind.SessionSettingsHandler, AttributeKind.StrMenuHandler, AttributeKind.Test };

public MethodSymbolAnalyzer(CompilationAnalysisContext compilationAnalysisContext)
{
NavAppManifest manifest = ManifestHelper.GetManifest(compilationAnalysisContext.Compilation);

if (manifest.InternalsVisibleTo != null && manifest.InternalsVisibleTo.Any())
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am surprised to see the curly brackets getting removed though (and all the non-redundant whitespace) 😮
It is good practice to have curly brackets in C# even if there is only a single statement.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a novice level C# developer I though I was improving readability of the code by removing the curly brackets.

I thought if it is a one liner, the code is cleaner to have it without curly braces. Even maybe put it in one nice and short line. After all, C# compiler allows it, who am I to disagree with that? Boy, was I wrong 😯

https://www.armannotes.com/2020/11/23/why-you-should-always-use-curly-braces-for-single-statement-blocks-in-csharp/

@rvanbekkum, I genuinely appreciate you feedback on this. I've learned why you should always use curly braces for single statement blocks in C#. I'll revert these changes with a new PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect it wouldn't be as much of a problem with C# if it would always apply a formatter automatically like what happens for AL code. 🙂

return;
}

ImmutableArray<IApplicationObjectTypeSymbol>.Enumerator objectEnumerator = compilationAnalysisContext.Compilation.GetDeclaredApplicationObjectSymbols().GetEnumerator();
while (objectEnumerator.MoveNext())
{
IApplicationObjectTypeSymbol applicationSymbol = objectEnumerator.Current;

if (applicationSymbol.GetNavTypeKindSafe() == NavTypeKind.Codeunit)
{
// If the containing object is an codeunit and implements an interface, then we do not need to check for references for this procedure.
ICodeunitTypeSymbol codeunitSymbol = applicationSymbol as ICodeunitTypeSymbol;
if (codeunitSymbol != null && codeunitSymbol.ImplementedInterfaces.Any()) continue;
}
ImmutableArray<ISymbol>.Enumerator objectMemberEnumerator = applicationSymbol.GetMembers().GetEnumerator();
while (objectMemberEnumerator.MoveNext())
{
Expand All @@ -67,83 +51,64 @@ public MethodSymbolAnalyzer(CompilationAnalysisContext compilationAnalysisContex
private bool MethodNeedsReferenceCheck(IMethodSymbol methodSymbol)
{
if (methodSymbol.MethodKind != MethodKind.Method)
{
return false;
}

if (methodSymbol.IsObsoletePending)
{
return false;
}

if (methodSymbol.Attributes.Any(attr => attributeKindsOfMethodsToSkip.Contains(attr.AttributeKind)))
{
return false;
}

IApplicationObjectTypeSymbol objectSymbol = methodSymbol.GetContainingApplicationObjectTypeSymbol();
if (HelperFunctions.MethodImplementsInterfaceMethod(objectSymbol, methodSymbol))
return false;

if (!methodSymbol.IsInternal)
{
// Check if public procedure in internal object
if (methodSymbol.DeclaredAccessibility == Accessibility.Public && methodSymbol.ContainingSymbol is IApplicationObjectTypeSymbol)
{
var objectSymbol = methodSymbol.GetContainingApplicationObjectTypeSymbol();

// If the containing object is not an internal object, then we do not need to check for references for this public procedure.
if (objectSymbol.DeclaredAccessibility != Accessibility.Internal)
{
return false;
}

if (HelperFunctions.MethodImplementsInterfaceMethod(objectSymbol, methodSymbol))
{
return false;
}
}
else
{
return false;
}
}

// If the procedure has signature ProcedureName(HostNotification: Notification) or ProcedureName(ErrorInfo: ErrorInfo), then the procedure does not need a reference check
if (methodSymbol.Parameters.Length == 1)
{
ITypeSymbol firstParameterTypeSymbol = methodSymbol.Parameters[0].ParameterType;
if (firstParameterTypeSymbol.GetNavTypeKindSafe() == NavTypeKind.Notification || firstParameterTypeSymbol.GetNavTypeKindSafe() == NavTypeKind.ErrorInfo)
{
return false;
}
}

return true;
}

public void AnalyzeObjectSyntax(CompilationAnalysisContext compilationAnalysisContext)
{
if (methodSymbols.Count == 0)
{
return;
}

Compilation compilation = compilationAnalysisContext.Compilation;
ImmutableArray<SyntaxTree>.Enumerator enumerator = compilation.SyntaxTrees.GetEnumerator();
while (enumerator.MoveNext())
{
if (methodSymbols.Count == 0)
{
break;
}

SyntaxTree syntaxTree = enumerator.Current;
SemanticModel semanticModel = compilation.GetSemanticModel(syntaxTree);

syntaxTree.GetRoot().WalkDescendantsAndPerformAction(delegate (SyntaxNode syntaxNode)
{
if (methodSymbols.Count == 0)
{
return;
}

if (syntaxNode.Parent.IsKind(SyntaxKind.MethodDeclaration) || !syntaxNode.IsKind(SyntaxKind.IdentifierName))
{
return;
}

IdentifierNameSyntax identifierNameSyntax = (IdentifierNameSyntax)syntaxNode;
if (methodSymbols.ContainsValue(identifierNameSyntax.Identifier.ValueText.ToLowerInvariant()) && TryGetSymbolFromIdentifier(semanticModel, (IdentifierNameSyntax)syntaxNode, SymbolKind.Method, out var methodSymbol))
{
Expand Down Expand Up @@ -179,23 +144,20 @@ internal static bool TryGetSymbolFromIdentifier(SemanticModel semanticModel, Ide
SymbolInfo symbolInfo = semanticModel.GetSymbolInfo(identifierName);
ISymbol symbol = symbolInfo.Symbol;
if (symbol == null || symbol.Kind != symbolKind)
{
return false;
}

methodSymbol = symbolInfo.Symbol as IMethodSymbol;
if (methodSymbol == null)
{
return false;
}

return true;
}

public void ReportUnchangedReferencePassedParameters(Action<Diagnostic> action)
{
if (internalMethodsUnused.Count == 0)
{
return;
}

foreach (KeyValuePair<IMethodSymbol, string> unusedInternalMethod in internalMethodsUnused)
{
IMethodSymbol methodSymbol = unusedInternalMethod.Key;
Expand Down