From fbf65dc482192121d890de4e8154ceb45360bc15 Mon Sep 17 00:00:00 2001 From: Cristian Ambrosini Date: Thu, 11 Apr 2024 12:18:23 +0200 Subject: [PATCH 1/4] Fix S1144 FN: Unused local functions --- .../Helpers/CSharpSyntaxHelper.cs | 24 +++++++------------ .../Rules/UnusedPrivateMember.cs | 9 +++++-- .../Helpers/SymbolHelper.cs | 1 + .../Helpers/SymbolHelperTest.cs | 2 +- .../TestCases/UnusedPrivateMember.CSharp10.cs | 2 +- .../TestCases/UnusedPrivateMember.CSharp7.cs | 9 ------- .../TestCases/UnusedPrivateMember.CSharp9.cs | 4 ++-- .../UnusedPrivateMember.Fixed.Batch.cs | 6 +++-- .../TestCases/UnusedPrivateMember.Fixed.cs | 6 +++-- .../TestCases/UnusedPrivateMember.cs | 8 ++++++- 10 files changed, 35 insertions(+), 36 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs index c4a27e7b544..b527ab22575 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs @@ -167,23 +167,15 @@ public static bool ContainsMethodInvocation(this BaseMethodDeclarationSyntax met .Any(symbolPredicate); } - public static SyntaxToken? GetIdentifierOrDefault(this BaseMethodDeclarationSyntax methodDeclaration) - { - switch (methodDeclaration?.Kind()) + public static SyntaxToken? GetIdentifierOrDefault(this BaseMethodDeclarationSyntax methodDeclaration) => + methodDeclaration?.Kind() switch { - case SyntaxKind.ConstructorDeclaration: - return ((ConstructorDeclarationSyntax)methodDeclaration).Identifier; - - case SyntaxKind.DestructorDeclaration: - return ((DestructorDeclarationSyntax)methodDeclaration).Identifier; - - case SyntaxKind.MethodDeclaration: - return ((MethodDeclarationSyntax)methodDeclaration).Identifier; - - default: - return null; - } - } + SyntaxKind.ConstructorDeclaration => ((ConstructorDeclarationSyntax)methodDeclaration)?.Identifier, + SyntaxKind.DestructorDeclaration => ((DestructorDeclarationSyntax)methodDeclaration)?.Identifier, + SyntaxKind.MethodDeclaration => ((MethodDeclarationSyntax)methodDeclaration)?.Identifier, + _ when LocalFunctionStatementSyntaxWrapper.IsInstance(methodDeclaration) => ((LocalFunctionStatementSyntaxWrapper)methodDeclaration).Identifier, + _ => null, + }; public static bool IsMethodInvocation(this InvocationExpressionSyntax invocation, KnownType type, string methodName, SemanticModel semanticModel) => invocation.Expression.NameIs(methodName) && diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs index eadf289d229..8329378664a 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs @@ -320,7 +320,7 @@ public CSharpRemovableSymbolWalker(Func getSema this.containingTypeAccessibility = containingTypeAccessibility; } - // This override is needed because VisitRecordDeclaration is not available due to the Roslyn version. + // This override is needed because VisitRecordDeclaration and LocalFunctionStatementSyntax are not available due to the Roslyn version. public override void Visit(SyntaxNode node) { if (node.IsAnyKind(SyntaxKindEx.RecordClassDeclaration, SyntaxKindEx.RecordStructDeclaration)) @@ -328,6 +328,11 @@ public override void Visit(SyntaxNode node) VisitBaseTypeDeclaration(node); } + if (LocalFunctionStatementSyntaxWrapper.IsInstance(node)) + { + ConditionalStore((IMethodSymbol)GetDeclaredSymbol(node), IsRemovableMethod); + } + base.Visit(node); } @@ -488,7 +493,7 @@ static bool IsPartial(TypeDeclarationSyntax typeDeclaration) => private static bool IsRemovableMethod(IMethodSymbol methodSymbol) => IsRemovableMember(methodSymbol) - && (methodSymbol.MethodKind == MethodKind.Ordinary || methodSymbol.MethodKind == MethodKind.Constructor) + && (methodSymbol.MethodKind is MethodKind.Ordinary or MethodKind.Constructor or MethodKindEx.LocalFunction) && !methodSymbol.IsMainMethod() && (!methodSymbol.IsEventHandler() || !IsDeclaredInPartialClass(methodSymbol)) // Event handlers could be added in XAML and no method reference will be generated in the .g.cs file. && !methodSymbol.IsSerializationConstructor() diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/SymbolHelper.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/SymbolHelper.cs index 77b6c38e9a0..92edd49dd36 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/SymbolHelper.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/SymbolHelper.cs @@ -315,6 +315,7 @@ public static string GetClassification(this ISymbol symbol) => { MethodKind: MethodKind.Destructor } => "destructor", { MethodKind: MethodKind.PropertyGet } => "getter", { MethodKind: MethodKind.PropertySet } => "setter", + { MethodKind: MethodKindEx.LocalFunction } => "local function", _ => "method", }, INamedTypeSymbol namedTypeSymbol => namedTypeSymbol switch diff --git a/analyzers/tests/SonarAnalyzer.Test/Helpers/SymbolHelperTest.cs b/analyzers/tests/SonarAnalyzer.Test/Helpers/SymbolHelperTest.cs index d74bbbab90d..98e24ec59cd 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Helpers/SymbolHelperTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Helpers/SymbolHelperTest.cs @@ -398,7 +398,7 @@ public void GetClassification_Record(TypeKind typeKind, string expected) [DataRow(MethodKind.ExplicitInterfaceImplementation, "method")] [DataRow(MethodKind.FunctionPointerSignature, "method")] [DataRow(MethodKind.LambdaMethod, "method")] - [DataRow(MethodKind.LocalFunction, "method")] + [DataRow(MethodKind.LocalFunction, "local function")] [DataRow(MethodKind.Ordinary, "method")] [DataRow(MethodKind.PropertyGet, "getter")] [DataRow(MethodKind.PropertySet, "setter")] diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp10.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp10.cs index 7e1cee0fc40..20970032551 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp10.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp10.cs @@ -70,7 +70,7 @@ static void Quix() { } [Obsolete] static void ForCoverage() { } - static void NoAttribute() { } + static void NoAttribute() { } // Noncompliant } } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp7.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp7.cs index dc4fa168871..31319f69da1 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp7.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp7.cs @@ -198,15 +198,6 @@ private enum Y } } -// https://github.com/SonarSource/sonar-dotnet/issues/6699 -public class Repro_6699 -{ - public void MethodUsingLocalMethod() - { - void LocalMethod() { } // FN - } -} - // https://github.com/SonarSource/sonar-dotnet/issues/6724 public class Repro_6724 { diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs index eb65156b832..1ccd42adf73 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs @@ -69,7 +69,7 @@ static void Quix() { } [Obsolete] static void ForCoverage() { } - static void NoAttribute() { } + static void NoAttribute() { } // Noncompliant } } @@ -158,7 +158,7 @@ public bool MethodWithLocalFunction() { return false; - bool PrintMembers(StringBuilder builder) => true; // FN - local functions are not handled + bool PrintMembers(StringBuilder builder) => true; // Noncompliant } } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.Fixed.Batch.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.Fixed.Batch.cs index f27dbf66f6a..05503920818 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.Fixed.Batch.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.Fixed.Batch.cs @@ -94,11 +94,13 @@ private interface MyInterface void Method(); } + // https://github.com/SonarSource/sonar-dotnet/issues/6699 public void MethodUsingLocalMethod() { - void LocalMethod() // FN: local function is never used - { + LocalMethod(); + void LocalMethod() + { } } } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.Fixed.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.Fixed.cs index c45b707c95b..7deb5606f78 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.Fixed.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.Fixed.cs @@ -78,11 +78,13 @@ private int MyProperty [My] private class Class1 { } + // https://github.com/SonarSource/sonar-dotnet/issues/6699 public void MethodUsingLocalMethod() { - void LocalMethod() // FN: local function is never used - { + LocalMethod(); + void LocalMethod() + { } } } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.cs index dad4f7fdd86..310299baa71 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.cs @@ -144,11 +144,17 @@ internal class Class4 : MyInterface // Noncompliant {{Remove the unused internal public void Method() { } } + // https://github.com/SonarSource/sonar-dotnet/issues/6699 public void MethodUsingLocalMethod() { - void LocalMethod() // FN: local function is never used + LocalMethod(); + + void LocalMethod() { + } + void UnusedLocalMethod() // Noncompliant {{Remove the unused private local function 'UnusedLocalMethod'.}} + { } } } From 85b39ab90ff32bbfe5ea27dae19c236d7de452b8 Mon Sep 17 00:00:00 2001 From: Cristian Ambrosini Date: Thu, 11 Apr 2024 14:59:17 +0200 Subject: [PATCH 2/4] Address 1st round of comments --- .../src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs | 6 +++--- .../src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs index b527ab22575..cb56f10fd11 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs @@ -170,9 +170,9 @@ public static bool ContainsMethodInvocation(this BaseMethodDeclarationSyntax met public static SyntaxToken? GetIdentifierOrDefault(this BaseMethodDeclarationSyntax methodDeclaration) => methodDeclaration?.Kind() switch { - SyntaxKind.ConstructorDeclaration => ((ConstructorDeclarationSyntax)methodDeclaration)?.Identifier, - SyntaxKind.DestructorDeclaration => ((DestructorDeclarationSyntax)methodDeclaration)?.Identifier, - SyntaxKind.MethodDeclaration => ((MethodDeclarationSyntax)methodDeclaration)?.Identifier, + SyntaxKind.ConstructorDeclaration => ((ConstructorDeclarationSyntax)methodDeclaration).Identifier, + SyntaxKind.DestructorDeclaration => ((DestructorDeclarationSyntax)methodDeclaration).Identifier, + SyntaxKind.MethodDeclaration => ((MethodDeclarationSyntax)methodDeclaration).Identifier, _ when LocalFunctionStatementSyntaxWrapper.IsInstance(methodDeclaration) => ((LocalFunctionStatementSyntaxWrapper)methodDeclaration).Identifier, _ => null, }; diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs index 8329378664a..2efe588a1a0 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs @@ -328,7 +328,7 @@ public override void Visit(SyntaxNode node) VisitBaseTypeDeclaration(node); } - if (LocalFunctionStatementSyntaxWrapper.IsInstance(node)) + if (node.IsKind(SyntaxKindEx.LocalFunctionStatement)) { ConditionalStore((IMethodSymbol)GetDeclaredSymbol(node), IsRemovableMethod); } From d6f2ce275a1beb1307c203cf6b182136c5f36cf3 Mon Sep 17 00:00:00 2001 From: Cristian Ambrosini Date: Thu, 11 Apr 2024 16:37:59 +0200 Subject: [PATCH 3/4] Revert changes in CSharpSyntaxHelper --- .../Helpers/CSharpSyntaxHelper.cs | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs index cb56f10fd11..8be82479397 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs @@ -167,15 +167,23 @@ public static bool ContainsMethodInvocation(this BaseMethodDeclarationSyntax met .Any(symbolPredicate); } - public static SyntaxToken? GetIdentifierOrDefault(this BaseMethodDeclarationSyntax methodDeclaration) => - methodDeclaration?.Kind() switch + public static SyntaxToken? GetIdentifierOrDefault(this BaseMethodDeclarationSyntax methodDeclaration) + { + switch (methodDeclaration?.Kind()) { - SyntaxKind.ConstructorDeclaration => ((ConstructorDeclarationSyntax)methodDeclaration).Identifier, - SyntaxKind.DestructorDeclaration => ((DestructorDeclarationSyntax)methodDeclaration).Identifier, - SyntaxKind.MethodDeclaration => ((MethodDeclarationSyntax)methodDeclaration).Identifier, - _ when LocalFunctionStatementSyntaxWrapper.IsInstance(methodDeclaration) => ((LocalFunctionStatementSyntaxWrapper)methodDeclaration).Identifier, - _ => null, - }; + case SyntaxKind.ConstructorDeclaration: + return ((ConstructorDeclarationSyntax) methodDeclaration).Identifier; + + case SyntaxKind.DestructorDeclaration: + return ((DestructorDeclarationSyntax) methodDeclaration).Identifier; + + case SyntaxKind.MethodDeclaration: + return ((MethodDeclarationSyntax) methodDeclaration).Identifier; + + default: + return null; + } + } public static bool IsMethodInvocation(this InvocationExpressionSyntax invocation, KnownType type, string methodName, SemanticModel semanticModel) => invocation.Expression.NameIs(methodName) && From 9c32ef0b387d5dc748540bbc0260f6803617bfc6 Mon Sep 17 00:00:00 2001 From: Cristian Ambrosini Date: Thu, 11 Apr 2024 16:38:28 +0200 Subject: [PATCH 4/4] Remove spaces --- .../src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs index 8be82479397..c4a27e7b544 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSyntaxHelper.cs @@ -172,13 +172,13 @@ public static bool ContainsMethodInvocation(this BaseMethodDeclarationSyntax met switch (methodDeclaration?.Kind()) { case SyntaxKind.ConstructorDeclaration: - return ((ConstructorDeclarationSyntax) methodDeclaration).Identifier; + return ((ConstructorDeclarationSyntax)methodDeclaration).Identifier; case SyntaxKind.DestructorDeclaration: - return ((DestructorDeclarationSyntax) methodDeclaration).Identifier; + return ((DestructorDeclarationSyntax)methodDeclaration).Identifier; case SyntaxKind.MethodDeclaration: - return ((MethodDeclarationSyntax) methodDeclaration).Identifier; + return ((MethodDeclarationSyntax)methodDeclaration).Identifier; default: return null;