diff --git a/src/Analyzers/Core/Analyzers/RemoveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs index 53f687fb9cd9a..d942741c5cea8 100644 --- a/src/Analyzers/Core/Analyzers/RemoveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/RemoveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs @@ -759,62 +759,78 @@ private async Task ProcessSuppressMessageAttributesAsync( continue; } - var symbols = SemanticFacts.GetDeclaredSymbols(semanticModel, node, cancellationToken); - foreach (var symbol in symbols) + // In the case of declaration nodes that can have more than one symbol e.g. fields and events, + // the attributes are shared between then. Given this, we only need to inspect the first symbol + // of the node. + var symbol = SemanticFacts + .GetDeclaredSymbols(semanticModel, node, cancellationToken) + .FirstOrDefault(); + + // If we somehow do not have a symbol, we can't do anything. Otherwise, check if our symbol is + // a partial definition. If it is, skip it in favor of checking the implementation. + if (symbol is null or + IMethodSymbol { IsPartialDefinition: true } or + IPropertySymbol { IsPartialDefinition: true }) { - switch (symbol?.Kind) - { - // Local SuppressMessageAttributes are only applicable for types and members. - case SymbolKind.NamedType: - case SymbolKind.Method: - case SymbolKind.Field: - case SymbolKind.Property: - case SymbolKind.Event: - break; - - default: - continue; - } + continue; + } - // Skip already processed symbols from partial declarations - var isPartial = symbol.Locations.Length > 1; - if (isPartial && !processedPartialSymbols.Add(symbol)) - { + switch (symbol?.Kind) + { + // Local SuppressMessageAttributes are only applicable for types and members. + case SymbolKind.NamedType: + case SymbolKind.Method: + case SymbolKind.Field: + case SymbolKind.Property: + case SymbolKind.Event: + break; + + default: continue; - } + } + + // Skip already processed symbols from partial declarations + var isPartial = symbol.Locations.Length > 1; - foreach (var attribute in symbol.GetAttributes()) + if (isPartial && !processedPartialSymbols.Add(symbol)) + { + continue; + } + + foreach (var attribute in symbol.GetAttributes()) + { + if (attribute.ApplicationSyntaxReference != null && + TryGetSuppressedDiagnosticId(attribute, suppressMessageAttributeType, out var id, out var category)) { - if (attribute.ApplicationSyntaxReference != null && - TryGetSuppressedDiagnosticId(attribute, suppressMessageAttributeType, out var id, out var category)) + // Ignore unsupported IDs and those excluded through user option. + if (!IsSupportedAnalyzerDiagnosticId(id) || + userIdExclusions.Contains(id, StringComparer.OrdinalIgnoreCase) || + category?.Length > 0 && userCategoryExclusions.Contains(category, StringComparer.OrdinalIgnoreCase)) + { + continue; + } + + if (!idToSuppressMessageAttributesMap.TryGetValue(id, out var nodesForId)) { - // Ignore unsupported IDs and those excluded through user option. - if (!IsSupportedAnalyzerDiagnosticId(id) || - userIdExclusions.Contains(id, StringComparer.OrdinalIgnoreCase) || - category?.Length > 0 && userCategoryExclusions.Contains(category, StringComparer.OrdinalIgnoreCase)) - { - continue; - } - - if (!idToSuppressMessageAttributesMap.TryGetValue(id, out var nodesForId)) - { - nodesForId = []; - idToSuppressMessageAttributesMap.Add(id, nodesForId); - } - - var attributeNode = await attribute.ApplicationSyntaxReference.GetSyntaxAsync(cancellationToken).ConfigureAwait(false); - nodesForId.Add(attributeNode); - - // Initialize the attribute node as unnecessary at the start of the algorithm. - // Later processing will identify attributes which are indeed responsible for suppressing diagnostics - // and mark them as used. - // NOTE: For attributes on partial symbols with multiple declarations, we conservatively - // consider them as used and avoid unnecessary attribute analysis because that would potentially - // require analysis across multiple files, which can be expensive from a performance standpoint. - suppressMessageAttributesToIsUsedMap.Add(attributeNode, isPartial); + nodesForId = []; + idToSuppressMessageAttributesMap.Add(id, nodesForId); } + + var attributeNode = await attribute.ApplicationSyntaxReference.GetSyntaxAsync(cancellationToken).ConfigureAwait(false); + nodesForId.Add(attributeNode); + + // Initialize the attribute node as unnecessary at the start of the algorithm. + // Later processing will identify attributes which are indeed responsible for suppressing diagnostics + // and mark them as used. + // NOTE: For attributes on partial symbols with multiple declarations, we conservatively + // consider them as used and avoid unnecessary attribute analysis because that would potentially + // require analysis across multiple files, which can be expensive from a performance standpoint. + suppressMessageAttributesToIsUsedMap.Add(attributeNode, isPartial); } } + + // Individual variables within a variable declaration cannot be decorated with distinct attributes, so we + // should avoid looking at any of the subsequent symbols for this node as they will be the same. } } diff --git a/src/Features/CSharpTest/Diagnostics/Suppression/RemoveUnnecessaryPragmaSuppressionsTests.cs b/src/Features/CSharpTest/Diagnostics/Suppression/RemoveUnnecessaryPragmaSuppressionsTests.cs index ae41ca3c4801f..3271d0b4b302e 100644 --- a/src/Features/CSharpTest/Diagnostics/Suppression/RemoveUnnecessaryPragmaSuppressionsTests.cs +++ b/src/Features/CSharpTest/Diagnostics/Suppression/RemoveUnnecessaryPragmaSuppressionsTests.cs @@ -461,6 +461,156 @@ void M() |]", new TestParameters(options: options)); } + [Theory] + [InlineData("event", "EventHandler")] + [InlineData("static", "int")] + [WorkItem("https://github.com/dotnet/roslyn/issues/78786")] + public async Task TestRemoveDiagnosticSuppression_Attribute_MultiVariableDeclaration(string keyword, string type) + { + await TestInRegularAndScript1Async( + $$""" + public class C + { + [|[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1720:Identifier contains type name", Justification = "")]|] + public {{keyword}} {{type}} A, B; + } + """, + $$""" + public class C + { + public {{keyword}} {{type}} A, B; + } + """); + } + + [Fact] + [WorkItem("https://github.com/dotnet/roslyn/issues/78786")] + public async Task TestRemoveDiagnosticSuppression_Attribute_PartialMethodDefinition() + { + await TestInRegularAndScript1Async( + """ + public partial class C + { + [|[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1720:Identifier contains type name", Justification = "")]|] + partial void M(); + } + + public partial class C + { + partial void M() + { + } + } + """, + """ + public partial class C + { + partial void M(); + } + + public partial class C + { + partial void M() + { + } + } + """); + } + + [Fact] + [WorkItem("https://github.com/dotnet/roslyn/issues/78786")] + public async Task TestRemoveDiagnosticSuppression_Attribute_PartialMethodImplementation() + { + await TestInRegularAndScript1Async( + """ + public partial class C + { + partial void M(); + } + + public partial class C + { + [|[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1720:Identifier contains type name", Justification = "")]|] + partial void M() + { + } + } + """, + """ + public partial class C + { + partial void M(); + } + + public partial class C + { + partial void M() + { + } + } + """); + } + + [Fact] + [WorkItem("https://github.com/dotnet/roslyn/issues/78786")] + public async Task TestRemoveDiagnosticSuppression_Attribute_PartialPropertyDefinition() + { + await TestInRegularAndScript1Async( + """ + public partial class C + { + [|[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1720:Identifier contains type name", Justification = "")]|] + partial int P { get; } + } + + public partial class C + { + partial int P => 5230; + } + """, + """ + public partial class C + { + partial int P { get; } + } + + public partial class C + { + partial int P => 5230; + } + """); + } + + [Fact] + [WorkItem("https://github.com/dotnet/roslyn/issues/78786")] + public async Task TestRemoveDiagnosticSuppression_Attribute_PartialPropertyImplementation() + { + await TestInRegularAndScript1Async( + """ + public partial class C + { + partial int P { get; } + } + + public partial class C + { + [|[System.Diagnostics.CodeAnalysis.SuppressMessage("Naming", "CA1720:Identifier contains type name", Justification = "")]|] + partial int P => 5230; + } + """, + """ + public partial class C + { + partial int P { get; } + } + + public partial class C + { + partial int P => 5230; + } + """); + } + [Fact] public async Task TestDoNotRemoveDiagnosticSuppression_Attribute_OnPartialDeclarations() {