diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Document.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Document.cs index ba601240cab4c..0d7fa9786fed0 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Document.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Document.cs @@ -383,41 +383,37 @@ async Task GetSemanticModelWorkerAsync() /// Creates a new instance of this document updated to have the source code kind specified. /// public Document WithSourceCodeKind(SourceCodeKind kind) - => this.Project.Solution.WithDocumentSourceCodeKind(this.Id, kind).GetDocument(this.Id)!; + => this.Project.Solution.WithDocumentSourceCodeKind(this.Id, kind).GetRequiredDocument(Id); /// /// Creates a new instance of this document updated to have the text specified. /// 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); /// /// Creates a new instance of this document updated to have a syntax tree rooted by the specified syntax node. /// 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); /// /// Creates a new instance of this document updated to have the specified name. /// public Document WithName(string name) - => this.Project.Solution.WithDocumentName(this.Id, name).GetDocument(this.Id)!; + => this.Project.Solution.WithDocumentName(this.Id, name).GetRequiredDocument(Id); /// /// Creates a new instance of this document updated to have the specified folders. /// public Document WithFolders(IEnumerable folders) - => this.Project.Solution.WithDocumentFolders(this.Id, folders).GetDocument(this.Id)!; + => this.Project.Solution.WithDocumentFolders(this.Id, folders).GetRequiredDocument(Id); /// /// Creates a new instance of this document updated to have the specified file path. /// - /// - // 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); /// /// Get the text changes between this document and a prior version of the same document. diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs index 5eaacf839d0b6..285e12ba49633 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentState.cs +++ b/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 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, diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs index e102e911df007..438640169d706 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs @@ -1169,18 +1169,9 @@ public Solution WithDocumentFolders(DocumentId documentId, IEnumerable? /// /// Creates a new solution instance with the document specified updated to have the specified file path. /// - 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)); } diff --git a/src/Workspaces/Core/Portable/Workspace/Workspace.cs b/src/Workspaces/Core/Portable/Workspace/Workspace.cs index 6bf7e5efe4372..7f7e86d82952e 100644 --- a/src/Workspaces/Core/Portable/Workspace/Workspace.cs +++ b/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) diff --git a/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs b/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs index 2bf869fa41587..b068688cc7798 100644 --- a/src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs +++ b/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(() => solution.WithDocumentFilePath(documentId, filePath: null!)); + var newSolution4 = solution.WithDocumentFilePath(documentId, null); + Assert.Null(newSolution4.GetRequiredDocument(documentId).FilePath); + Assert.Empty(newSolution4.GetDocumentIdsWithFilePath(null)); Assert.Throws(() => solution.WithDocumentFilePath(null!, path)); Assert.Throws(() => 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() {