diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/NotAssignedPrivateMember.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/NotAssignedPrivateMember.cs index 4aaa6a455a3..5c4a3853a22 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/NotAssignedPrivateMember.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/NotAssignedPrivateMember.cs @@ -20,190 +20,189 @@ using MemberUsage = SonarAnalyzer.Common.NodeSymbolAndModel; -namespace SonarAnalyzer.Rules.CSharp +namespace SonarAnalyzer.Rules.CSharp; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class NotAssignedPrivateMember : SonarDiagnosticAnalyzer { - [DiagnosticAnalyzer(LanguageNames.CSharp)] - public sealed class NotAssignedPrivateMember : SonarDiagnosticAnalyzer - { - /* - CS0649 reports the same on internal fields. So that's wider in scope, but that's not a live Roslyn analyzer, - the issue only shows up at build time and not during editing. - */ + /* + CS0649 reports the same on internal fields. So that's wider in scope, but that's not a live Roslyn analyzer, + the issue only shows up at build time and not during editing. + */ - private const string DiagnosticId = "S3459"; - private const string MessageFormat = "Remove unassigned {0} '{1}', or set its value."; - private const Accessibility MaxAccessibility = Accessibility.Private; + private const string DiagnosticId = "S3459"; + private const string MessageFormat = "Remove unassigned {0} '{1}', or set its value."; + private const Accessibility MaxAccessibility = Accessibility.Private; - private static readonly DiagnosticDescriptor Rule = - DescriptorFactory.Create(DiagnosticId, MessageFormat); + private static readonly DiagnosticDescriptor Rule = + DescriptorFactory.Create(DiagnosticId, MessageFormat); - private static readonly ISet PreOrPostfixOpSyntaxKinds = new HashSet - { - SyntaxKind.PostDecrementExpression, - SyntaxKind.PostIncrementExpression, - SyntaxKind.PreDecrementExpression, - SyntaxKind.PreIncrementExpression, - }; + private static readonly ISet PreOrPostfixOpSyntaxKinds = new HashSet + { + SyntaxKind.PostDecrementExpression, + SyntaxKind.PostIncrementExpression, + SyntaxKind.PreDecrementExpression, + SyntaxKind.PreIncrementExpression, + }; - public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); + public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); - protected override void Initialize(SonarAnalysisContext context) => - context.RegisterSymbolAction( - c => + protected override void Initialize(SonarAnalysisContext context) => + context.RegisterSymbolAction( + c => + { + var namedType = (INamedTypeSymbol)c.Symbol; + if (TypeDefinitionShouldBeSkipped(namedType)) { - var namedType = (INamedTypeSymbol)c.Symbol; - if (TypeDefinitionShouldBeSkipped(namedType)) - { - return; - } + return; + } - var removableDeclarationCollector = new CSharpRemovableDeclarationCollector(namedType, c.Compilation); + var removableDeclarationCollector = new CSharpRemovableDeclarationCollector(namedType, c.Compilation); - var allCandidateMembers = GetCandidateDeclarations(removableDeclarationCollector); - if (!allCandidateMembers.Any()) - { - return; - } + var allCandidateMembers = GetCandidateDeclarations(removableDeclarationCollector); + if (!allCandidateMembers.Any()) + { + return; + } - var usedMembers = GetMemberUsages(removableDeclarationCollector, new HashSet(allCandidateMembers.Select(t => t.Symbol))); - var usedMemberSymbols = new HashSet(usedMembers.Select(tuple => tuple.Symbol)); + var usedMembers = GetMemberUsages(removableDeclarationCollector, new HashSet(allCandidateMembers.Select(t => t.Symbol))); + var usedMemberSymbols = new HashSet(usedMembers.Select(tuple => tuple.Symbol)); - var assignedMemberSymbols = GetAssignedMemberSymbols(usedMembers); - var unassignedUsedMemberSymbols = allCandidateMembers.Where(x => usedMemberSymbols.Contains(x.Symbol) && !assignedMemberSymbols.Contains(x.Symbol)); + var assignedMemberSymbols = GetAssignedMemberSymbols(usedMembers); + var unassignedUsedMemberSymbols = allCandidateMembers.Where(x => usedMemberSymbols.Contains(x.Symbol) && !assignedMemberSymbols.Contains(x.Symbol)); - foreach (var candidateMember in unassignedUsedMemberSymbols) - { - var field = candidateMember.Node as VariableDeclaratorSyntax; - var property = candidateMember.Node as PropertyDeclarationSyntax; + foreach (var candidateMember in unassignedUsedMemberSymbols) + { + var field = candidateMember.Node as VariableDeclaratorSyntax; + var property = candidateMember.Node as PropertyDeclarationSyntax; - var memberType = field == null ? "auto-property" : "field"; + var memberType = field is null ? "auto-property" : "field"; - var location = field == null - ? property.Identifier.GetLocation() - : field.Identifier.GetLocation(); + var location = field is null + ? property.Identifier.GetLocation() + : field.Identifier.GetLocation(); - c.ReportIssue(Rule, location, memberType, candidateMember.Symbol.Name); - } - }, - SymbolKind.NamedType); + c.ReportIssue(Rule, location, memberType, candidateMember.Symbol.Name); + } + }, + SymbolKind.NamedType); - private static List> GetCandidateDeclarations(CSharpRemovableDeclarationCollector removableDeclarationCollector) - { - var candidateFields = removableDeclarationCollector.GetRemovableFieldLikeDeclarations(new HashSet { SyntaxKind.FieldDeclaration }, MaxAccessibility) - .Where(tuple => !IsInitializedOrFixed((VariableDeclaratorSyntax)tuple.Node) - && !HasStructLayoutAttribute(tuple.Symbol.ContainingType)); + private static List> GetCandidateDeclarations(CSharpRemovableDeclarationCollector removableDeclarationCollector) + { + var candidateFields = removableDeclarationCollector.GetRemovableFieldLikeDeclarations(new HashSet { SyntaxKind.FieldDeclaration }, MaxAccessibility) + .Where(x => !IsInitializedOrFixed((VariableDeclaratorSyntax)x.Node) + && !HasStructLayoutAttribute(x.Symbol.ContainingType)); - var candidateProperties = removableDeclarationCollector.GetRemovableDeclarations(new HashSet { SyntaxKind.PropertyDeclaration }, MaxAccessibility) - .Where(tuple => IsAutoPropertyWithNoInitializer((PropertyDeclarationSyntax)tuple.Node) - && !HasStructLayoutAttribute(tuple.Symbol.ContainingType)); + var candidateProperties = removableDeclarationCollector.GetRemovableDeclarations(new HashSet { SyntaxKind.PropertyDeclaration }, MaxAccessibility) + .Where(x => IsAutoPropertyWithNoInitializer((PropertyDeclarationSyntax)x.Node) + && !HasStructLayoutAttribute(x.Symbol.ContainingType)); - return candidateFields.Concat(candidateProperties).ToList(); - } + return candidateFields.Concat(candidateProperties).ToList(); + } - private static bool TypeDefinitionShouldBeSkipped(ITypeSymbol namedType) => - !namedType.IsClassOrStruct() - || HasStructLayoutAttribute(namedType) - || namedType.ContainingType != null - || namedType.HasAttribute(KnownType.System_SerializableAttribute); + private static bool TypeDefinitionShouldBeSkipped(ITypeSymbol namedType) => + !namedType.IsClassOrStruct() + || HasStructLayoutAttribute(namedType) + || namedType.ContainingType is not null + || namedType.HasAttribute(KnownType.System_SerializableAttribute); - private static bool HasStructLayoutAttribute(ISymbol namedTypeSymbol) => - namedTypeSymbol.HasAttribute(KnownType.System_Runtime_InteropServices_StructLayoutAttribute); + private static bool HasStructLayoutAttribute(ISymbol namedTypeSymbol) => + namedTypeSymbol.HasAttribute(KnownType.System_Runtime_InteropServices_StructLayoutAttribute); - private static bool IsInitializedOrFixed(VariableDeclaratorSyntax declarator) + private static bool IsInitializedOrFixed(VariableDeclaratorSyntax declarator) + { + if (declarator.Parent.Parent is BaseFieldDeclarationSyntax fieldDeclaration + && fieldDeclaration.Modifiers.Any(SyntaxKind.FixedKeyword)) { - if (declarator.Parent.Parent is BaseFieldDeclarationSyntax fieldDeclaration - && fieldDeclaration.Modifiers.Any(SyntaxKind.FixedKeyword)) - { - return true; - } - - return declarator.Initializer != null; + return true; } - private static bool IsAutoPropertyWithNoInitializer(PropertyDeclarationSyntax declaration) => - declaration.Initializer == null - && declaration.AccessorList != null - && declaration.AccessorList.Accessors.All(acc => acc.Body == null && acc.ExpressionBody() == null); + return declarator.Initializer is not null; + } - private static IList GetMemberUsages(CSharpRemovableDeclarationCollector removableDeclarationCollector, HashSet declaredPrivateSymbols) - { - var symbolNames = declaredPrivateSymbols.Select(s => s.Name).ToHashSet(); - - var identifiers = removableDeclarationCollector.TypeDeclarations - .SelectMany(container => container.Node.DescendantNodes() - .Where(node => node.IsKind(SyntaxKind.IdentifierName)) - .Cast() - .Where(x => symbolNames.Contains(x.Identifier.ValueText)) - .Select(x => new MemberUsage(container.Model, x, container.Model.GetSymbolInfo(x).Symbol))); - - var generic = removableDeclarationCollector.TypeDeclarations - .SelectMany(container => container.Node.DescendantNodes() - .Where(node => node.IsKind(SyntaxKind.GenericName)) - .Cast() - .Where(x => symbolNames.Contains(x.Identifier.ValueText)) - .Select(x => new MemberUsage(container.Model, x, container.Model.GetSymbolInfo(x).Symbol))); - - return identifiers.Concat(generic) - .Where(tuple => tuple.Symbol is IFieldSymbol || tuple.Symbol is IPropertySymbol) - .ToList(); - } + private static bool IsAutoPropertyWithNoInitializer(PropertyDeclarationSyntax declaration) => + declaration.Initializer is null + && declaration.AccessorList is not null + && declaration.AccessorList.Accessors.All(acc => acc.Body is null && acc.ExpressionBody() is null); - private static ISet GetAssignedMemberSymbols(IList memberUsages) + private static IList GetMemberUsages(CSharpRemovableDeclarationCollector removableDeclarationCollector, HashSet declaredPrivateSymbols) + { + var symbolNames = declaredPrivateSymbols.Select(s => s.Name).ToHashSet(); + + var identifiers = removableDeclarationCollector.TypeDeclarations + .SelectMany(container => container.Node.DescendantNodes() + .Where(node => node.IsKind(SyntaxKind.IdentifierName)) + .Cast() + .Where(x => symbolNames.Contains(x.Identifier.ValueText)) + .Select(x => new MemberUsage(container.Model, x, container.Model.GetSymbolInfo(x).Symbol))); + + var generic = removableDeclarationCollector.TypeDeclarations + .SelectMany(container => container.Node.DescendantNodes() + .Where(node => node.IsKind(SyntaxKind.GenericName)) + .Cast() + .Where(x => symbolNames.Contains(x.Identifier.ValueText)) + .Select(x => new MemberUsage(container.Model, x, container.Model.GetSymbolInfo(x).Symbol))); + + return identifiers.Concat(generic) + .Where(x => x.Symbol is IFieldSymbol or IPropertySymbol) + .ToList(); + } + + private static ISet GetAssignedMemberSymbols(IList memberUsages) + { + var assignedMembers = new HashSet(); + + foreach (var memberUsage in memberUsages) { - var assignedMembers = new HashSet(); + ExpressionSyntax node = memberUsage.Node; + var memberSymbol = memberUsage.Symbol; - foreach (var memberUsage in memberUsages) + // Handle "expr.FieldName" + if (node.Parent is MemberAccessExpressionSyntax simpleMemberAccess + && simpleMemberAccess.Name == node) { - ExpressionSyntax node = memberUsage.Node; - var memberSymbol = memberUsage.Symbol; + node = simpleMemberAccess; + } - // Handle "expr.FieldName" - if (node.Parent is MemberAccessExpressionSyntax simpleMemberAccess - && simpleMemberAccess.Name == node) + // Handle "((expr.FieldName))" + node = node.GetSelfOrTopParenthesizedExpression(); + + if (IsValueType(memberSymbol)) + { + // Handle (((exp.FieldName)).Member1).Member2 + var parentMemberAccess = node.Parent as MemberAccessExpressionSyntax; + while (IsParentMemberAccess(parentMemberAccess, node)) { - node = simpleMemberAccess; + node = parentMemberAccess.GetSelfOrTopParenthesizedExpression(); + parentMemberAccess = node.Parent as MemberAccessExpressionSyntax; } - // Handle "((expr.FieldName))" node = node.GetSelfOrTopParenthesizedExpression(); + } - if (IsValueType(memberSymbol)) - { - // Handle (((exp.FieldName)).Member1).Member2 - var parentMemberAccess = node.Parent as MemberAccessExpressionSyntax; - while (IsParentMemberAccess(parentMemberAccess, node)) - { - node = parentMemberAccess.GetSelfOrTopParenthesizedExpression(); - parentMemberAccess = node.Parent as MemberAccessExpressionSyntax; - } - - node = node.GetSelfOrTopParenthesizedExpression(); - } - - var parentNode = node.Parent; + var parentNode = node.Parent; - if (PreOrPostfixOpSyntaxKinds.Contains(parentNode.Kind()) - || (parentNode is AssignmentExpressionSyntax assignment && assignment.Left == node) - || (parentNode is ArgumentSyntax argument && (!argument.RefOrOutKeyword.IsKind(SyntaxKind.None) || TupleExpressionSyntaxWrapper.IsInstance(argument.Parent)))) - { - assignedMembers.Add(memberSymbol); - assignedMembers.Add(memberSymbol.OriginalDefinition); - } + if (PreOrPostfixOpSyntaxKinds.Contains(parentNode.Kind()) + || (parentNode is AssignmentExpressionSyntax assignment && assignment.Left == node) + || (parentNode is ArgumentSyntax argument && (!argument.RefOrOutKeyword.IsKind(SyntaxKind.None) || TupleExpressionSyntaxWrapper.IsInstance(argument.Parent))) + || RefExpressionSyntaxWrapper.IsInstance(parentNode)) + { + assignedMembers.Add(memberSymbol); + assignedMembers.Add(memberSymbol.OriginalDefinition); } - - return assignedMembers; } - private static bool IsParentMemberAccess(MemberAccessExpressionSyntax parent, ExpressionSyntax node) => - parent != null - && parent.Expression == node; - - private static bool IsValueType(ISymbol symbol) => - symbol switch - { - IFieldSymbol field => field.Type.IsValueType, - IPropertySymbol property => property.Type.IsValueType, - _ => false - }; + return assignedMembers; } + + private static bool IsParentMemberAccess(MemberAccessExpressionSyntax parent, ExpressionSyntax node) => + parent?.Expression == node; + + private static bool IsValueType(ISymbol symbol) => + symbol switch + { + IFieldSymbol field => field.Type.IsValueType, + IPropertySymbol property => property.Type.IsValueType, + _ => false + }; } diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/NotAssignedPrivateMemberTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/NotAssignedPrivateMemberTest.cs index 8ebb60b93bd..8fa5e7ebe2b 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/NotAssignedPrivateMemberTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/NotAssignedPrivateMemberTest.cs @@ -21,37 +21,37 @@ using Microsoft.CodeAnalysis.CSharp; using SonarAnalyzer.Rules.CSharp; -namespace SonarAnalyzer.Test.Rules +namespace SonarAnalyzer.Test.Rules; + +[TestClass] +public class NotAssignedPrivateMemberTest { - [TestClass] - public class NotAssignedPrivateMemberTest - { - private readonly VerifierBuilder builder = new VerifierBuilder(); + private readonly VerifierBuilder builder = new VerifierBuilder(); - [TestMethod] - public void NotAssignedPrivateMember() => - builder.AddPaths("NotAssignedPrivateMember.cs").Verify(); + [TestMethod] + public void NotAssignedPrivateMember() => + builder.AddPaths("NotAssignedPrivateMember.cs").Verify(); #if NET - [TestMethod] - public void NotAssignedPrivateMember_CSharp9() => - builder.AddPaths("NotAssignedPrivateMember.CSharp9.cs").WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); + [TestMethod] + public void NotAssignedPrivateMember_CSharp9() => + builder.AddPaths("NotAssignedPrivateMember.CSharp9.cs").WithOptions(ParseOptionsHelper.FromCSharp9).Verify(); #endif - [TestMethod] - public void NotAssignedPrivateMember_IndexingMovableFixedBuffer() => - builder.AddSnippet(@" -unsafe struct FixedArray -{ - private fixed int a[42]; // Compliant, because of the fixed modifier + [TestMethod] + public void NotAssignedPrivateMember_IndexingMovableFixedBuffer() => + builder.AddSnippet(""" + unsafe struct FixedArray + { + private fixed int a[42]; // Compliant, because of the fixed modifier - private int[] b; // Noncompliant + private int[] b; // Noncompliant - void M() - { - a[0] = 42; - b[0] = 42; - } -}").WithLanguageVersion(LanguageVersion.CSharp7_3).Verify(); - } + void M() + { + a[0] = 42; + b[0] = 42; + } + } + """).WithLanguageVersion(LanguageVersion.CSharp7_3).Verify(); } diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/NotAssignedPrivateMember.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/NotAssignedPrivateMember.cs index d6f76f7e750..0b1a739fdb6 100644 --- a/analyzers/tests/SonarAnalyzer.Test/TestCases/NotAssignedPrivateMember.cs +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/NotAssignedPrivateMember.cs @@ -205,7 +205,7 @@ public void Print() // Reproducer: https://github.com/SonarSource/sonar-dotnet/issues/9106 public class Repro_9106 { - private int _foo; // Noncompliant FP + private int _foo; // Compliant public ref int Foo => ref _foo; }