Allow Document.FilePath to be set to null

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

@@ -383,41 +383,37 @@ async Task<SemanticModel> GetSemanticModelWorkerAsync()
     /// Creates a new instance of this document updated to have the source code kind specified.
     /// </summary>
     public Document WithSourceCodeKind(SourceCodeKind kind)
-        => this.Project.Solution.WithDocumentSourceCodeKind(this.Id, kind).GetDocument(this.Id)!;
+        => this.Project.Solution.WithDocumentSourceCodeKind(this.Id, kind).GetRequiredDocument(Id);
 
     /// <summary>
     /// Creates a new instance of this document updated to have the text specified.
     /// </summary>
     public Document WithText(SourceText text)
-        => this.Project.Solution.WithDocumentText(this.Id, text, PreservationMode.PreserveIdentity).GetDocument(this.Id)!;
+        => this.Project.Solution.WithDocumentText(this.Id, text, PreservationMode.PreserveIdentity).GetRequiredDocument(Id);
 
     /// <summary>
     /// Creates a new instance of this document updated to have a syntax tree rooted by the specified syntax node.
     /// </summary>
     public Document WithSyntaxRoot(SyntaxNode root)
-        => this.Project.Solution.WithDocumentSyntaxRoot(this.Id, root, PreservationMode.PreserveIdentity).GetDocument(this.Id)!;
+        => this.Project.Solution.WithDocumentSyntaxRoot(this.Id, root, PreservationMode.PreserveIdentity).GetRequiredDocument(Id);
 
     /// <summary>
     /// Creates a new instance of this document updated to have the specified name.
     /// </summary>
     public Document WithName(string name)
-        => this.Project.Solution.WithDocumentName(this.Id, name).GetDocument(this.Id)!;
+        => this.Project.Solution.WithDocumentName(this.Id, name).GetRequiredDocument(Id);
 
     /// <summary>
     /// Creates a new instance of this document updated to have the specified folders.
     /// </summary>
     public Document WithFolders(IEnumerable<string> folders)
-        => this.Project.Solution.WithDocumentFolders(this.Id, folders).GetDocument(this.Id)!;
+        => this.Project.Solution.WithDocumentFolders(this.Id, folders).GetRequiredDocument(Id);
 
     /// <summary>
     /// Creates a new instance of this document updated to have the specified file path.
     /// </summary>
-    /// <param name="filePath"></param>
-    // TODO (https://github.com/dotnet/roslyn/issues/37125): Solution.WithDocumentFilePath will throw if
-    // filePath is null, but it's odd because we *do* support null file paths. Why can't you switch a
-    // document back to null?
-    public Document WithFilePath(string filePath)
-        => this.Project.Solution.WithDocumentFilePath(this.Id, filePath).GetDocument(this.Id)!;
+    public Document WithFilePath(string? filePath)
+        => this.Project.Solution.WithDocumentFilePath(this.Id, filePath).GetRequiredDocument(Id);
 
     /// <summary>
     /// Get the text changes between this document and a prior version of the same document.

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

@@ -350,15 +350,7 @@ public DocumentState UpdateParseOptions(ParseOptions options, bool onlyPreproces
 
     private DocumentState SetParseOptions(ParseOptions options, bool onlyPreprocessorDirectiveChange)
     {
-        if (options == null)
-        {
-            throw new ArgumentNullException(nameof(options));
-        }
-
-        if (!SupportsSyntaxTree)
-        {
-            throw new InvalidOperationException();
-        }
+        Contract.ThrowIfFalse(SupportsSyntaxTree);
 
         ITreeAndVersionSource? newTreeSource = null;
 
@@ -413,42 +405,37 @@ public DocumentState UpdateSourceCodeKind(SourceCodeKind kind)
         return this.SetParseOptions(this.ParseOptions.WithKind(kind), onlyPreprocessorDirectiveChange: false);
     }
 
-    // TODO: https://github.com/dotnet/roslyn/issues/37125
-    // if FilePath is null, then this will change the name of the underlying tree, but we aren't producing a new tree in that case.
     public DocumentState UpdateName(string name)
         => UpdateAttributes(Attributes.With(name: name));
 
+    public DocumentState UpdateFilePath(string? path)
+        => UpdateAttributes(Attributes.With(filePath: path));
+
     public DocumentState UpdateFolders(IReadOnlyList<string> folders)
         => UpdateAttributes(Attributes.With(folders: folders));
 
-    private DocumentState UpdateAttributes(DocumentInfo.DocumentAttributes attributes)
-    {
-        Debug.Assert(attributes != Attributes);
-
-        return new DocumentState(
-            LanguageServices,
-            Services,
-            attributes,
-            _options,
-            TextAndVersionSource,
-            LoadTextOptions,
-            _treeSource);
-    }
-
-    public DocumentState UpdateFilePath(string? filePath)
+    private DocumentState UpdateAttributes(DocumentInfo.DocumentAttributes newAttributes)
     {
-        var newAttributes = Attributes.With(filePath: filePath);
         Debug.Assert(newAttributes != Attributes);
 
-        // TODO: it's overkill to fully reparse the tree if we had the tree already; all we have to do is update the
-        // file path and diagnostic options for that tree.
-        var newTreeSource = SupportsSyntaxTree ?
-            CreateLazyFullyParsedTree(
-                TextAndVersionSource,
-                LoadTextOptions,
-                newAttributes.SyntaxTreeFilePath,
-                _options,
-                LanguageServices) : null;
+        ITreeAndVersionSource? newTreeSource;
+
+        if (newAttributes.SyntaxTreeFilePath != Attributes.SyntaxTreeFilePath)
+        {
+            // TODO: it's overkill to fully reparse the tree if we had the tree already; all we have to do is update the
+            // file path and diagnostic options for that tree.
+            newTreeSource = SupportsSyntaxTree ?
+                CreateLazyFullyParsedTree(
+                    TextAndVersionSource,
+                    LoadTextOptions,
+                    newAttributes.SyntaxTreeFilePath,
+                    _options,
+                    LanguageServices) : null;
+        }
+        else
+        {
+            newTreeSource = _treeSource;
+        }
 
         return new DocumentState(
             LanguageServices,

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

@@ -1169,18 +1169,9 @@ public Solution WithDocumentFolders(DocumentId documentId, IEnumerable<string>?
     /// <summary>
     /// Creates a new solution instance with the document specified updated to have the specified file path.
     /// </summary>
-    public Solution WithDocumentFilePath(DocumentId documentId, string filePath)
+    public Solution WithDocumentFilePath(DocumentId documentId, string? filePath)
     {
         CheckContainsDocument(documentId);
-
-        // TODO (https://github.com/dotnet/roslyn/issues/37125): 
-        // We *do* support null file paths. Why can't you switch a document back to null?
-        // See DocumentState.GetSyntaxTreeFilePath
-        if (filePath == null)
-        {
-            throw new ArgumentNullException(nameof(filePath));
-        }
-
         return WithCompilationState(_compilationState.WithDocumentFilePath(documentId, filePath));
     }
 

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

@@ -1088,10 +1088,7 @@ protected internal void OnDocumentInfoChanged(DocumentId documentId, DocumentInf
 
                 if (oldAttributes.FilePath != newInfo.FilePath)
                 {
-                    // TODO (https://github.com/dotnet/roslyn/issues/37125): Solution.WithDocumentFilePath will throw if
-                    // filePath is null, but it's odd because we *do* support null file paths. The suppression here is to silence it
-                    // but should be removed when the bug is fixed.
-                    newSolution = newSolution.WithDocumentFilePath(documentId, newInfo.FilePath!);
+                    newSolution = newSolution.WithDocumentFilePath(documentId, newInfo.FilePath);
                 }
 
                 if (oldAttributes.SourceCodeKind != newInfo.SourceCodeKind)

src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs

@@ -226,19 +226,19 @@ public void WithDocumentFilePath()
             var path = "new path";
 
             var newSolution1 = solution.WithDocumentFilePath(documentId, path);
-            Assert.Equal(path, newSolution1.GetDocument(documentId)!.FilePath);
+            Assert.Equal(path, newSolution1.GetRequiredDocument(documentId).FilePath);
             AssertEx.Equal(new[] { documentId }, newSolution1.GetDocumentIdsWithFilePath(path));
 
             var newSolution2 = newSolution1.WithDocumentFilePath(documentId, path);
             Assert.Same(newSolution1, newSolution2);
 
-            // empty path (TODO https://github.com/dotnet/roslyn/issues/37125):
             var newSolution3 = solution.WithDocumentFilePath(documentId, "");
-            Assert.Equal("", newSolution3.GetDocument(documentId)!.FilePath);
+            Assert.Equal("", newSolution3.GetRequiredDocument(documentId).FilePath);
             Assert.Empty(newSolution3.GetDocumentIdsWithFilePath(""));
 
-            // TODO: https://github.com/dotnet/roslyn/issues/37125
-            Assert.Throws<ArgumentNullException>(() => solution.WithDocumentFilePath(documentId, filePath: null!));
+            var newSolution4 = solution.WithDocumentFilePath(documentId, null);
+            Assert.Null(newSolution4.GetRequiredDocument(documentId).FilePath);
+            Assert.Empty(newSolution4.GetDocumentIdsWithFilePath(null));
 
             Assert.Throws<ArgumentNullException>(() => solution.WithDocumentFilePath(null!, path));
             Assert.Throws<InvalidOperationException>(() => solution.WithDocumentFilePath(s_unrelatedDocumentId, path));
@@ -1341,6 +1341,67 @@ public async Task ChangingPreprocessorDirectivesMayReparse(string source, bool e
             Assert.Equal(expectReuse, oldRoot.IsIncrementallyIdenticalTo(newTree.GetRoot()));
         }
 
+        [Theory]
+        [InlineData(null, "test.cs")]
+        [InlineData("test.cs", null)]
+        [InlineData("", null)]
+        [InlineData("test.cs", "")]
+        public async Task ChangingFilePathReparses(string oldPath, string newPath)
+        {
+            var projectId = ProjectId.CreateNewId();
+            var documentId = DocumentId.CreateNewId(projectId);
+
+            using var workspace = CreateWorkspace();
+            var document = workspace.CurrentSolution
+                            .AddProject(projectId, "proj1", "proj1.dll", LanguageNames.CSharp)
+                            .AddDocument(documentId, name: "Test.cs", text: "// File", filePath: oldPath)
+                            .GetRequiredDocument(documentId);
+
+            var oldTree = await document.GetRequiredSyntaxTreeAsync(CancellationToken.None);
+            var newDocument = document.WithFilePath(newPath);
+            var newTree = await newDocument.GetRequiredSyntaxTreeAsync(CancellationToken.None);
+
+            Assert.False(oldTree.GetRoot().IsIncrementallyIdenticalTo(newTree.GetRoot()));
+        }
+
+        [Fact]
+        public async Task ChangingName_ReparsesWhenPathIsNull()
+        {
+            var projectId = ProjectId.CreateNewId();
+            var documentId = DocumentId.CreateNewId(projectId);
+
+            using var workspace = CreateWorkspace();
+            var document = workspace.CurrentSolution
+                            .AddProject(projectId, "proj1", "proj1.dll", LanguageNames.CSharp)
+                            .AddDocument(documentId, name: "name1", text: "// File", filePath: null)
+                            .GetRequiredDocument(documentId);
+
+            var oldTree = await document.GetRequiredSyntaxTreeAsync(CancellationToken.None);
+            var newDocument = document.WithName("name2");
+            var newTree = await newDocument.GetRequiredSyntaxTreeAsync(CancellationToken.None);
+
+            Assert.False(oldTree.GetRoot().IsIncrementallyIdenticalTo(newTree.GetRoot()));
+        }
+
+        [Fact]
+        public async Task ChangingName_NoReparse()
+        {
+            var projectId = ProjectId.CreateNewId();
+            var documentId = DocumentId.CreateNewId(projectId);
+
+            using var workspace = CreateWorkspace();
+            var document = workspace.CurrentSolution
+                            .AddProject(projectId, "proj1", "proj1.dll", LanguageNames.CSharp)
+                            .AddDocument(documentId, name: "name1", text: "// File", filePath: "")
+                            .GetRequiredDocument(documentId);
+
+            var oldTree = await document.GetRequiredSyntaxTreeAsync(CancellationToken.None);
+            var newDocument = document.WithName("name2");
+            var newTree = await newDocument.GetRequiredSyntaxTreeAsync(CancellationToken.None);
+
+            Assert.True(oldTree.GetRoot().IsIncrementallyIdenticalTo(newTree.GetRoot()));
+        }
+
         [Fact]
         public void WithProjectReferences()
         {