Fix StringAssertToAssertFixer to swap StringComparison and message arguments by @Youssef1313 in #6481 (backport to rel/3.10) (#6487)

Co-authored-by: Youssef1313 <youssefvictor00@gmail.com>

Author: youssef-backport-bot

src/Analyzers/MSTest.Analyzers.CodeFixes/StringAssertToAssertFixer.cs

@@ -11,7 +11,6 @@
 using Microsoft.CodeAnalysis.CodeFixes;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
-using Microsoft.CodeAnalysis.Editing;
 
 using MSTest.Analyzers.Helpers;
 
@@ -78,13 +77,30 @@ private static async Task<Document> FixStringAssertAsync(
             return document;
         }
 
-        DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
+        SyntaxNode root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
 
         // Create new argument list with swapped first two arguments
-        ArgumentSyntax[] newArguments = [.. arguments];
-        (newArguments[0], newArguments[1]) = (newArguments[1], newArguments[0]);
+        // We keep the existing separators in case there is trivia attached to them.
+        var newArguments = arguments.GetWithSeparators().ToList();
+        // NOTE: Index 1 has the "separator" (comma) between the first and second arguments.
+        (newArguments[0], newArguments[2]) = (newArguments[2], newArguments[0]);
+
+        // StringAssert has overloads with parameter types (string, string, string, StringComparison)
+        // In Assert class, these are (string, string, StringComparison, string)
+        // So we need to do the swap.
+        if (arguments.Count >= 4)
+        {
+            SemanticModel semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+            if (semanticModel.GetSymbolInfo(invocationExpr, cancellationToken).Symbol is IMethodSymbol stringAssertMethod &&
+                stringAssertMethod.Parameters.Length >= 4 &&
+                stringAssertMethod.Parameters[2].Type.SpecialType == SpecialType.System_String &&
+                stringAssertMethod.Parameters[3].Type.Name == "StringComparison")
+            {
+                (newArguments[4], newArguments[6]) = (newArguments[6], newArguments[4]);
+            }
+        }
 
-        ArgumentListSyntax newArgumentList = SyntaxFactory.ArgumentList(SyntaxFactory.SeparatedList(newArguments));
+        ArgumentListSyntax newArgumentList = invocationExpr.ArgumentList.WithArguments(SyntaxFactory.SeparatedList<ArgumentSyntax>(newArguments));
         InvocationExpressionSyntax newInvocationExpr = invocationExpr.WithArgumentList(newArgumentList);
 
         // Replace StringAssert with Assert in the member access expression
@@ -93,7 +109,9 @@ private static async Task<Document> FixStringAssertAsync(
             .WithName(SyntaxFactory.IdentifierName(properAssertMethodName));
         newInvocationExpr = newInvocationExpr.WithExpression(newMemberAccess);
 
-        editor.ReplaceNode(invocationExpr, newInvocationExpr);
-        return editor.GetChangedDocument();
+        // Preserve leading trivia (including empty lines) from the original invocation
+        newInvocationExpr = newInvocationExpr.WithLeadingTrivia(invocationExpr.GetLeadingTrivia());
+
+        return document.WithSyntaxRoot(root.ReplaceNode(invocationExpr, newInvocationExpr));
     }
 }

test/UnitTests/MSTest.Analyzers.UnitTests/StringAssertToAssertAnalyzerTests.cs

@@ -14,6 +14,7 @@ public sealed class StringAssertToAssertAnalyzerTests
     public async Task WhenStringAssertContains()
     {
         string code = """
+            using System;
             using Microsoft.VisualStudio.TestTools.UnitTesting;
 
             [TestClass]
@@ -25,11 +26,17 @@ public void MyTestMethod()
                     string value = "Hello World";
                     string substring = "World";
                     {|#0:StringAssert.Contains(value, substring)|};
+                    {|#1:StringAssert.Contains(value, substring, "message")|};
+                    {|#2:StringAssert.Contains(value, substring, StringComparison.Ordinal)|};
+                    {|#3:StringAssert.Contains(value, substring, "message", StringComparison.Ordinal)|};
+                    {|#4:StringAssert.Contains(value, substring, "message {0}", "arg")|};
+                    {|#5:StringAssert.Contains(value, substring, "message {0}", StringComparison.Ordinal, "arg")|};
                 }
             }
             """;
 
         string fixedCode = """
+            using System;
             using Microsoft.VisualStudio.TestTools.UnitTesting;
 
             [TestClass]
@@ -41,14 +48,31 @@ public void MyTestMethod()
                     string value = "Hello World";
                     string substring = "World";
                     Assert.Contains(substring, value);
+                    Assert.Contains(substring, value, "message");
+                    Assert.Contains(substring, value, StringComparison.Ordinal);
+                    Assert.Contains(substring, value, StringComparison.Ordinal, "message");
+                    Assert.Contains(substring, value, "message {0}", "arg");
+                    Assert.Contains(substring, value, StringComparison.Ordinal, "message {0}", "arg");
                 }
             }
             """;
 
         await VerifyCS.VerifyCodeFixAsync(
             code,
-            // /0/Test0.cs(11,9): info MSTEST0046: Use 'Assert.Contains' instead of 'StringAssert.Contains'
-            VerifyCS.DiagnosticIgnoringAdditionalLocations().WithLocation(0).WithArguments("Contains", "Contains"),
+            [
+                // /0/Test0.cs(12,9): info MSTEST0046: Use 'Assert.Contains' instead of 'StringAssert.Contains'
+                VerifyCS.DiagnosticIgnoringAdditionalLocations().WithLocation(0).WithArguments("Contains", "Contains"),
+                // /0/Test0.cs(13,9): info MSTEST0046: Use 'Assert.Contains' instead of 'StringAssert.Contains'
+                VerifyCS.DiagnosticIgnoringAdditionalLocations().WithLocation(1).WithArguments("Contains", "Contains"),
+                // /0/Test0.cs(14,9): info MSTEST0046: Use 'Assert.Contains' instead of 'StringAssert.Contains'
+                VerifyCS.DiagnosticIgnoringAdditionalLocations().WithLocation(2).WithArguments("Contains", "Contains"),
+                // /0/Test0.cs(15,9): info MSTEST0046: Use 'Assert.Contains' instead of 'StringAssert.Contains'
+                VerifyCS.DiagnosticIgnoringAdditionalLocations().WithLocation(3).WithArguments("Contains", "Contains"),
+                // /0/Test0.cs(16,9): info MSTEST0046: Use 'Assert.Contains' instead of 'StringAssert.Contains'
+                VerifyCS.DiagnosticIgnoringAdditionalLocations().WithLocation(4).WithArguments("Contains", "Contains"),
+                // /0/Test0.cs(17,9): info MSTEST0046: Use 'Assert.Contains' instead of 'StringAssert.Contains'
+                VerifyCS.DiagnosticIgnoringAdditionalLocations().WithLocation(5).WithArguments("Contains", "Contains"),
+            ],
             fixedCode);
     }