Skip to content

Commit

Permalink
Add an option to control if unused members shoudl be faded out. (#76520)
Browse files Browse the repository at this point in the history
  • Loading branch information
CyrusNajmabadi authored Dec 19, 2024
2 parents 9a27ef7 + 2c922c6 commit fbdd94c
Show file tree
Hide file tree
Showing 29 changed files with 152 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;
using Microsoft.CodeAnalysis.CodeQuality;
using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.LanguageService;
Expand All @@ -27,10 +26,9 @@ internal abstract class AbstractRemoveUnusedMembersDiagnosticAnalyzer<
TIdentifierNameSyntax,
TTypeDeclarationSyntax,
TMemberDeclarationSyntax>()
: AbstractCodeQualityDiagnosticAnalyzer(
: AbstractBuiltInUnnecessaryCodeStyleDiagnosticAnalyzer(
[s_removeUnusedMembersRule, s_removeUnreadMembersRule],
// We want to analyze references in generated code, but not report unused members in generated code.
GeneratedCodeAnalysisFlags.Analyze)
FadingOptions.FadeOutUnusedMembers)
where TDocumentationCommentTriviaSyntax : SyntaxNode
where TIdentifierNameSyntax : SyntaxNode
where TTypeDeclarationSyntax : TMemberDeclarationSyntax
Expand All @@ -44,35 +42,43 @@ internal abstract class AbstractRemoveUnusedMembersDiagnosticAnalyzer<
memberOptions: SymbolDisplayMemberOptions.IncludeContainingType);

// IDE0051: "Remove unused members" (Symbol is declared but never referenced)
private static readonly DiagnosticDescriptor s_removeUnusedMembersRule = CreateDescriptor(
private static readonly DiagnosticDescriptor s_removeUnusedMembersRule = CreateDescriptorWithId(
IDEDiagnosticIds.RemoveUnusedMembersDiagnosticId,
EnforceOnBuildValues.RemoveUnusedMembers,
hasAnyCodeStyleOption: false,
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);
isUnnecessary: true);

// IDE0052: "Remove unread members" (Value is written and/or symbol is referenced, but the assigned value is never read)
// Internal for testing
internal static readonly DiagnosticDescriptor s_removeUnreadMembersRule = CreateDescriptor(
internal static readonly DiagnosticDescriptor s_removeUnreadMembersRule = CreateDescriptorWithId(
IDEDiagnosticIds.RemoveUnreadMembersDiagnosticId,
EnforceOnBuildValues.RemoveUnreadMembers,
hasAnyCodeStyleOption: false,
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);
isUnnecessary: true);

protected abstract ISemanticFacts SemanticFacts { get; }

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.
// For example, if we had an unused field with no references, then editing any single method body
// to reference this field should clear the unused field diagnostic.
// Hence, we need to re-analyze the declarations in the whole file for any edits within the document.
/// <summary>
/// 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. For example, if we had an unused field with no references, then
/// editing any single method body to reference this field should clear the unused field diagnostic. Hence, we need
/// to re-analyze the declarations in the whole file for any edits within the document.
/// </summary>
public override DiagnosticAnalyzerCategory GetAnalyzerCategory() => DiagnosticAnalyzerCategory.SemanticDocumentAnalysis;

/// <summary>
/// We want to analyze references in generated code, but not report unused members in generated code.
/// </summary>
protected override GeneratedCodeAnalysisFlags GeneratedCodeAnalysisFlags => GeneratedCodeAnalysisFlags.Analyze;

protected sealed override void InitializeWorker(AnalysisContext context)
=> context.RegisterCompilationStartAction(compilationStartContext
=> CompilationAnalyzer.CreateAndRegisterActions(compilationStartContext, this));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1621,7 +1621,7 @@ parseOptions:=Nothing,
compilationOptions:=Nothing,
options:=Nothing,
"IDE0051",
DiagnosticSeverity.Info,
DiagnosticSeverity.Hidden,
diagnosticMessage:=String.Format(AnalyzersResources.Private_member_0_is_unused, "C.New"))
End Function
End Class
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.QuickInfo;

[UseExportProvider]
[Trait(Traits.Feature, Traits.Features.QuickInfo)]
public class DiagnosticAnalyzerQuickInfoSourceTests
public sealed class DiagnosticAnalyzerQuickInfoSourceTests
{
[WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/46604")]
public async Task ErrorTitleIsShownOnDisablePragma()
Expand Down Expand Up @@ -156,26 +156,26 @@ public async Task QuickInfoSuppressMessageAttributeUseCases(string suppressMessa
? GetFormattedIDEAnalyzerTitle(51, nameof(AnalyzersResources.Remove_unused_private_members))
: null;
await TestAsync(
@$"
using System.Diagnostics.CodeAnalysis;
using SM = System.Diagnostics.CodeAnalysis.SuppressMessageAttribute;
namespace T
{{
public static class DiagnosticIds
{{
public const string IDE0051 = ""IDE0051"";
}}
$$"""
using System.Diagnostics.CodeAnalysis;
using SM = System.Diagnostics.CodeAnalysis.SuppressMessageAttribute;
namespace T
{
public static class DiagnosticIds
{
public const string IDE0051 = "IDE0051";
}
{suppressMessageAttribute}
public class C
{{
private int _i;
}}
}}
", description, ImmutableArray<TextSpan>.Empty);
{{suppressMessageAttribute}}
public class C
{
private int _i;
}
}
""", description, ImmutableArray<TextSpan>.Empty);
}

protected static async Task AssertContentIsAsync(EditorTestWorkspace workspace, Document document, int position, string expectedDescription,
private static async Task AssertContentIsAsync(EditorTestWorkspace workspace, Document document, int position, string expectedDescription,
ImmutableArray<TextSpan> relatedSpans)
{
var info = await GetQuickinfo(workspace, document, position);
Expand All @@ -194,13 +194,13 @@ private static async Task<QuickInfoItem> GetQuickinfo(EditorTestWorkspace worksp
return info;
}

protected static async Task AssertNoContentAsync(EditorTestWorkspace workspace, Document document, int position)
private static async Task AssertNoContentAsync(EditorTestWorkspace workspace, Document document, int position)
{
var info = await GetQuickinfo(workspace, document, position);
Assert.Null(info);
}

protected static async Task TestAsync(
private static async Task TestAsync(
string code,
string expectedDescription,
ImmutableArray<TextSpan> relatedSpans,
Expand Down Expand Up @@ -237,7 +237,7 @@ private static string GetFormattedIDEAnalyzerTitle(int ideDiagnosticId, string n
return $"IDE{ideDiagnosticId:0000}: {localizable}";
}

protected static Task TestInClassAsync(string code, string expectedDescription, params TextSpan[] relatedSpans)
private static Task TestInClassAsync(string code, string expectedDescription, params TextSpan[] relatedSpans)
=> TestAsync(
$$"""
class C
Expand All @@ -246,7 +246,7 @@ class C
}
""", expectedDescription, relatedSpans.ToImmutableArray());

protected static Task TestInMethodAsync(string code, string expectedDescription, params TextSpan[] relatedSpans)
private static Task TestInMethodAsync(string code, string expectedDescription, params TextSpan[] relatedSpans)
=> TestInClassAsync(
$$"""
void M()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@
<StackPanel>
<CheckBox x:Name="Fade_out_unused_usings"
Content="{x:Static local:AdvancedOptionPageStrings.Option_Fade_out_unused_usings}" />
<CheckBox x:Name="Fade_out_unused_members"
Content="{x:Static local:AdvancedOptionPageStrings.Option_Fade_out_unused_members}" />
<CheckBox x:Name="Fade_out_unreachable_code"
Content="{x:Static local:AdvancedOptionPageStrings.Option_Fade_out_unreachable_code}" />
</StackPanel>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ public AdvancedOptionPageControl(OptionStore optionStore) : base(optionStore)

// Fading
BindToOption(Fade_out_unused_usings, FadingOptions.FadeOutUnusedImports, LanguageNames.CSharp);
BindToOption(Fade_out_unused_members, FadingOptions.FadeOutUnusedMembers, LanguageNames.CSharp);
BindToOption(Fade_out_unreachable_code, FadingOptions.FadeOutUnreachableCode, LanguageNames.CSharp);

// Block Structure Guides
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,9 @@ public static string Option_Fading
public static string Option_Fade_out_unused_usings
=> CSharpVSResources.Fade_out_unused_usings;

public static string Option_Fade_out_unused_members
=> ServicesVSResources.Fade_out_unused_members;

public static string Option_Fade_out_unreachable_code
=> ServicesVSResources.Fade_out_unreachable_code;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,25 @@

using Microsoft.CodeAnalysis.CodeStyle;

namespace Microsoft.VisualStudio.LanguageServices.CSharp.Options
namespace Microsoft.VisualStudio.LanguageServices.CSharp.Options;

public partial class AutomationObject
{
public partial class AutomationObject
public int Fading_FadeOutUnreachableCode
{
get { return GetBooleanOption(FadingOptions.FadeOutUnreachableCode); }
set { SetBooleanOption(FadingOptions.FadeOutUnreachableCode, value); }
}

public int Fading_FadeOutUnusedImports
{
public int Fading_FadeOutUnreachableCode
{
get { return GetBooleanOption(FadingOptions.FadeOutUnreachableCode); }
set { SetBooleanOption(FadingOptions.FadeOutUnreachableCode, value); }
}
get { return GetBooleanOption(FadingOptions.FadeOutUnusedImports); }
set { SetBooleanOption(FadingOptions.FadeOutUnusedImports, value); }
}

public int Fading_FadeOutUnusedImports
{
get { return GetBooleanOption(FadingOptions.FadeOutUnusedImports); }
set { SetBooleanOption(FadingOptions.FadeOutUnusedImports, value); }
}
public int Fading_FadeOutUnusedMembers
{
get { return GetBooleanOption(FadingOptions.FadeOutUnusedMembers); }
set { SetBooleanOption(FadingOptions.FadeOutUnusedMembers, value); }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
namespace Microsoft.VisualStudio.LanguageServices.CSharp.Options
{
[ComVisible(true)]
public partial class AutomationObject : AbstractAutomationObject
public sealed partial class AutomationObject : AbstractAutomationObject
{
internal AutomationObject(ILegacyGlobalOptionService legacyGlobalOptions)
: base(legacyGlobalOptions, LanguageNames.CSharp)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ public bool TryFetch(LocalUserRegistryOptionPersister persister, OptionKey2 opti
{"dotnet_allow_best_effort_when_extracting_method", new RoamingProfileStorage("TextEditor.%LANGUAGE%.Specific.Allow Best Effort")},
{"dotnet_fade_out_unreachable_code", new RoamingProfileStorage("TextEditor.%LANGUAGE%.Specific.FadeOutUnreachableCode")},
{"dotnet_fade_out_unused_imports", new RoamingProfileStorage("TextEditor.%LANGUAGE%.Specific.FadeOutUnusedImports")},
{"dotnet_fade_out_unused_members", new RoamingProfileStorage("TextEditor.%LANGUAGE%.Specific.FadeOutUnusedMembers")},
{"dotnet_add_imports_on_paste", new RoamingProfileStorage("TextEditor.%LANGUAGE%.Specific.AddImportsOnPaste2")},
{"dotnet_always_use_default_symbol_servers", new RoamingProfileStorage("TextEditor.AlwaysUseDefaultSymbolServers")},
{"csharp_insert_block_comment_start_string", new RoamingProfileStorage("TextEditor.%LANGUAGE%.Specific.Auto Insert Block Comment Start String")},
Expand Down
3 changes: 3 additions & 0 deletions src/VisualStudio/Core/Def/ServicesVSResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,9 @@ Additional information: {1}</value>
<data name="Analysis" xml:space="preserve">
<value>Analysis</value>
</data>
<data name="Fade_out_unused_members" xml:space="preserve">
<value>Fade out unused members</value>
</data>
<data name="Fade_out_unreachable_code" xml:space="preserve">
<value>Fade out unreachable code</value>
</data>
Expand Down
5 changes: 5 additions & 0 deletions src/VisualStudio/Core/Def/xlf/ServicesVSResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/VisualStudio/Core/Def/xlf/ServicesVSResources.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/VisualStudio/Core/Def/xlf/ServicesVSResources.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/VisualStudio/Core/Def/xlf/ServicesVSResources.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/VisualStudio/Core/Def/xlf/ServicesVSResources.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/VisualStudio/Core/Def/xlf/ServicesVSResources.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/VisualStudio/Core/Def/xlf/ServicesVSResources.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/VisualStudio/Core/Def/xlf/ServicesVSResources.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/VisualStudio/Core/Def/xlf/ServicesVSResources.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/VisualStudio/Core/Def/xlf/ServicesVSResources.ru.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/VisualStudio/Core/Def/xlf/ServicesVSResources.tr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit fbdd94c

Please sign in to comment.