From e1f01e9a45fd94524acc0fbea4067f4e48a68225 Mon Sep 17 00:00:00 2001 From: Shahar Prish Date: Fri, 27 Sep 2024 15:41:39 +0300 Subject: [PATCH] Fix issue with adding the namespace if it already exists, as well as adding it in the correct location. --- .../CorrectObsolescenceCodeFixTests.cs | 152 +++++++++++++++++- .../CorrectObsolescenceCodeFix.cs | 68 +++++++- 2 files changed, 213 insertions(+), 7 deletions(-) diff --git a/src/SubtleEngineering.Analyzers.Tests/Obsolescence/CorrectObsolescenceCodeFixTests.cs b/src/SubtleEngineering.Analyzers.Tests/Obsolescence/CorrectObsolescenceCodeFixTests.cs index 2ce5bd4..481c740 100644 --- a/src/SubtleEngineering.Analyzers.Tests/Obsolescence/CorrectObsolescenceCodeFixTests.cs +++ b/src/SubtleEngineering.Analyzers.Tests/Obsolescence/CorrectObsolescenceCodeFixTests.cs @@ -37,8 +37,7 @@ public void ObsoleteMethod() { } } - """ - ; + """; var expectedDiagnostics = new List { @@ -229,6 +228,155 @@ public async Task ObsoleteDelegateWithoutEditorBrowsable_ShouldAddAttribute() await sut.RunAsync(); } + [Fact] + public async Task ObsoleteMethodWithoutEditorBrowsableDoesNotAddImport_ShouldAddAttribute() + { + const string code = """ + using System; + using System.ComponentModel; + + class TestClass + { + [Obsolete] + public void {|#0:ObsoleteMethod|}() + { + } + } + """; + + const string fixedCode = """ + using System; + using System.ComponentModel; + + class TestClass + { + [Obsolete] + [EditorBrowsable(EditorBrowsableState.Never)] + public void ObsoleteMethod() + { + } + } + """; + + var expectedDiagnostics = new List + { + VerifyCf.Diagnostic(CorrectObsolescenceAnalyzer.Rules[0]) + .WithLocation(0) + .WithArguments("ObsoleteMethod") + }; + + var sut = CreateSut(code, fixedCode, expectedDiagnostics); + + sut.CodeFixTestBehaviors = CodeFixTestBehaviors.SkipFixAllInDocumentCheck; // Ensures only the specific fix is applied + sut.TestBehaviors |= TestBehaviors.SkipSuppressionCheck; // If suppression is involved + + await sut.RunAsync(); + } + + [Fact] + public async Task ObsoleteMethodWithoutEditorBrowsableButWithCommentOnTop_ShouldAddAttribute() + { + const string code = """ + // This should stay the top comment. + + namespace MyNamespace + { + using System; + + class TestClass + { + [Obsolete] + public void {|#0:ObsoleteMethod|}() + { + } + } + } + """; + + const string fixedCode = """ + // This should stay the top comment. + + namespace MyNamespace + { + using System; + using System.ComponentModel; + + class TestClass + { + [Obsolete] + [EditorBrowsable(EditorBrowsableState.Never)] + public void ObsoleteMethod() + { + } + } + } + """; + + var expectedDiagnostics = new List + { + VerifyCf.Diagnostic(CorrectObsolescenceAnalyzer.Rules[0]) + .WithLocation(0) + .WithArguments("ObsoleteMethod") + }; + + var sut = CreateSut(code, fixedCode, expectedDiagnostics); + + sut.CodeFixTestBehaviors = CodeFixTestBehaviors.SkipFixAllInDocumentCheck; // Ensures only the specific fix is applied + sut.TestBehaviors |= TestBehaviors.SkipSuppressionCheck; // If suppression is involved + + await sut.RunAsync(); + } + + [Fact] + public async Task ObsoleteMethodWithoutEditorBrowsableInsideANamespace_ShouldAddAttribute() + { + const string code = """ + namespace MyNamespace + { + using System; + + class TestClass + { + [Obsolete] + public void {|#0:ObsoleteMethod|}() + { + } + } + } + """; + + const string fixedCode = """ + namespace MyNamespace + { + using System; + using System.ComponentModel; + + class TestClass + { + [Obsolete] + [EditorBrowsable(EditorBrowsableState.Never)] + public void ObsoleteMethod() + { + } + } + } + """; + + var expectedDiagnostics = new List + { + VerifyCf.Diagnostic(CorrectObsolescenceAnalyzer.Rules[0]) + .WithLocation(0) + .WithArguments("ObsoleteMethod") + }; + + var sut = CreateSut(code, fixedCode, expectedDiagnostics); + + sut.CodeFixTestBehaviors = CodeFixTestBehaviors.SkipFixAllInDocumentCheck; // Ensures only the specific fix is applied + sut.TestBehaviors |= TestBehaviors.SkipSuppressionCheck; // If suppression is involved + + await sut.RunAsync(); + } + private VerifyCf.Test CreateSut(string code, string fixedCode, List expected) { var test = new VerifyCf.Test diff --git a/src/SubtleEngineering.Analyzers/Obsolescence/CorrectObsolescenceCodeFix.cs b/src/SubtleEngineering.Analyzers/Obsolescence/CorrectObsolescenceCodeFix.cs index 321be75..ea262ed 100644 --- a/src/SubtleEngineering.Analyzers/Obsolescence/CorrectObsolescenceCodeFix.cs +++ b/src/SubtleEngineering.Analyzers/Obsolescence/CorrectObsolescenceCodeFix.cs @@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CodeActions; + using Microsoft.CodeAnalysis.Formatting; [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(CorrectObsolescenceCodeFix)), Shared] public class CorrectObsolescenceCodeFix : CodeFixProvider @@ -97,21 +98,78 @@ private async Task AddEditorBrowsableAttributeAsync(Document document, private SyntaxNode AddUsingDirectiveIfMissing(SyntaxNode root, string namespaceName) { + var editorBrowsableNamespace = SyntaxFactory.ParseName(namespaceName); + + // Check for existing using directives at the compilation unit level if (root is CompilationUnitSyntax compilationUnit) { var hasUsingDirective = compilationUnit.Usings.Any(u => u.Name.ToString() == namespaceName); - if (!hasUsingDirective) + if (hasUsingDirective) + { + // The using directive already exists at the compilation unit level + return root; + } + + // Check if there are any existing using directives at the compilation unit level + if (compilationUnit.Usings.Any()) { - var usingDirective = SyntaxFactory.UsingDirective(SyntaxFactory.ParseName(namespaceName)); - //.WithTrailingTrivia(SyntaxFactory.CarriageReturnLineFeed); + // Add the using directive after the existing usings at the compilation unit level + var newUsing = SyntaxFactory.UsingDirective(editorBrowsableNamespace); + var newUsings = compilationUnit.Usings.Add(newUsing); + var newCompilationUnit = compilationUnit.WithUsings(newUsings) + .WithAdditionalAnnotations(Formatter.Annotation); - compilationUnit = compilationUnit.AddUsings(usingDirective); - return compilationUnit; + return newCompilationUnit; } + + // No usings at the compilation unit level, check inside namespace declarations + var firstNamespace = compilationUnit.Members.OfType().FirstOrDefault(); + if (firstNamespace != null) + { + var newFirstNamespace = AddUsingToNamespace(firstNamespace, namespaceName); + var newRoot = root.ReplaceNode(firstNamespace, newFirstNamespace); + return newRoot; + } + + // No namespaces, add using directive at the compilation unit level after any leading trivia (e.g., comments) + var leadingTrivia = compilationUnit.GetLeadingTrivia(); + compilationUnit = compilationUnit.WithoutLeadingTrivia(); + + var newUsingWhenNoNamespace = SyntaxFactory.UsingDirective(editorBrowsableNamespace); + compilationUnit = compilationUnit.AddUsings(newUsingWhenNoNamespace) + .WithAdditionalAnnotations(Formatter.Annotation); + + return compilationUnit; } + return root; } + private NamespaceDeclarationSyntax AddUsingToNamespace(NamespaceDeclarationSyntax namespaceDeclaration, string namespaceName) + { + var editorBrowsableNamespace = SyntaxFactory.ParseName(namespaceName); + + var hasUsingDirective = namespaceDeclaration.Usings.Any(u => u.Name.ToString() == namespaceName); + if (hasUsingDirective) + { + // The using directive already exists inside the namespace + return namespaceDeclaration; + } + + // Create the new using directive + var newUsing = SyntaxFactory.UsingDirective(editorBrowsableNamespace); + + // Add the new using directive to the namespace's using directives + // Place it after existing using directives, or at the top if there are none + var newUsings = namespaceDeclaration.Usings.Add(newUsing); + + // Update the namespace declaration with the new usings + var newNamespaceDeclaration = namespaceDeclaration.WithUsings(newUsings) + .WithAdditionalAnnotations(Formatter.Annotation); + + return newNamespaceDeclaration; + } + private SyntaxNode AddAttributeToDeclaration(SyntaxNode declarationNode, AttributeListSyntax attributeList) { switch (declarationNode)