Merge pull request #18890 from dpoeschl/LinkedFileRace

Ensure linked files are kept in sync in the workspace

src/EditorFeatures/CSharpTest/Workspaces/WorkspaceTests.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
+using System.Composition;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -11,11 +13,10 @@
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Shared.TestHooks;
 using Microsoft.CodeAnalysis.Text;
+using Microsoft.VisualStudio.Composition;
 using Microsoft.VisualStudio.Text;
 using Roslyn.Test.Utilities;
 using Xunit;
-using System.Composition;
-using Microsoft.VisualStudio.Composition;
 
 namespace Microsoft.CodeAnalysis.UnitTests.Workspaces
 {
@@ -1003,5 +1004,51 @@ public void TestAdditionalFile_AddRemove_FromProject()
                 Assert.Equal("original.config", workspace.CurrentSolution.GetProject(project1.Id).AdditionalDocuments.Single().Name);
             }
         }
+
+        [Fact, WorkItem(209299, "https://devdiv.visualstudio.com/DevDiv/_workitems?id=209299")]
+        public async Task TestLinkedFilesStayInSync()
+        {
+            var originalText = "class Program1 { }";
+            var updatedText = "class Program2 { }";
+
+            var input = $@"
+<Workspace>
+    <Project Language=""C#"" AssemblyName=""Assembly1"" CommonReferences=""true"">
+        <Document FilePath=""Test.cs"">{ originalText }</Document>
+    </Project>
+    <Project Language=""C#"" AssemblyName=""Assembly2"" CommonReferences=""true"">
+        <Document IsLinkFile=""true"" LinkAssemblyName=""Assembly1"" LinkFilePath=""Test.cs"" />
+    </Project>
+</Workspace>";
+
+            using (var workspace = TestWorkspace.Create(input, exportProvider: s_exportProvider.Value))
+            {
+                var eventArgs = new List<WorkspaceChangeEventArgs>();
+
+                workspace.WorkspaceChanged += (s, e) =>
+                {
+                    Assert.Equal(WorkspaceChangeKind.DocumentChanged, e.Kind);
+                    eventArgs.Add(e);
+                };
+
+                var originalDocumentId = workspace.GetOpenDocumentIds().Single(id => !workspace.GetTestDocument(id).IsLinkFile);
+                var linkedDocumentId = workspace.GetOpenDocumentIds().Single(id => workspace.GetTestDocument(id).IsLinkFile);
+
+                workspace.GetTestDocument(originalDocumentId).Update(SourceText.From("class Program2 { }"));
+                await WaitForWorkspaceOperationsToComplete(workspace);
+
+                Assert.Equal(2, eventArgs.Count);
+                AssertEx.SetEqual(workspace.Projects.SelectMany(p => p.Documents).Select(d => d.Id), eventArgs.Select(e => e.DocumentId));
+
+                Assert.Equal(eventArgs[0].OldSolution, eventArgs[1].OldSolution);
+                Assert.Equal(eventArgs[0].NewSolution, eventArgs[1].NewSolution);
+
+                Assert.Equal(originalText, (await eventArgs[0].OldSolution.GetDocument(originalDocumentId).GetTextAsync().ConfigureAwait(false)).ToString());
+                Assert.Equal(originalText, (await eventArgs[1].OldSolution.GetDocument(originalDocumentId).GetTextAsync().ConfigureAwait(false)).ToString());
+
+                Assert.Equal(updatedText, (await eventArgs[0].NewSolution.GetDocument(originalDocumentId).GetTextAsync().ConfigureAwait(false)).ToString());
+                Assert.Equal(updatedText, (await eventArgs[1].NewSolution.GetDocument(originalDocumentId).GetTextAsync().ConfigureAwait(false)).ToString());
+            }
+        }
     }
 }

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

@@ -162,6 +162,10 @@ public Solution CurrentSolution
         /// <summary>
         /// Sets the <see cref="CurrentSolution"/> of this workspace. This method does not raise a <see cref="WorkspaceChanged"/> event.
         /// </summary>
+        /// <remarks>
+        /// This method does not guarantee that linked files will have the same contents. Callers 
+        /// should enforce that policy before passing in the new solution.
+        /// </remarks>
         protected Solution SetCurrentSolution(Solution solution)
         {
             var currentSolution = Volatile.Read(ref _latestSolution);
@@ -830,35 +834,97 @@ protected internal void OnDocumentInfoChanged(DocumentId documentId, DocumentInf
         /// </summary>
         protected internal void OnDocumentTextChanged(DocumentId documentId, SourceText newText, PreservationMode mode)
         {
-            using (_serializationLock.DisposableWait())
-            {
-                CheckDocumentIsInCurrentSolution(documentId);
-
-                var oldSolution = this.CurrentSolution;
-                var newSolution = this.SetCurrentSolution(oldSolution.WithDocumentText(documentId, newText, mode));
-
-                var newDocument = newSolution.GetDocument(documentId);
-                this.OnDocumentTextChanged(newDocument);
-
-                this.RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind.DocumentChanged, oldSolution, newSolution, documentId: documentId);
-            }
+            OnAnyDocumentTextChanged(
+                documentId,
+                newText,
+                mode,
+                CheckDocumentIsInCurrentSolution,
+                (solution, docId) => solution.GetRelatedDocumentIds(docId),
+                (solution, docId, text, preservationMode) => solution.WithDocumentText(docId, text, preservationMode),
+                WorkspaceChangeKind.DocumentChanged,
+                isCodeDocument: true);
         }
 
         /// <summary>
         /// Call this method when the text of a document is updated in the host environment.
         /// </summary>
         protected internal void OnAdditionalDocumentTextChanged(DocumentId documentId, SourceText newText, PreservationMode mode)
+        {
+            OnAnyDocumentTextChanged(
+                documentId,
+                newText,
+                mode,
+                CheckAdditionalDocumentIsInCurrentSolution,
+                (solution, docId) => ImmutableArray.Create(docId), // We do not support the concept of linked additional documents
+                (solution, docId, text, preservationMode) => solution.WithAdditionalDocumentText(docId, text, preservationMode),
+                WorkspaceChangeKind.AdditionalDocumentChanged,
+                isCodeDocument: false);
+        }
+
+        /// <summary>
+        /// When a <see cref="Document"/>s text is changed, we need to make sure all of the linked
+        /// files also have their content updated in the new solution before applying it to the
+        /// workspace to avoid the workspace having solutions with linked files where the contents
+        /// do not match.
+        /// </summary>
+        private void OnAnyDocumentTextChanged(
+            DocumentId documentId, 
+            SourceText newText, 
+            PreservationMode mode, 
+            Action<DocumentId> checkIsInCurrentSolution,
+            Func<Solution, DocumentId, ImmutableArray<DocumentId>> getRelatedDocuments,
+            Func<Solution, DocumentId, SourceText, PreservationMode, Solution> updateSolutionWithText,
+            WorkspaceChangeKind changeKind,
+            bool isCodeDocument)
         {
             using (_serializationLock.DisposableWait())
             {
-                CheckAdditionalDocumentIsInCurrentSolution(documentId);
+                checkIsInCurrentSolution(documentId);
 
-                var oldSolution = this.CurrentSolution;
-                var newSolution = this.SetCurrentSolution(oldSolution.WithAdditionalDocumentText(documentId, newText, mode));
+                var originalSolution = CurrentSolution;
+                var updatedSolution = CurrentSolution;
+                var previousSolution = updatedSolution;
 
-                var newDocument = newSolution.GetAdditionalDocument(documentId);
+                var linkedDocuments = getRelatedDocuments(updatedSolution, documentId);
+                var updatedDocumentIds = new List<DocumentId>();
 
-                this.RaiseWorkspaceChangedEventAsync(WorkspaceChangeKind.AdditionalDocumentChanged, oldSolution, newSolution, documentId: documentId);
+                foreach (var linkedDocument in linkedDocuments)
+                {
+                    previousSolution = updatedSolution;
+                    updatedSolution = updateSolutionWithText(updatedSolution, linkedDocument, newText, mode);
+                    if (previousSolution != updatedSolution)
+                    {
+                        updatedDocumentIds.Add(linkedDocument);
+                    }
+                }
+
+                // In the case of linked files, we may have already updated all of the linked
+                // documents during an earlier call to this method. We may have no work to do here.
+                if (updatedDocumentIds.Count > 0)
+                {
+                    var newSolution = SetCurrentSolution(updatedSolution);
+
+                    // Prior to the unification of the callers of this method, the
+                    // OnAdditionalDocumentTextChanged method did not fire any sort of synchronous
+                    // update notification event, so we preserve that behavior here.
+                    if (isCodeDocument)
+                    {
+                        foreach (var updatedDocumentId in updatedDocumentIds)
+                        {
+                            var newDocument = newSolution.GetDocument(updatedDocumentId);
+                            OnDocumentTextChanged(newDocument);
+                        }
+                    }
+
+                    foreach (var updatedDocumentInfo in updatedDocumentIds)
+                    {
+                        RaiseWorkspaceChangedEventAsync(
+                            changeKind,
+                            originalSolution,
+                            newSolution,
+                            documentId: updatedDocumentInfo);
+                    }
+                }
             }
         }
 

src/Workspaces/Core/Portable/Workspace/WorkspaceChangeEventArgs.cs

@@ -4,11 +4,32 @@
 
 namespace Microsoft.CodeAnalysis
 {
+    /// <summary>
+    /// The <see cref="EventArgs"/> describing any kind of workspace change.
+    /// </summary>
+    /// <remarks>
+    /// When linked files are edited, one document change event is fired per linked file. All of
+    /// these events contain the same <see cref="OldSolution"/>, and they all contain the same
+    /// <see cref="NewSolution"/>. This is so that we can trigger document change events on all
+    /// affected documents without reporting intermediate states in which the linked file contents
+    /// do not match.
+    /// </remarks>
     public class WorkspaceChangeEventArgs : EventArgs
     {
         public WorkspaceChangeKind Kind { get; }
+
+        /// <remarks>
+        /// If linked documents are being changed, there may be multiple events with the same
+        /// <see cref="OldSolution"/> and <see cref="NewSolution"/>.
+        /// </remarks>
         public Solution OldSolution { get; }
+
+        /// <remarks>
+        /// If linked documents are being changed, there may be multiple events with the same
+        /// <see cref="OldSolution"/> and <see cref="NewSolution"/>.
+        /// </remarks>
         public Solution NewSolution { get; }
+
         public ProjectId ProjectId { get; }
         public DocumentId DocumentId { get; }
 

src/Workspaces/Core/Portable/Workspace/WorkspaceChangeKind.cs

@@ -68,6 +68,14 @@ public enum WorkspaceChangeKind
         /// <summary>
         /// A document in the current solution was changed.
         /// </summary>
+        /// <remarks>
+        /// When linked files are edited, one <see cref="DocumentChanged"/> event is fired per
+        /// linked file. All of these events contain the same OldSolution, and they all contain
+        /// the same NewSolution. This is so that we can trigger document change events on all
+        /// affected documents without reporting intermediate states in which the linked file
+        /// contents do not match. Each <see cref="DocumentChanged"/> event does not represent
+        /// an incremental update from the previous event in this special case.
+        /// </remarks>
         DocumentChanged = 12,
 
         /// <summary>