Fix MSTEST0002 codefix to not change modifier order (#6479)

Author: Youssef1313

src/Analyzers/MSTest.Analyzers.CodeFixes/TestClassShouldBeValidFixer.cs

@@ -9,7 +9,6 @@
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CodeActions;
 using Microsoft.CodeAnalysis.CodeFixes;
-using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Editing;
 using Microsoft.CodeAnalysis.Text;
@@ -62,32 +61,24 @@ private static async Task<Document> FixClassDeclarationAsync(Document document,
     {
         cancellationToken.ThrowIfCancellationRequested();
 
-        // Get the SemanticModel and Compilation
+        SyntaxNode root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
         SemanticModel semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
         bool canDiscoverInternals = semanticModel.Compilation.CanDiscoverInternals();
 
-        DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
+        var generator = SyntaxGenerator.GetGenerator(document);
 
-        // Remove the static modifier if it exists
-        SyntaxTokenList modifiers = SyntaxFactory.TokenList(
-            typeDeclaration.Modifiers.Where(modifier => !modifier.IsKind(SyntaxKind.StaticKeyword)));
+        Accessibility existingAccessibility = generator.GetAccessibility(typeDeclaration);
+        bool isGoodAccessibility = existingAccessibility == Accessibility.Public ||
+            (canDiscoverInternals && existingAccessibility == Accessibility.Internal);
 
-        if (!typeDeclaration.Modifiers.Any(SyntaxKind.PublicKeyword))
-        {
-            // Determine the visibility modifier
-            SyntaxToken visibilityModifier = canDiscoverInternals
-                ? SyntaxFactory.Token(SyntaxKind.InternalKeyword)
-                : SyntaxFactory.Token(SyntaxKind.PublicKeyword);
+        SyntaxNode newTypeDeclaration = generator
+            .WithModifiers(typeDeclaration, generator.GetModifiers(typeDeclaration).WithIsStatic(false));
 
-            modifiers = SyntaxFactory.TokenList(
-                modifiers.Where(modifier => !modifier.IsKind(SyntaxKind.PrivateKeyword) && !modifier.IsKind(SyntaxKind.InternalKeyword))).Add(visibilityModifier);
+        if (!isGoodAccessibility)
+        {
+            newTypeDeclaration = generator.WithAccessibility(newTypeDeclaration, Accessibility.Public);
         }
 
-        // Create a new class declaration with the updated modifiers.
-        TypeDeclarationSyntax newTypeDeclaration = typeDeclaration.WithModifiers(modifiers);
-        editor.ReplaceNode(typeDeclaration, newTypeDeclaration);
-        SyntaxNode newRoot = editor.GetChangedRoot();
-
-        return document.WithSyntaxRoot(newRoot);
+        return document.WithSyntaxRoot(root.ReplaceNode(typeDeclaration, newTypeDeclaration));
     }
 }

test/UnitTests/MSTest.Analyzers.UnitTests/TestClassShouldBeValidAnalyzerTests.cs

@@ -192,6 +192,7 @@ internal class MyTestClass
     [TestMethod]
     public async Task WhenDiscoverInternalsAndTypeIsPrivate_Diagnostic()
     {
+        // NOTE: As we are fixing the modifier, we still prefer to make it public, even if discover internals is set.
         string code = """
             using Microsoft.VisualStudio.TestTools.UnitTesting;
 
@@ -214,7 +215,7 @@ private class {|#0:MyTestClass|}
             public class A
             {
                 [TestClass]
-                internal class MyTestClass
+                public class MyTestClass
                 {
                 }
             }
@@ -460,4 +461,88 @@ await VerifyCS.VerifyCodeFixAsync(
                 .WithArguments("MyTestClass"),
             fixedCode);
     }
+
+    [TestMethod]
+    public async Task WhenClassIsInternalSealed_Diagnostic()
+    {
+        string code = """
+            using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+            namespace MyNamespace
+            {
+                [TestClass]
+                internal sealed class {|#0:MyTestClass|}
+                {
+                    [TestMethod]
+                    public void TestMethod()
+                    {
+                    }
+                }
+            }
+            """;
+        string fixedCode = """
+            using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+            namespace MyNamespace
+            {
+                [TestClass]
+                public sealed class MyTestClass
+                {
+                    [TestMethod]
+                    public void TestMethod()
+                    {
+                    }
+                }
+            }
+            """;
+
+        await VerifyCS.VerifyCodeFixAsync(
+            code,
+            VerifyCS.Diagnostic(TestClassShouldBeValidAnalyzer.TestClassShouldBeValidRule)
+                .WithLocation(0)
+                .WithArguments("MyTestClass"),
+            fixedCode);
+    }
+
+    [TestMethod]
+    public async Task WhenClassIsInternalStatic_Diagnostic()
+    {
+        string code = """
+            using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+            namespace MyNamespace
+            {
+                [TestClass]
+                internal static class {|#0:MyTestClass|}
+                {
+                    [TestMethod]
+                    public static void TestMethod()
+                    {
+                    }
+                }
+            }
+            """;
+        string fixedCode = """
+            using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+            namespace MyNamespace
+            {
+                [TestClass]
+                public class MyTestClass
+                {
+                    [TestMethod]
+                    public static void TestMethod()
+                    {
+                    }
+                }
+            }
+            """;
+
+        await VerifyCS.VerifyCodeFixAsync(
+            code,
+            VerifyCS.Diagnostic(TestClassShouldBeValidAnalyzer.TestClassShouldBeValidRule)
+                .WithLocation(0)
+                .WithArguments("MyTestClass"),
+            fixedCode);
+    }
 }