Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -28,4 +29,18 @@ protected override IEnumerable<TypeDeclarationSyntax> GetTypeDeclarations(INamed

protected override SyntaxList<MemberDeclarationSyntax> 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,
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -3176,8 +3176,8 @@ class C
private C(int i) { }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private C(int i) { }
{|#0:private C(int i) { }|}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that doesn't seem to help.

}
""",
// /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"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
VerifyCS.Diagnostic("IDE0051").WithSpan(3, 5, 3, 25).WithArguments("C.C"));
VerifyCS.Diagnostic("IDE0051").WithLocation(0).WithArguments("C.C"));

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that doesn't seem to fix things.

}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/62856")]
Expand Down
97 changes: 57 additions & 40 deletions src/Analyzers/Core/Analyzers/Helpers/DiagnosticHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/// <summary>
Expand Down Expand Up @@ -92,11 +93,28 @@ public static Diagnostic CreateWithLocationTags(
ImmutableArray<Location> additionalLocations,
ImmutableArray<Location> 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<Location> additionalLocations,
ImmutableArray<Location> additionalUnnecessaryLocations)
{
if (additionalUnnecessaryLocations.IsEmpty)
{
return Create(descriptor, location, notificationOption, analyzerOptions, additionalLocations, ImmutableDictionary<string, string?>.Empty, messageArgs);
}
return CreateWithMessage(descriptor, location, notificationOption, analyzerOptions, additionalLocations, ImmutableDictionary<string, string?>.Empty, message);

var tagIndices = ImmutableDictionary<string, IEnumerable<int>>.Empty
.Add(WellKnownDiagnosticTags.Unnecessary, Enumerable.Range(additionalLocations.Length, additionalUnnecessaryLocations.Length));
Expand All @@ -105,10 +123,10 @@ public static Diagnostic CreateWithLocationTags(
location,
notificationOption,
analyzerOptions,
message,
additionalLocations.AddRange(additionalUnnecessaryLocations),
tagIndices,
ImmutableDictionary<string, string?>.Empty,
messageArgs);
ImmutableDictionary<string, string?>.Empty);
}

/// <summary>
Expand Down Expand Up @@ -144,11 +162,30 @@ public static Diagnostic CreateWithLocationTags(
ImmutableArray<Location> additionalUnnecessaryLocations,
ImmutableDictionary<string, string?>? 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<Location> additionalLocations,
ImmutableArray<Location> additionalUnnecessaryLocations,
ImmutableDictionary<string, string?>? 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<string, IEnumerable<int>>.Empty
.Add(WellKnownDiagnosticTags.Unnecessary, Enumerable.Range(additionalLocations.Length, additionalUnnecessaryLocations.Length));
Expand All @@ -157,49 +194,29 @@ public static Diagnostic CreateWithLocationTags(
location,
notificationOption,
analyzerOptions,
message,
additionalLocations.AddRange(additionalUnnecessaryLocations),
tagIndices,
properties,
messageArgs);
properties);
}

/// <summary>
/// Create a diagnostic that adds properties specifying a tag for a set of locations.
/// </summary>
/// <param name="descriptor">A <see cref="DiagnosticDescriptor"/> describing the diagnostic.</param>
/// <param name="location">An optional primary location of the diagnostic. If null, <see cref="Location"/> will return <see cref="Location.None"/>.</param>
/// <param name="notificationOption">Notification option for the diagnostic.</param>
/// <param name="additionalLocations">
/// An optional set of additional locations related to the diagnostic.
/// Typically, these are locations of other items referenced in the message.
/// </param>
/// <param name="tagIndices">
/// a map of location tag to index in additional locations.
/// "AbstractRemoveUnnecessaryParenthesesDiagnosticAnalyzer" for an example of usage.
/// </param>
/// <param name="properties">
/// 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.
/// </param>
/// <param name="messageArgs">Arguments to the message of the diagnostic.</param>
/// <returns>The <see cref="Diagnostic"/> instance.</returns>
private static Diagnostic CreateWithLocationTags(
DiagnosticDescriptor descriptor,
Location location,
NotificationOption2 notificationOption,
AnalyzerOptions analyzerOptions,
LocalizableString message,
IEnumerable<Location> additionalLocations,
IDictionary<string, IEnumerable<int>> tagIndices,
ImmutableDictionary<string, string?>? properties,
params object[] messageArgs)
ImmutableDictionary<string, string?>? properties)
{
Contract.ThrowIfTrue(additionalLocations.IsEmpty());
Contract.ThrowIfTrue(tagIndices.IsEmpty());

properties ??= ImmutableDictionary<string, string?>.Empty;
properties = properties.AddRange(tagIndices.Select(kvp => new KeyValuePair<string, string?>(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<int> indices, int additionalLocationsLength)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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],
Expand All @@ -64,6 +64,7 @@ protected AbstractRemoveUnusedMembersDiagnosticAnalyzer()

protected abstract IEnumerable<TTypeDeclarationSyntax> GetTypeDeclarations(INamedTypeSymbol namedType, CancellationToken cancellationToken);
protected abstract SyntaxList<TMemberDeclarationSyntax> 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.
Expand Down Expand Up @@ -428,6 +429,7 @@ private void AnalyzeObjectCreationOperation(OperationAnalysisContext operationCo

private void OnSymbolEnd(SymbolAnalysisContext symbolEndContext, bool hasUnsupportedOperation)
{
var cancellationToken = symbolEndContext.CancellationToken;
if (hasUnsupportedOperation)
{
return;
Expand All @@ -445,7 +447,7 @@ private void OnSymbolEnd(SymbolAnalysisContext symbolEndContext, bool hasUnsuppo
using var _1 = PooledHashSet<ISymbol>.GetInstance(out var symbolsReferencedInDocComments);
using var _2 = ArrayBuilder<string>.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())
Expand All @@ -467,15 +469,15 @@ 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;
}

// 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.
Expand Down Expand Up @@ -509,17 +511,24 @@ member is IPropertySymbol property &&
continue;
}

var diagnosticLocation = GetDiagnosticLocation(member);
var fadingLocation = member.DeclaringSyntaxReferences.FirstOrDefault(
r => r.SyntaxTree == diagnosticLocation.SourceTree && r.Span.Contains(diagnosticLocation.SourceSpan));

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.
// 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,
properties: null,
GetMessage(rule, member));
symbolEndContext.ReportDiagnostic(diagnostic);
message: GetMessage(rule, member),
additionalLocations: [],
additionalUnnecessaryLocations: fadingNode is null ? [] : [fadingNode.GetLocation()],
properties: null));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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