Update SA1649CodeFixProvider to remove linked files before re-adding …

…them (to not end up with two identical files in those projects) and also to re-add files with correct path (to not get a copy where there was a link before)
DotNetAnalyzers · Jun 19, 2024 · 4f91b3e · 4f91b3e
1 parent 51c772e
commit 4f91b3e
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 10 deletions.
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/DocumentationRules/SA1649CodeFixProvider.cs b/StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/DocumentationRules/SA1649CodeFixProvider.cs
@@ -53,24 +53,30 @@ private static async Task<Solution> GetTransformedSolutionAsync(Document documen
         {
             var solution = document.Project.Solution;
             var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
-
             var expectedFileName = diagnostic.Properties[SA1649FileNameMustMatchTypeName.ExpectedFileNameKey];
-            var newPath = document.FilePath != null ? Path.Combine(Path.GetDirectoryName(document.FilePath), expectedFileName) : null;
-
-            var newDocumentId = DocumentId.CreateNewId(document.Id.ProjectId);
 
-            var newSolution = solution
-                .RemoveDocument(document.Id)
-                .AddDocument(newDocumentId, expectedFileName, syntaxRoot, document.Folders, newPath);
+            var newSolution = ReplaceDocument(solution, document, document.Id, syntaxRoot, expectedFileName);
 
-            // Make sure to also add the file to linked projects
+            // Make sure to also replace the file in linked projects
             foreach (var linkedDocumentId in document.GetLinkedDocumentIds())
             {
-                DocumentId linkedExtractedDocumentId = DocumentId.CreateNewId(linkedDocumentId.ProjectId);
-                newSolution = newSolution.AddDocument(linkedExtractedDocumentId, expectedFileName, syntaxRoot, document.Folders);
+                newSolution = ReplaceDocument(newSolution, null, linkedDocumentId, syntaxRoot, expectedFileName);
             }
 
             return newSolution;
         }
+
+        private static Solution ReplaceDocument(Solution solution, Document document, DocumentId documentId, SyntaxNode syntaxRoot, string expectedFileName)
+        {
+            document ??= solution.GetDocument(documentId);
+
+            var newDocumentFilePath = document.FilePath != null ? Path.Combine(Path.GetDirectoryName(document.FilePath), expectedFileName) : null;
+            var newDocumentId = DocumentId.CreateNewId(documentId.ProjectId);
+
+            var newSolution = solution
+                .RemoveDocument(documentId)
+                .AddDocument(newDocumentId, expectedFileName, syntaxRoot, document.Folders, newDocumentFilePath);
+            return newSolution;
+        }
     }
 }
diff --git a/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1649UnitTests.cs b/StyleCop.Analyzers/StyleCop.Analyzers.Test/DocumentationRules/SA1649UnitTests.cs
@@ -5,6 +5,7 @@
 
 namespace StyleCop.Analyzers.Test.DocumentationRules
 {
+    using System.IO;
     using System.Threading;
     using System.Threading.Tasks;
     using Microsoft.CodeAnalysis.Testing;
@@ -487,6 +488,59 @@ public class Class2
             await VerifyCSharpDiagnosticAsync("Class1.cs", testCode, testSettings: null, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
         }
 
+        [Fact]
+        [WorkItem(1693, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/1693")]
+        [WorkItem(1665, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/1665")]
+        public async Task VerifyWithLinkedFileAsync()
+        {
+            var dirName = "0";
+            var testCode = "public class [|Type1|] { }";
+
+            await new StyleCopCodeFixVerifier<SA1649FileNameMustMatchTypeName, SA1649CodeFixProvider>.CSharpTest()
+            {
+                TestState =
+                {
+                    Sources =
+                    {
+                        (BuildPath(dirName, "TestFile.cs"), testCode),
+                    },
+                    AdditionalProjects =
+                    {
+                        ["Project2"] =
+                        {
+                            Sources =
+                            {
+                                (BuildPath(dirName, "TestFile.cs"), testCode),
+                            },
+                        },
+                    },
+                },
+                FixedState =
+                {
+                    Sources =
+                    {
+                        (BuildPath(dirName, "Type1.cs"), testCode),
+                    },
+                    AdditionalProjects =
+                    {
+                        ["Project2"] =
+                        {
+                            Sources =
+                            {
+                                (BuildPath(dirName, "Type1.cs"), testCode),
+                            },
+                        },
+                    },
+                },
+
+                // Fails without this. Hard to be sure why this is needed, but one guess could be
+                // that the test framework does not respect the fact that both projects point to
+                // the same file, and only inserts '#pragma warning disable' in the primary project's file.
+                // Then we would still get a diagnostic in the additional project.
+                TestBehaviors = TestBehaviors.SkipSuppressionCheck,
+            }.RunAsync().ConfigureAwait(false);
+        }
+
         protected static string GetTypeDeclaration(string typeKind, string typeName, int? diagnosticKey = null)
         {
             if (diagnosticKey is not null)
@@ -550,5 +604,14 @@ protected static Task VerifyCSharpFixAsync(string oldFileName, string source, st
             test.ExpectedDiagnostics.AddRange(expected);
             return test.RunAsync(cancellationToken);
         }
+
+        // NOTE: Added to simplify the tests. After the fix has executed,
+        // the file paths will contain backslashes when running tests on Windows.
+        // Not really needed when setting up the test state, but handy in the fixed state.
+        // Might make tests pass on Linux if anyone is developing there.
+        private static string BuildPath(string part1, string part2)
+        {
+            return Path.Combine(part1, part2);
+        }
     }
 }