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

    diff --git a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs index b7dc1d889ee..33db5e93128 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Helpers/CSharpSymbolUsageCollector.cs @@ -20,413 +20,430 @@ 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; + None = 0, + Read = 1, + Write = 2, + ReadWrite = Read | Write + } - public ISet UsedSymbols { get; } = new HashSet(); - public IDictionary FieldSymbolUsages { get; } = new Dictionary(); - public HashSet DebuggerDisplayValues { get; } = []; - public Dictionary PropertyAccess { get; } = []; + 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 CSharpSymbolUsageCollector(Compilation compilation, IEnumerable knownSymbols) + public override void Visit(SyntaxNode node) + { + model = node.EnsureCorrectSemanticModelOrDefault(model ?? compilation.GetSemanticModel(node.SyntaxTree)); + if (model is not null) { - this.compilation = compilation; - knownSymbolNames = knownSymbols.Select(GetName).ToHashSet(); + if (node.IsKind(SyntaxKindEx.ImplicitObjectCreationExpression) + && knownSymbolNames.Contains(ObjectCreationFactory.Create(node).TypeAsString(model))) + { + UsedSymbols.UnionWith(GetSymbols(node)); + } + else if (node.IsKind(SyntaxKindEx.LocalFunctionStatement) + && ((LocalFunctionStatementSyntaxWrapper)node) is { AttributeLists.Count: > 0 } + && model.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); } + } - public override void Visit(SyntaxNode 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) { - semanticModel = node.EnsureCorrectSemanticModelOrDefault(semanticModel ?? compilation.GetSemanticModel(node.SyntaxTree)); - if (semanticModel != null) + var assignmentRight = node.Right; + var namedTypeSymbol = model.GetSymbolInfo(assignmentRight).Symbol?.GetSymbolType(); + if (namedTypeSymbol is not null) { - 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) + var deconstructors = namedTypeSymbol.GetMembers("Deconstruct"); + if (deconstructors.Length == 1) { - UsedSymbols.UnionWith(localFunctionSymbol - .GetAttributes() - .Where(a => knownSymbolNames.Contains(a.AttributeClass.Name)) - .Select(a => a.AttributeClass)); + UsedSymbols.Add(deconstructors.First()); } - else if (node.IsKind(SyntaxKindEx.PrimaryConstructorBaseType) - && knownSymbolNames.Contains(((PrimaryConstructorBaseTypeSyntaxWrapper)node).Type.GetName())) + else if (deconstructors.Length > 1 && FindDeconstructor(deconstructors, leftTupleCount) is {} deconstructor) { - UsedSymbols.UnionWith(GetSymbols(node)); + UsedSymbols.Add(deconstructor); } - base.Visit(node); } } + base.VisitAssignmentExpression(node); - // TupleExpression "(a, b) = qix" - // ParenthesizedVariableDesignation "var (a, b) = quix" inside a DeclarationExpression - public override void VisitAssignmentExpression(AssignmentExpressionSyntax node) + static int GetTupleCount(ExpressionSyntax assignmentLeft) { - var leftTupleCount = GetTupleCount(node.Left); - if (leftTupleCount != 0) + var result = 0; + if (TupleExpressionSyntaxWrapper.IsInstance(assignmentLeft)) { - 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); - } - } + result = ((TupleExpressionSyntaxWrapper)assignmentLeft).Arguments.Count; } - base.VisitAssignmentExpression(node); - - static int GetTupleCount(ExpressionSyntax assignmentLeft) + else if (DeclarationExpressionSyntaxWrapper.IsInstance(assignmentLeft) + && (DeclarationExpressionSyntaxWrapper)assignmentLeft is { } leftDeclaration + && ParenthesizedVariableDesignationSyntaxWrapper.IsInstance(leftDeclaration.Designation)) { - 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; + result = ((ParenthesizedVariableDesignationSyntaxWrapper)leftDeclaration.Designation).Variables.Count; } - - static ISymbol FindDeconstructor(IEnumerable deconstructors, int numberOfArguments) => - deconstructors.FirstOrDefault(m => m.GetParameters().Count() == numberOfArguments && m.DeclaredAccessibility.IsAccessibleOutsideTheType()); + return result; } - 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) - { - 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); - } - 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"; - } + static ISymbol FindDeconstructor(IEnumerable deconstructors, int numberOfArguments) => + deconstructors.FirstOrDefault(m => m.GetParameters().Count() == numberOfArguments && m.DeclaredAccessibility.IsAccessibleOutsideTheType()); + } - public override void VisitIdentifierName(IdentifierNameSyntax node) + public override void VisitAttribute(AttributeSyntax node) + { + // 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) { - if (IsKnownIdentifier(node.Identifier)) - { - var symbols = GetSymbols(node); - TryStoreFieldAccess(node, symbols); - UsedSymbols.UnionWith(symbols); - TryStorePropertyAccess(node, symbols); - } - base.VisitIdentifierName(node); + TypesUsedWithReflection.Add(typeSymbol); } - - public override void VisitObjectCreationExpression(ObjectCreationExpressionSyntax node) + 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) { - if (knownSymbolNames.Contains(node.Type.GetName())) - { - UsedSymbols.UnionWith(GetSymbols(node)); - } - base.VisitObjectCreationExpression(node); + 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); - 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 VisitObjectCreationExpression(ObjectCreationExpressionSyntax node) + { + if (knownSymbolNames.Contains(node.Type.GetName())) { - if (node.Expression.IsKind(SyntaxKind.ThisExpression) || knownSymbolNames.Contains(node.Expression.GetIdentifier()?.ValueText)) - { - var symbols = GetSymbols(node); - UsedSymbols.UnionWith(symbols); - TryStorePropertyAccess(node, symbols); - } - base.VisitElementAccessExpression(node); + 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 - }; - - 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 - }; - - /// - /// 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 + public override void VisitPropertyDeclaration(PropertyDeclarationSyntax node) + { + if (node.Initializer is not null && IsKnownIdentifier(node.Identifier)) { - 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; + var symbol = model.GetDeclaredSymbol(node); + UsedSymbols.Add(symbol); + StorePropertyAccess(symbol, AccessorAccess.Set); } + base.VisitPropertyDeclaration(node); + } - private void TryStorePropertyAccess(ExpressionSyntax node, IEnumerable identifierSymbols) + private SymbolAccess ParentAccessType(SyntaxNode node) => + node.Parent switch { - var propertySymbols = identifierSymbols.OfType().ToList(); - if (propertySymbols.Any()) + // (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 + }; + + 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 + }; + + /// + /// 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) + { + 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)) { - 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()) - { - 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 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 SymbolUsage GetFieldSymbolUsage(ISymbol symbol) => + FieldSymbolUsages.GetOrAdd(symbol, x => new SymbolUsage(x)); + + 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 9c8617113b3..976afa1d1ad 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) => @@ -232,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, @@ -278,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/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/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(); - } } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/UnusedPrivateMember.CSharp9.cs index 1ccd42adf73..36b94dcc98a 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,146 @@ 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(); + var instance2 = CreateInstance(typeof(AnotherClassInstantiatedThroughReflection)); + + A classViaReflection = new(); + InitValue(classViaReflection); + } + + 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 (see line 183) + } + + class C + { + private int usedByReflection; // Noncompliant - FP + C Create() => Program.CreateInstance>(); // Noncompliant - FP + } + + private class ClassInstantiatedThroughReflection + { + 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; // Noncompliant - FP + + public ClassInstantiatedThroughReflection(int arg) + { + } + + private class NestedType // Noncompliant - FP + { + private int privateField; // Noncompliant - FP + } + } + + private class AnotherClassInstantiatedThroughReflection + { + private int privateField; // Noncompliant - FP + } + + [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 { } + + [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 + + private void Method([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] NestedType arg) { } + + private class NestedType + { + private NestedType(int arg) { } + } + + public record RecordWithAttribute([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicFields)] string NotUnused); // Noncompliant + } + + 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 + } + } +} + +namespace CustomDynamicallyAccessedMembersAttribute +{ + public class DynamicallyAccessedMembersAttribute : Attribute { } + + [DynamicallyAccessedMembers] + public class UnusedClassMarkedWithCustomAttribute + { + private int privateField; + } +}