From c8195292820a406dcf480752d894b14bed506105 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 10:15:46 -0700 Subject: [PATCH 01/15] In progress --- ...ctRemoveUnusedMembersDiagnosticAnalyzer.cs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs index 7dcd15beb0c86..7ce575132f49a 100644 --- a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs @@ -45,7 +45,7 @@ internal abstract class AbstractRemoveUnusedMembersDiagnosticAnalyzer< EnforceOnBuildValues.RemoveUnusedMembers, new LocalizableResourceString(nameof(AnalyzersResources.Remove_unused_private_members), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)), new LocalizableResourceString(nameof(AnalyzersResources.Private_member_0_is_unused), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)), - hasAnyCodeStyleOption: false, isUnnecessary: true); + hasAnyCodeStyleOption: false, isUnnecessary: false); // IDE0052: "Remove unread members" (Value is written and/or symbol is referenced, but the assigned value is never read) // Internal for testing @@ -54,7 +54,7 @@ internal abstract class AbstractRemoveUnusedMembersDiagnosticAnalyzer< EnforceOnBuildValues.RemoveUnreadMembers, new LocalizableResourceString(nameof(AnalyzersResources.Remove_unread_private_members), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)), new LocalizableResourceString(nameof(AnalyzersResources.Private_member_0_can_be_removed_as_the_value_assigned_to_it_is_never_read), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)), - hasAnyCodeStyleOption: false, isUnnecessary: true); + hasAnyCodeStyleOption: false, isUnnecessary: false); protected AbstractRemoveUnusedMembersDiagnosticAnalyzer() : base([s_removeUnusedMembersRule, s_removeUnreadMembersRule], @@ -509,17 +509,22 @@ member is IPropertySymbol property && continue; } + var diagnosticLocation = GetDiagnosticLocation(member); + var fadingLocation = member.DeclaringSyntaxReferences.FirstOrDefault( + r => r.SyntaxTree == diagnosticLocation.SourceTree && r.Span.Contains(diagnosticLocation.SourceSpan)); + + // Most of the members should have a single location, except for partial methods. // We report the diagnostic on the first location of the member. - var diagnostic = DiagnosticHelper.CreateWithMessage( + symbolEndContext.ReportDiagnostic(DiagnosticHelper.CreateWithLocationTags( rule, - GetDiagnosticLocation(member), + diagnosticLocation, NotificationOption2.ForSeverity(rule.DefaultSeverity), symbolEndContext.Options, - additionalLocations: null, + additionalLocations: default, + additionalUnnecessaryLocations: default, properties: null, - GetMessage(rule, member)); - symbolEndContext.ReportDiagnostic(diagnostic); + GetMessage(rule, member))); } } } From 805992c7768e81dc2735c7ff5b4ef0b315e39772 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 10:30:24 -0700 Subject: [PATCH 02/15] Properly gray out --- ...harpRemoveUnusedMembersDiagnosticAnalyzer.cs | 17 ++++++++++++++++- ...ractRemoveUnusedMembersDiagnosticAnalyzer.cs | 12 ++++++++---- ...asicRemoveUnusedMembersDiagnosticAnalyzer.vb | 12 ++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/RemoveUnusedMembers/CSharpRemoveUnusedMembersDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/RemoveUnusedMembers/CSharpRemoveUnusedMembersDiagnosticAnalyzer.cs index 2bb616b174829..55ed47da767b2 100644 --- a/src/Analyzers/CSharp/Analyzers/RemoveUnusedMembers/CSharpRemoveUnusedMembersDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/RemoveUnusedMembers/CSharpRemoveUnusedMembersDiagnosticAnalyzer.cs @@ -8,11 +8,12 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.RemoveUnusedMembers; +using Microsoft.CodeAnalysis.Shared.Extensions; namespace Microsoft.CodeAnalysis.CSharp.RemoveUnusedMembers; [DiagnosticAnalyzer(LanguageNames.CSharp)] -internal class CSharpRemoveUnusedMembersDiagnosticAnalyzer +internal sealed class CSharpRemoveUnusedMembersDiagnosticAnalyzer : AbstractRemoveUnusedMembersDiagnosticAnalyzer< DocumentationCommentTriviaSyntax, IdentifierNameSyntax, @@ -28,4 +29,18 @@ protected override IEnumerable GetTypeDeclarations(INamed protected override SyntaxList GetMembers(TypeDeclarationSyntax typeDeclaration) => typeDeclaration.Members; + + protected override SyntaxNode GetParentIfSoleDeclarator(SyntaxNode node) + { + return node switch + { + VariableDeclaratorSyntax variableDeclarator + => variableDeclarator.Parent is VariableDeclarationSyntax + { + Parent: FieldDeclarationSyntax { Declaration.Variables.Count: 0 } or + EventFieldDeclarationSyntax { Declaration.Variables.Count: 0 } + } declaration ? declaration.GetRequiredParent() : node, + _ => node, + }; + } } diff --git a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs index 7ce575132f49a..69d4986c17ed9 100644 --- a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs @@ -64,6 +64,7 @@ protected AbstractRemoveUnusedMembersDiagnosticAnalyzer() protected abstract IEnumerable GetTypeDeclarations(INamedTypeSymbol namedType, CancellationToken cancellationToken); protected abstract SyntaxList GetMembers(TTypeDeclarationSyntax typeDeclaration); + protected abstract SyntaxNode GetParentIfSoleDeclarator(SyntaxNode declaration); // We need to analyze the whole document even for edits within a method body, // because we might add or remove references to members in executable code. @@ -428,6 +429,7 @@ private void AnalyzeObjectCreationOperation(OperationAnalysisContext operationCo private void OnSymbolEnd(SymbolAnalysisContext symbolEndContext, bool hasUnsupportedOperation) { + var cancellationToken = symbolEndContext.CancellationToken; if (hasUnsupportedOperation) { return; @@ -445,7 +447,7 @@ private void OnSymbolEnd(SymbolAnalysisContext symbolEndContext, bool hasUnsuppo using var _1 = PooledHashSet.GetInstance(out var symbolsReferencedInDocComments); using var _2 = ArrayBuilder.GetInstance(out var debuggerDisplayAttributeArguments); - var entryPoint = symbolEndContext.Compilation.GetEntryPoint(symbolEndContext.CancellationToken); + var entryPoint = symbolEndContext.Compilation.GetEntryPoint(cancellationToken); var namedType = (INamedTypeSymbol)symbolEndContext.Symbol; foreach (var member in namedType.GetMembers()) @@ -467,7 +469,7 @@ private void OnSymbolEnd(SymbolAnalysisContext symbolEndContext, bool hasUnsuppo { // Bail out if there are syntax errors in any of the declarations of the containing type. // Note that we check this only for the first time that we report an unused or unread member for the containing type. - if (HasSyntaxErrors(namedType, symbolEndContext.CancellationToken)) + if (HasSyntaxErrors(namedType, cancellationToken)) { return; } @@ -475,7 +477,7 @@ private void OnSymbolEnd(SymbolAnalysisContext symbolEndContext, bool hasUnsuppo // Compute the set of candidate symbols referenced in all the documentation comments within the named type declarations. // This set is computed once and used for all the iterations of the loop. AddCandidateSymbolsReferencedInDocComments( - namedType, symbolEndContext.Compilation, symbolsReferencedInDocComments, symbolEndContext.CancellationToken); + namedType, symbolEndContext.Compilation, symbolsReferencedInDocComments, cancellationToken); // Compute the set of string arguments to DebuggerDisplay attributes applied to any symbol within the named type declaration. // These strings may have an embedded reference to the symbol. @@ -513,6 +515,8 @@ member is IPropertySymbol property && var fadingLocation = member.DeclaringSyntaxReferences.FirstOrDefault( r => r.SyntaxTree == diagnosticLocation.SourceTree && r.Span.Contains(diagnosticLocation.SourceSpan)); + var fadingNode = fadingLocation?.GetSyntax(cancellationToken); + fadingNode = fadingNode != null ? this._analyzer.GetParentIfSoleDeclarator(fadingNode) : null; // Most of the members should have a single location, except for partial methods. // We report the diagnostic on the first location of the member. @@ -522,7 +526,7 @@ member is IPropertySymbol property && NotificationOption2.ForSeverity(rule.DefaultSeverity), symbolEndContext.Options, additionalLocations: default, - additionalUnnecessaryLocations: default, + additionalUnnecessaryLocations: fadingNode is null ? [] : [fadingNode.GetLocation()], properties: null, GetMessage(rule, member))); } diff --git a/src/Analyzers/VisualBasic/Analyzers/RemoveUnusedMembers/VisualBasicRemoveUnusedMembersDiagnosticAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/RemoveUnusedMembers/VisualBasicRemoveUnusedMembersDiagnosticAnalyzer.vb index e5a7cff41e3c1..21a7da1fd0061 100644 --- a/src/Analyzers/VisualBasic/Analyzers/RemoveUnusedMembers/VisualBasicRemoveUnusedMembersDiagnosticAnalyzer.vb +++ b/src/Analyzers/VisualBasic/Analyzers/RemoveUnusedMembers/VisualBasicRemoveUnusedMembersDiagnosticAnalyzer.vb @@ -59,5 +59,17 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnusedMembers Protected Overrides Function GetMembers(typeDeclaration As TypeBlockSyntax) As SyntaxList(Of StatementSyntax) Return typeDeclaration.Members End Function + + Protected Overrides Function GetParentIfSoleDeclarator(node As SyntaxNode) As SyntaxNode + Dim modifiedIdentifier = TryCast(node, ModifiedIdentifierSyntax) + Dim declarator = TryCast(modifiedIdentifier?.Parent, VariableDeclaratorSyntax) + Dim field = TryCast(declarator?.Parent, FieldDeclarationSyntax) + + If declarator?.Names.Count = 1 AndAlso field?.Declarators.Count = 1 Then + Return field + End If + + Return node + End Function End Class End Namespace From bab626355225f55b0eb4cd4e243e6183a7c35063 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 10:33:49 -0700 Subject: [PATCH 03/15] fix --- .../AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs index 69d4986c17ed9..ae45da7159327 100644 --- a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs @@ -525,7 +525,7 @@ member is IPropertySymbol property && diagnosticLocation, NotificationOption2.ForSeverity(rule.DefaultSeverity), symbolEndContext.Options, - additionalLocations: default, + additionalLocations: [], additionalUnnecessaryLocations: fadingNode is null ? [] : [fadingNode.GetLocation()], properties: null, GetMessage(rule, member))); From 6dbaa4e815223f60dc21f20830526867df7f4a7d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 10:43:54 -0700 Subject: [PATCH 04/15] Share code --- .../Analyzers/Helpers/DiagnosticHelper.cs | 97 +++++++++++-------- ...ctRemoveUnusedMembersDiagnosticAnalyzer.cs | 4 +- 2 files changed, 59 insertions(+), 42 deletions(-) diff --git a/src/Analyzers/Core/Analyzers/Helpers/DiagnosticHelper.cs b/src/Analyzers/Core/Analyzers/Helpers/DiagnosticHelper.cs index 151a02031558b..e108c76b24797 100644 --- a/src/Analyzers/Core/Analyzers/Helpers/DiagnosticHelper.cs +++ b/src/Analyzers/Core/Analyzers/Helpers/DiagnosticHelper.cs @@ -47,21 +47,22 @@ public static Diagnostic Create( params object[] messageArgs) { if (descriptor == null) - { throw new ArgumentNullException(nameof(descriptor)); - } - LocalizableString message; + var message = CreateMessage(descriptor, messageArgs); + return CreateWithMessage(descriptor, location, notificationOption, analyzerOptions, additionalLocations, properties, message); + } + + private static LocalizableString CreateMessage(DiagnosticDescriptor descriptor, object[] messageArgs) + { if (messageArgs == null || messageArgs.Length == 0) { - message = descriptor.MessageFormat; + return descriptor.MessageFormat; } else { - message = new LocalizableStringWithArguments(descriptor.MessageFormat, messageArgs); + return new LocalizableStringWithArguments(descriptor.MessageFormat, messageArgs); } - - return CreateWithMessage(descriptor, location, notificationOption, analyzerOptions, additionalLocations, properties, message); } /// @@ -92,11 +93,28 @@ public static Diagnostic CreateWithLocationTags( ImmutableArray additionalLocations, ImmutableArray additionalUnnecessaryLocations, params object[] messageArgs) + { + return CreateWithLocationTags( + descriptor, + location, + notificationOption, + analyzerOptions, + CreateMessage(descriptor, messageArgs), + additionalLocations, + additionalUnnecessaryLocations); + } + + private static Diagnostic CreateWithLocationTags( + DiagnosticDescriptor descriptor, + Location location, + NotificationOption2 notificationOption, + AnalyzerOptions analyzerOptions, + LocalizableString message, + ImmutableArray additionalLocations, + ImmutableArray additionalUnnecessaryLocations) { if (additionalUnnecessaryLocations.IsEmpty) - { - return Create(descriptor, location, notificationOption, analyzerOptions, additionalLocations, ImmutableDictionary.Empty, messageArgs); - } + return CreateWithMessage(descriptor, location, notificationOption, analyzerOptions, additionalLocations, ImmutableDictionary.Empty, message); var tagIndices = ImmutableDictionary>.Empty .Add(WellKnownDiagnosticTags.Unnecessary, Enumerable.Range(additionalLocations.Length, additionalUnnecessaryLocations.Length)); @@ -105,10 +123,10 @@ public static Diagnostic CreateWithLocationTags( location, notificationOption, analyzerOptions, + message, additionalLocations.AddRange(additionalUnnecessaryLocations), tagIndices, - ImmutableDictionary.Empty, - messageArgs); + ImmutableDictionary.Empty); } /// @@ -144,11 +162,30 @@ public static Diagnostic CreateWithLocationTags( ImmutableArray additionalUnnecessaryLocations, ImmutableDictionary? properties, params object[] messageArgs) + { + return CreateWithLocationTags( + descriptor, + location, + notificationOption, + analyzerOptions, + CreateMessage(descriptor, messageArgs), + additionalLocations, + additionalUnnecessaryLocations, + properties); + } + + public static Diagnostic CreateWithLocationTags( + DiagnosticDescriptor descriptor, + Location location, + NotificationOption2 notificationOption, + AnalyzerOptions analyzerOptions, + LocalizableString message, + ImmutableArray additionalLocations, + ImmutableArray additionalUnnecessaryLocations, + ImmutableDictionary? properties) { if (additionalUnnecessaryLocations.IsEmpty) - { - return Create(descriptor, location, notificationOption, analyzerOptions, additionalLocations, properties, messageArgs); - } + return CreateWithMessage(descriptor, location, notificationOption, analyzerOptions, additionalLocations, properties, message); var tagIndices = ImmutableDictionary>.Empty .Add(WellKnownDiagnosticTags.Unnecessary, Enumerable.Range(additionalLocations.Length, additionalUnnecessaryLocations.Length)); @@ -157,41 +194,21 @@ public static Diagnostic CreateWithLocationTags( location, notificationOption, analyzerOptions, + message, additionalLocations.AddRange(additionalUnnecessaryLocations), tagIndices, - properties, - messageArgs); + properties); } - /// - /// Create a diagnostic that adds properties specifying a tag for a set of locations. - /// - /// A describing the diagnostic. - /// An optional primary location of the diagnostic. If null, will return . - /// Notification option for the diagnostic. - /// - /// An optional set of additional locations related to the diagnostic. - /// Typically, these are locations of other items referenced in the message. - /// - /// - /// a map of location tag to index in additional locations. - /// "AbstractRemoveUnnecessaryParenthesesDiagnosticAnalyzer" for an example of usage. - /// - /// - /// An optional set of name-value pairs by means of which the analyzer that creates the diagnostic - /// can convey more detailed information to the fixer. - /// - /// Arguments to the message of the diagnostic. - /// The instance. private static Diagnostic CreateWithLocationTags( DiagnosticDescriptor descriptor, Location location, NotificationOption2 notificationOption, AnalyzerOptions analyzerOptions, + LocalizableString message, IEnumerable additionalLocations, IDictionary> tagIndices, - ImmutableDictionary? properties, - params object[] messageArgs) + ImmutableDictionary? properties) { Contract.ThrowIfTrue(additionalLocations.IsEmpty()); Contract.ThrowIfTrue(tagIndices.IsEmpty()); @@ -199,7 +216,7 @@ private static Diagnostic CreateWithLocationTags( properties ??= ImmutableDictionary.Empty; properties = properties.AddRange(tagIndices.Select(kvp => new KeyValuePair(kvp.Key, EncodeIndices(kvp.Value, additionalLocations.Count())))); - return Create(descriptor, location, notificationOption, analyzerOptions, additionalLocations, properties, messageArgs); + return CreateWithMessage(descriptor, location, notificationOption, analyzerOptions, additionalLocations, properties, message); static string EncodeIndices(IEnumerable indices, int additionalLocationsLength) { diff --git a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs index ae45da7159327..0546f37968438 100644 --- a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs @@ -525,10 +525,10 @@ member is IPropertySymbol property && diagnosticLocation, NotificationOption2.ForSeverity(rule.DefaultSeverity), symbolEndContext.Options, + message: GetMessage(rule, member), additionalLocations: [], additionalUnnecessaryLocations: fadingNode is null ? [] : [fadingNode.GetLocation()], - properties: null, - GetMessage(rule, member))); + properties: null)); } } } From 6a91190b6bd2acb615bd35280451b7e5408aef5b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 10:51:58 -0700 Subject: [PATCH 05/15] Tests --- .../Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs | 8 ++++---- .../AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs index 2bcf767b9dfeb..eab8c99b9d395 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs @@ -1745,7 +1745,7 @@ class MyClass } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] - public async Task PropertyIsIncrementedAndValueDropped_VerifyAnalizerMessage() + public async Task PropertyIsIncrementedAndValueDropped_VerifyAnalyzerMessage() { var code = """ class MyClass @@ -1805,7 +1805,7 @@ class MyClass } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] - public async Task IndexerIsIncrementedAndValueDropped_VerifyAnalizerMessage() + public async Task IndexerIsIncrementedAndValueDropped_VerifyAnalyzerMessage() { var code = """ class MyClass @@ -3176,8 +3176,8 @@ class C private C(int i) { } } """, -// /0/Test0.cs(3,13): info IDE0051: Private member 'C.C' is unused -VerifyCS.Diagnostic("IDE0051").WithSpan(3, 13, 3, 14).WithArguments("C.C")); + // /0/Test0.cs(3,13): info IDE0051: Private member 'C.C' is unused + VerifyCS.Diagnostic("IDE0051").WithSpan(3, 5, 3, 25).WithArguments("C.C")); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/62856")] diff --git a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs index 0546f37968438..2cfeef8920cc2 100644 --- a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs @@ -515,7 +515,7 @@ member is IPropertySymbol property && var fadingLocation = member.DeclaringSyntaxReferences.FirstOrDefault( r => r.SyntaxTree == diagnosticLocation.SourceTree && r.Span.Contains(diagnosticLocation.SourceSpan)); - var fadingNode = fadingLocation?.GetSyntax(cancellationToken); + var fadingNode = fadingLocation?.GetSyntax(cancellationToken) ?? diagnosticLocation.FindNode(cancellationToken); fadingNode = fadingNode != null ? this._analyzer.GetParentIfSoleDeclarator(fadingNode) : null; // Most of the members should have a single location, except for partial methods. From 0c143a889d5605977adfa37b0a8d1e53a060497a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 12:21:02 -0700 Subject: [PATCH 06/15] in progress --- .../RemoveUnusedMembersTests.cs | 54 ++++++------------- ...ctRemoveUnusedMembersDiagnosticAnalyzer.cs | 16 ++++-- .../CodeActions/CSharpCodeFixVerifier`2.cs | 4 +- 3 files changed, 32 insertions(+), 42 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs index eab8c99b9d395..519cef4d023c4 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs @@ -1466,7 +1466,7 @@ public async Task PropertyIsOnlyWritten() """ class MyClass { - private int {|#0:P|} { get; set; } + private int {|IDE0052:P|} { get; set; } public void M() { P = 0; @@ -1747,18 +1747,13 @@ class MyClass [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] public async Task PropertyIsIncrementedAndValueDropped_VerifyAnalyzerMessage() { - var code = """ + await VerifyCS.VerifyAnalyzerAsync(""" class MyClass { - private int P { get; set; } + private int {|IDE0052:P|} { get; set; } public void M1() { ++P; } } - """; - - await VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult( - CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) - .WithSpan(3, 17, 3, 18) - .WithArguments("MyClass.P")); + """); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] @@ -1807,18 +1802,13 @@ class MyClass [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] public async Task IndexerIsIncrementedAndValueDropped_VerifyAnalyzerMessage() { - var code = """ + await VerifyCS.VerifyAnalyzerAsync(""" class MyClass { - private int this[int x] { get { return 0; } set { } } + private int {|IDE0052:this|}[int x] { get { return 0; } set { } } public void M1(int x) => ++this[x]; } - """; - - await VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult( - CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) - .WithSpan(3, 17, 3, 21) - .WithArguments("MyClass.this")); + """); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] @@ -1921,20 +1911,15 @@ class MyClass } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] - public async Task PropertyIsTargetOfCompoundAssignmentAndValueDropped_VerifyAnalizerMessage() + public async Task PropertyIsTargetOfCompoundAssignmentAndValueDropped_VerifyAnalyzerMessage() { - var code = """ + await VerifyCS.VerifyAnalyzerAsync(""" class MyClass { - private int P { get; set; } + private int {|IDE0052:P|} { get; set; } public void M1(int x) { P += x; } } - """; - - await VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult( - CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) - .WithSpan(3, 17, 3, 18) - .WithArguments("MyClass.P")); + """); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] @@ -1983,18 +1968,13 @@ class MyClass [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] public async Task IndexerIsTargetOfCompoundAssignmentAndValueDropped_VerifyAnalyzerMessage() { - var code = """ + await VerifyCS.VerifyAnalyzerAsync(""" class MyClass { - private int this[int x] { get { return 0; } set { } } + private int {|IDE0052:this|}[int x] { get { return 0; } set { } } public void M1(int x, int y) => this[x] += y; } - """; - - await VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult( - CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) - .WithSpan(3, 17, 3, 21) - .WithArguments("MyClass.this")); + """); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] @@ -3173,11 +3153,9 @@ await VerifyCS.VerifyAnalyzerAsync( """ class C { - private C(int i) { } + private [|C|](int i) { } } - """, - // /0/Test0.cs(3,13): info IDE0051: Private member 'C.C' is unused - VerifyCS.Diagnostic("IDE0051").WithSpan(3, 5, 3, 25).WithArguments("C.C")); + """); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/62856")] diff --git a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs index 2cfeef8920cc2..0217666be99fd 100644 --- a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs @@ -511,6 +511,12 @@ member is IPropertySymbol property && continue; } + // We change the message only if both 'get' and 'set' accessors are present and + // there are no shadow 'get' accessor usages. Otherwise the message will be confusing + var isConvertibleProperty = + member is IPropertySymbol { GetMethod: not null, SetMethod: not null } property2 && + !_propertiesWithShadowGetAccessorUsages.Contains(property2); + var diagnosticLocation = GetDiagnosticLocation(member); var fadingLocation = member.DeclaringSyntaxReferences.FirstOrDefault( r => r.SyntaxTree == diagnosticLocation.SourceTree && r.Span.Contains(diagnosticLocation.SourceSpan)); @@ -518,6 +524,10 @@ member is IPropertySymbol property && var fadingNode = fadingLocation?.GetSyntax(cancellationToken) ?? diagnosticLocation.FindNode(cancellationToken); fadingNode = fadingNode != null ? this._analyzer.GetParentIfSoleDeclarator(fadingNode) : null; + var additionalUnnecessaryLocations = !isConvertibleProperty && fadingNode is not null + ? [fadingNode.GetLocation()] + : ImmutableArray.Empty; + // Most of the members should have a single location, except for partial methods. // We report the diagnostic on the first location of the member. symbolEndContext.ReportDiagnostic(DiagnosticHelper.CreateWithLocationTags( @@ -525,9 +535,9 @@ member is IPropertySymbol property && diagnosticLocation, NotificationOption2.ForSeverity(rule.DefaultSeverity), symbolEndContext.Options, - message: GetMessage(rule, member), + message: GetMessage(rule, member, isConvertibleProperty), additionalLocations: [], - additionalUnnecessaryLocations: fadingNode is null ? [] : [fadingNode.GetLocation()], + additionalUnnecessaryLocations: additionalUnnecessaryLocations, properties: null)); } } @@ -547,7 +557,7 @@ private LocalizableString GetMessage( messageFormat = AnalyzersResources.Private_method_0_can_be_removed_as_it_is_never_invoked; break; - case IPropertySymbol property: + case IPropertySymbol property when isConvertibleProperty: // We change the message only if both 'get' and 'set' accessors are present and // there are no shadow 'get' accessor usages. Otherwise the message will be confusing if (property.GetMethod != null && property.SetMethod != null && diff --git a/src/Features/DiagnosticsTestUtilities/CodeActions/CSharpCodeFixVerifier`2.cs b/src/Features/DiagnosticsTestUtilities/CodeActions/CSharpCodeFixVerifier`2.cs index f84a43aa103a1..5c9637bd4b190 100644 --- a/src/Features/DiagnosticsTestUtilities/CodeActions/CSharpCodeFixVerifier`2.cs +++ b/src/Features/DiagnosticsTestUtilities/CodeActions/CSharpCodeFixVerifier`2.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Diagnostics.CodeAnalysis; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Testing; @@ -36,7 +37,8 @@ public static void VerifyStandardProperty(AnalyzerProperty property) => CodeFixVerifierHelper.VerifyStandardProperty(new TAnalyzer(), property); /// - public static async Task VerifyAnalyzerAsync(string source, params DiagnosticResult[] expected) + public static async Task VerifyAnalyzerAsync( + [StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string source, params DiagnosticResult[] expected) { var test = new Test { From dd8401255ca8bc503116fb87e63b5b48e0301e25 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 12:28:31 -0700 Subject: [PATCH 07/15] tests --- .../RemoveUnusedMembersTests.cs | 2 +- ...ctRemoveUnusedMembersDiagnosticAnalyzer.cs | 31 +++++++++---------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs index 519cef4d023c4..7ed74c31af35b 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs @@ -1466,7 +1466,7 @@ public async Task PropertyIsOnlyWritten() """ class MyClass { - private int {|IDE0052:P|} { get; set; } + private int {|#0:P|} { get; set; } public void M() { P = 0; diff --git a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs index 0217666be99fd..3b9ab552c14f2 100644 --- a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs @@ -543,35 +543,32 @@ member is IPropertySymbol property && } } - private LocalizableString GetMessage( + private static LocalizableString GetMessage( DiagnosticDescriptor rule, - ISymbol member) + ISymbol member, + bool isConvertibleProperty) { - var messageFormat = rule.MessageFormat; + var memberString = member.ToDisplayString(ContainingTypeAndNameOnlyFormat); + if (rule == s_removeUnreadMembersRule) { // IDE0052 has a different message for method and property symbols. switch (member) { case IMethodSymbol: - messageFormat = AnalyzersResources.Private_method_0_can_be_removed_as_it_is_never_invoked; - break; - - case IPropertySymbol property when isConvertibleProperty: - // We change the message only if both 'get' and 'set' accessors are present and - // there are no shadow 'get' accessor usages. Otherwise the message will be confusing - if (property.GetMethod != null && property.SetMethod != null && - !_propertiesWithShadowGetAccessorUsages.Contains(property)) - { - messageFormat = AnalyzersResources.Private_property_0_can_be_converted_to_a_method_as_its_get_accessor_is_never_invoked; - } - - break; + return new DiagnosticHelper.LocalizableStringWithArguments( + AnalyzersResources.Private_method_0_can_be_removed_as_it_is_never_invoked, + memberString); + + case IPropertySymbol when isConvertibleProperty: + return new DiagnosticHelper.LocalizableStringWithArguments( + AnalyzersResources.Private_property_0_can_be_converted_to_a_method_as_its_get_accessor_is_never_invoked, + memberString); } } return new DiagnosticHelper.LocalizableStringWithArguments( - messageFormat, member.ToDisplayString(ContainingTypeAndNameOnlyFormat)); + rule.MessageFormat, memberString); } private static bool HasSyntaxErrors(INamedTypeSymbol namedTypeSymbol, CancellationToken cancellationToken) From 2d8a12c60280b8dae7b32f83fc12fe28cf5d00a7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 12:29:31 -0700 Subject: [PATCH 08/15] tests --- .../RemoveUnusedMembersTests.cs | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs index 7ed74c31af35b..ace1e150d7871 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; using System.Linq; using System.Threading.Tasks; using Microsoft.CodeAnalysis.CSharp; @@ -1462,28 +1461,25 @@ public void M() [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/33994")] public async Task PropertyIsOnlyWritten() { - var source = - """ - class MyClass - { - private int {|#0:P|} { get; set; } - public void M() - { - P = 0; - } - } - """; - - var descriptor = new CSharpRemoveUnusedMembersDiagnosticAnalyzer().SupportedDiagnostics.First(x => x.Id == "IDE0052"); - var expectedMessage = string.Format(AnalyzersResources.Private_property_0_can_be_converted_to_a_method_as_its_get_accessor_is_never_invoked, "MyClass.P"); - await new VerifyCS.Test { - TestCode = source, + TestCode = """ + class MyClass + { + private int {|#0:P|} { get; set; } + public void M() + { + P = 0; + } + } + """, ExpectedDiagnostics = { // Test0.cs(3,17): info IDE0052: Private property 'MyClass.P' can be converted to a method as its get accessor is never invoked. - VerifyCS.Diagnostic(descriptor).WithMessage(expectedMessage).WithLocation(0), + VerifyCS + .Diagnostic(new CSharpRemoveUnusedMembersDiagnosticAnalyzer().SupportedDiagnostics.First(x => x.Id == "IDE0052")) + .WithMessage(string.Format(AnalyzersResources.Private_property_0_can_be_converted_to_a_method_as_its_get_accessor_is_never_invoked, "MyClass.P")) + .WithLocation(0), }, FixedCode = source, }.RunAsync(); From 4f782a0a7831dc913bb26f9c327b3ffebf257c99 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 12:32:52 -0700 Subject: [PATCH 09/15] tests --- .../RemoveUnusedMembersTests.cs | 75 ++++++++++--------- .../CodeActions/CSharpCodeFixVerifier`2.cs | 4 +- 2 files changed, 43 insertions(+), 36 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs index ace1e150d7871..b09ce23112b7d 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs @@ -29,19 +29,20 @@ public void TestStandardProperty(AnalyzerProperty property) [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/31582")] public async Task FieldReadViaSuppression() { - var code = """ - #nullable enable - class MyClass - { - string? _field = null; - public void M() + await new VerifyCS.Test + { + TestCode = """ + #nullable enable + class MyClass { - _field!.ToString(); + string? _field = null; + public void M() + { + _field!.ToString(); + } } - } - """; - - await VerifyCS.VerifyCodeFixAsync(code, code); + """ + }.RunAsync(); } [Theory] @@ -52,14 +53,16 @@ public void M() [InlineData("private protected")] public async Task NonPrivateField(string accessibility) { - var code = $$""" - class MyClass - { - {{accessibility}} int _goo; - } - """; + await new VerifyCS.Test + { + TestCode = $$""" + class MyClass + { + {{accessibility}} int _goo; + } + """, + }.RunAsync(); - await VerifyCS.VerifyCodeFixAsync(code, code); } [Theory] @@ -70,14 +73,15 @@ class MyClass [InlineData("private protected")] public async Task NonPrivateFieldWithConstantInitializer(string accessibility) { - var code = $$""" - class MyClass - { - {{accessibility}} int _goo = 0; - } - """; - - await VerifyCS.VerifyCodeFixAsync(code, code); + await new VerifyCS.Test + { + TestCode = $$""" + class MyClass + { + {{accessibility}} int _goo = 0; + } + """, + }.RunAsync(); } [Theory] @@ -88,15 +92,16 @@ class MyClass [InlineData("private protected")] public async Task NonPrivateFieldWithNonConstantInitializer(string accessibility) { - var code = $$""" - class MyClass - { - {{accessibility}} int _goo = _goo2; - private static readonly int _goo2 = 0; - } - """; - - await VerifyCS.VerifyCodeFixAsync(code, code); + await new VerifyCS.Test + { + TestCode = $$""" + class MyClass + { + {{accessibility}} int _goo = _goo2; + private static readonly int _goo2 = 0; + } + """, + }.RunAsync(); } [Theory] diff --git a/src/Features/DiagnosticsTestUtilities/CodeActions/CSharpCodeFixVerifier`2.cs b/src/Features/DiagnosticsTestUtilities/CodeActions/CSharpCodeFixVerifier`2.cs index 5c9637bd4b190..d534c9087c602 100644 --- a/src/Features/DiagnosticsTestUtilities/CodeActions/CSharpCodeFixVerifier`2.cs +++ b/src/Features/DiagnosticsTestUtilities/CodeActions/CSharpCodeFixVerifier`2.cs @@ -50,7 +50,9 @@ public static async Task VerifyAnalyzerAsync( } /// - public static async Task VerifyCodeFixAsync(string source, string fixedSource) + public static async Task VerifyCodeFixAsync( + [StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string source, + [StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] string fixedSource) => await VerifyCodeFixAsync(source, DiagnosticResult.EmptyDiagnosticResults, fixedSource); /// From 34b8b3192e69387827ee9d80638157f7a24015bc Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 12:39:41 -0700 Subject: [PATCH 10/15] tests --- .../RemoveUnusedMembersTests.cs | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs index b09ce23112b7d..7da6b79eb7924 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs @@ -15,6 +15,7 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.RemoveUnusedMembers; +using static Microsoft.CodeAnalysis.CSharp.UsePatternCombinators.AnalyzedPattern; using VerifyCS = CSharpCodeFixVerifier< CSharpRemoveUnusedMembersDiagnosticAnalyzer, CSharpRemoveUnusedMembersCodeFixProvider>; @@ -1486,7 +1487,6 @@ public void M() .WithMessage(string.Format(AnalyzersResources.Private_property_0_can_be_converted_to_a_method_as_its_get_accessor_is_never_invoked, "MyClass.P")) .WithLocation(0), }, - FixedCode = source, }.RunAsync(); } @@ -1748,13 +1748,24 @@ class MyClass [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] public async Task PropertyIsIncrementedAndValueDropped_VerifyAnalyzerMessage() { - await VerifyCS.VerifyAnalyzerAsync(""" - class MyClass + await new VerifyCS.Test + { + TestCode = """ + class MyClass + { + private int {|#0:P|} { get; set; } + public void M1() { ++P; } + } + """, + ExpectedDiagnostics = { - private int {|IDE0052:P|} { get; set; } - public void M1() { ++P; } - } - """); + // Test0.cs(3,17): info IDE0052: Private property 'MyClass.P' can be converted to a method as its get accessor is never invoked. + VerifyCS + .Diagnostic(new CSharpRemoveUnusedMembersDiagnosticAnalyzer().SupportedDiagnostics.First(x => x.Id == "IDE0052")) + .WithMessage(string.Format(AnalyzersResources.Private_member_0_can_be_removed_as_the_value_assigned_to_it_is_never_read, "MyClass.P")) + .WithLocation(0), + }, + }.RunAsync(); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] From cd7c5edec3ed651542cf82441758212500ddca04 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 12:45:07 -0700 Subject: [PATCH 11/15] update --- .../RemoveUnusedMembersTests.cs | 67 +++++++++++-------- 1 file changed, 39 insertions(+), 28 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs index 7da6b79eb7924..d6fc924d4f6f2 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs @@ -1748,24 +1748,18 @@ class MyClass [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] public async Task PropertyIsIncrementedAndValueDropped_VerifyAnalyzerMessage() { - await new VerifyCS.Test - { - TestCode = """ - class MyClass - { - private int {|#0:P|} { get; set; } - public void M1() { ++P; } - } - """, - ExpectedDiagnostics = + var code = """ + class MyClass { - // Test0.cs(3,17): info IDE0052: Private property 'MyClass.P' can be converted to a method as its get accessor is never invoked. - VerifyCS - .Diagnostic(new CSharpRemoveUnusedMembersDiagnosticAnalyzer().SupportedDiagnostics.First(x => x.Id == "IDE0052")) - .WithMessage(string.Format(AnalyzersResources.Private_member_0_can_be_removed_as_the_value_assigned_to_it_is_never_read, "MyClass.P")) - .WithLocation(0), - }, - }.RunAsync(); + private int {|#0:P|} { get; set; } + public void M1() { ++P; } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult( + CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) + .WithLocation(0) + .WithArguments("MyClass.P")); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] @@ -1814,13 +1808,18 @@ class MyClass [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] public async Task IndexerIsIncrementedAndValueDropped_VerifyAnalyzerMessage() { - await VerifyCS.VerifyAnalyzerAsync(""" + var code = """ class MyClass { - private int {|IDE0052:this|}[int x] { get { return 0; } set { } } + private int {|#0:this|}[int x] { get { return 0; } set { } } public void M1(int x) => ++this[x]; } - """); + """; + + await VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult( + CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) + .WithLocation(0) + .WithArguments("MyClass.this")); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] @@ -1925,13 +1924,18 @@ class MyClass [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] public async Task PropertyIsTargetOfCompoundAssignmentAndValueDropped_VerifyAnalyzerMessage() { - await VerifyCS.VerifyAnalyzerAsync(""" + var code = """ class MyClass { - private int {|IDE0052:P|} { get; set; } + private int {|#0:P|} { get; set; } public void M1(int x) { P += x; } } - """); + """; + + await VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult( + CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) + .WithLocation(0) + .WithArguments("MyClass.P")); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] @@ -1980,13 +1984,18 @@ class MyClass [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] public async Task IndexerIsTargetOfCompoundAssignmentAndValueDropped_VerifyAnalyzerMessage() { - await VerifyCS.VerifyAnalyzerAsync(""" + var code = """ class MyClass { - private int {|IDE0052:this|}[int x] { get { return 0; } set { } } + private int {|#0:this|}[int x] { get { return 0; } set { } } public void M1(int x, int y) => this[x] += y; } - """); + """; + + await VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult( + CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) + .WithLocation(0) + .WithArguments("MyClass.this")); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] @@ -3165,9 +3174,11 @@ await VerifyCS.VerifyAnalyzerAsync( """ class C { - private [|C|](int i) { } + private {|#0:C|}(int i) { } } - """); + """, + // /0/Test0.cs(3,13): info IDE0051: Private member 'C.C' is unused + VerifyCS.Diagnostic("IDE0051").WithLocation(0).WithArguments("C.C")); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/62856")] From 9cb5580922493840283d2ac16a2978de7daac549 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 12:50:03 -0700 Subject: [PATCH 12/15] trying other stuff --- .../RemoveUnusedMembersTests.cs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs index d6fc924d4f6f2..4537d7ca517a6 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs @@ -1808,18 +1808,16 @@ class MyClass [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] public async Task IndexerIsIncrementedAndValueDropped_VerifyAnalyzerMessage() { - var code = """ + await VerifyCS.VerifyAnalyzerAsync(""" class MyClass { - private int {|#0:this|}[int x] { get { return 0; } set { } } + [|private int {|#0:this|}[int x] { get { return 0; } set { } }|] public void M1(int x) => ++this[x]; } - """; - - await VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult( - CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) - .WithLocation(0) - .WithArguments("MyClass.this")); + """, new DiagnosticResult( + CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) + .WithLocation(0) + .WithArguments("MyClass.this")); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] From fd3edc17c10113850fac097e71a8f66974e2d1ad Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 12:57:40 -0700 Subject: [PATCH 13/15] fix tests --- .../RemoveUnusedMembersTests.cs | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs index 4537d7ca517a6..e7ed9594d69e0 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs @@ -1758,8 +1758,9 @@ class MyClass await VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult( CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) - .WithLocation(0) - .WithArguments("MyClass.P")); + .WithLocation(0) + .WithArguments("MyClass.P") + .WithOptions(DiagnosticOptions.IgnoreAdditionalLocations)); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] @@ -1811,13 +1812,14 @@ public async Task IndexerIsIncrementedAndValueDropped_VerifyAnalyzerMessage() await VerifyCS.VerifyAnalyzerAsync(""" class MyClass { - [|private int {|#0:this|}[int x] { get { return 0; } set { } }|] + private int {|#0:this|}[int x] { get { return 0; } set { } } public void M1(int x) => ++this[x]; } """, new DiagnosticResult( CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) .WithLocation(0) - .WithArguments("MyClass.this")); + .WithArguments("MyClass.this") + .WithOptions(DiagnosticOptions.IgnoreAdditionalLocations)); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] @@ -1932,8 +1934,9 @@ class MyClass await VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult( CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) - .WithLocation(0) - .WithArguments("MyClass.P")); + .WithLocation(0) + .WithArguments("MyClass.P") + .WithOptions(DiagnosticOptions.IgnoreAdditionalLocations)); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] @@ -1992,8 +1995,9 @@ class MyClass await VerifyCS.VerifyAnalyzerAsync(code, new DiagnosticResult( CSharpRemoveUnusedMembersDiagnosticAnalyzer.s_removeUnreadMembersRule) - .WithLocation(0) - .WithArguments("MyClass.this")); + .WithLocation(0) + .WithArguments("MyClass.this") + .WithOptions(DiagnosticOptions.IgnoreAdditionalLocations)); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/43191")] @@ -3176,7 +3180,10 @@ class C } """, // /0/Test0.cs(3,13): info IDE0051: Private member 'C.C' is unused - VerifyCS.Diagnostic("IDE0051").WithLocation(0).WithArguments("C.C")); + VerifyCS.Diagnostic("IDE0051") + .WithLocation(0) + .WithArguments("C.C") + .WithOptions(DiagnosticOptions.IgnoreAdditionalLocations)); } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/62856")] From 21c29e4e82d4ae1990aa0e4b2e062f519764f5f9 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 12:58:14 -0700 Subject: [PATCH 14/15] Rename --- .../Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs index e7ed9594d69e0..c06801cfe9adc 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs @@ -3187,7 +3187,7 @@ class C } [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/62856")] - public async Task DontWarnForAwaiterMethods() + public async Task DoNotWarnForAwaiterMethods() { const string code = """ using System; From 74e6730b4e38c1ad763ef2ce99ce97f940570c40 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 29 Jul 2024 13:14:09 -0700 Subject: [PATCH 15/15] Apply suggestions from code review --- .../AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs index 3b9ab552c14f2..814efb24c087c 100644 --- a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs @@ -45,7 +45,7 @@ internal abstract class AbstractRemoveUnusedMembersDiagnosticAnalyzer< EnforceOnBuildValues.RemoveUnusedMembers, new LocalizableResourceString(nameof(AnalyzersResources.Remove_unused_private_members), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)), new LocalizableResourceString(nameof(AnalyzersResources.Private_member_0_is_unused), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)), - hasAnyCodeStyleOption: false, isUnnecessary: false); + hasAnyCodeStyleOption: false, isUnnecessary: true); // IDE0052: "Remove unread members" (Value is written and/or symbol is referenced, but the assigned value is never read) // Internal for testing @@ -54,7 +54,7 @@ internal abstract class AbstractRemoveUnusedMembersDiagnosticAnalyzer< EnforceOnBuildValues.RemoveUnreadMembers, new LocalizableResourceString(nameof(AnalyzersResources.Remove_unread_private_members), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)), new LocalizableResourceString(nameof(AnalyzersResources.Private_member_0_can_be_removed_as_the_value_assigned_to_it_is_never_read), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)), - hasAnyCodeStyleOption: false, isUnnecessary: false); + hasAnyCodeStyleOption: false, isUnnecessary: true); protected AbstractRemoveUnusedMembersDiagnosticAnalyzer() : base([s_removeUnusedMembersRule, s_removeUnreadMembersRule],