Ensure we remove project references before adding them in a batch

It's possible to have a batch remove and re-add a project reference to the same project, even if you're removing different reference paths. If that happened, we'd try to add the duplicate reference first which would crash. Fixes #39032 Fixes #43632
dotnet · Apr 30, 2020 · 6bc0538 · 6bc0538
1 parent 17fd0b6
commit 6bc0538
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 27 deletions.
diff --git a/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs b/src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs
@@ -427,6 +427,35 @@ private void OnBatchScopeDisposed()
                         (s, ids) => s.RemoveAnalyzerConfigDocuments(ids),
                         WorkspaceChangeKind.AnalyzerConfigDocumentRemoved);
 
+                    // Metadata reference removing. Do this before adding in case this removes a project reference that
+                    // we are also going to add in the same batch. This could happen if case is changing, or we're targeting
+                    // a different output path (say bin vs. obj vs. ref).
+                    foreach (var (path, properties) in _metadataReferencesRemovedInBatch)
+                    {
+                        var projectReference = _workspace.TryRemoveConvertedProjectReference_NoLock(Id, path, properties);
+
+                        if (projectReference != null)
+                        {
+                            solutionChanges.UpdateSolutionForProjectAction(
+                                Id,
+                                solutionChanges.Solution.RemoveProjectReference(Id, projectReference));
+                        }
+                        else
+                        {
+                            // TODO: find a cleaner way to fetch this
+                            var metadataReference = _workspace.CurrentSolution.GetRequiredProject(Id).MetadataReferences.Cast<PortableExecutableReference>()
+                                                                                    .Single(m => m.FilePath == path && m.Properties == properties);
+
+                            _workspace.FileWatchedReferenceFactory.StopWatchingReference(metadataReference);
+
+                            solutionChanges.UpdateSolutionForProjectAction(
+                                Id,
+                                newSolution: solutionChanges.Solution.RemoveMetadataReference(Id, metadataReference));
+                        }
+                    }
+
+                    ClearAndZeroCapacity(_metadataReferencesRemovedInBatch);
+
                     // Metadata reference adding...
                     if (_metadataReferencesAddedInBatch.Count > 0)
                     {
@@ -456,33 +485,6 @@ private void OnBatchScopeDisposed()
                         ClearAndZeroCapacity(_metadataReferencesAddedInBatch);
                     }
 
-                    // Metadata reference removing...
-                    foreach (var (path, properties) in _metadataReferencesRemovedInBatch)
-                    {
-                        var projectReference = _workspace.TryRemoveConvertedProjectReference_NoLock(Id, path, properties);
-
-                        if (projectReference != null)
-                        {
-                            solutionChanges.UpdateSolutionForProjectAction(
-                                Id,
-                                solutionChanges.Solution.RemoveProjectReference(Id, projectReference));
-                        }
-                        else
-                        {
-                            // TODO: find a cleaner way to fetch this
-                            var metadataReference = _workspace.CurrentSolution.GetRequiredProject(Id).MetadataReferences.Cast<PortableExecutableReference>()
-                                                                                    .Single(m => m.FilePath == path && m.Properties == properties);
-
-                            _workspace.FileWatchedReferenceFactory.StopWatchingReference(metadataReference);
-
-                            solutionChanges.UpdateSolutionForProjectAction(
-                                Id,
-                                newSolution: solutionChanges.Solution.RemoveMetadataReference(Id, metadataReference));
-                        }
-                    }
-
-                    ClearAndZeroCapacity(_metadataReferencesRemovedInBatch);
-
                     // Project reference adding...
                     solutionChanges.UpdateSolutionForProjectAction(
                         Id,

diff --git a/...t/ProjectSystemShim/VisualStudioProjectTests/MetadataToProjectReferenceConversionTests.vb b/...t/ProjectSystemShim/VisualStudioProjectTests/MetadataToProjectReferenceConversionTests.vb
@@ -234,5 +234,33 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
                 Assert.Empty(getReferencingProject().MetadataReferences)
             End Using
         End Sub
+
+        <WpfFact>
+        <WorkItem(39032, "https://github.com/dotnet/roslyn/issues/39032")>
+        <WorkItem(43632, "https://github.com/dotnet/roslyn/issues/43632")>
+        Public Sub RemoveAndReAddReferenceInSingleBatchWhileChangingCase()
+            Using environment = New TestEnvironment()
+                Dim referencingProject = environment.ProjectFactory.CreateAndAddToWorkspace("referencingProject", LanguageNames.CSharp)
+                Dim referencedProject = environment.ProjectFactory.CreateAndAddToWorkspace("referencedProject", LanguageNames.CSharp)
+
+                Const ReferencePath = "C:\project.dll"
+                referencingProject.AddMetadataReference(ReferencePath, MetadataReferenceProperties.Assembly)
+                referencedProject.OutputFilePath = ReferencePath
+
+                Dim getReferencingProject = Function() environment.Workspace.CurrentSolution.GetProject(referencingProject.Id)
+
+                Assert.Single(getReferencingProject().ProjectReferences)
+                Assert.Empty(getReferencingProject().MetadataReferences)
+
+                Using referencingProject.CreateBatchScope()
+                    referencingProject.RemoveMetadataReference(ReferencePath, MetadataReferenceProperties.Assembly)
+                    referencingProject.AddMetadataReference(ReferencePath.ToUpper(), MetadataReferenceProperties.Assembly)
+                End Using
+
+                ' We should still have a project reference
+                Assert.Single(getReferencingProject().ProjectReferences)
+                Assert.Empty(getReferencingProject().MetadataReferences)
+            End Using
+        End Sub
     End Class
 End Namespace