Ordering syntax trees

src/VisualStudio/CSharp/Test/ProjectSystemShim/CPS/SourceFileHandlingTests.cs

@@ -67,6 +67,7 @@ public void ReorderSourceFiles_CPS()
             using (var project = CreateCSharpCPSProject(environment, "project1"))
             {
                 IEnumerable<Document> GetCurrentDocuments() => environment.Workspace.CurrentSolution.Projects.Single().Documents;
+                VersionStamp GetVersion() => environment.Workspace.CurrentSolution.Projects.Single().Version;
 
                 Assert.Empty(GetCurrentDocuments());
 
@@ -87,15 +88,27 @@ public void ReorderSourceFiles_CPS()
                 project.RemoveSourceFile(sourceFileFullPath4);
                 project.AddSourceFile(sourceFileFullPath4);
 
-                project.ReorderSourceFiles(new[] { sourceFileFullPath1, sourceFileFullPath2, sourceFileFullPath3, sourceFileFullPath4, sourceFileFullPath5 });
+                var oldVersion = GetVersion();
+
+                project.ReorderSourceFiles(new[] { sourceFileFullPath5, sourceFileFullPath4, sourceFileFullPath3, sourceFileFullPath2, sourceFileFullPath1 });
+
+                var newVersion = GetVersion();
+
+                project.ReorderSourceFiles(new[] { sourceFileFullPath5, sourceFileFullPath4, sourceFileFullPath3, sourceFileFullPath2, sourceFileFullPath1 });
+
+                var newSameVersion = GetVersion();
+
+                // Reordering should result in a new version if the order is different. If it's the same, the version should stay the same.
+                Assert.NotEqual(oldVersion, newVersion);
+                Assert.Equal(newVersion, newSameVersion);
 
                 var documents = GetCurrentDocuments().ToArray();
 
-                Assert.Equal(documents[0].FilePath, sourceFileFullPath1, StringComparer.OrdinalIgnoreCase);
-                Assert.Equal(documents[1].FilePath, sourceFileFullPath2, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[0].FilePath, sourceFileFullPath5, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[1].FilePath, sourceFileFullPath4, StringComparer.OrdinalIgnoreCase);
                 Assert.Equal(documents[2].FilePath, sourceFileFullPath3, StringComparer.OrdinalIgnoreCase);
-                Assert.Equal(documents[3].FilePath, sourceFileFullPath4, StringComparer.OrdinalIgnoreCase);
-                Assert.Equal(documents[4].FilePath, sourceFileFullPath5, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[3].FilePath, sourceFileFullPath2, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[4].FilePath, sourceFileFullPath1, StringComparer.OrdinalIgnoreCase);
             }
         }
 
@@ -131,15 +144,15 @@ public void ReorderSourceFilesBatch_CPS()
                 // Removing path4 to test remove of a file when it was also added in a batch.
                 project.RemoveSourceFile(sourceFileFullPath4);
 
-                project.ReorderSourceFiles(new[] { sourceFileFullPath1, sourceFileFullPath3, sourceFileFullPath5 });
+                project.ReorderSourceFiles(new[] { sourceFileFullPath5, sourceFileFullPath3, sourceFileFullPath1 });
 
                 project.EndBatch();
 
                 var documents = GetCurrentDocuments().ToArray();
 
-                Assert.Equal(documents[0].FilePath, sourceFileFullPath1, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[0].FilePath, sourceFileFullPath5, StringComparer.OrdinalIgnoreCase);
                 Assert.Equal(documents[1].FilePath, sourceFileFullPath3, StringComparer.OrdinalIgnoreCase);
-                Assert.Equal(documents[2].FilePath, sourceFileFullPath5, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[2].FilePath, sourceFileFullPath1, StringComparer.OrdinalIgnoreCase);
             }
         }
 
@@ -175,7 +188,7 @@ public void ReorderSourceFilesBatchWithReAdding_CPS()
                 // Removing path4 to test remove of a file when it was also added in a batch.
                 project.RemoveSourceFile(sourceFileFullPath4);
 
-                project.ReorderSourceFiles(new[] { sourceFileFullPath1, sourceFileFullPath3, sourceFileFullPath5 });
+                project.ReorderSourceFiles(new[] { sourceFileFullPath5, sourceFileFullPath3, sourceFileFullPath1 });
 
                 // Re-adding / re-removing / re-adding again.
                 project.AddSourceFile(sourceFileFullPath2);
@@ -185,17 +198,17 @@ public void ReorderSourceFilesBatchWithReAdding_CPS()
                 project.AddSourceFile(sourceFileFullPath2);
                 project.AddSourceFile(sourceFileFullPath4);
 
-                project.ReorderSourceFiles(new[] { sourceFileFullPath1, sourceFileFullPath2, sourceFileFullPath3, sourceFileFullPath4, sourceFileFullPath5 });
+                project.ReorderSourceFiles(new[] { sourceFileFullPath5, sourceFileFullPath4, sourceFileFullPath3, sourceFileFullPath2, sourceFileFullPath1 });
 
                 project.EndBatch();
 
                 var documents = GetCurrentDocuments().ToArray();
 
-                Assert.Equal(documents[0].FilePath, sourceFileFullPath1, StringComparer.OrdinalIgnoreCase);
-                Assert.Equal(documents[1].FilePath, sourceFileFullPath2, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[0].FilePath, sourceFileFullPath5, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[1].FilePath, sourceFileFullPath4, StringComparer.OrdinalIgnoreCase);
                 Assert.Equal(documents[2].FilePath, sourceFileFullPath3, StringComparer.OrdinalIgnoreCase);
-                Assert.Equal(documents[3].FilePath, sourceFileFullPath4, StringComparer.OrdinalIgnoreCase);
-                Assert.Equal(documents[4].FilePath, sourceFileFullPath5, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[3].FilePath, sourceFileFullPath2, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[4].FilePath, sourceFileFullPath1, StringComparer.OrdinalIgnoreCase);
             }
         }
 
@@ -221,7 +234,7 @@ public void ReorderSourceFilesBatchAddAfterReorder_CPS()
                 project.AddSourceFile(sourceFileFullPath1);
                 project.AddSourceFile(sourceFileFullPath2);
 
-                project.ReorderSourceFiles(new[] { sourceFileFullPath1, sourceFileFullPath2 });
+                project.ReorderSourceFiles(new[] { sourceFileFullPath2, sourceFileFullPath1 });
 
                 project.AddSourceFile(sourceFileFullPath3);
                 project.AddSourceFile(sourceFileFullPath4);
@@ -229,15 +242,15 @@ public void ReorderSourceFilesBatchAddAfterReorder_CPS()
 
                 project.EndBatch();
 
-                project.ReorderSourceFiles(new[] { sourceFileFullPath1, sourceFileFullPath2, sourceFileFullPath3, sourceFileFullPath4, sourceFileFullPath5 });
+                project.ReorderSourceFiles(new[] { sourceFileFullPath5, sourceFileFullPath4, sourceFileFullPath3, sourceFileFullPath2, sourceFileFullPath1 });
 
                 var documents = GetCurrentDocuments().ToArray();
 
-                Assert.Equal(documents[0].FilePath, sourceFileFullPath1, StringComparer.OrdinalIgnoreCase);
-                Assert.Equal(documents[1].FilePath, sourceFileFullPath2, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[0].FilePath, sourceFileFullPath5, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[1].FilePath, sourceFileFullPath4, StringComparer.OrdinalIgnoreCase);
                 Assert.Equal(documents[2].FilePath, sourceFileFullPath3, StringComparer.OrdinalIgnoreCase);
-                Assert.Equal(documents[3].FilePath, sourceFileFullPath4, StringComparer.OrdinalIgnoreCase);
-                Assert.Equal(documents[4].FilePath, sourceFileFullPath5, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[3].FilePath, sourceFileFullPath2, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[4].FilePath, sourceFileFullPath1, StringComparer.OrdinalIgnoreCase);
             }
         }
 
@@ -266,7 +279,7 @@ public void ReorderSourceFilesBatchRemoveAfterReorder_CPS()
                 project.AddSourceFile(sourceFileFullPath4);
                 project.AddSourceFile(sourceFileFullPath5);
 
-                project.ReorderSourceFiles(new[] { sourceFileFullPath1, sourceFileFullPath2, sourceFileFullPath3, sourceFileFullPath4, sourceFileFullPath5 });
+                project.ReorderSourceFiles(new[] { sourceFileFullPath5, sourceFileFullPath4, sourceFileFullPath3, sourceFileFullPath2, sourceFileFullPath1 });
 
                 project.RemoveSourceFile(sourceFileFullPath3);
                 project.RemoveSourceFile(sourceFileFullPath4);
@@ -274,12 +287,12 @@ public void ReorderSourceFilesBatchRemoveAfterReorder_CPS()
 
                 project.EndBatch();
 
-                project.ReorderSourceFiles(new[] { sourceFileFullPath1, sourceFileFullPath2 });
+                project.ReorderSourceFiles(new[] { sourceFileFullPath2, sourceFileFullPath1 });
 
                 var documents = GetCurrentDocuments().ToArray();
 
-                Assert.Equal(documents[0].FilePath, sourceFileFullPath1, StringComparer.OrdinalIgnoreCase);
-                Assert.Equal(documents[1].FilePath, sourceFileFullPath2, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[0].FilePath, sourceFileFullPath2, StringComparer.OrdinalIgnoreCase);
+                Assert.Equal(documents[1].FilePath, sourceFileFullPath1, StringComparer.OrdinalIgnoreCase);
             }
         }
 

src/VisualStudio/Core/Def/Implementation/ProjectSystem/VisualStudioProject.cs

@@ -1080,9 +1080,9 @@ private sealed class BatchingDocumentCollection
             private readonly List<DocumentId> _documentsRemovedInBatch = new List<DocumentId>();
 
             /// <summary>
-            /// The current
+            /// The current list of document file paths that will be ordered in a batch.
             /// </summary>
-            private Nullable<ImmutableArray<string>> _orderedFilesInBatch = null;
+            private List<string> _orderedFilesInBatch = null;
 
             private readonly Func<Solution, DocumentId, bool> _documentAlreadyInWorkspace;
             private readonly Action<Workspace, DocumentInfo> _documentAddAction;
@@ -1124,8 +1124,8 @@ public DocumentId AddFile(string fullPath, SourceCodeKind sourceCodeKind, Immuta
                         throw new ArgumentException($"'{fullPath}' has already been added to this project.", nameof(fullPath));
                     }
 
-                    // Adding a file messes up the ordering and does not gaurantee any kind of order, so invalidate any ordered files in batch.
-                    _orderedFilesInBatch = null;
+                    // If we have an ordered files batch, we need to add the file to the end of it as well.
+                    _orderedFilesInBatch?.Add(fullPath);
 
                     _documentPathsToDocumentIds.Add(fullPath, documentId);
                     _project._documentFileWatchingTokens.Add(documentId, _project._documentFileChangeContext.EnqueueWatchingFile(fullPath));
@@ -1213,8 +1213,8 @@ public void AddDynamicFile(IDynamicFileInfoProvider fileInfoProvider, DynamicFil
                         throw new ArgumentException($"'{filePath}' has already been added to this project.", nameof(filePath));
                     }
 
-                    // Adding a dynamic file messes up the ordering and does not gaurantee any kind of order, so invalidate any ordered files in batch.
-                    _orderedFilesInBatch = null;
+                    // If we have an ordered files batch, we need to add the file to the end of it as well.
+                    _orderedFilesInBatch?.Add(filePath);
 
                     _documentPathsToDocumentIds.Add(filePath, documentId);
 
@@ -1284,8 +1284,17 @@ public void RemoveFile(string fullPath)
 
             private void RemoveFileInternal(DocumentId documentId, string fullPath)
             {
-                // Removing a file messes up the ordering and does not gaurantee any kind of order, so invalidate any ordered files in batch.
-                _orderedFilesInBatch = null;
+                // If we have an ordered files batch, we need to remove the file from the batch.
+                if (_orderedFilesInBatch != null)
+                {
+                    for (var i = 0; i < _orderedFilesInBatch.Count; i++)
+                    {
+                        if (_orderedFilesInBatch[i].Equals(fullPath, StringComparison.OrdinalIgnoreCase))
+                        {
+                            _orderedFilesInBatch.RemoveAt(i);
+                        }
+                    }
+                }
 
                 _documentPathsToDocumentIds.Remove(fullPath);
 
@@ -1483,7 +1492,7 @@ public void ReorderFiles(ImmutableArray<string> filePaths)
 
                     if (_project._activeBatchScopes > 0)
                     {
-                        _orderedFilesInBatch = filePaths;
+                        _orderedFilesInBatch = new List<string>(filePaths);
                     }
                     else
                     {
@@ -1523,31 +1532,23 @@ public void ReorderFiles(ImmutableArray<string> filePaths)
                 ClearAndZeroCapacity(_documentsRemovedInBatch);
 
                 // Update project's order of documents.
-                solution = UpdateProjectDocumentsOrderForBatch(solution);
-
-                return solution;
-            }
-
-            internal Solution UpdateProjectDocumentsOrderForBatch(Solution solution)
-            {
-                // The action of ordering documents was not included in the batch. Simply return the solution.
-                if (_orderedFilesInBatch == null)
+                if (_orderedFilesInBatch != null)
                 {
-                    return solution;
-                }
+                    var orderedFiles = _orderedFilesInBatch;
 
-                var orderedFiles = _orderedFilesInBatch;
+                    _orderedFilesInBatch = null;
 
-                _orderedFilesInBatch = null;
+                    solution = UpdateProjectDocumentsOrder(solution, orderedFiles);
+                }
 
-                return UpdateProjectDocumentsOrder(solution, orderedFiles);
+                return solution;
             }
 
             private Solution UpdateProjectDocumentsOrder(Solution solution, IEnumerable<string> filePaths)
             {
                 var projectId = _project.Id;
                 var documentIds =
-                    filePaths.Select(x => solution.GetDocumentIdsWithFilePath(x).Single(id => id.ProjectId == projectId));
+                    filePaths.Select(x => _documentPathsToDocumentIds[x]);
 
                 return solution.WithProjectDocumentsOrder(projectId, documentIds.ToImmutableList());
             }

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

@@ -683,15 +683,30 @@ public ProjectState UpdateDocumentsOrder(ImmutableList<DocumentId> documentIds)
                 throw new ArgumentException($"The specified documents do not equal the project document count.", nameof(documentIds));
             }
 
-            foreach (var documentId in documentIds)
+            var hasOrderChanged = false;
+
+            for (var i = 0; i < documentIds.Count; ++i)
             {
-                if (!this.ContainsDocument(documentId))
+                var documentId = documentIds[i];
+
+                if (!ContainsDocument(documentId))
                 {
                     throw new InvalidOperationException($"The document '{documentId}' does not exist in the project.");
                 }
+
+                if (DocumentIds[i] != documentId)
+                {
+                    hasOrderChanged = true;
+                }
+            }
+
+            if (!hasOrderChanged)
+            {
+                return this;
             }
 
             return this.With(
+                projectInfo: this.ProjectInfo.WithVersion(this.Version.GetNewerVersion()),
                 documentIds: documentIds);
         }
 

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

@@ -946,7 +946,12 @@ public SolutionState WithProjectDocumentsOrder(ProjectId projectId, ImmutableLis
             var oldProject = this.GetProjectState(projectId);
             var newProject = oldProject.UpdateDocumentsOrder(documentIds);
 
-            return this.ForkProject(newProject);
+            if (oldProject == newProject)
+            {
+                return this;
+            }
+
+            return this.ForkProject(newProject, CompilationTranslationAction.ProjectParseOptions(newProject));
         }
 
         /// <summary>

src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs

@@ -1526,7 +1526,14 @@ public void TestUpdateDocumentsOrder()
             var solution = CreateSolution();
             var pid = ProjectId.CreateNewId();
 
-            Func<ImmutableArray<Document>> GetDocuments = () => solution.GetProject(pid).Documents.ToImmutableArray();
+            Func<VersionStamp> GetVersion = () => solution.GetProject(pid).Version;
+            Func<ImmutableArray<DocumentId>> GetDocumentIds = () => solution.GetProject(pid).DocumentIds.ToImmutableArray();
+            Func<DocumentId, Document> GetDocument = documentId => solution.GetProject(pid).GetDocument(documentId);
+            Func<ImmutableArray<SyntaxTree>> GetSyntaxTrees = () =>
+                {
+                    var cancellationToken = new CancellationToken();
+                    return solution.State.GetCompilationAsync(solution.GetProject(pid).State, cancellationToken).Result.SyntaxTrees.ToImmutableArray();
+                };
 
             solution = solution.AddProject(pid, "test", "test.dll", LanguageNames.CSharp);
 
@@ -1550,15 +1557,39 @@ public void TestUpdateDocumentsOrder()
             var did5 = DocumentId.CreateNewId(pid);
             solution = solution.AddDocument(did5, "test5.cs", text5);
 
+            var oldVersion = GetVersion();
+
+            solution = solution.WithProjectDocumentsOrder(pid, ImmutableList.CreateRange(new[] { did5, did4, did3, did2, did1 }));
+
+            var newVersion = GetVersion();
+
+            // Make sure we have a new version because the order changed.
+            Assert.NotEqual(oldVersion, newVersion);
+
+            var documentIds = GetDocumentIds();
+
+            Assert.Equal(did5, documentIds[0]);
+            Assert.Equal(did4, documentIds[1]);
+            Assert.Equal(did3, documentIds[2]);
+            Assert.Equal(did2, documentIds[3]);
+            Assert.Equal(did1, documentIds[4]);
+
+            var syntaxTrees = GetSyntaxTrees();
+
+            Assert.Equal(documentIds.Count(), syntaxTrees.Count());
+
+            Assert.Equal("test5.cs", syntaxTrees[0].FilePath, StringComparer.OrdinalIgnoreCase);
+            Assert.Equal("test4.cs", syntaxTrees[1].FilePath, StringComparer.OrdinalIgnoreCase);
+            Assert.Equal("test3.cs", syntaxTrees[2].FilePath, StringComparer.OrdinalIgnoreCase);
+            Assert.Equal("test2.cs", syntaxTrees[3].FilePath, StringComparer.OrdinalIgnoreCase);
+            Assert.Equal("test1.cs", syntaxTrees[4].FilePath, StringComparer.OrdinalIgnoreCase);
+
             solution = solution.WithProjectDocumentsOrder(pid, ImmutableList.CreateRange(new[] { did5, did4, did3, did2, did1 }));
 
-            var documents = GetDocuments();
+            var newSameVersion = GetVersion();
 
-            Assert.Equal("test5.cs", documents[0].Name, StringComparer.OrdinalIgnoreCase);
-            Assert.Equal("test4.cs", documents[1].Name, StringComparer.OrdinalIgnoreCase);
-            Assert.Equal("test3.cs", documents[2].Name, StringComparer.OrdinalIgnoreCase);
-            Assert.Equal("test2.cs", documents[3].Name, StringComparer.OrdinalIgnoreCase);
-            Assert.Equal("test1.cs", documents[4].Name, StringComparer.OrdinalIgnoreCase);
+            // Make sure we have the same new version because the order hasn't changed.
+            Assert.Equal(newVersion, newSameVersion);
         }
 
         [Fact, Trait(Traits.Feature, Traits.Features.Workspace)]