Merge pull request #43527 from jasonmalinowski/fix-api-review-issues

Fix API issues identified during API review

src/Workspaces/Core/Portable/PublicAPI.Unshipped.txt

@@ -7,6 +7,8 @@ Microsoft.CodeAnalysis.CompilationOutputFilePaths.AssemblyPath.get -> string
 Microsoft.CodeAnalysis.CompilationOutputFilePaths.Equals(Microsoft.CodeAnalysis.CompilationOutputFilePaths other) -> bool
 Microsoft.CodeAnalysis.CompilationOutputFilePaths.WithAssemblyPath(string path) -> Microsoft.CodeAnalysis.CompilationOutputFilePaths
 Microsoft.CodeAnalysis.Project.CompilationOutputFilePaths.get -> Microsoft.CodeAnalysis.CompilationOutputFilePaths
+Microsoft.CodeAnalysis.Project.RemoveAdditionalDocuments(System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DocumentId> documentIds) -> Microsoft.CodeAnalysis.Project
+Microsoft.CodeAnalysis.Project.RemoveAnalyzerConfigDocuments(System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.DocumentId> documentIds) -> Microsoft.CodeAnalysis.Project
 Microsoft.CodeAnalysis.ProjectInfo.CompilationOutputFilePaths.get -> Microsoft.CodeAnalysis.CompilationOutputFilePaths
 Microsoft.CodeAnalysis.ProjectInfo.WithCompilationOutputFilePaths(in Microsoft.CodeAnalysis.CompilationOutputFilePaths paths) -> Microsoft.CodeAnalysis.ProjectInfo
 Microsoft.CodeAnalysis.Rename.Renamer.RenameDocumentAction

src/Workspaces/Core/Portable/Workspace/Solution/Project.cs

@@ -555,14 +555,7 @@ public Project RemoveDocument(DocumentId documentId)
         /// </summary>
         public Project RemoveDocuments(ImmutableArray<DocumentId> documentIds)
         {
-            foreach (var documentId in documentIds)
-            {
-                // Handling of null entries is handled by Solution.RemoveDocuments.
-                if (documentId?.ProjectId != this.Id)
-                {
-                    throw new ArgumentException(string.Format(WorkspacesResources._0_is_in_a_different_project, documentId));
-                }
-            }
+            CheckIdsContainedInProject(documentIds);
 
             return this.Solution.RemoveDocuments(documentIds).GetRequiredProject(this.Id);
         }
@@ -571,14 +564,50 @@ public Project RemoveDocuments(ImmutableArray<DocumentId> documentIds)
         /// Creates a new instance of this project updated to no longer include the specified additional document.
         /// </summary>
         public Project RemoveAdditionalDocument(DocumentId documentId)
+            // NOTE: the method isn't checking if documentId belongs to the project. This probably should be done, but may be a compat change.
+            // https://github.com/dotnet/roslyn/issues/41211 tracks this investigation.
             => this.Solution.RemoveAdditionalDocument(documentId).GetProject(this.Id)!;
 
+        /// <summary>
+        /// Creates a new instance of this project updated to no longer include the specified additional documents.
+        /// </summary>
+        public Project RemoveAdditionalDocuments(ImmutableArray<DocumentId> documentIds)
+        {
+            CheckIdsContainedInProject(documentIds);
+
+            return this.Solution.RemoveAdditionalDocuments(documentIds).GetRequiredProject(this.Id);
+        }
+
         /// <summary>
         /// Creates a new instance of this project updated to no longer include the specified analyzer config document.
         /// </summary>
         public Project RemoveAnalyzerConfigDocument(DocumentId documentId)
+            // NOTE: the method isn't checking if documentId belongs to the project. This probably should be done, but may be a compat change.
+            // https://github.com/dotnet/roslyn/issues/41211 tracks this investigation.
             => this.Solution.RemoveAnalyzerConfigDocument(documentId).GetProject(this.Id)!;
 
+        /// <summary>
+        /// Creates a new solution instance that no longer includes the specified <see cref="AnalyzerConfigDocument"/>s.
+        /// </summary>
+        public Project RemoveAnalyzerConfigDocuments(ImmutableArray<DocumentId> documentIds)
+        {
+            CheckIdsContainedInProject(documentIds);
+
+            return this.Solution.RemoveAnalyzerConfigDocuments(documentIds).GetRequiredProject(this.Id);
+        }
+
+        private void CheckIdsContainedInProject(ImmutableArray<DocumentId> documentIds)
+        {
+            foreach (var documentId in documentIds)
+            {
+                // Dealing with nulls is handled by the caller of this
+                if (documentId?.ProjectId != this.Id)
+                {
+                    throw new ArgumentException(string.Format(WorkspacesResources._0_is_in_a_different_project, documentId));
+                }
+            }
+        }
+
         internal ImmutableDictionary<string, ReportDiagnostic> GetAnalyzerConfigSpecialDiagnosticOptions()
             => _projectState.GetAnalyzerConfigSpecialDiagnosticOptions();
 

src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs

@@ -1251,6 +1251,42 @@ public void RemoveDocumentFromUnrelatedProject()
             Assert.Throws<ArgumentException>(() => solution.GetProject(projectId2).RemoveDocuments(ImmutableArray.Create(documentInfo1.Id)));
         }
 
+        [Fact]
+        public void RemoveAdditionalDocumentFromUnrelatedProject()
+        {
+            var projectId1 = ProjectId.CreateNewId();
+            var projectId2 = ProjectId.CreateNewId();
+
+            var documentInfo1 = DocumentInfo.Create(DocumentId.CreateNewId(projectId1), "file1.txt");
+
+            var solution = CreateSolution()
+                .AddProject(projectId1, "project1", "project1.dll", LanguageNames.CSharp)
+                .AddProject(projectId2, "project2", "project2.dll", LanguageNames.CSharp)
+                .AddAdditionalDocument(documentInfo1);
+
+            // This should throw if we're removing one document from the wrong project. Right now we don't test the RemoveAdditionalDocument
+            // API due to https://github.com/dotnet/roslyn/issues/41211.
+            Assert.Throws<ArgumentException>(() => solution.GetProject(projectId2).RemoveAdditionalDocuments(ImmutableArray.Create(documentInfo1.Id)));
+        }
+
+        [Fact]
+        public void RemoveAnalyzerConfigDocumentFromUnrelatedProject()
+        {
+            var projectId1 = ProjectId.CreateNewId();
+            var projectId2 = ProjectId.CreateNewId();
+
+            var documentInfo1 = DocumentInfo.Create(DocumentId.CreateNewId(projectId1), ".editorconfig");
+
+            var solution = CreateSolution()
+                .AddProject(projectId1, "project1", "project1.dll", LanguageNames.CSharp)
+                .AddProject(projectId2, "project2", "project2.dll", LanguageNames.CSharp)
+                .AddAnalyzerConfigDocuments(ImmutableArray.Create(documentInfo1));
+
+            // This should throw if we're removing one document from the wrong project. Right now we don't test the RemoveAdditionalDocument
+            // API due to https://github.com/dotnet/roslyn/issues/41211.
+            Assert.Throws<ArgumentException>(() => solution.GetProject(projectId2).RemoveAnalyzerConfigDocuments(ImmutableArray.Create(documentInfo1.Id)));
+        }
+
         [Fact]
         public async Task TestOneCSharpProjectAsync()
         {