From 88f6d112abec3bb33eec96fdaa6421d0f8377d0f Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Wed, 5 Jun 2024 14:37:40 +0200 Subject: [PATCH 01/13] Fix S1144 FP: types used through reflection --- .../Helpers/CSharpSymbolUsageCollector.cs | 48 ++++++--- .../Rules/UnusedPrivateMember.cs | 22 ++++- .../SonarAnalyzer.Common/Helpers/KnownType.cs | 1 + .../TestCases/UnusedPrivateMember.CSharp9.cs | 98 +++++++++++++++++++ 4 files changed, 155 insertions(+), 14 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs index b7dc1d889ee..281ffad0f72 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs @@ -53,6 +53,7 @@ private enum SymbolAccess public IDictionary FieldSymbolUsages { get; } = new Dictionary(); public HashSet DebuggerDisplayValues { get; } = []; public Dictionary PropertyAccess { get; } = []; + public HashSet TypesUsedWithReflection { get; } = []; public CSharpSymbolUsageCollector(Compilation compilation, IEnumerable knownSymbols) { @@ -134,19 +135,26 @@ static ISymbol FindDeconstructor(IEnumerable deconstructors, int number public override void VisitAttribute(AttributeSyntax node) { - var symbol = semanticModel.GetSymbolInfo(node).Symbol; - if (symbol != null - && symbol.ContainingType.Is(KnownType.System_Diagnostics_DebuggerDisplayAttribute) - && node.ArgumentList != null) + if (semanticModel.GetSymbolInfo(node).Symbol is { } symbol) { - var arguments = node.ArgumentList.Arguments - .Where(IsValueNameOrType) - .Select(a => semanticModel.GetConstantValue(a.Expression)) - .Where(o => o.HasValue) - .Select(o => o.Value) - .OfType(); - - DebuggerDisplayValues.UnionWith(arguments); + if (symbol.ContainingType.Is(KnownType.System_Diagnostics_DebuggerDisplayAttribute) + && node.ArgumentList != null) + { + var arguments = node.ArgumentList.Arguments + .Where(IsValueNameOrType) + .Select(a => semanticModel.GetConstantValue(a.Expression)) + .Where(o => o.HasValue) + .Select(o => o.Value) + .OfType(); + + DebuggerDisplayValues.UnionWith(arguments); + } + else if (symbol.ContainingType.Is(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute) + && node.Parent.Parent is BaseTypeDeclarationSyntax typeDeclaration + && semanticModel.GetDeclaredSymbol(typeDeclaration) is { } typeSymbol) + { + TypesUsedWithReflection.Add(typeSymbol); + } } base.VisitAttribute(node); @@ -169,6 +177,22 @@ public override void VisitIdentifierName(IdentifierNameSyntax node) base.VisitIdentifierName(node); } + public override void VisitTypeArgumentList(TypeArgumentListSyntax node) + { + if (semanticModel.GetSymbolInfo(node.Parent).Symbol is { } symbol) + { + var typeArgumentSymbols = symbol is IMethodSymbol method + ? method.TypeParameters + : ((INamedTypeSymbol)symbol).TypeParameters; + var typesWithDynamicallyAccessedMembers = typeArgumentSymbols.Zip(node.Arguments, (symbol, argument) => (symbol, argument)) + .Where(x => x.symbol.HasAttribute(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute)) + .Select(x => semanticModel.GetSymbolInfo(x.argument).Symbol) + .Where(x => x is INamedTypeSymbol { IsUnboundGenericType: false }); + TypesUsedWithReflection.UnionWith(typesWithDynamicallyAccessedMembers); + } + base.VisitTypeArgumentList(node); + } + public override void VisitObjectCreationExpression(ObjectCreationExpressionSyntax node) { if (knownSymbolNames.Contains(node.Type.GetName())) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs index 9c8617113b3..bf439580d60 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs @@ -151,6 +151,20 @@ private static void ReportUnusedPrivateMembers(SonarCompilationReporti ReportDiagnosticsForMembers(context, unusedSymbols, accessibility, fieldLikeSymbols); } + private static bool IsUsedWithReflection(ISymbol symbol, HashSet symbolsUsedWithReflection) + { + var currentSymbol = symbol; + while (currentSymbol is not null) + { + if (symbolsUsedWithReflection.Contains(currentSymbol)) + { + return true; + } + currentSymbol = currentSymbol.ContainingSymbol; + } + return false; + } + private static bool IsMentionedInDebuggerDisplay(ISymbol symbol, CSharpSymbolUsageCollector usageCollector) => usageCollector.DebuggerDisplayValues.Any(x => x.Contains(symbol.Name)); @@ -161,7 +175,9 @@ private static void ReportUsedButUnreadFields(SonarSymbolReportingContext contex var usedButUnreadFields = usageCollector.FieldSymbolUsages.Values .Where(x => x.Symbol.DeclaredAccessibility == Accessibility.Private || x.Symbol.ContainingType?.DeclaredAccessibility == Accessibility.Private) .Where(x => x.Symbol.Kind == SymbolKind.Field || x.Symbol.Kind == SymbolKind.Event) - .Where(x => !unusedSymbols.Contains(x.Symbol) && !IsMentionedInDebuggerDisplay(x.Symbol, usageCollector)) + .Where(x => !unusedSymbols.Contains(x.Symbol) + && !IsMentionedInDebuggerDisplay(x.Symbol, usageCollector) + && !IsUsedWithReflection(x.Symbol, usageCollector.TypesUsedWithReflection)) .Where(x => x.Declaration is not null && !x.Readings.Any()); foreach (var usage in usedButUnreadFields) @@ -173,7 +189,9 @@ private static void ReportUsedButUnreadFields(SonarSymbolReportingContext contex private static HashSet GetUnusedSymbols(CSharpSymbolUsageCollector usageCollector, IEnumerable removableSymbols) => removableSymbols .Except(usageCollector.UsedSymbols) - .Where(x => !IsMentionedInDebuggerDisplay(x, usageCollector) && !IsAccessorUsed(x, usageCollector)) + .Where(x => !IsMentionedInDebuggerDisplay(x, usageCollector) + && !IsAccessorUsed(x, usageCollector) + && !IsUsedWithReflection(x, usageCollector.TypesUsedWithReflection)) .ToHashSet(); private static bool IsAccessorUsed(ISymbol symbol, CSharpSymbolUsageCollector usageCollector) => diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs index e7e27702090..a3f18294b1e 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs @@ -364,6 +364,7 @@ public sealed partial class KnownType public static readonly KnownType System_DateTimeOffset = new("System.DateTimeOffset"); public static readonly KnownType System_Decimal = new("System.Decimal"); public static readonly KnownType System_Delegate = new("System.Delegate"); + public static readonly KnownType System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute = new("System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute"); public static readonly KnownType System_Diagnostics_CodeAnalysis_ExcludeFromCodeCoverageAttribute = new("System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverageAttribute"); public static readonly KnownType System_Diagnostics_CodeAnalysis_SuppressMessageAttribute = new("System.Diagnostics.CodeAnalysis.SuppressMessageAttribute"); public static readonly KnownType System_Diagnostics_CodeAnalysis_StringSyntaxAttribute = new("System.Diagnostics.CodeAnalysis.StringSyntaxAttribute"); diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs index 1ccd42adf73..cb9eb73574b 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs @@ -1,5 +1,6 @@ using System; using System.Text; +using System.Diagnostics.CodeAnalysis; using System.Runtime.InteropServices; namespace Tests.Diagnostics @@ -168,3 +169,100 @@ public class ClassWithPrintMembers } } } + +// https://github.com/SonarSource/sonar-dotnet/issues/9379 +namespace Repro_9379 +{ + public static class Program + { + public static void Method() + { + var instance = CreateInstance(); + } + + public static T CreateInstance<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>() => + (T)Activator.CreateInstance(typeof(T), 42); + + private class ClassInstantiatedThroughReflection + { + private const int PrivateConst = 42; // Compliant - we assume all members are used through reflection + private int privateField; + private int PrivateProperty { get; set; } + private void PrivateMethod() { } + private ClassInstantiatedThroughReflection() { } + private event EventHandler PrivateEvent; + + public ClassInstantiatedThroughReflection(int arg) // Compliant - the constructor used in CreateInstance through Reflection + { + } + + private class NestedType + { + private int privateField; + } + } + + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] + private class TypeDecoratedWithDynamicallyAccessedMembers + { + private const int PrivateConst = 42; + private int privateField; + private int PrivateProperty { get; set; } + private void PrivateMethod() { } + public TypeDecoratedWithDynamicallyAccessedMembers() { } + private event EventHandler PrivateEvent; + + private class NestedType + { + private const int PrivateConst = 42; + private int privateField; + private int PrivateProperty { get; set; } + private void PrivateMethod() { } + private NestedType() { } + private event EventHandler PrivateEvent; + } + } + + private class MembersDecoratedWithAttribute // Noncompliant + { + private const int PrivateConst = 42; // Noncompliant + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicFields)] private const int PrivateConstWithAttribute = 42; + + private int privateField; // Noncompliant + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicFields)] private int privateFieldWithAttribute; + + private int PrivateProperty { get; set; } // Noncompliant + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicProperties)] private int PrivatePropertyWithAttribute { get; set; } + + private void PrivateMethod() { } // Noncompliant + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicMethods)] private void PrivateMethodWithAttribute() { } + + private class NestedType { } // Noncompliant + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicNestedTypes)] private class NestedTypeWithAttribute { } + } + + private class ArgumentsDecoratedWithAttribute // Noncompliant + { + public void PublicMethod() => Method(null); // Noncompliant + + private void Method([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] NestedType arg) { } + + private class NestedType + { + private NestedType(int arg) { } + } + } + + private class DerivedFromTypeDecoratedWithDynamicallyAccessedMembers : TypeDecoratedWithDynamicallyAccessedMembers // Noncompliant + { + private int privateField; // Noncompliant - [DynamicallyAccessedMembers] attribute is not inherited + } + + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] + private class DynamicallyAccessedMembersOnlyForConstructors + { + private int privateField; // FN - only public constructors are used are indicated to be used through reflection, not fields + // The analyzer assumens when the [DynamicallyAccessedMembers] attribute is used, then all members are used through reflection + } + } +} From a954c41fec4b1af453c16b01973715ab8b4527a9 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Wed, 5 Jun 2024 16:21:01 +0200 Subject: [PATCH 02/13] Fix QA issue --- .../Helpers/CSharpSymbolUsageCollector.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs index 281ffad0f72..06505d2f77f 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs @@ -184,9 +184,9 @@ public override void VisitTypeArgumentList(TypeArgumentListSyntax node) var typeArgumentSymbols = symbol is IMethodSymbol method ? method.TypeParameters : ((INamedTypeSymbol)symbol).TypeParameters; - var typesWithDynamicallyAccessedMembers = typeArgumentSymbols.Zip(node.Arguments, (symbol, argument) => (symbol, argument)) - .Where(x => x.symbol.HasAttribute(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute)) - .Select(x => semanticModel.GetSymbolInfo(x.argument).Symbol) + var typesWithDynamicallyAccessedMembers = typeArgumentSymbols.Zip(node.Arguments, (symbol, argument) => new Tuple(symbol, argument)) + .Where(x => x.Item1.HasAttribute(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute)) + .Select(x => semanticModel.GetSymbolInfo(x.Item2).Symbol) .Where(x => x is INamedTypeSymbol { IsUnboundGenericType: false }); TypesUsedWithReflection.UnionWith(typesWithDynamicallyAccessedMembers); } From 20bfa79a256220660c31b08d60feba032436cd01 Mon Sep 17 00:00:00 2001 From: Cristian Ambrosini Date: Thu, 6 Jun 2024 10:59:21 +0200 Subject: [PATCH 03/13] Address comments review 1 (partial) --- .../Helpers/CSharpSymbolUsageCollector.cs | 19 ++++++++++++------- .../TestCases/UnusedPrivateMember.CSharp9.cs | 9 +++++++-- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs index 06505d2f77f..0ba14fb5308 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs @@ -179,18 +179,23 @@ public override void VisitIdentifierName(IdentifierNameSyntax node) public override void VisitTypeArgumentList(TypeArgumentListSyntax node) { - if (semanticModel.GetSymbolInfo(node.Parent).Symbol is { } symbol) + if (semanticModel.GetSymbolInfo(node.Parent).Symbol is { } symbol) // symbol that the attribute is applied to { - var typeArgumentSymbols = symbol is IMethodSymbol method - ? method.TypeParameters - : ((INamedTypeSymbol)symbol).TypeParameters; - var typesWithDynamicallyAccessedMembers = typeArgumentSymbols.Zip(node.Arguments, (symbol, argument) => new Tuple(symbol, argument)) + var typesWithDynamicallyAccessedMembers = GetMethodTypeParameters(symbol) + .Zip(node.Arguments, (symbol, argument) => new Tuple(symbol, argument)) // map T to Person in void M() { } .. M(); .Where(x => x.Item1.HasAttribute(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute)) - .Select(x => semanticModel.GetSymbolInfo(x.Item2).Symbol) - .Where(x => x is INamedTypeSymbol { IsUnboundGenericType: false }); + .Select(x => semanticModel.GetSymbolInfo(x.Item2).Symbol); TypesUsedWithReflection.UnionWith(typesWithDynamicallyAccessedMembers); } base.VisitTypeArgumentList(node); + + static ImmutableArray GetMethodTypeParameters(ISymbol symbol) => + symbol switch + { + IMethodSymbol method => method.TypeParameters, + INamedTypeSymbol namedType => namedType.TypeParameters, + _ => ImmutableArray.Empty + }; } public override void VisitObjectCreationExpression(ObjectCreationExpressionSyntax node) diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs index cb9eb73574b..a614353ed4b 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs @@ -239,6 +239,9 @@ private void PrivateMethod() { } // Noncompli private class NestedType { } // Noncompliant [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicNestedTypes)] private class NestedTypeWithAttribute { } + + [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicMethods)] + private int PrivateMethodWithReturn() { return 0; } // Noncompliant } private class ArgumentsDecoratedWithAttribute // Noncompliant @@ -251,6 +254,8 @@ private class NestedType { private NestedType(int arg) { } } + + public record RecordWithAttribute([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicFields)] string NotUnused); // Noncompliant } private class DerivedFromTypeDecoratedWithDynamicallyAccessedMembers : TypeDecoratedWithDynamicallyAccessedMembers // Noncompliant @@ -261,8 +266,8 @@ private class DerivedFromTypeDecoratedWithDynamicallyAccessedMembers : TypeDecor [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] private class DynamicallyAccessedMembersOnlyForConstructors { - private int privateField; // FN - only public constructors are used are indicated to be used through reflection, not fields - // The analyzer assumens when the [DynamicallyAccessedMembers] attribute is used, then all members are used through reflection + private int privateField; // FN - only public constructors are used are indicated to be used through reflection, not fields + // The analyzer assumens when the [DynamicallyAccessedMembers] attribute is used, then all members are used through reflection } } } From af66774b9a5269d2ccca8b70585e113d3036c980 Mon Sep 17 00:00:00 2001 From: Cristian Ambrosini Date: Thu, 6 Jun 2024 11:30:00 +0200 Subject: [PATCH 04/13] Add repro --- .../TestCases/UnusedPrivateMember.CSharp9.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs index a614353ed4b..6f12cebe707 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs @@ -178,11 +178,27 @@ public static class Program public static void Method() { var instance = CreateInstance(); + + A classViaReflection = new(); + InitValue(classViaReflection); } public static T CreateInstance<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>() => (T)Activator.CreateInstance(typeof(T), 42); + public static void InitValue<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>(T value) { } + + private class A + { + private bool a = true; // Noncompliant FP: type argument is inferred and ArgumentList is not present on syntax level + } + + class C + { + private int usedByReflection; + C Create() => Program.CreateInstance>(); + } + private class ClassInstantiatedThroughReflection { private const int PrivateConst = 42; // Compliant - we assume all members are used through reflection From f50d5861b1f7850c6a70fc6acac759db2bac475b Mon Sep 17 00:00:00 2001 From: Cristian Ambrosini Date: Thu, 6 Jun 2024 16:48:39 +0200 Subject: [PATCH 05/13] Review 2 --- .../Helpers/CSharpSymbolUsageCollector.cs | 10 +++++----- .../TestCases/UnusedPrivateMember.CSharp9.cs | 11 ++++++++++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs index 0ba14fb5308..fda4496cf16 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs @@ -135,9 +135,9 @@ static ISymbol FindDeconstructor(IEnumerable deconstructors, int number public override void VisitAttribute(AttributeSyntax node) { - if (semanticModel.GetSymbolInfo(node).Symbol is { } symbol) + if (semanticModel.GetSymbolInfo(node).Symbol is IMethodSymbol { MethodKind: MethodKind.Constructor, ContainingType: ITypeSymbol attribute }) { - if (symbol.ContainingType.Is(KnownType.System_Diagnostics_DebuggerDisplayAttribute) + if (attribute.Is(KnownType.System_Diagnostics_DebuggerDisplayAttribute) && node.ArgumentList != null) { var arguments = node.ArgumentList.Arguments @@ -149,7 +149,7 @@ public override void VisitAttribute(AttributeSyntax node) DebuggerDisplayValues.UnionWith(arguments); } - else if (symbol.ContainingType.Is(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute) + else if (attribute.Is(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute) && node.Parent.Parent is BaseTypeDeclarationSyntax typeDeclaration && semanticModel.GetDeclaredSymbol(typeDeclaration) is { } typeSymbol) { @@ -181,7 +181,7 @@ public override void VisitTypeArgumentList(TypeArgumentListSyntax node) { if (semanticModel.GetSymbolInfo(node.Parent).Symbol is { } symbol) // symbol that the attribute is applied to { - var typesWithDynamicallyAccessedMembers = GetMethodTypeParameters(symbol) + var typesWithDynamicallyAccessedMembers = GetTypeParameters(symbol) .Zip(node.Arguments, (symbol, argument) => new Tuple(symbol, argument)) // map T to Person in void M() { } .. M(); .Where(x => x.Item1.HasAttribute(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute)) .Select(x => semanticModel.GetSymbolInfo(x.Item2).Symbol); @@ -189,7 +189,7 @@ public override void VisitTypeArgumentList(TypeArgumentListSyntax node) } base.VisitTypeArgumentList(node); - static ImmutableArray GetMethodTypeParameters(ISymbol symbol) => + static ImmutableArray GetTypeParameters(ISymbol symbol) => symbol switch { IMethodSymbol method => method.TypeParameters, diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs index 6f12cebe707..b9b791bbd21 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs @@ -178,6 +178,7 @@ public static class Program public static void Method() { var instance = CreateInstance(); + var instance2 = CreateInstance(typeof(ClassInstantiatedThroughReflection)); A classViaReflection = new(); InitValue(classViaReflection); @@ -186,11 +187,14 @@ public static void Method() public static T CreateInstance<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>() => (T)Activator.CreateInstance(typeof(T), 42); + public static object CreateInstance([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type type) => + Activator.CreateInstance(type, 42); + public static void InitValue<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>(T value) { } private class A { - private bool a = true; // Noncompliant FP: type argument is inferred and ArgumentList is not present on syntax level + private bool a = true; // Noncompliant FP: type argument is inferred and ArgumentList is not present on syntax level (see line 183) } class C @@ -258,8 +262,13 @@ private class NestedType { } // Noncompli [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicMethods)] private int PrivateMethodWithReturn() { return 0; } // Noncompliant + + [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicMethods)] + private PrivateClass PrivateMethodWithReturnCustomClass() { return null; } // Noncompliant FP } + private class PrivateClass { } + private class ArgumentsDecoratedWithAttribute // Noncompliant { public void PublicMethod() => Method(null); // Noncompliant From 213dbcb5dc611cd63ed50c29ebb32b048937ca41 Mon Sep 17 00:00:00 2001 From: Cristian Ambrosini Date: Thu, 6 Jun 2024 17:07:14 +0200 Subject: [PATCH 06/13] Cleanup --- .../Helpers/CSharpSymbolUsageCollector.cs | 701 +++++++++--------- .../Rules/UnusedPrivateMember.cs | 15 +- .../UnusedPrivateMemberTest.Constructors.cs | 145 ++-- .../Rules/UnusedPrivateMemberTest.Fields.cs | 229 +++--- .../Rules/UnusedPrivateMemberTest.Methods.cs | 113 ++- .../Rules/UnusedPrivateMemberTest.Types.cs | 65 +- 6 files changed, 633 insertions(+), 635 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs index fda4496cf16..ea068961230 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs @@ -20,442 +20,447 @@ using SonarAnalyzer.Common.Walkers; -namespace SonarAnalyzer.Helpers +namespace SonarAnalyzer.Helpers; + +/// +/// Collects all symbol usages from a class declaration. Ignores symbols whose names are not present +/// in the knownSymbolNames collection for performance reasons. +/// +internal class CSharpSymbolUsageCollector : SafeCSharpSyntaxWalker { - /// - /// Collects all symbol usages from a class declaration. Ignores symbols whose names are not present - /// in the knownSymbolNames collection for performance reasons. - /// - internal class CSharpSymbolUsageCollector : SafeCSharpSyntaxWalker + [Flags] + private enum SymbolAccess { - [Flags] - private enum SymbolAccess - { - None = 0, - Read = 1, - Write = 2, - ReadWrite = Read | Write - } - - private static readonly ISet IncrementKinds = new HashSet - { - SyntaxKind.PostIncrementExpression, - SyntaxKind.PreIncrementExpression, - SyntaxKind.PostDecrementExpression, - SyntaxKind.PreDecrementExpression - }; - - private readonly Compilation compilation; - private readonly HashSet knownSymbolNames; - private SemanticModel semanticModel; - - public ISet UsedSymbols { get; } = new HashSet(); - public IDictionary FieldSymbolUsages { get; } = new Dictionary(); - public HashSet DebuggerDisplayValues { get; } = []; - public Dictionary PropertyAccess { get; } = []; - public HashSet TypesUsedWithReflection { get; } = []; + None = 0, + Read = 1, + Write = 2, + ReadWrite = Read | Write + } - public CSharpSymbolUsageCollector(Compilation compilation, IEnumerable knownSymbols) - { - this.compilation = compilation; - knownSymbolNames = knownSymbols.Select(GetName).ToHashSet(); - } + private static readonly ISet IncrementKinds = new HashSet + { + SyntaxKind.PostIncrementExpression, + SyntaxKind.PreIncrementExpression, + SyntaxKind.PostDecrementExpression, + SyntaxKind.PreDecrementExpression + }; + + private readonly Compilation compilation; + private readonly HashSet knownSymbolNames; + private SemanticModel model; + + public ISet UsedSymbols { get; } = new HashSet(); + public IDictionary FieldSymbolUsages { get; } = new Dictionary(); + public HashSet DebuggerDisplayValues { get; } = []; + public Dictionary PropertyAccess { get; } = []; + public HashSet TypesUsedWithReflection { get; } = []; + + public CSharpSymbolUsageCollector(Compilation compilation, IEnumerable knownSymbols) + { + this.compilation = compilation; + knownSymbolNames = knownSymbols.Select(GetName).ToHashSet(); + } - public override void Visit(SyntaxNode node) + public override void Visit(SyntaxNode node) + { + model = node.EnsureCorrectSemanticModelOrDefault(model ?? compilation.GetSemanticModel(node.SyntaxTree)); + if (model is not null) { - semanticModel = node.EnsureCorrectSemanticModelOrDefault(semanticModel ?? compilation.GetSemanticModel(node.SyntaxTree)); - if (semanticModel != null) + if (node.IsKind(SyntaxKindEx.ImplicitObjectCreationExpression) + && knownSymbolNames.Contains(ObjectCreationFactory.Create(node).TypeAsString(model))) { - if (node.IsKind(SyntaxKindEx.ImplicitObjectCreationExpression) - && knownSymbolNames.Contains(ObjectCreationFactory.Create(node).TypeAsString(semanticModel))) - { - UsedSymbols.UnionWith(GetSymbols(node)); - } - else if (node.IsKind(SyntaxKindEx.LocalFunctionStatement) - && ((LocalFunctionStatementSyntaxWrapper)node) is { AttributeLists: { Count: > 0 } } - && semanticModel.GetDeclaredSymbol(node) is IMethodSymbol localFunctionSymbol) - { - UsedSymbols.UnionWith(localFunctionSymbol - .GetAttributes() - .Where(a => knownSymbolNames.Contains(a.AttributeClass.Name)) - .Select(a => a.AttributeClass)); - } - else if (node.IsKind(SyntaxKindEx.PrimaryConstructorBaseType) - && knownSymbolNames.Contains(((PrimaryConstructorBaseTypeSyntaxWrapper)node).Type.GetName())) - { - UsedSymbols.UnionWith(GetSymbols(node)); - } - base.Visit(node); + UsedSymbols.UnionWith(GetSymbols(node)); } - } - - // TupleExpression "(a, b) = qix" - // ParenthesizedVariableDesignation "var (a, b) = quix" inside a DeclarationExpression - public override void VisitAssignmentExpression(AssignmentExpressionSyntax node) - { - var leftTupleCount = GetTupleCount(node.Left); - if (leftTupleCount != 0) + else if (node.IsKind(SyntaxKindEx.LocalFunctionStatement) + && ((LocalFunctionStatementSyntaxWrapper)node) is { AttributeLists.Count: > 0 } + && model.GetDeclaredSymbol(node) is IMethodSymbol localFunctionSymbol) { - var assignmentRight = node.Right; - var namedTypeSymbol = semanticModel.GetSymbolInfo(assignmentRight).Symbol?.GetSymbolType(); - if (namedTypeSymbol != null) - { - var deconstructors = namedTypeSymbol.GetMembers("Deconstruct"); - if (deconstructors.Length == 1) - { - UsedSymbols.Add(deconstructors.First()); - } - else if (deconstructors.Length > 1 && FindDeconstructor(deconstructors, leftTupleCount) is {} deconstructor) - { - UsedSymbols.Add(deconstructor); - } - } + UsedSymbols.UnionWith(localFunctionSymbol + .GetAttributes() + .Where(a => knownSymbolNames.Contains(a.AttributeClass.Name)) + .Select(a => a.AttributeClass)); } - base.VisitAssignmentExpression(node); - - static int GetTupleCount(ExpressionSyntax assignmentLeft) + else if (node.IsKind(SyntaxKindEx.PrimaryConstructorBaseType) + && knownSymbolNames.Contains(((PrimaryConstructorBaseTypeSyntaxWrapper)node).Type.GetName())) { - var result = 0; - if (TupleExpressionSyntaxWrapper.IsInstance(assignmentLeft)) - { - result = ((TupleExpressionSyntaxWrapper)assignmentLeft).Arguments.Count; - } - else if (DeclarationExpressionSyntaxWrapper.IsInstance(assignmentLeft) - && (DeclarationExpressionSyntaxWrapper)assignmentLeft is { } leftDeclaration - && ParenthesizedVariableDesignationSyntaxWrapper.IsInstance(leftDeclaration.Designation)) - { - result = ((ParenthesizedVariableDesignationSyntaxWrapper)leftDeclaration.Designation).Variables.Count; - } - return result; + UsedSymbols.UnionWith(GetSymbols(node)); } - - static ISymbol FindDeconstructor(IEnumerable deconstructors, int numberOfArguments) => - deconstructors.FirstOrDefault(m => m.GetParameters().Count() == numberOfArguments && m.DeclaredAccessibility.IsAccessibleOutsideTheType()); + base.Visit(node); } + } - public override void VisitAttribute(AttributeSyntax node) + // TupleExpression "(a, b) = qix" + // ParenthesizedVariableDesignation "var (a, b) = quix" inside a DeclarationExpression + public override void VisitAssignmentExpression(AssignmentExpressionSyntax node) + { + var leftTupleCount = GetTupleCount(node.Left); + if (leftTupleCount != 0) { - if (semanticModel.GetSymbolInfo(node).Symbol is IMethodSymbol { MethodKind: MethodKind.Constructor, ContainingType: ITypeSymbol attribute }) + var assignmentRight = node.Right; + var namedTypeSymbol = model.GetSymbolInfo(assignmentRight).Symbol?.GetSymbolType(); + if (namedTypeSymbol is not null) { - if (attribute.Is(KnownType.System_Diagnostics_DebuggerDisplayAttribute) - && node.ArgumentList != null) + var deconstructors = namedTypeSymbol.GetMembers("Deconstruct"); + if (deconstructors.Length == 1) { - var arguments = node.ArgumentList.Arguments - .Where(IsValueNameOrType) - .Select(a => semanticModel.GetConstantValue(a.Expression)) - .Where(o => o.HasValue) - .Select(o => o.Value) - .OfType(); - - DebuggerDisplayValues.UnionWith(arguments); + UsedSymbols.Add(deconstructors.First()); } - else if (attribute.Is(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute) - && node.Parent.Parent is BaseTypeDeclarationSyntax typeDeclaration - && semanticModel.GetDeclaredSymbol(typeDeclaration) is { } typeSymbol) + else if (deconstructors.Length > 1 && FindDeconstructor(deconstructors, leftTupleCount) is {} deconstructor) { - TypesUsedWithReflection.Add(typeSymbol); + UsedSymbols.Add(deconstructor); } } - base.VisitAttribute(node); - - static bool IsValueNameOrType(AttributeArgumentSyntax a) => - a.NameColon == null // Value - || a.NameColon.Name.Identifier.ValueText == "Value" - || a.NameColon.Name.Identifier.ValueText == "Name" - || a.NameColon.Name.Identifier.ValueText == "Type"; } + base.VisitAssignmentExpression(node); - public override void VisitIdentifierName(IdentifierNameSyntax node) + static int GetTupleCount(ExpressionSyntax assignmentLeft) { - if (IsKnownIdentifier(node.Identifier)) + var result = 0; + if (TupleExpressionSyntaxWrapper.IsInstance(assignmentLeft)) { - var symbols = GetSymbols(node); - TryStoreFieldAccess(node, symbols); - UsedSymbols.UnionWith(symbols); - TryStorePropertyAccess(node, symbols); + result = ((TupleExpressionSyntaxWrapper)assignmentLeft).Arguments.Count; } - base.VisitIdentifierName(node); - } - - public override void VisitTypeArgumentList(TypeArgumentListSyntax node) - { - if (semanticModel.GetSymbolInfo(node.Parent).Symbol is { } symbol) // symbol that the attribute is applied to + else if (DeclarationExpressionSyntaxWrapper.IsInstance(assignmentLeft) + && (DeclarationExpressionSyntaxWrapper)assignmentLeft is { } leftDeclaration + && ParenthesizedVariableDesignationSyntaxWrapper.IsInstance(leftDeclaration.Designation)) { - var typesWithDynamicallyAccessedMembers = GetTypeParameters(symbol) - .Zip(node.Arguments, (symbol, argument) => new Tuple(symbol, argument)) // map T to Person in void M() { } .. M(); - .Where(x => x.Item1.HasAttribute(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute)) - .Select(x => semanticModel.GetSymbolInfo(x.Item2).Symbol); - TypesUsedWithReflection.UnionWith(typesWithDynamicallyAccessedMembers); + result = ((ParenthesizedVariableDesignationSyntaxWrapper)leftDeclaration.Designation).Variables.Count; } - base.VisitTypeArgumentList(node); - - static ImmutableArray GetTypeParameters(ISymbol symbol) => - symbol switch - { - IMethodSymbol method => method.TypeParameters, - INamedTypeSymbol namedType => namedType.TypeParameters, - _ => ImmutableArray.Empty - }; + return result; } - public override void VisitObjectCreationExpression(ObjectCreationExpressionSyntax node) + static ISymbol FindDeconstructor(IEnumerable deconstructors, int numberOfArguments) => + deconstructors.FirstOrDefault(m => m.GetParameters().Count() == numberOfArguments && m.DeclaredAccessibility.IsAccessibleOutsideTheType()); + } + + public override void VisitAttribute(AttributeSyntax node) + { + if (model.GetSymbolInfo(node).Symbol is IMethodSymbol { MethodKind: MethodKind.Constructor, ContainingType: ITypeSymbol attribute }) { - if (knownSymbolNames.Contains(node.Type.GetName())) + if (attribute.Is(KnownType.System_Diagnostics_DebuggerDisplayAttribute) + && node.ArgumentList is not null) { - UsedSymbols.UnionWith(GetSymbols(node)); + var arguments = node.ArgumentList.Arguments + .Where(IsValueNameOrType) + .Select(a => model.GetConstantValue(a.Expression)) + .Where(o => o.HasValue) + .Select(o => o.Value) + .OfType(); + + DebuggerDisplayValues.UnionWith(arguments); + } + else if (attribute.Is(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute) + && node.Parent.Parent is BaseTypeDeclarationSyntax typeDeclaration + && model.GetDeclaredSymbol(typeDeclaration) is { } typeSymbol) + { + TypesUsedWithReflection.Add(typeSymbol); } - base.VisitObjectCreationExpression(node); } + base.VisitAttribute(node); - public override void VisitGenericName(GenericNameSyntax node) + static bool IsValueNameOrType(AttributeArgumentSyntax a) => + a.NameColon is null // Value + || a.NameColon.Name.Identifier.ValueText == "Value" + || a.NameColon.Name.Identifier.ValueText == "Name" + || a.NameColon.Name.Identifier.ValueText == "Type"; + } + + public override void VisitIdentifierName(IdentifierNameSyntax node) + { + if (IsKnownIdentifier(node.Identifier)) { - if (IsKnownIdentifier(node.Identifier)) - { - UsedSymbols.UnionWith(GetSymbols(node)); - } - base.VisitGenericName(node); + var symbols = GetSymbols(node); + TryStoreFieldAccess(node, symbols); + UsedSymbols.UnionWith(symbols); + TryStorePropertyAccess(node, symbols); } + base.VisitIdentifierName(node); + } - public override void VisitElementAccessExpression(ElementAccessExpressionSyntax node) + public override void VisitTypeArgumentList(TypeArgumentListSyntax node) + { + if (model.GetSymbolInfo(node.Parent).Symbol is { } symbol) // symbol that the attribute is applied to { - if (node.Expression.IsKind(SyntaxKind.ThisExpression) || knownSymbolNames.Contains(node.Expression.GetIdentifier()?.ValueText)) + var typesWithDynamicallyAccessedMembers = GetTypeParameters(symbol) + .Zip(node.Arguments, (symbol, argument) => new Tuple(symbol, argument)) // map T to Person in void M() { } .. M(); + .Where(x => x.Item1.HasAttribute(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute)) + .Select(x => model.GetSymbolInfo(x.Item2).Symbol); + TypesUsedWithReflection.UnionWith(typesWithDynamicallyAccessedMembers); + } + base.VisitTypeArgumentList(node); + + static ImmutableArray GetTypeParameters(ISymbol symbol) => + symbol switch { - var symbols = GetSymbols(node); - UsedSymbols.UnionWith(symbols); - TryStorePropertyAccess(node, symbols); - } - base.VisitElementAccessExpression(node); + IMethodSymbol method => method.TypeParameters, + INamedTypeSymbol namedType => namedType.TypeParameters, + _ => ImmutableArray.Empty + }; + } + + public override void VisitObjectCreationExpression(ObjectCreationExpressionSyntax node) + { + if (knownSymbolNames.Contains(node.Type.GetName())) + { + UsedSymbols.UnionWith(GetSymbols(node)); } + base.VisitObjectCreationExpression(node); + } - public override void VisitConstructorInitializer(ConstructorInitializerSyntax node) + public override void VisitGenericName(GenericNameSyntax node) + { + if (IsKnownIdentifier(node.Identifier)) { - // In this case (":base()") we cannot check at the syntax level if the constructor name is in the list - // of known names so we have to check for symbols. UsedSymbols.UnionWith(GetSymbols(node)); - base.VisitConstructorInitializer(node); } + base.VisitGenericName(node); + } - public override void VisitConstructorDeclaration(ConstructorDeclarationSyntax node) + public override void VisitElementAccessExpression(ElementAccessExpressionSyntax node) + { + if (node.Expression.IsKind(SyntaxKind.ThisExpression) || knownSymbolNames.Contains(node.Expression.GetIdentifier()?.ValueText)) { - // We are visiting a ctor with no initializer and the compiler will automatically - // call the default constructor of the type if declared, or the base type if the - // current type does not declare a default constructor. - if (node.Initializer == null && IsKnownIdentifier(node.Identifier)) - { - var constructor = semanticModel.GetDeclaredSymbol(node); - var implicitlyCalledConstructor = GetImplicitlyCalledConstructor(constructor); - if (implicitlyCalledConstructor != null) - { - UsedSymbols.Add(implicitlyCalledConstructor); - } - } - base.VisitConstructorDeclaration(node); + var symbols = GetSymbols(node); + UsedSymbols.UnionWith(symbols); + TryStorePropertyAccess(node, symbols); } + base.VisitElementAccessExpression(node); + } - public override void VisitVariableDeclarator(VariableDeclaratorSyntax node) + public override void VisitConstructorInitializer(ConstructorInitializerSyntax node) + { + // In this case (":base()") we cannot check at the syntax level if the constructor name is in the list + // of known names so we have to check for symbols. + UsedSymbols.UnionWith(GetSymbols(node)); + base.VisitConstructorInitializer(node); + } + + public override void VisitConstructorDeclaration(ConstructorDeclarationSyntax node) + { + // We are visiting a ctor with no initializer and the compiler will automatically + // call the default constructor of the type if declared, or the base type if the + // current type does not declare a default constructor. + if (node.Initializer is null && IsKnownIdentifier(node.Identifier)) { - if (IsKnownIdentifier(node.Identifier)) + var constructor = model.GetDeclaredSymbol(node); + var implicitlyCalledConstructor = GetImplicitlyCalledConstructor(constructor); + if (implicitlyCalledConstructor is not null) { - var usage = GetFieldSymbolUsage(semanticModel.GetDeclaredSymbol(node)); - usage.Declaration = node; - if (node.Initializer != null) - { - usage.Initializer = node; - } + UsedSymbols.Add(implicitlyCalledConstructor); } - base.VisitVariableDeclarator(node); } + base.VisitConstructorDeclaration(node); + } - public override void VisitPropertyDeclaration(PropertyDeclarationSyntax node) + public override void VisitVariableDeclarator(VariableDeclaratorSyntax node) + { + if (IsKnownIdentifier(node.Identifier)) { - if (node.Initializer != null && IsKnownIdentifier(node.Identifier)) + var usage = GetFieldSymbolUsage(model.GetDeclaredSymbol(node)); + usage.Declaration = node; + if (node.Initializer is not null) { - var symbol = semanticModel.GetDeclaredSymbol(node); - UsedSymbols.Add(symbol); - StorePropertyAccess(symbol, AccessorAccess.Set); + usage.Initializer = node; } - base.VisitPropertyDeclaration(node); } + base.VisitVariableDeclarator(node); + } - private SymbolAccess ParentAccessType(SyntaxNode node) => - node.Parent switch - { - // (node) - ParenthesizedExpressionSyntax parenthesizedExpression => ParentAccessType(parenthesizedExpression), - // node; - ExpressionStatementSyntax _ => SymbolAccess.None, - // node(_) : - InvocationExpressionSyntax invocation => node == invocation.Expression ? SymbolAccess.Read : SymbolAccess.None, - // _.node : node._ - MemberAccessExpressionSyntax memberAccess => node == memberAccess.Name ? ParentAccessType(memberAccess) : SymbolAccess.Read, - // _?.node : node?._ - MemberBindingExpressionSyntax memberBinding => node == memberBinding.Name ? ParentAccessType(memberBinding) : SymbolAccess.Read, - // node ??= _ : _ ??= node - AssignmentExpressionSyntax assignment when assignment.IsKind(SyntaxKindEx.CoalesceAssignmentExpression) => - node == assignment.Left ? SymbolAccess.ReadWrite : SymbolAccess.Read, - // Ignoring distinction assignmentExpression.IsKind(SyntaxKind.SimpleAssignmentExpression) between - // "node = _" and "node += _" both are considered as Write and rely on the parent to know if its read. - // node = _ : _ = node - AssignmentExpressionSyntax assignment => node == assignment.Left ? SymbolAccess.Write | ParentAccessType(assignment) : SymbolAccess.Read, - // Invocation(node), Invocation(out node), Invocation(ref node) - ArgumentSyntax argument => ArgumentAccessType(argument), - // node++ - ExpressionSyntax expressionSyntax when expressionSyntax.IsAnyKind(IncrementKinds) => SymbolAccess.Write | ParentAccessType(expressionSyntax), - // => node - ArrowExpressionClauseSyntax arrowExpressionClause when arrowExpressionClause.Parent is MethodDeclarationSyntax arrowMethod => - arrowMethod.ReturnType != null && arrowMethod.ReturnType.IsKnownType(KnownType.Void, semanticModel) - ? SymbolAccess.None - : SymbolAccess.Read, - _ => SymbolAccess.Read - }; + public override void VisitPropertyDeclaration(PropertyDeclarationSyntax node) + { + if (node.Initializer is not null && IsKnownIdentifier(node.Identifier)) + { + var symbol = model.GetDeclaredSymbol(node); + UsedSymbols.Add(symbol); + StorePropertyAccess(symbol, AccessorAccess.Set); + } + base.VisitPropertyDeclaration(node); + } - private static SymbolAccess ArgumentAccessType(ArgumentSyntax argument) => - argument.RefOrOutKeyword.Kind() switch - { - // out Type node : out node - SyntaxKind.OutKeyword => SymbolAccess.Write, - // ref node - SyntaxKind.RefKeyword => SymbolAccess.ReadWrite, - _ => SymbolAccess.Read - }; + private SymbolAccess ParentAccessType(SyntaxNode node) => + node.Parent switch + { + // (node) + ParenthesizedExpressionSyntax parenthesizedExpression => ParentAccessType(parenthesizedExpression), + // node; + ExpressionStatementSyntax _ => SymbolAccess.None, + // node(_) : + InvocationExpressionSyntax invocation => node == invocation.Expression ? SymbolAccess.Read : SymbolAccess.None, + // _.node : node._ + MemberAccessExpressionSyntax memberAccess => node == memberAccess.Name ? ParentAccessType(memberAccess) : SymbolAccess.Read, + // _?.node : node?._ + MemberBindingExpressionSyntax memberBinding => node == memberBinding.Name ? ParentAccessType(memberBinding) : SymbolAccess.Read, + // node ??= _ : _ ??= node + AssignmentExpressionSyntax assignment when assignment.IsKind(SyntaxKindEx.CoalesceAssignmentExpression) => + node == assignment.Left ? SymbolAccess.ReadWrite : SymbolAccess.Read, + // Ignoring distinction assignmentExpression.IsKind(SyntaxKind.SimpleAssignmentExpression) between + // "node = _" and "node += _" both are considered as Write and rely on the parent to know if its read. + // node = _ : _ = node + AssignmentExpressionSyntax assignment => node == assignment.Left ? SymbolAccess.Write | ParentAccessType(assignment) : SymbolAccess.Read, + // Invocation(node), Invocation(out node), Invocation(ref node) + ArgumentSyntax argument => ArgumentAccessType(argument), + // node++ + ExpressionSyntax expressionSyntax when expressionSyntax.IsAnyKind(IncrementKinds) => SymbolAccess.Write | ParentAccessType(expressionSyntax), + // => node + ArrowExpressionClauseSyntax arrowExpressionClause when arrowExpressionClause.Parent is MethodDeclarationSyntax arrowMethod => + arrowMethod.ReturnType is not null && arrowMethod.ReturnType.IsKnownType(KnownType.Void, model) + ? SymbolAccess.None + : SymbolAccess.Read, + _ => SymbolAccess.Read + }; - /// - /// Given a node, it tries to get the symbol or the candidate symbols (if the compiler cannot find the symbol, - /// .e.g when the code cannot compile). - /// - /// List of symbols - private ImmutableArray GetSymbols(TSyntaxNode node) - where TSyntaxNode : SyntaxNode + private static SymbolAccess ArgumentAccessType(ArgumentSyntax argument) => + argument.RefOrOutKeyword.Kind() switch { - var symbolInfo = semanticModel.GetSymbolInfo(node); - - return new[] { symbolInfo.Symbol } - .Concat(symbolInfo.CandidateSymbols) - .Select(GetOriginalDefinition) - .WhereNotNull() - .ToImmutableArray(); - - static ISymbol GetOriginalDefinition(ISymbol candidateSymbol) => - candidateSymbol is IMethodSymbol methodSymbol && methodSymbol.MethodKind == MethodKind.ReducedExtension - ? methodSymbol.ReducedFrom?.OriginalDefinition - : candidateSymbol?.OriginalDefinition; - } + // out Type node : out node + SyntaxKind.OutKeyword => SymbolAccess.Write, + // ref node + SyntaxKind.RefKeyword => SymbolAccess.ReadWrite, + _ => SymbolAccess.Read + }; + + /// + /// Given a node, it tries to get the symbol or the candidate symbols (if the compiler cannot find the symbol, + /// .e.g when the code cannot compile). + /// + /// List of symbols + private ImmutableArray GetSymbols(TSyntaxNode node) + where TSyntaxNode : SyntaxNode + { + var symbolInfo = model.GetSymbolInfo(node); + + return new[] { symbolInfo.Symbol } + .Concat(symbolInfo.CandidateSymbols) + .Select(GetOriginalDefinition) + .WhereNotNull() + .ToImmutableArray(); + + static ISymbol GetOriginalDefinition(ISymbol candidateSymbol) => + candidateSymbol is IMethodSymbol methodSymbol && methodSymbol.MethodKind == MethodKind.ReducedExtension + ? methodSymbol.ReducedFrom?.OriginalDefinition + : candidateSymbol?.OriginalDefinition; + } - private void TryStorePropertyAccess(ExpressionSyntax node, IEnumerable identifierSymbols) + private void TryStorePropertyAccess(ExpressionSyntax node, IEnumerable identifierSymbols) + { + var propertySymbols = identifierSymbols.OfType().ToList(); + if (propertySymbols.Any()) { - var propertySymbols = identifierSymbols.OfType().ToList(); - if (propertySymbols.Any()) + var access = EvaluatePropertyAccesses(node); + foreach (var propertySymbol in propertySymbols) { - var access = EvaluatePropertyAccesses(node); - foreach (var propertySymbol in propertySymbols) - { - StorePropertyAccess(propertySymbol, access); - } + StorePropertyAccess(propertySymbol, access); } } + } - private void StorePropertyAccess(IPropertySymbol propertySymbol, AccessorAccess access) + private void StorePropertyAccess(IPropertySymbol propertySymbol, AccessorAccess access) + { + if (PropertyAccess.ContainsKey(propertySymbol)) { - if (PropertyAccess.ContainsKey(propertySymbol)) - { - PropertyAccess[propertySymbol] |= access; - } - else - { - PropertyAccess[propertySymbol] = access; - } + PropertyAccess[propertySymbol] |= access; } + else + { + PropertyAccess[propertySymbol] = access; + } + } - private AccessorAccess EvaluatePropertyAccesses(ExpressionSyntax node) + private AccessorAccess EvaluatePropertyAccesses(ExpressionSyntax node) + { + var topmostSyntax = GetTopmostSyntaxWithTheSameSymbol(node); + if (topmostSyntax.Parent is AssignmentExpressionSyntax assignmentExpression) { - var topmostSyntax = GetTopmostSyntaxWithTheSameSymbol(node); - if (topmostSyntax.Parent is AssignmentExpressionSyntax assignmentExpression) - { - if (assignmentExpression.IsKind(SyntaxKind.SimpleAssignmentExpression)) - { - // Prop = value --> set - // value = Prop --> get - return assignmentExpression.Left == topmostSyntax ? AccessorAccess.Set : AccessorAccess.Get; - } - else - { - // Prop += value --> get/set - return AccessorAccess.Both; - } - } - else if (topmostSyntax.Parent is ArgumentSyntax argument && argument.IsInTupleAssignmentTarget()) + if (assignmentExpression.IsKind(SyntaxKind.SimpleAssignmentExpression)) { - return AccessorAccess.Set; - } - else if (node.IsInNameOfArgument(semanticModel)) - { - // nameof(Prop) --> get/set - return AccessorAccess.Both; + // Prop = value --> set + // value = Prop --> get + return assignmentExpression.Left == topmostSyntax ? AccessorAccess.Set : AccessorAccess.Get; } else { - // Prop++ --> get/set - return topmostSyntax.Parent.IsAnyKind(IncrementKinds) ? AccessorAccess.Both : AccessorAccess.Get; + // Prop += value --> get/set + return AccessorAccess.Both; } } + else if (topmostSyntax.Parent is ArgumentSyntax argument && argument.IsInTupleAssignmentTarget()) + { + return AccessorAccess.Set; + } + else if (node.IsInNameOfArgument(model)) + { + // nameof(Prop) --> get/set + return AccessorAccess.Both; + } + else + { + // Prop++ --> get/set + return topmostSyntax.Parent.IsAnyKind(IncrementKinds) ? AccessorAccess.Both : AccessorAccess.Get; + } + } - private bool IsKnownIdentifier(SyntaxToken identifier) => - knownSymbolNames.Contains(identifier.ValueText); + private bool IsKnownIdentifier(SyntaxToken identifier) => + knownSymbolNames.Contains(identifier.ValueText); - private void TryStoreFieldAccess(IdentifierNameSyntax node, IEnumerable symbols) + private void TryStoreFieldAccess(IdentifierNameSyntax node, IEnumerable symbols) + { + var access = ParentAccessType(node); + var fieldSymbolUsagesList = GetFieldSymbolUsagesList(symbols); + if (HasFlag(access, SymbolAccess.Read)) { - var access = ParentAccessType(node); - var fieldSymbolUsagesList = GetFieldSymbolUsagesList(symbols); - if (HasFlag(access, SymbolAccess.Read)) + foreach (var symbolUsage in fieldSymbolUsagesList) { - fieldSymbolUsagesList.ForEach(usage => usage.Readings.Add(node)); + symbolUsage.Readings.Add(node); } + } - if (HasFlag(access, SymbolAccess.Write)) + if (HasFlag(access, SymbolAccess.Write)) + { + foreach (var symbolUsage in fieldSymbolUsagesList) { - fieldSymbolUsagesList.ForEach(usage => usage.Writings.Add(node)); + symbolUsage.Writings.Add(node); } - - static bool HasFlag(SymbolAccess symbolAccess, SymbolAccess flag) => (symbolAccess & flag) != 0; } - private List GetFieldSymbolUsagesList(IEnumerable symbols) => - symbols.Select(GetFieldSymbolUsage).ToList(); + static bool HasFlag(SymbolAccess symbolAccess, SymbolAccess flag) => (symbolAccess & flag) != 0; + } - private SymbolUsage GetFieldSymbolUsage(ISymbol symbol) => - FieldSymbolUsages.GetOrAdd(symbol, s => new SymbolUsage(s)); + private List GetFieldSymbolUsagesList(IEnumerable symbols) => + symbols.Select(GetFieldSymbolUsage).ToList(); - private static SyntaxNode GetTopmostSyntaxWithTheSameSymbol(SyntaxNode identifier) => - // All of the cases below could be parts of invocation or other expressions - identifier.Parent switch - { - // this.identifier or a.identifier or ((a)).identifier, but not identifier.other - MemberAccessExpressionSyntax memberAccess when memberAccess.Name == identifier => memberAccess.GetSelfOrTopParenthesizedExpression(), - // this?.identifier or a?.identifier or ((a))?.identifier, but not identifier?.other - MemberBindingExpressionSyntax memberBinding when memberBinding.Name == identifier => memberBinding.Parent.GetSelfOrTopParenthesizedExpression(), - // identifier or ((identifier)) - _ => identifier.GetSelfOrTopParenthesizedExpression() - }; + private SymbolUsage GetFieldSymbolUsage(ISymbol symbol) => + FieldSymbolUsages.GetOrAdd(symbol, x => new SymbolUsage(x)); - private static IMethodSymbol GetImplicitlyCalledConstructor(IMethodSymbol constructor) => - // In case there is no other explicitly called constructor in a constructor declaration - // the compiler will automatically put a call to the current class' default constructor, - // or if the declaration is the default constructor or there is no default constructor, - // the compiler will put a call the base class' default constructor. - IsDefaultConstructor(constructor) - ? GetDefaultConstructor(constructor.ContainingType.BaseType) - : GetDefaultConstructor(constructor.ContainingType) ?? GetDefaultConstructor(constructor.ContainingType.BaseType); - - private static IMethodSymbol GetDefaultConstructor(INamedTypeSymbol namedType) => - // See https://github.com/SonarSource/sonar-dotnet/issues/3155 - namedType != null && namedType.InstanceConstructors != null - ? namedType.InstanceConstructors.FirstOrDefault(IsDefaultConstructor) - : null; - - private static bool IsDefaultConstructor(IMethodSymbol constructor) => - constructor.Parameters.Length == 0; - - private static string GetName(ISymbol symbol) => - symbol.IsConstructor() ? symbol.ContainingType.Name : symbol.Name; - } + private static SyntaxNode GetTopmostSyntaxWithTheSameSymbol(SyntaxNode identifier) => + // All of the cases below could be parts of invocation or other expressions + identifier.Parent switch + { + // this.identifier or a.identifier or ((a)).identifier, but not identifier.other + MemberAccessExpressionSyntax memberAccess when memberAccess.Name == identifier => memberAccess.GetSelfOrTopParenthesizedExpression(), + // this?.identifier or a?.identifier or ((a))?.identifier, but not identifier?.other + MemberBindingExpressionSyntax memberBinding when memberBinding.Name == identifier => memberBinding.Parent.GetSelfOrTopParenthesizedExpression(), + // identifier or ((identifier)) + _ => identifier.GetSelfOrTopParenthesizedExpression() + }; + + private static IMethodSymbol GetImplicitlyCalledConstructor(IMethodSymbol constructor) => + // In case there is no other explicitly called constructor in a constructor declaration + // the compiler will automatically put a call to the current class' default constructor, + // or if the declaration is the default constructor or there is no default constructor, + // the compiler will put a call the base class' default constructor. + IsDefaultConstructor(constructor) + ? GetDefaultConstructor(constructor.ContainingType.BaseType) + : GetDefaultConstructor(constructor.ContainingType) ?? GetDefaultConstructor(constructor.ContainingType.BaseType); + + private static IMethodSymbol GetDefaultConstructor(INamedTypeSymbol namedType) => + // See https://github.com/SonarSource/sonar-dotnet/issues/3155 + namedType != null && namedType.InstanceConstructors != null + ? namedType.InstanceConstructors.FirstOrDefault(IsDefaultConstructor) + : null; + + private static bool IsDefaultConstructor(IMethodSymbol constructor) => + constructor.Parameters.Length == 0; + + private static string GetName(ISymbol symbol) => + symbol.IsConstructor() ? symbol.ContainingType.Name : symbol.Name; } diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs index bf439580d60..976afa1d1ad 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/UnusedPrivateMember.cs @@ -250,10 +250,10 @@ static IEnumerable GetSiblingDeclarators(SyntaxNode va _ => Enumerable.Empty(), }; - static Location GetIdentifierLocation(SyntaxNode syntaxNode) => - syntaxNode.GetIdentifier() is { } identifier + static Location GetIdentifierLocation(SyntaxNode node) => + node.GetIdentifier() is { } identifier ? identifier.GetLocation() - : syntaxNode.GetLocation(); + : node.GetLocation(); } private static void ReportProperty(SonarCompilationReportingContextBase context, @@ -296,12 +296,9 @@ bool IsGenerated(SyntaxReference syntaxReference) => private static CSharpRemovableSymbolWalker RetrieveRemovableSymbols(INamedTypeSymbol namedType, Compilation compilation, SonarSymbolReportingContext context) { var removableSymbolsCollector = new CSharpRemovableSymbolWalker(compilation.GetSemanticModel, namedType.DeclaredAccessibility); - if (!VisitDeclaringReferences(namedType, removableSymbolsCollector, context, includeGeneratedFile: false)) - { - return null; - } - - return removableSymbolsCollector; + return VisitDeclaringReferences(namedType, removableSymbolsCollector, context, includeGeneratedFile: false) + ? removableSymbolsCollector + : null; } private static void CopyRetrievedSymbols(CSharpRemovableSymbolWalker removableSymbolCollector, diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Constructors.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Constructors.cs index 188d0f74ac0..4595fdb65cd 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Constructors.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Constructors.cs @@ -18,13 +18,13 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -namespace SonarAnalyzer.Test.Rules +namespace SonarAnalyzer.Test.Rules; + +public partial class UnusedPrivateMemberTest { - public partial class UnusedPrivateMemberTest - { - [TestMethod] - public void UnusedPrivateMember_Constructor_Accessibility() => - builder.AddSnippet(@" + [TestMethod] + public void UnusedPrivateMember_Constructor_Accessibility() => + builder.AddSnippet(@" public class PrivateConstructors { private PrivateConstructors(int i) { var x = 5; } // Noncompliant {{Remove the unused private constructor 'PrivateConstructors'.}} @@ -62,40 +62,40 @@ public class InnerPublicClass } ").Verify(); - [TestMethod] - public void UnusedPrivateMember_Constructor_DirectReferences() => - builder.AddSnippet(""" - public abstract class PrivateConstructors + [TestMethod] + public void UnusedPrivateMember_Constructor_DirectReferences() => + builder.AddSnippet(""" + public abstract class PrivateConstructors + { + public class Constructor1 + { + public static readonly Constructor1 Instance = new Constructor1(); + private Constructor1() { var x = 5; } + } + + public class Constructor2 + { + public Constructor2(int a) { } + private Constructor2() { var x = 5; } // Compliant - FN + } + + public class Constructor3 + { + public Constructor3(int a) : this() { } + private Constructor3() { var x = 5; } + } + + public class Constructor4 { - public class Constructor1 - { - public static readonly Constructor1 Instance = new Constructor1(); - private Constructor1() { var x = 5; } - } - - public class Constructor2 - { - public Constructor2(int a) { } - private Constructor2() { var x = 5; } // Compliant - FN - } - - public class Constructor3 - { - public Constructor3(int a) : this() { } - private Constructor3() { var x = 5; } - } - - public class Constructor4 - { - static Constructor4() { var x = 5; } - } + static Constructor4() { var x = 5; } } + } - """).VerifyNoIssues(); + """).VerifyNoIssues(); - [TestMethod] - public void UnusedPrivateMember_Constructor_Inheritance() => - builder.AddSnippet(@" + [TestMethod] + public void UnusedPrivateMember_Constructor_Inheritance() => + builder.AddSnippet(@" public class Inheritance { private abstract class BaseClass1 @@ -121,21 +121,21 @@ public DerivedClass2() { } } ").Verify(); - [TestMethod] - public void UnusedPrivateMember_Empty_Constructors() => - builder.AddSnippet(""" - public class PrivateConstructors - { - private PrivateConstructors(int i) { } // Compliant, empty ctors are reported from another rule - } + [TestMethod] + public void UnusedPrivateMember_Empty_Constructors() => + builder.AddSnippet(""" + public class PrivateConstructors + { + private PrivateConstructors(int i) { } // Compliant, empty ctors are reported from another rule + } - """).VerifyNoIssues(); + """).VerifyNoIssues(); - [TestMethod] - public void UnusedPrivateMember_Illegal_Interface_Constructor() => - // While typing code in IDE, we can end up in a state where an interface has a constructor defined. - // Even though this results in a compiler error (CS0526), IDE will still trigger rules on the interface. - builder.AddSnippet(@" + [TestMethod] + public void UnusedPrivateMember_Illegal_Interface_Constructor() => + // While typing code in IDE, we can end up in a state where an interface has a constructor defined. + // Even though this results in a compiler error (CS0526), IDE will still trigger rules on the interface. + builder.AddSnippet(@" public interface IInterface { // UnusedPrivateMember rule does not trigger AD0001 error from NullReferenceException @@ -143,13 +143,13 @@ public interface IInterface } ").Verify(); - [DataTestMethod] - [DataRow("private", "Remove the unused private constructor 'Foo'.")] - [DataRow("protected", "Remove unused constructor of private type 'Foo'.")] - [DataRow("internal", "Remove unused constructor of private type 'Foo'.")] - [DataRow("public", "Remove unused constructor of private type 'Foo'.")] - public void UnusedPrivateMember_NonPrivateConstructorInPrivateClass(string accessModifier, string expectedMessage) => - builder.AddSnippet($$$""" + [DataTestMethod] + [DataRow("private", "Remove the unused private constructor 'Foo'.")] + [DataRow("protected", "Remove unused constructor of private type 'Foo'.")] + [DataRow("internal", "Remove unused constructor of private type 'Foo'.")] + [DataRow("public", "Remove unused constructor of private type 'Foo'.")] + public void UnusedPrivateMember_NonPrivateConstructorInPrivateClass(string accessModifier, string expectedMessage) => + builder.AddSnippet($$$""" public class Some { private class Foo // Noncompliant @@ -164,26 +164,26 @@ private class Foo // Noncompliant #if NET - [TestMethod] - public void UnusedPrivateMember_RecordPositionalConstructor() => - builder.AddSnippet(""" - // https://github.com/SonarSource/sonar-dotnet/issues/5381 - public abstract record Foo + [TestMethod] + public void UnusedPrivateMember_RecordPositionalConstructor() => + builder.AddSnippet(""" + // https://github.com/SonarSource/sonar-dotnet/issues/5381 + public abstract record Foo + { + Foo(string value) { - Foo(string value) - { - Value = value; - } + Value = value; + } - public string Value { get; } + public string Value { get; } - public sealed record Bar(string Value) : Foo(Value); - } - """).WithOptions(ParseOptionsHelper.FromCSharp9).VerifyNoIssues(); + public sealed record Bar(string Value) : Foo(Value); + } + """).WithOptions(ParseOptionsHelper.FromCSharp9).VerifyNoIssues(); - [TestMethod] - public void UnusedPrivateMember_NonExistentRecordPositionalConstructor() => - builder.AddSnippet(@" + [TestMethod] + public void UnusedPrivateMember_NonExistentRecordPositionalConstructor() => + builder.AddSnippet(@" public abstract record Foo { public sealed record Bar(string Value) : RandomRecord(Value); // Error [CS0246, CS1729] no suitable method found to override @@ -191,5 +191,4 @@ public sealed record Bar(string Value) : RandomRecord(Value); // Error [CS0246, #endif - } } diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Fields.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Fields.cs index 6a3cae33ffa..d22b852ffe5 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Fields.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Fields.cs @@ -18,33 +18,44 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -namespace SonarAnalyzer.Test.Rules +namespace SonarAnalyzer.Test.Rules; + +public partial class UnusedPrivateMemberTest { - public partial class UnusedPrivateMemberTest - { - [TestMethod] - public void UnusedPrivateMember_Field_Accessibility() => - builder.AddSnippet(""" - public class PrivateMembers + [TestMethod] + public void UnusedPrivateMember_Field_Accessibility() => + builder.AddSnippet(""" + public class PrivateMembers + { + private int privateField; // Noncompliant {{Remove the unused private field 'privateField'.}} + // ^^^^^^^^^^^^^^^^^^^^^^^^^ + private static int privateStaticField; // Noncompliant + + private class InnerPrivateClass // Noncompliant { - private int privateField; // Noncompliant {{Remove the unused private field 'privateField'.}} - // ^^^^^^^^^^^^^^^^^^^^^^^^^ - private static int privateStaticField; // Noncompliant - - private class InnerPrivateClass // Noncompliant - { - internal int internalField; // Noncompliant - protected int protectedField; // Noncompliant - protected internal int protectedInternalField; // Noncompliant - public int publicField; // Noncompliant - internal static int internalStaticField; // Noncompliant - protected static int protectedStaticField; // Noncompliant - protected internal static int protectedInternalStaticField; // Noncompliant - public static int publicStaticField; // Noncompliant - } + internal int internalField; // Noncompliant + protected int protectedField; // Noncompliant + protected internal int protectedInternalField; // Noncompliant + public int publicField; // Noncompliant + internal static int internalStaticField; // Noncompliant + protected static int protectedStaticField; // Noncompliant + protected internal static int protectedInternalStaticField; // Noncompliant + public static int publicStaticField; // Noncompliant } - - public class NonPrivateMembers + } + + public class NonPrivateMembers + { + internal int internalField; + protected int protectedField; + protected internal int protectedInternalField; + public int publicField; + internal static int internalStaticField; + protected static int protectedStaticField; + protected internal static int protectedInternalStaticField; + public static int publicStaticField; + + public class InnerPublicClass { internal int internalField; protected int protectedField; @@ -54,109 +65,97 @@ public class NonPrivateMembers protected static int protectedStaticField; protected internal static int protectedInternalStaticField; public static int publicStaticField; - - public class InnerPublicClass - { - internal int internalField; - protected int protectedField; - protected internal int protectedInternalField; - public int publicField; - internal static int internalStaticField; - protected static int protectedStaticField; - protected internal static int protectedInternalStaticField; - public static int publicStaticField; - } } - """).Verify(); - - [TestMethod] - public void UnusedPrivateMember_Field_MultipleDeclarations() => - builder.AddSnippet(""" - public class PrivateMembers + } + """).Verify(); + + [TestMethod] + public void UnusedPrivateMember_Field_MultipleDeclarations() => + builder.AddSnippet(""" + public class PrivateMembers + { + private int x, y, z; // Noncompliant {{Remove the unused private field 'x'.}} + // ^^^^^^^^^^^^^^^^^^^^ + + private int a, b, c; + // ^ {{Remove the unused private field 'a'.}} + // ^ @-1 {{Remove the unused private field 'c'.}} + + public int Method1() => b; + } + """).Verify(); + + [TestMethod] + public void UnusedPrivateMember_Fields_DirectReferences() => + builder.AddSnippet(""" + using System; + + public class FieldUsages + { + private int field1; // Noncompliant {{Remove this unread private field 'field1' or refactor the code to use its value.}} + private int field2; // Noncompliant {{Remove this unread private field 'field2' or refactor the code to use its value.}} + private int field3; // Noncompliant {{Remove this unread private field 'field3' or refactor the code to use its value.}} + private int field4; + private int field5; + private int field6; + public int Method1() { - private int x, y, z; // Noncompliant {{Remove the unused private field 'x'.}} - // ^^^^^^^^^^^^^^^^^^^^ - - private int a, b, c; - // ^ {{Remove the unused private field 'a'.}} - // ^ @-1 {{Remove the unused private field 'c'.}} - - public int Method1() => b; + field1 = 0; + this.field2 = 0; + int.TryParse("1", out field3); + Console.Write(field4); + Func x = () => field5; + return field6; } - """).Verify(); - - [TestMethod] - public void UnusedPrivateMember_Fields_DirectReferences() => - builder.AddSnippet(""" - using System; - public class FieldUsages - { - private int field1; // Noncompliant {{Remove this unread private field 'field1' or refactor the code to use its value.}} - private int field2; // Noncompliant {{Remove this unread private field 'field2' or refactor the code to use its value.}} - private int field3; // Noncompliant {{Remove this unread private field 'field3' or refactor the code to use its value.}} - private int field4; - private int field5; - private int field6; - public int Method1() - { - field1 = 0; - this.field2 = 0; - int.TryParse("1", out field3); - Console.Write(field4); - Func x = () => field5; - return field6; - } - - private int field7; - public int ExpressionBodyMethod() => field7; + private int field7; + public int ExpressionBodyMethod() => field7; - private static int field8; - public int Property { get; set; } = field8; + private static int field8; + public int Property { get; set; } = field8; - public FieldUsages(int number) { } + public FieldUsages(int number) { } - private static int field9; - public FieldUsages() : this(field9) { } + private static int field9; + public FieldUsages() : this(field9) { } - private int field10; - private int field11; // Compliant nameof(field11) - public object Method2() - { - var x = new[] { field10 }; - var name = nameof(field11); - return null; - } + private int field10; + private int field11; // Compliant nameof(field11) + public object Method2() + { + var x = new[] { field10 }; + var name = nameof(field11); + return null; } - """).Verify(); - - [TestMethod] - public void UnusedPrivateMember_Fields_StructLayout() => - builder.AddSnippet(""" - // https://github.com/SonarSource/sonar-dotnet/issues/6912 - using System.Runtime.InteropServices; - - public class Foo + } + """).Verify(); + + [TestMethod] + public void UnusedPrivateMember_Fields_StructLayout() => + builder.AddSnippet(""" + // https://github.com/SonarSource/sonar-dotnet/issues/6912 + using System.Runtime.InteropServices; + + public class Foo + { + [StructLayout(LayoutKind.Sequential)] + private struct NetResource { - [StructLayout(LayoutKind.Sequential)] - private struct NetResource - { - public string LocalName; // Compliant: Unused members in a struct with StructLayout attribute are compliant - public string RemoteName; - } + public string LocalName; // Compliant: Unused members in a struct with StructLayout attribute are compliant + public string RemoteName; + } - [DllImport("mpr.dll")] - private static extern int WNetAddConnection2(NetResource netResource, string password, string username, int flags); + [DllImport("mpr.dll")] + private static extern int WNetAddConnection2(NetResource netResource, string password, string username, int flags); - public void Bar() + public void Bar() + { + var netResource = new NetResource { - var netResource = new NetResource - { - RemoteName = "foo" - }; - WNetAddConnection2(netResource, "password", "username", 0); - } + RemoteName = "foo" + }; + WNetAddConnection2(netResource, "password", "username", 0); } - """).VerifyNoIssues(); - } + } + """).VerifyNoIssues(); } diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Methods.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Methods.cs index 8efbdba276e..15c39533aba 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Methods.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Methods.cs @@ -18,13 +18,13 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -namespace SonarAnalyzer.Test.Rules +namespace SonarAnalyzer.Test.Rules; + +public partial class UnusedPrivateMemberTest { - public partial class UnusedPrivateMemberTest - { - [TestMethod] - public void UnusedPrivateMember_Method_Accessibility() => - builder.AddSnippet(@" + [TestMethod] + public void UnusedPrivateMember_Method_Accessibility() => + builder.AddSnippet(@" public class PrivateMembers { private int PrivateMethod() { return 0; } // Noncompliant {{Remove the unused private method 'PrivateMethod'.}} @@ -79,60 +79,60 @@ public class InterfaceImpl : IInterface } ").Verify(); - [TestMethod] - public void UnusedPrivateMember_Methods_DirectReferences() => - builder.AddSnippet(""" - using System; - using System.Linq; - public class MethodUsages + [TestMethod] + public void UnusedPrivateMember_Methods_DirectReferences() => + builder.AddSnippet(""" + using System; + using System.Linq; + public class MethodUsages + { + private int Method1() { return 0; } + private int Method2() { return 0; } + private int Method3() { return 0; } + private int Method4() { return 0; } + private int Method5() { return 0; } + private int Method6() { return 0; } + private int Method7() { return 0; } + public int Test1(MethodUsages other) { - private int Method1() { return 0; } - private int Method2() { return 0; } - private int Method3() { return 0; } - private int Method4() { return 0; } - private int Method5() { return 0; } - private int Method6() { return 0; } - private int Method7() { return 0; } - public int Test1(MethodUsages other) - { - int i; - i = Method1(); - i = this.Method2(); - Console.Write(Method3()); - new MethodUsages().Method4(); - Func x = () => Method5(); - other.Method6(); - return Method7(); - } - - private int Method8() { return 0; } - public int ExpressionBodyMethod() => Method8(); - - private static int Method9() { return 0; } - public MethodUsages(int number) { } - public MethodUsages() : this(Method9()) { } - - private int Method10() { return 0; } - private int Method11() { return 0; } - public object Test2() - { - var x = new[] { Method10() }; - var name = nameof(Method11); - return null; - } - - private int Method12(int i) { return 0; } - public void Test3() - { - new[] { 1, 2, 3 }.Select(Method12); - } + int i; + i = Method1(); + i = this.Method2(); + Console.Write(Method3()); + new MethodUsages().Method4(); + Func x = () => Method5(); + other.Method6(); + return Method7(); } - """).VerifyNoIssues(); + private int Method8() { return 0; } + public int ExpressionBodyMethod() => Method8(); + + private static int Method9() { return 0; } + public MethodUsages(int number) { } + public MethodUsages() : this(Method9()) { } + + private int Method10() { return 0; } + private int Method11() { return 0; } + public object Test2() + { + var x = new[] { Method10() }; + var name = nameof(Method11); + return null; + } - [TestMethod] - public void UnusedPrivateMember_Methods_Main() => - builder.AddSnippet(@" + private int Method12(int i) { return 0; } + public void Test3() + { + new[] { 1, 2, 3 }.Select(Method12); + } + } + + """).VerifyNoIssues(); + + [TestMethod] + public void UnusedPrivateMember_Methods_Main() => + builder.AddSnippet(@" using System.Threading.Tasks; public class NewClass1 { @@ -160,5 +160,4 @@ public class NewClass5 static async Task Main(string[] args) { return ""a""; } // Noncompliant } ").Verify(); - } } diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Types.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Types.cs index ebcc9d101b4..618769f6e19 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Types.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/UnusedPrivateMemberTest.Types.cs @@ -18,13 +18,13 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -namespace SonarAnalyzer.Test.Rules +namespace SonarAnalyzer.Test.Rules; + +public partial class UnusedPrivateMemberTest { - public partial class UnusedPrivateMemberTest - { - [TestMethod] - public void UnusedPrivateMember_Types_Accessibility() => - builder.AddSnippet(@" + [TestMethod] + public void UnusedPrivateMember_Types_Accessibility() => + builder.AddSnippet(@" public class PrivateTypes { private class InnerPrivateClass // Noncompliant {{Remove the unused private class 'InnerPrivateClass'.}} @@ -54,9 +54,9 @@ public struct PublicStruct { } } ").Verify(); - [TestMethod] - public void UnusedPrivateMember_Types_InternalsVisibleTo() => - builder.AddSnippet(@" + [TestMethod] + public void UnusedPrivateMember_Types_InternalsVisibleTo() => + builder.AddSnippet(@" [assembly:System.Runtime.CompilerServices.InternalsVisibleTo("""")] public class PrivateTypes { @@ -65,32 +65,32 @@ internal class InternalClass { } // Compliant, internal types are not reported w } ").Verify(); - [TestMethod] - public void UnusedPrivateMember_Types_Internals() => - builder.AddSnippet(""" - // https://github.com/SonarSource/sonar-dotnet/issues/1225 - // https://github.com/SonarSource/sonar-dotnet/issues/904 - using System; - public class Class1 + [TestMethod] + public void UnusedPrivateMember_Types_Internals() => + builder.AddSnippet(""" + // https://github.com/SonarSource/sonar-dotnet/issues/1225 + // https://github.com/SonarSource/sonar-dotnet/issues/904 + using System; + public class Class1 + { + public void Method1() { - public void Method1() - { - var x = Sample.Constants.X; - } + var x = Sample.Constants.X; } + } - public class Sample + public class Sample + { + internal class Constants { - internal class Constants - { - public const int X = 5; - } + public const int X = 5; } - """).VerifyNoIssues(); + } + """).VerifyNoIssues(); - [TestMethod] - public void UnusedPrivateMember_Types_DirectReferences() => - builder.AddSnippet(@" + [TestMethod] + public void UnusedPrivateMember_Types_DirectReferences() => + builder.AddSnippet(@" using System.Linq; public class PrivateTypes { @@ -117,9 +117,9 @@ public void Test1() } ").Verify(); - [TestMethod] - public void UnusedPrivateMember_SupportTypeKinds() => - builder.AddSnippet(@" + [TestMethod] + public void UnusedPrivateMember_SupportTypeKinds() => + builder.AddSnippet(@" public class PrivateTypes { private class MyPrivateClass { } // Noncompliant @@ -149,5 +149,4 @@ public void Foo() public static int PerformCalculation(int x, int y) => x + y; } ").Verify(); - } } From f0b7dbcb968093b756afb2ac61b66f607147370a Mon Sep 17 00:00:00 2001 From: Cristian Ambrosini Date: Thu, 6 Jun 2024 17:41:25 +0200 Subject: [PATCH 07/13] Fix QA --- .../Helpers/CSharpSymbolUsageCollector.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs index ea068961230..92fa7eef4d1 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs @@ -142,9 +142,9 @@ public override void VisitAttribute(AttributeSyntax node) { var arguments = node.ArgumentList.Arguments .Where(IsValueNameOrType) - .Select(a => model.GetConstantValue(a.Expression)) - .Where(o => o.HasValue) - .Select(o => o.Value) + .Select(x => model.GetConstantValue(x.Expression)) + .Where(x => x.HasValue) + .Select(x => x.Value) .OfType(); DebuggerDisplayValues.UnionWith(arguments); From ffd0accc862ef7c7f7fe5ee6e57b4c18d441a02a Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Mon, 10 Jun 2024 11:29:40 +0200 Subject: [PATCH 08/13] Remove check through type parameters (too slow) --- .../Helpers/CSharpSymbolUsageCollector.cs | 21 ------------------- .../TestCases/UnusedPrivateMember.CSharp9.cs | 20 +++++++++--------- 2 files changed, 10 insertions(+), 31 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs index 92fa7eef4d1..d31cf990d04 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs @@ -177,27 +177,6 @@ public override void VisitIdentifierName(IdentifierNameSyntax node) base.VisitIdentifierName(node); } - public override void VisitTypeArgumentList(TypeArgumentListSyntax node) - { - if (model.GetSymbolInfo(node.Parent).Symbol is { } symbol) // symbol that the attribute is applied to - { - var typesWithDynamicallyAccessedMembers = GetTypeParameters(symbol) - .Zip(node.Arguments, (symbol, argument) => new Tuple(symbol, argument)) // map T to Person in void M() { } .. M(); - .Where(x => x.Item1.HasAttribute(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute)) - .Select(x => model.GetSymbolInfo(x.Item2).Symbol); - TypesUsedWithReflection.UnionWith(typesWithDynamicallyAccessedMembers); - } - base.VisitTypeArgumentList(node); - - static ImmutableArray GetTypeParameters(ISymbol symbol) => - symbol switch - { - IMethodSymbol method => method.TypeParameters, - INamedTypeSymbol namedType => namedType.TypeParameters, - _ => ImmutableArray.Empty - }; - } - public override void VisitObjectCreationExpression(ObjectCreationExpressionSyntax node) { if (knownSymbolNames.Contains(node.Type.GetName())) diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs index b9b791bbd21..35415037b91 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs @@ -181,7 +181,7 @@ public static void Method() var instance2 = CreateInstance(typeof(ClassInstantiatedThroughReflection)); A classViaReflection = new(); - InitValue(classViaReflection); + InitValue(classViaReflection); } public static T CreateInstance<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] T>() => @@ -199,26 +199,26 @@ private class A class C { - private int usedByReflection; - C Create() => Program.CreateInstance>(); + private int usedByReflection; // Noncompliant - FP + C Create() => Program.CreateInstance>(); // Noncompliant - FP } private class ClassInstantiatedThroughReflection { - private const int PrivateConst = 42; // Compliant - we assume all members are used through reflection - private int privateField; - private int PrivateProperty { get; set; } - private void PrivateMethod() { } + private const int PrivateConst = 42; // Noncompliant - FP + private int privateField; // Noncompliant - FP + private int PrivateProperty { get; set; } // Noncompliant - FP + private void PrivateMethod() { } // Noncompliant - FP private ClassInstantiatedThroughReflection() { } - private event EventHandler PrivateEvent; + private event EventHandler PrivateEvent; // Noncompliant - FP public ClassInstantiatedThroughReflection(int arg) // Compliant - the constructor used in CreateInstance through Reflection { } - private class NestedType + private class NestedType // Noncompliant - FP { - private int privateField; + private int privateField; // Noncompliant - FP } } From 667d9fba610061a231a084f01116744aa7110cdd Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Thu, 13 Jun 2024 10:17:53 +0200 Subject: [PATCH 09/13] Review --- .../Helpers/CSharpSymbolUsageCollector.cs | 2 +- .../TestCases/UnusedPrivateMember.CSharp9.cs | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs index d31cf990d04..ae52cdf3e4b 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs @@ -150,7 +150,7 @@ public override void VisitAttribute(AttributeSyntax node) DebuggerDisplayValues.UnionWith(arguments); } else if (attribute.Is(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute) - && node.Parent.Parent is BaseTypeDeclarationSyntax typeDeclaration + && node is { Parent: AttributeListSyntax { Parent: BaseTypeDeclarationSyntax typeDeclaration } } && model.GetDeclaredSymbol(typeDeclaration) is { } typeSymbol) { TypesUsedWithReflection.Add(typeSymbol); diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs index 35415037b91..a307799676c 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs @@ -178,7 +178,7 @@ public static class Program public static void Method() { var instance = CreateInstance(); - var instance2 = CreateInstance(typeof(ClassInstantiatedThroughReflection)); + var instance2 = CreateInstance(typeof(AnotherClassInstantiatedThroughReflection)); A classViaReflection = new(); InitValue(classViaReflection); @@ -212,7 +212,7 @@ private void PrivateMethod() { } // Noncompliant - FP private ClassInstantiatedThroughReflection() { } private event EventHandler PrivateEvent; // Noncompliant - FP - public ClassInstantiatedThroughReflection(int arg) // Compliant - the constructor used in CreateInstance through Reflection + public ClassInstantiatedThroughReflection(int arg) { } @@ -222,6 +222,11 @@ private class NestedType // Noncompliant - FP } } + private class AnotherClassInstantiatedThroughReflection + { + private int privateField; // Noncompliant - FP + } + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] private class TypeDecoratedWithDynamicallyAccessedMembers { From 5b96a358137e1731cd6121c5577ef6c59688e0b8 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Thu, 13 Jun 2024 10:36:36 +0200 Subject: [PATCH 10/13] Loosen check for DynamicallyAccessedMembersAttribute --- .../Helpers/CSharpSymbolUsageCollector.cs | 35 +++++++++---------- .../TestCases/UnusedPrivateMember.CSharp9.cs | 11 ++++++ 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs index ae52cdf3e4b..5939b2d691d 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs @@ -18,6 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using Microsoft.CodeAnalysis; using SonarAnalyzer.Common.Walkers; namespace SonarAnalyzer.Helpers; @@ -135,26 +136,24 @@ static ISymbol FindDeconstructor(IEnumerable deconstructors, int number public override void VisitAttribute(AttributeSyntax node) { - if (model.GetSymbolInfo(node).Symbol is IMethodSymbol { MethodKind: MethodKind.Constructor, ContainingType: ITypeSymbol attribute }) + if (node.GetName() is "DynamicallyAccessedMembersAttribute" or "DynamicallyAccessedMembers" + && node is { Parent: AttributeListSyntax { Parent: BaseTypeDeclarationSyntax typeDeclaration } } + && model.GetDeclaredSymbol(typeDeclaration) is { } typeSymbol) { - if (attribute.Is(KnownType.System_Diagnostics_DebuggerDisplayAttribute) + TypesUsedWithReflection.Add(typeSymbol); + } + else if (model.GetSymbolInfo(node).Symbol is IMethodSymbol { MethodKind: MethodKind.Constructor, ContainingType: ITypeSymbol attribute } + && attribute.Is(KnownType.System_Diagnostics_DebuggerDisplayAttribute) && node.ArgumentList is not null) - { - var arguments = node.ArgumentList.Arguments - .Where(IsValueNameOrType) - .Select(x => model.GetConstantValue(x.Expression)) - .Where(x => x.HasValue) - .Select(x => x.Value) - .OfType(); - - DebuggerDisplayValues.UnionWith(arguments); - } - else if (attribute.Is(KnownType.System_Diagnostics_CodeAnalysis_DynamicallyAccessedMembersAttribute) - && node is { Parent: AttributeListSyntax { Parent: BaseTypeDeclarationSyntax typeDeclaration } } - && model.GetDeclaredSymbol(typeDeclaration) is { } typeSymbol) - { - TypesUsedWithReflection.Add(typeSymbol); - } + { + var arguments = node.ArgumentList.Arguments + .Where(IsValueNameOrType) + .Select(x => model.GetConstantValue(x.Expression)) + .Where(x => x.HasValue) + .Select(x => x.Value) + .OfType(); + + DebuggerDisplayValues.UnionWith(arguments); } base.VisitAttribute(node); diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs index a307799676c..36b94dcc98a 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs @@ -301,3 +301,14 @@ private class DynamicallyAccessedMembersOnlyForConstructors } } } + +namespace CustomDynamicallyAccessedMembersAttribute +{ + public class DynamicallyAccessedMembersAttribute : Attribute { } + + [DynamicallyAccessedMembers] + public class UnusedClassMarkedWithCustomAttribute + { + private int privateField; + } +} From b84a2ea842108ec185aa7971de230a299e3258b5 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Thu, 13 Jun 2024 12:46:07 +0200 Subject: [PATCH 11/13] Update RSPEC --- analyzers/rspec/cs/S1144.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/analyzers/rspec/cs/S1144.html b/analyzers/rspec/cs/S1144.html index 0bf9b1254a4..9284f1debd9 100644 --- a/analyzers/rspec/cs/S1144.html +++ b/analyzers/rspec/cs/S1144.html @@ -43,6 +43,8 @@

Exceptions

href="https://learn.microsoft.com/en-us/dotnet/api/system.serializableattribute">System.SerializableAttribute attribute
  • internal members in assemblies that have a System.Runtime.CompilerServices.InternalsVisibleToAttribute attribute.
    • +
  • types and members decorated with the System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute attribute (available in .NET 5.0+) or a custom attribute named DynamicallyAccessedMembersAttribute.

    • Resources

    Documentation

    From 219213e36d986ac48fafeca8411bdaaf3669f5cf Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Thu, 13 Jun 2024 13:48:25 +0200 Subject: [PATCH 12/13] Review --- .../Helpers/CSharpSymbolUsageCollector.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs index 5939b2d691d..628cdda30ab 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs @@ -136,7 +136,13 @@ static ISymbol FindDeconstructor(IEnumerable deconstructors, int number public override void VisitAttribute(AttributeSyntax node) { - if (node.GetName() is "DynamicallyAccessedMembersAttribute" or "DynamicallyAccessedMembers" + // Some members that seem unused might be dynamically accessed through reflection. + // The DynamicallyAccessedMembersAttribute was introduced to inform tools about such uses. + // The attribute is not available on NetFramework and we want to enable this mechanism for these users. + // Therefore, we only check the name, but not the namespace and let the users define their own custom version. + // https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.codeanalysis.dynamicallyaccessedmembersattribute + const string DynamicallyAccessedMembers = "DynamicallyAccessedMembers"; + if (node.GetName() is DynamicallyAccessedMembers or $"{DynamicallyAccessedMembers}Attribute" && node is { Parent: AttributeListSyntax { Parent: BaseTypeDeclarationSyntax typeDeclaration } } && model.GetDeclaredSymbol(typeDeclaration) is { } typeSymbol) { From e07a90f7ac4e3130e9df68325229d021fdff8410 Mon Sep 17 00:00:00 2001 From: Zsolt Kolbay Date: Thu, 13 Jun 2024 13:49:41 +0200 Subject: [PATCH 13/13] Remove namespace --- .../SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs index 628cdda30ab..33db5e93128 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs @@ -18,7 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using Microsoft.CodeAnalysis; using SonarAnalyzer.Common.Walkers; namespace SonarAnalyzer.Helpers;