From ea8a82f7a2ec99af4e199620450ccdc052fa5112 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Thu, 15 Jun 2017 00:57:12 -0700 Subject: [PATCH] Initial implementation of the Remove-Unreachable-Code fix. --- .../CSharp/Portable/CSharpFeatures.csproj | 3 + ...arpRemoveUnreachableCodeCodeFixProvider.cs | 76 +++++++++++ ...RemoveUnreachableCodeDiagnosticAnalyzer.cs | 121 ++++++++++++++++++ .../RemoveUnreachableCodeHelpers.cs | 60 +++++++++ .../PredefinedCodeFixProviderNames.cs | 1 + .../Diagnostics/Analyzers/IDEDiagnosticIds.cs | 2 + .../Portable/FeaturesResources.Designer.cs | 18 +++ .../Core/Portable/FeaturesResources.resx | 6 + 8 files changed, 287 insertions(+) create mode 100644 src/Features/CSharp/Portable/RemoveUnreachableCode/CSharpRemoveUnreachableCodeCodeFixProvider.cs create mode 100644 src/Features/CSharp/Portable/RemoveUnreachableCode/CSharpRemoveUnreachableCodeDiagnosticAnalyzer.cs create mode 100644 src/Features/CSharp/Portable/RemoveUnreachableCode/RemoveUnreachableCodeHelpers.cs diff --git a/src/Features/CSharp/Portable/CSharpFeatures.csproj b/src/Features/CSharp/Portable/CSharpFeatures.csproj index e8d8b591ed8..3aecc8c378f 100644 --- a/src/Features/CSharp/Portable/CSharpFeatures.csproj +++ b/src/Features/CSharp/Portable/CSharpFeatures.csproj @@ -87,6 +87,9 @@ + + + diff --git a/src/Features/CSharp/Portable/RemoveUnreachableCode/CSharpRemoveUnreachableCodeCodeFixProvider.cs b/src/Features/CSharp/Portable/RemoveUnreachableCode/CSharpRemoveUnreachableCodeCodeFixProvider.cs new file mode 100644 index 00000000000..049fe2d4361 --- /dev/null +++ b/src/Features/CSharp/Portable/RemoveUnreachableCode/CSharpRemoveUnreachableCodeCodeFixProvider.cs @@ -0,0 +1,76 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Composition; +using System.Diagnostics; +using System.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CodeStyle; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Text; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis.CSharp.RemoveUnreachableCode +{ + [ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.RemoveUnreachableCode), Shared] + internal class CSharpRemoveUnreachableCodeCodeFixProvider : SyntaxEditorBasedCodeFixProvider + { + public override ImmutableArray FixableDiagnosticIds { get; } = + ImmutableArray.Create(IDEDiagnosticIds.RemoveUnreachableCodeDiagnosticId); + + public override Task RegisterCodeFixesAsync(CodeFixContext context) + { + var diagnostic = context.Diagnostics[0]; + context.RegisterCodeFix(new MyCodeAction( + FeaturesResources.Remove_unreachable_code, + c => FixAsync(context.Document, diagnostic, c)), diagnostic); + + return SpecializedTasks.EmptyTask; + } + + protected override bool IncludeDiagnosticDuringFixAll(Diagnostic diagnostic) + => !diagnostic.Properties.ContainsKey(CSharpRemoveUnreachableCodeDiagnosticAnalyzer.IsCascadedStatement); + + protected override Task FixAllAsync( + Document document, + ImmutableArray diagnostics, + SyntaxEditor editor, + CancellationToken cancellationToken) + { + var syntaxRoot = editor.OriginalRoot; + + foreach (var diagnostic in diagnostics) + { + var firstUnreachableStatementLocation = diagnostic.AdditionalLocations.Single(); + var firstUnreachableStatement = (StatementSyntax)firstUnreachableStatementLocation.FindNode(cancellationToken); + + editor.RemoveNode(firstUnreachableStatement, SyntaxRemoveOptions.KeepUnbalancedDirectives); + + var subsequentUnreachableStatements = RemoveUnreachableCodeHelpers.GetSubsequentUnreachableStatements(firstUnreachableStatement); + foreach (var nextStatement in subsequentUnreachableStatements) + { + editor.RemoveNode(nextStatement, SyntaxRemoveOptions.KeepUnbalancedDirectives); + } + } + + return SpecializedTasks.EmptyTask; + } + + private class MyCodeAction : CodeAction.DocumentChangeAction + { + public MyCodeAction(string title, Func> createChangedDocument) + : base(title, createChangedDocument, title) + { + } + } + } +} \ No newline at end of file diff --git a/src/Features/CSharp/Portable/RemoveUnreachableCode/CSharpRemoveUnreachableCodeDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/RemoveUnreachableCode/CSharpRemoveUnreachableCodeDiagnosticAnalyzer.cs new file mode 100644 index 00000000000..c9182ce2311 --- /dev/null +++ b/src/Features/CSharp/Portable/RemoveUnreachableCode/CSharpRemoveUnreachableCodeDiagnosticAnalyzer.cs @@ -0,0 +1,121 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis.CodeStyle; +using Microsoft.CodeAnalysis.CSharp.Extensions; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Text; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis.CSharp.RemoveUnreachableCode +{ + [DiagnosticAnalyzer(LanguageNames.CSharp)] + internal class CSharpRemoveUnreachableCodeDiagnosticAnalyzer : AbstractCodeStyleDiagnosticAnalyzer + { + public const string IsCascadedStatement = nameof(IsCascadedStatement); + + private const string CS0162 = nameof(CS0162); // Unreachable code detected + + public CSharpRemoveUnreachableCodeDiagnosticAnalyzer() + : base(IDEDiagnosticIds.RemoveUnreachableCodeDiagnosticId, + new LocalizableResourceString(nameof(FeaturesResources.Unreachable_code_detected), FeaturesResources.ResourceManager, typeof(FeaturesResources))) + { + } + + public override DiagnosticAnalyzerCategory GetAnalyzerCategory() + => DiagnosticAnalyzerCategory.SemanticSpanAnalysis; + + public override bool OpenFileOnly(Workspace workspace) + => false; + + protected override void InitializeWorker(AnalysisContext context) + { + context.RegisterSyntaxNodeAction(AnalyzeBlock, ImmutableArray.Create(SyntaxKind.Block)); + } + + private void AnalyzeBlock(SyntaxNodeAnalysisContext context) + { + var block = (BlockSyntax)context.Node; + if (HasOuterBlock(block)) + { + // Don't bother processing inner blocks. We'll have already checked then when + // we processed the outer block + return; + } + + var semanticModel = context.SemanticModel; + var cancellationToken = context.CancellationToken; + + var root = semanticModel.SyntaxTree.GetRoot(cancellationToken); + var diagnostics = semanticModel.GetDiagnostics(block.Span, cancellationToken); + foreach (var diagnostic in diagnostics) + { + cancellationToken.ThrowIfCancellationRequested(); + + if (diagnostic.Id == CS0162) + { + ProcessUnreachableDiagnostic(context, root, diagnostic.Location.SourceSpan); + } + } + } + + private bool HasOuterBlock(SyntaxNode block) + { + for (var current = block.Parent; current != null; current = current.Parent) + { + if (current.Kind() == SyntaxKind.Block) + { + return true; + } + } + + return false; + } + + private void ProcessUnreachableDiagnostic( + SyntaxNodeAnalysisContext context, SyntaxNode root, TextSpan sourceSpan) + { + var node = root.FindNode(sourceSpan); + var firstUnreachableStatement = node.FirstAncestorOrSelf(); + if (firstUnreachableStatement == null) + { + return; + } + + // Fade out the statement that the unreachable code warning is placed on. + var firstStatementLocation = root.SyntaxTree.GetLocation(firstUnreachableStatement.FullSpan); + + if (!firstUnreachableStatement.IsParentKind(SyntaxKind.Block) && + !firstUnreachableStatement.IsParentKind(SyntaxKind.SwitchSection)) + { + // Can't actually remove this statement (it's an embedded statement in something + // like an 'if-statement'. Just fade the code out, but don't offer to remove it. + context.ReportDiagnostic( + Diagnostic.Create(UnnecessaryWithoutSuggestionDescriptor, firstStatementLocation)); + return; + } + + var additionalLocations = SpecializedCollections.SingletonEnumerable(firstStatementLocation); + context.ReportDiagnostic( + Diagnostic.Create(UnnecessaryWithSuggestionDescriptor, firstStatementLocation, additionalLocations)); + + // Now try to fade out subsequent statements as necessary. + var subsequentUnreachableStatements = RemoveUnreachableCodeHelpers.GetSubsequentUnreachableStatements(firstUnreachableStatement); + + if (subsequentUnreachableStatements.Length > 0) + { + var additionalProperties = ImmutableDictionary.Empty.Add(IsCascadedStatement, ""); + + foreach (var nextStatement in subsequentUnreachableStatements) + { + context.ReportDiagnostic( + Diagnostic.Create(UnnecessaryWithSuggestionDescriptor, + root.SyntaxTree.GetLocation(nextStatement.FullSpan), + additionalLocations, + additionalProperties)); + } + } + } + } +} \ No newline at end of file diff --git a/src/Features/CSharp/Portable/RemoveUnreachableCode/RemoveUnreachableCodeHelpers.cs b/src/Features/CSharp/Portable/RemoveUnreachableCode/RemoveUnreachableCodeHelpers.cs new file mode 100644 index 00000000000..c198e920a89 --- /dev/null +++ b/src/Features/CSharp/Portable/RemoveUnreachableCode/RemoveUnreachableCodeHelpers.cs @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +namespace Microsoft.CodeAnalysis.CSharp.RemoveUnreachableCode +{ + internal static class RemoveUnreachableCodeHelpers + { + public static ImmutableArray GetSubsequentUnreachableStatements(StatementSyntax firstUnreachableStatement) + { + SyntaxList siblingStatements; + switch (firstUnreachableStatement.Parent) + { + case BlockSyntax block: + siblingStatements = block.Statements; + break; + + case SwitchSectionSyntax switchSection: + siblingStatements = switchSection.Statements; + break; + + default: + return ImmutableArray.Empty; + } + + var result = ArrayBuilder.GetInstance(); + + // Keep consuming statements after the statement that was reported unreachable and + // fade them out as appropriate. + var firstUnreachableStatementIndex = siblingStatements.IndexOf(firstUnreachableStatement); + for (int i = firstUnreachableStatementIndex + 1, n = siblingStatements.Count; i < n; i++) + { + var nextStatement = siblingStatements[i]; + if (nextStatement.IsKind(SyntaxKind.LabeledStatement)) + { + // In the case of a labeled statement, we don't want to consider it unreachable as + // there may be a 'goto' somewhere else to that label. If the compiler thinks that + // label is actually unreachable, it will give an diagnostic on that label itself + // and we can use that diagnostic to fade the label and any subsequent statements. + break; + } + + if (nextStatement.IsKind(SyntaxKind.LocalFunctionStatement)) + { + // In the case of local functions, it is legal for a local function to be declared + // in code that is otherwise unreachable. It can still be called elsewhere. If + // the local function itself is not called, there will be a particular diagnostic + // for that ("The variable XXX is declared but never used") and the user can choose + // if they want to remove it or not. + continue; + } + + result.Add(nextStatement); + } + + return result.ToImmutableAndFree(); + } + } +} diff --git a/src/Features/Core/Portable/CodeFixes/PredefinedCodeFixProviderNames.cs b/src/Features/Core/Portable/CodeFixes/PredefinedCodeFixProviderNames.cs index a3fa2ae9d3f..163f9170418 100644 --- a/src/Features/Core/Portable/CodeFixes/PredefinedCodeFixProviderNames.cs +++ b/src/Features/Core/Portable/CodeFixes/PredefinedCodeFixProviderNames.cs @@ -37,6 +37,7 @@ internal static class PredefinedCodeFixProviderNames public const string QualifyMemberAccess = nameof(QualifyMemberAccess); public const string RemoveUnnecessaryCast = nameof(RemoveUnnecessaryCast); public const string RemoveUnnecessaryImports = nameof(RemoveUnnecessaryImports); + public const string RemoveUnreachableCode = nameof(RemoveUnreachableCode); public const string RemoveUnusedVariable = nameof(RemoveUnusedVariable); public const string RenameTracking = nameof(RenameTracking); public const string SimplifyNames = nameof(SimplifyNames); diff --git a/src/Features/Core/Portable/Diagnostics/Analyzers/IDEDiagnosticIds.cs b/src/Features/Core/Portable/Diagnostics/Analyzers/IDEDiagnosticIds.cs index 0af7854797e..d62dbf4dade 100644 --- a/src/Features/Core/Portable/Diagnostics/Analyzers/IDEDiagnosticIds.cs +++ b/src/Features/Core/Portable/Diagnostics/Analyzers/IDEDiagnosticIds.cs @@ -46,6 +46,8 @@ internal static class IDEDiagnosticIds public const string UseDefaultLiteralDiagnosticId = "IDE0034"; + public const string RemoveUnreachableCodeDiagnosticId = "IDE0035"; + // Analyzer error Ids public const string AnalyzerChangedId = "IDE1001"; public const string AnalyzerDependencyConflictId = "IDE1002"; diff --git a/src/Features/Core/Portable/FeaturesResources.Designer.cs b/src/Features/Core/Portable/FeaturesResources.Designer.cs index c85759f7a6e..ec4e580b883 100644 --- a/src/Features/Core/Portable/FeaturesResources.Designer.cs +++ b/src/Features/Core/Portable/FeaturesResources.Designer.cs @@ -2472,6 +2472,15 @@ internal class FeaturesResources { } } + /// + /// Looks up a localized string similar to Remove unreachable code. + /// + internal static string Remove_unreachable_code { + get { + return ResourceManager.GetString("Remove_unreachable_code", resourceCulture); + } + } + /// /// Looks up a localized string similar to Remove unused variable. /// @@ -3036,6 +3045,15 @@ internal class FeaturesResources { } } + /// + /// Looks up a localized string similar to Unreachable code detected. + /// + internal static string Unreachable_code_detected { + get { + return ResourceManager.GetString("Unreachable_code_detected", resourceCulture); + } + } + /// /// Looks up a localized string similar to Updating '{0}' will prevent the debug session from continuing.. /// diff --git a/src/Features/Core/Portable/FeaturesResources.resx b/src/Features/Core/Portable/FeaturesResources.resx index e68790cec07..7e20e836b30 100644 --- a/src/Features/Core/Portable/FeaturesResources.resx +++ b/src/Features/Core/Portable/FeaturesResources.resx @@ -1247,4 +1247,10 @@ This version used in: {2} 'default' expression can be simplified + + Unreachable code detected + + + Remove unreachable code + \ No newline at end of file -- GitLab