Update SA1649CodeFixProvider to first remove the file in other projec…

…ts which reference the same file before adding them there (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) #1693 #3866
DotNetAnalyzers · Jul 2, 2024 · 282c5e1 · 282c5e1
1 parent ff5c432
commit 282c5e1
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 update other projects which reference the same file
             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(3866, "https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3866")]
+        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, since the error message is not so good,
+                // 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);
+        }
     }
 }