diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1402CodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1402CodeFixProvider.cs index 2f1d0740e..00ee3ceb2 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1402CodeFixProvider.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/MaintainabilityRules/SA1402CodeFixProvider.cs @@ -9,6 +9,7 @@ namespace StyleCop.Analyzers.MaintainabilityRules using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; + using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; @@ -109,6 +110,8 @@ private static async Task GetTransformedSolutionAsync(Document documen SyntaxNode extractedDocumentNode = root.RemoveNodes(nodesToRemoveFromExtracted, SyntaxRemoveOptions.KeepUnbalancedDirectives); Solution updatedSolution = document.Project.Solution.AddDocument(extractedDocumentId, extractedDocumentName, extractedDocumentNode, document.Folders); + updatedSolution = await RemoveUnnecessaryUsingsAsync(updatedSolution, extractedDocumentId, cancellationToken).ConfigureAwait(false); + // Make sure to also add the file to linked projects foreach (var linkedDocumentId in document.GetLinkedDocumentIds()) { @@ -117,9 +120,151 @@ private static async Task GetTransformedSolutionAsync(Document documen } // Remove the type from its original location - updatedSolution = updatedSolution.WithDocumentSyntaxRoot(document.Id, root.RemoveNode(node, SyntaxRemoveOptions.KeepUnbalancedDirectives)); + var newRootOriginal = root.RemoveNode(node, SyntaxRemoveOptions.KeepUnbalancedDirectives); + updatedSolution = updatedSolution.WithDocumentSyntaxRoot(document.Id, newRootOriginal); return updatedSolution; } + + private static async Task RemoveUnnecessaryUsingsAsync(Solution solution, DocumentId documentId, CancellationToken cancellationToken) + { + var document = solution.GetDocument(documentId); + var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + + var unnecessaryUsings = await GetUnnecessaryUsingsAsync(document, root, cancellationToken).ConfigureAwait(false); + + var hasPreprocessorDirectives = root.DescendantTrivia().Any(t => t.IsKind(SyntaxKind.IfDirectiveTrivia) || t.IsKind(SyntaxKind.EndIfDirectiveTrivia)); + + if (hasPreprocessorDirectives) + { + root = RemoveUnnecessaryPreprocessorDirectives(root, unnecessaryUsings); + + root = StripMultipleBlankLines(root); + + // Recalculate unnecessary usings after modifying the root + unnecessaryUsings = await GetUnnecessaryUsingsAsync(document, root, cancellationToken).ConfigureAwait(false); + } + + var newRoot = root.RemoveNodes(unnecessaryUsings, SyntaxRemoveOptions.KeepNoTrivia); + + return solution.WithDocumentSyntaxRoot(documentId, newRoot); + } + + private static async Task> GetUnnecessaryUsingsAsync(Document document, SyntaxNode root, CancellationToken cancellationToken) + { + var semanticModel = await document.WithSyntaxRoot(root).GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); + var diagnostics = semanticModel.GetDiagnostics(); + var cs8019Diagnostics = diagnostics.Where(d => d.Id == "CS8019").ToList(); + + return cs8019Diagnostics.Select(diagnostic => + { + var diagnosticSpan = diagnostic.Location.SourceSpan; + var usingDirective = root.FindToken(diagnosticSpan.Start).Parent.AncestorsAndSelf().OfType().First(); + return usingDirective; + }).ToList(); + } + + private static SyntaxNode RemoveUnnecessaryPreprocessorDirectives(SyntaxNode root, List unnecessaryUsings) + { + var nodesToRemove = new List(); + + foreach (var usingDirective in unnecessaryUsings) + { + var ifDirective = usingDirective.GetLeadingTrivia().FirstOrDefault(t => t.IsKind(SyntaxKind.IfDirectiveTrivia)); + var elifDirective = root.DescendantTrivia() + .FirstOrDefault(t => t.IsKind(SyntaxKind.ElifDirectiveTrivia) && + t.SpanStart > usingDirective.Span.End); + var elseDirective = root.DescendantTrivia() + .FirstOrDefault(t => t.IsKind(SyntaxKind.ElseDirectiveTrivia) && + t.SpanStart > usingDirective.Span.End); + var endIfDirective = root.DescendantTrivia() + .FirstOrDefault(t => t.IsKind(SyntaxKind.EndIfDirectiveTrivia) && + t.SpanStart > usingDirective.Span.End); + + if (ifDirective != default) + { + nodesToRemove.Add(ifDirective.GetStructure() as DirectiveTriviaSyntax); + } + + if (elifDirective != default) + { + nodesToRemove.Add(elifDirective.GetStructure() as DirectiveTriviaSyntax); + } + + if (elseDirective != default) + { + nodesToRemove.Add(elseDirective.GetStructure() as DirectiveTriviaSyntax); + } + + if (endIfDirective != default) + { + nodesToRemove.Add(endIfDirective.GetStructure() as DirectiveTriviaSyntax); + } + } + + root = root.RemoveNodes(nodesToRemove, SyntaxRemoveOptions.KeepNoTrivia); + return root; + } + + private static SyntaxNode StripMultipleBlankLines(SyntaxNode syntaxRoot) + { + var replaceMap = new Dictionary(); + + var usingDirectives = syntaxRoot.DescendantNodes().OfType(); + + foreach (var usingDirective in usingDirectives) + { + var nextToken = usingDirective.GetLastToken().GetNextToken(); + + // Start at -1 to compensate for the always present end-of-line. + var trailingCount = -1; + + // Count the blank lines at the end of the using statement. + foreach (var trivia in usingDirective.GetTrailingTrivia().Reverse()) + { + if (!trivia.IsKind(SyntaxKind.EndOfLineTrivia)) + { + break; + } + + trailingCount++; + } + + // Count the blank lines at the start of the next token + var leadingCount = 0; + + foreach (var trivia in nextToken.LeadingTrivia) + { + if (!trivia.IsKind(SyntaxKind.EndOfLineTrivia)) + { + break; + } + + leadingCount++; + } + + if ((trailingCount + leadingCount) > 1) + { + var totalStripCount = trailingCount + leadingCount - 1; + + if (trailingCount > 0) + { + var trailingStripCount = Math.Min(totalStripCount, trailingCount); + + var trailingTrivia = usingDirective.GetTrailingTrivia(); + replaceMap[usingDirective.GetLastToken()] = usingDirective.GetLastToken().WithTrailingTrivia(trailingTrivia.Take(trailingTrivia.Count - trailingStripCount)); + totalStripCount -= trailingStripCount; + } + + if (totalStripCount > 0) + { + replaceMap[nextToken] = nextToken.WithLeadingTrivia(nextToken.LeadingTrivia.Skip(totalStripCount)); + } + } + } + + var newSyntaxRoot = syntaxRoot.ReplaceTokens(replaceMap.Keys, (original, rewritten) => replaceMap[original]); + return newSyntaxRoot; + } } } diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/MaintainabilityRules/FileMayOnlyContainTestBase.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/MaintainabilityRules/FileMayOnlyContainTestBase.cs index 2e899e531..489f12427 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers.Test/MaintainabilityRules/FileMayOnlyContainTestBase.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers.Test/MaintainabilityRules/FileMayOnlyContainTestBase.cs @@ -306,6 +306,694 @@ public async Task TestPreservePreprocessorDirectivesAsync() } } + [Fact] + [WorkItem(3109, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3109")] + public async Task TestCodeFixRemovesUnnecessaryUsingsAsync() + { + var testCode = @" +namespace TestNamespace +{ + using System; + using System.Collections.Generic; + + public class TestClass + { + public List Items { get; set; } + } + + public class {|#0:TestClass2|} + { + public DateTime Date { get; set; } + } +} +"; + + var fixedCode = new[] + { + ("/0/Test0.cs", @" +namespace TestNamespace +{ + using System; + using System.Collections.Generic; + + public class TestClass + { + public List Items { get; set; } + } +} +"), + ("TestClass2.cs", @" +namespace TestNamespace +{ + using System; + + public class TestClass2 + { + public DateTime Date { get; set; } + } +} +"), + }; + + var expected = new[] + { + this.Diagnostic().WithLocation(0).WithArguments("not", "preceded"), + }; + + await this.VerifyCSharpFixAsync(testCode, this.GetSettings(), expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestCodeFixKeepsNeededUsingsAsync() + { + var testCode = @" +namespace TestNamespace +{ + using System.Collections.Generic; + + public class TestClass + { + public List Items { get; set; } + } + + public class {|#0:TestClass2|} + { + public List Items2 { get; set; } + } +} +"; + + var fixedCode = new[] + { + ("/0/Test0.cs", @" +namespace TestNamespace +{ + using System.Collections.Generic; + + public class TestClass + { + public List Items { get; set; } + } +} +"), + ("TestClass2.cs", @" +namespace TestNamespace +{ + using System.Collections.Generic; + + public class TestClass2 + { + public List Items2 { get; set; } + } +} +"), + }; + + var expected = new[] + { + this.Diagnostic().WithLocation(0).WithArguments("not", "preceded"), + }; + + await this.VerifyCSharpFixAsync(testCode, this.GetSettings(), expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestCodeFixRemovesUnnecessaryUsingsFromSecondFileOnlyAsync() + { + var testCode = @" +namespace TestNamespace +{ + using System.Collections.Generic; + + public class TestClass + { + public string Items { get; set; } + } + + public class {|#0:TestClass2|} + { + public string Items2 { get; set; } + } +} +"; + + var fixedCode = new[] + { + ("/0/Test0.cs", @" +namespace TestNamespace +{ + using System.Collections.Generic; + + public class TestClass + { + public string Items { get; set; } + } +} +"), + ("TestClass2.cs", @" +namespace TestNamespace +{ + + public class TestClass2 + { + public string Items2 { get; set; } + } +} +"), + }; + + var expected = new[] + { + this.Diagnostic().WithLocation(0).WithArguments("not", "preceded"), + }; + + await this.VerifyCSharpFixAsync(testCode, this.GetSettings(), expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestCodeWithNoUsingsAsync() + { + var testCode = @" +namespace TestNamespace +{ + public class TestClass + { + public int MyProperty { get; set; } + } + + public class {|#0:TestClass2|} + { + public string MyProperty { get; set; } + } +} +"; + + var fixedCode = new[] + { + ("/0/Test0.cs", @" +namespace TestNamespace +{ + public class TestClass + { + public int MyProperty { get; set; } + } +} +"), + ("TestClass2.cs", @" +namespace TestNamespace +{ + + public class TestClass2 + { + public string MyProperty { get; set; } + } +} +"), + }; + + var expected = new[] + { + this.Diagnostic().WithLocation(0).WithArguments("not", "preceded"), + }; + + await this.VerifyCSharpFixAsync(testCode, this.GetSettings(), expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestCodeWithPreprocessorDirectivesAsync() + { + var testCode = @" +namespace TestNamespace +{ +#if true + using System; +#endif + + public class TestClass + { + public DateTime MyDate { get; set; } + } + + public class {|#0:TestClass2|} + { + public string MyString { get; set; } + } +} +"; + + var fixedCode = new[] + { + ("/0/Test0.cs", @" +namespace TestNamespace +{ +#if true + using System; +#endif + + public class TestClass + { + public DateTime MyDate { get; set; } + } +} +"), + ("TestClass2.cs", @" +namespace TestNamespace +{ + + public class TestClass2 + { + public string MyString { get; set; } + } +} +"), + }; + + var expected = new[] + { + this.Diagnostic().WithLocation(0).WithArguments("not", "preceded"), + }; + + await this.VerifyCSharpFixAsync(testCode, this.GetSettings(), expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestCodeFixRemovesUnnecessaryUsingsAndPreprocessorDirectivesFromSecondFileOnlyAsync() + { + var testCode = @" +namespace TestNamespace +{ +#if true + using System.Collections.Generic; +#endif + + public class TestClass + { + public string Items { get; set; } + } + + public class {|#0:TestClass2|} + { + public string Items2 { get; set; } + } +} +"; + + var fixedCode = new[] + { + ("/0/Test0.cs", @" +namespace TestNamespace +{ +#if true + using System.Collections.Generic; +#endif + + public class TestClass + { + public string Items { get; set; } + } +} +"), + ("TestClass2.cs", @" +namespace TestNamespace +{ + + public class TestClass2 + { + public string Items2 { get; set; } + } +} +"), + }; + + var expected = new[] + { + this.Diagnostic().WithLocation(0).WithArguments("not", "preceded"), + }; + + await this.VerifyCSharpFixAsync(testCode, this.GetSettings(), expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestCodeWithSameConditionalCompilationDirectivesAsync() + { + var testCode = @" +namespace TestNamespace +{ +#if true + using System; +#endif + + public class TestClass + { + public DateTime MyDate { get; set; } + } + + public class {|#0:TestClass2|} + { + public DateTime MyDate2 { get; set; } + } +} +"; + + var fixedCode = new[] + { + ("/0/Test0.cs", @" +namespace TestNamespace +{ +#if true + using System; +#endif + + public class TestClass + { + public DateTime MyDate { get; set; } + } +} +"), + ("TestClass2.cs", @" +namespace TestNamespace +{ +#if true + using System; + +#endif + + public class TestClass2 + { + public DateTime MyDate2 { get; set; } + } +} +"), + }; + + var expected = new[] + { + this.Diagnostic().WithLocation(0).WithArguments("not", "preceded"), + }; + + await this.VerifyCSharpFixAsync(testCode, this.GetSettings(), expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestCodeFixWithDifferentPreprocessorDirectivesAsync() + { + var testCode = @" +namespace TestNamespace +{ +#if true + using System; +#else + using System.Collections.Generic; +#endif + + public class TestClass + { + public DateTime MyDate { get; set; } + } + + public class {|#0:TestClass2|} + { + public string MyString { get; set; } + } +} +"; + + var fixedCode = new[] + { +("/0/Test0.cs", @" +namespace TestNamespace +{ +#if true + using System; +#else + using System.Collections.Generic; +#endif + + public class TestClass + { + public DateTime MyDate { get; set; } + } +} +"), +("TestClass2.cs", @" +namespace TestNamespace +{ + + public class TestClass2 + { + public string MyString { get; set; } + } +} +"), + }; + + var expected = new[] + { + this.Diagnostic().WithLocation(0).WithArguments("not", "preceded"), + }; + + await this.VerifyCSharpFixAsync(testCode, this.GetSettings(), expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestCodeWithElifPreprocessorDirectivesAsync() + { + var testCode = @" +namespace TestNamespace +{ +#if true + using System; +#elif false + using System.Collections.Generic; +#endif + + public class TestClass + { + public DateTime MyDate { get; set; } + } + + public class {|#0:TestClass2|} + { + public string MyString { get; set; } + } +} +"; + + var fixedCode = new[] + { + ("/0/Test0.cs", @" +namespace TestNamespace +{ +#if true + using System; +#elif false + using System.Collections.Generic; +#endif + + public class TestClass + { + public DateTime MyDate { get; set; } + } +} +"), + ("TestClass2.cs", @" +namespace TestNamespace +{ + + public class TestClass2 + { + public string MyString { get; set; } + } +} +"), + }; + + var expected = new[] + { + this.Diagnostic().WithLocation(0).WithArguments("not", "preceded"), + }; + + await this.VerifyCSharpFixAsync(testCode, this.GetSettings(), expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestCodeFixRemovesUsingsWithCommentsAsync() + { + var testCode = @" +namespace TestNamespace +{ + using System.Collections.Generic; // Comment + + public class TestClass + { + public string Items { get; set; } + } + + public class {|#0:TestClass2|} + { + public string Items2 { get; set; } + } +} +"; + + var fixedCode = new[] + { +("/0/Test0.cs", @" +namespace TestNamespace +{ + using System.Collections.Generic; // Comment + + public class TestClass + { + public string Items { get; set; } + } +} +"), +("TestClass2.cs", @" +namespace TestNamespace +{ + + public class TestClass2 + { + public string Items2 { get; set; } + } +} +"), + }; + + var expected = new[] + { + this.Diagnostic().WithLocation(0).WithArguments("not", "preceded"), + }; + + await this.VerifyCSharpFixAsync(testCode, this.GetSettings(), expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestCodeWithTrailingBlankLinesAfterPreprocessorDirectivesAsync() + { + var testCode = @" +namespace TestNamespace +{ +#if true + using System.Collections.Generic; + + +#endif + + + public class TestClass + { + public List Items { get; set; } + } + + public class {|#0:TestClass2|} + { + public List Items2 { get; set; } + } +} +"; + + var fixedCode = new[] + { + ("/0/Test0.cs", @" +namespace TestNamespace +{ +#if true + using System.Collections.Generic; + + +#endif + + + public class TestClass + { + public List Items { get; set; } + } +} +"), + ("TestClass2.cs", @" +namespace TestNamespace +{ +#if true + using System.Collections.Generic; + +#endif + + public class TestClass2 + { + public List Items2 { get; set; } + } +} +"), + }; + + var expected = new[] + { + this.Diagnostic().WithLocation(0).WithArguments("not", "preceded"), + }; + + await this.VerifyCSharpFixAsync(testCode, this.GetSettings(), expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + + [Fact] + public async Task TestCodeWithTrailingBlankLinesAfterUsingDirectiveAsync() + { + var testCode = @" +namespace TestNamespace +{ + using System.Collections.Generic; + + + + public class TestClass + { + public List Items { get; set; } + } + + public class {|#0:TestClass2|} + { + public List Items2 { get; set; } + } +} +"; + + var fixedCode = new[] + { + ("/0/Test0.cs", @" +namespace TestNamespace +{ + using System.Collections.Generic; + + + + public class TestClass + { + public List Items { get; set; } + } +} +"), + ("TestClass2.cs", @" +namespace TestNamespace +{ + using System.Collections.Generic; + + public class TestClass2 + { + public List Items2 { get; set; } + } +} +"), + }; + + var expected = new[] + { + this.Diagnostic().WithLocation(0).WithArguments("not", "preceded"), + }; + + await this.VerifyCSharpFixAsync(testCode, this.GetSettings(), expected, fixedCode, CancellationToken.None).ConfigureAwait(false); + } + protected DiagnosticResult Diagnostic() => new DiagnosticResult(this.Analyzer.SupportedDiagnostics.Single());