PR feedback

davidwengier · davidwengier · commit 33fde18028d1 · 2025-07-26T12:23:58.000+10:00
diff --git a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs
@@ -939,14 +939,18 @@ private async Task<Solution> GetFinalSolutionAsync(ImmutableArray<(DocumentId, s
         {
             if (documentId.IsSourceGenerated)
             {
-                var document = await finalSolution.GetDocumentAsync(documentId, includeSourceGenerated: true, CancellationToken.None).ConfigureAwait(false);
+                var document = await finalSolution.GetDocumentAsync(documentId, includeSourceGenerated: true, CancellationToken.None).ConfigureAwait(true);
                 Contract.ThrowIfFalse(document.IsRazorSourceGeneratedDocument());
             }
 
             finalSolution = newRoot != null
                 ? finalSolution.WithDocumentSyntaxRoot(documentId, newRoot)
                 : finalSolution.WithDocumentText(documentId, newText);
 
+            // WithDocumentName doesn't support source generated documents, and there should be no circumstance were they'd
+            // be renamed anyway. Given rename only supports Razor generated documents, and those are always named for the Razor
+            // file they come from, and nothing in Roslyn knows to rename those documents, we can safely skip this step. Razor
+            // may process the results of this rename and rename the document if it needs to later.
             if (!documentId.IsSourceGenerated)
             {
                 finalSolution = finalSolution.WithDocumentName(documentId, newName);
diff --git a/src/Tools/ExternalAccess/Razor/Features/RazorSourceGeneratedDocumentSpanMappingService.cs b/src/Tools/ExternalAccess/Razor/Features/RazorSourceGeneratedDocumentSpanMappingService.cs
@@ -54,7 +54,6 @@ public async Task<ImmutableArray<MappedTextChange>> GetMappedTextChangesAsync(Do
         }
 
         return changesBuilder.ToImmutableAndClear();
-
     }
 
     public async Task<ImmutableArray<MappedSpanResult>> MapSpansAsync(Document document, ImmutableArray<TextSpan> spans, CancellationToken cancellationToken)
diff --git a/src/VisualStudio/Core/Def/ProjectSystem/VisualStudioWorkspaceImpl.cs b/src/VisualStudio/Core/Def/ProjectSystem/VisualStudioWorkspaceImpl.cs
@@ -657,7 +657,7 @@ internal override void ApplyMappedFileChanges(SolutionChanges solutionChanges)
 
                     var newDocument = projectChanges.NewProject.GetRequiredDocument(changedDocumentId);
                     var mappedTextChanges = await mappingService.GetMappedTextChangesAsync(
-                        oldDocument, newDocument, cancellationToken).ConfigureAwait(false);
+                        oldDocument, newDocument, cancellationToken).ConfigureAwait(true);
                     foreach (var (filePath, textChange) in mappedTextChanges)
                     {
                         filePathToMappedTextChanges.Add(filePath, (textChange, projectChanges.ProjectId));
@@ -672,15 +672,15 @@ internal override void ApplyMappedFileChanges(SolutionChanges solutionChanges)
                 // so the mapper has something to compare to.
                 foreach (var (docId, state) in solutionChanges.NewSolution.CompilationState.FrozenSourceGeneratedDocumentStates.States)
                 {
-                    var document = await solutionChanges.OldSolution.GetRequiredDocumentAsync(docId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);
+                    var document = await solutionChanges.OldSolution.GetRequiredDocumentAsync(docId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(true);
                     Contract.ThrowIfFalse(document.IsRazorSourceGeneratedDocument());
                 }
 
                 foreach (var docId in solutionChanges.GetExplicitlyChangedSourceGeneratedDocuments())
                 {
                     var oldDocument = solutionChanges.OldSolution.GetRequiredSourceGeneratedDocumentForAlreadyGeneratedId(docId);
                     var newDocument = solutionChanges.NewSolution.GetRequiredSourceGeneratedDocumentForAlreadyGeneratedId(docId);
-                    var mappedTextChanges = await sourceGeneratedDocumentMappingService.GetMappedTextChangesAsync(oldDocument, newDocument, cancellationToken).ConfigureAwait(false);
+                    var mappedTextChanges = await sourceGeneratedDocumentMappingService.GetMappedTextChangesAsync(oldDocument, newDocument, cancellationToken).ConfigureAwait(true);
                     foreach (var (filePath, textChange) in mappedTextChanges)
                     {
                         filePathToMappedTextChanges.Add(filePath, (textChange, docId.ProjectId));
diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionChanges.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionChanges.cs
@@ -13,60 +13,57 @@ namespace Microsoft.CodeAnalysis;
 
 public readonly struct SolutionChanges
 {
-    private readonly Solution _newSolution;
-    private readonly Solution _oldSolution;
-
-    internal Solution OldSolution => _oldSolution;
-    internal Solution NewSolution => _newSolution;
+    internal Solution OldSolution { get; }
+    internal Solution NewSolution { get; }
 
     internal SolutionChanges(Solution newSolution, Solution oldSolution)
     {
-        _newSolution = newSolution;
-        _oldSolution = oldSolution;
+        NewSolution = newSolution;
+        OldSolution = oldSolution;
     }
 
     public IEnumerable<Project> GetAddedProjects()
     {
-        foreach (var id in _newSolution.ProjectIds)
+        foreach (var id in NewSolution.ProjectIds)
         {
-            if (!_oldSolution.ContainsProject(id))
+            if (!OldSolution.ContainsProject(id))
             {
-                yield return _newSolution.GetRequiredProject(id);
+                yield return NewSolution.GetRequiredProject(id);
             }
         }
     }
 
     public IEnumerable<ProjectChanges> GetProjectChanges()
     {
-        var old = _oldSolution;
+        var old = OldSolution;
 
         // if the project states are different then there is a change.
-        foreach (var id in _newSolution.ProjectIds)
+        foreach (var id in NewSolution.ProjectIds)
         {
-            var newState = _newSolution.GetProjectState(id);
+            var newState = NewSolution.GetProjectState(id);
             var oldState = old.GetProjectState(id);
             if (oldState != null && newState != null && newState != oldState)
             {
-                yield return _newSolution.GetRequiredProject(id).GetChanges(_oldSolution.GetRequiredProject(id));
+                yield return NewSolution.GetRequiredProject(id).GetChanges(OldSolution.GetRequiredProject(id));
             }
         }
     }
 
     public IEnumerable<Project> GetRemovedProjects()
     {
-        foreach (var id in _oldSolution.ProjectIds)
+        foreach (var id in OldSolution.ProjectIds)
         {
-            if (!_newSolution.ContainsProject(id))
+            if (!NewSolution.ContainsProject(id))
             {
-                yield return _oldSolution.GetRequiredProject(id);
+                yield return OldSolution.GetRequiredProject(id);
             }
         }
     }
 
     public IEnumerable<AnalyzerReference> GetAddedAnalyzerReferences()
     {
-        var oldAnalyzerReferences = new HashSet<AnalyzerReference>(_oldSolution.AnalyzerReferences);
-        foreach (var analyzerReference in _newSolution.AnalyzerReferences)
+        var oldAnalyzerReferences = new HashSet<AnalyzerReference>(OldSolution.AnalyzerReferences);
+        foreach (var analyzerReference in NewSolution.AnalyzerReferences)
         {
             if (!oldAnalyzerReferences.Contains(analyzerReference))
             {
@@ -77,8 +74,8 @@ public IEnumerable<AnalyzerReference> GetAddedAnalyzerReferences()
 
     public IEnumerable<AnalyzerReference> GetRemovedAnalyzerReferences()
     {
-        var newAnalyzerReferences = new HashSet<AnalyzerReference>(_newSolution.AnalyzerReferences);
-        foreach (var analyzerReference in _oldSolution.AnalyzerReferences)
+        var newAnalyzerReferences = new HashSet<AnalyzerReference>(NewSolution.AnalyzerReferences);
+        foreach (var analyzerReference in OldSolution.AnalyzerReferences)
         {
             if (!newAnalyzerReferences.Contains(analyzerReference))
             {
@@ -97,18 +94,18 @@ public IEnumerable<AnalyzerReference> GetRemovedAnalyzerReferences()
     /// </remarks>
     internal IEnumerable<DocumentId> GetExplicitlyChangedSourceGeneratedDocuments()
     {
-        if (_newSolution.CompilationState.FrozenSourceGeneratedDocumentStates.IsEmpty)
+        if (NewSolution.CompilationState.FrozenSourceGeneratedDocumentStates.IsEmpty)
             return [];
 
         using var _ = ArrayBuilder<SourceGeneratedDocumentState>.GetInstance(out var oldStateBuilder);
-        foreach (var (id, _) in _newSolution.CompilationState.FrozenSourceGeneratedDocumentStates.States)
+        foreach (var (id, _) in NewSolution.CompilationState.FrozenSourceGeneratedDocumentStates.States)
         {
-            var oldState = _oldSolution.CompilationState.TryGetSourceGeneratedDocumentStateForAlreadyGeneratedId(id);
+            var oldState = OldSolution.CompilationState.TryGetSourceGeneratedDocumentStateForAlreadyGeneratedId(id);
             oldStateBuilder.AddIfNotNull(oldState);
         }
 
         var oldStates = new TextDocumentStates<SourceGeneratedDocumentState>(oldStateBuilder);
-        return _newSolution.CompilationState.FrozenSourceGeneratedDocumentStates.GetChangedStateIds(
+        return NewSolution.CompilationState.FrozenSourceGeneratedDocumentStates.GetChangedStateIds(
             oldStates,
             ignoreUnchangedContent: true,
             ignoreUnchangeableDocuments: false);

Original file line number	Diff line number	Diff line change
`@@ -939,14 +939,18 @@ private async Task<Solution> GetFinalSolutionAsync(ImmutableArray<(DocumentId, s`
`939`	`939`	`{`
`940`	`940`	`if (documentId.IsSourceGenerated)`
`941`	`941`	`{`
`942`		`- var document = await finalSolution.GetDocumentAsync(documentId, includeSourceGenerated: true, CancellationToken.None).ConfigureAwait(false);`
	`942`	`+ var document = await finalSolution.GetDocumentAsync(documentId, includeSourceGenerated: true, CancellationToken.None).ConfigureAwait(true);`
`943`	`943`	`Contract.ThrowIfFalse(document.IsRazorSourceGeneratedDocument());`
`944`	`944`	`}`
`945`	`945`
`946`	`946`	`finalSolution = newRoot != null`
`947`	`947`	`? finalSolution.WithDocumentSyntaxRoot(documentId, newRoot)`
`948`	`948`	`: finalSolution.WithDocumentText(documentId, newText);`
`949`	`949`
	`950`	`+ // WithDocumentName doesn't support source generated documents, and there should be no circumstance were they'd`
	`951`	`+ // be renamed anyway. Given rename only supports Razor generated documents, and those are always named for the Razor`
	`952`	`+ // file they come from, and nothing in Roslyn knows to rename those documents, we can safely skip this step. Razor`
	`953`	`+ // may process the results of this rename and rename the document if it needs to later.`
`950`	`954`	`if (!documentId.IsSourceGenerated)`
`951`	`955`	`{`
`952`	`956`	`finalSolution = finalSolution.WithDocumentName(documentId, newName);`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,6 @@ public async Task<ImmutableArray<MappedTextChange>> GetMappedTextChangesAsync(Do`
`54`	`54`	`}`
`55`	`55`
`56`	`56`	`return changesBuilder.ToImmutableAndClear();`
`57`		`-`
`58`	`57`	`}`
`59`	`58`
`60`	`59`	`public async Task<ImmutableArray<MappedSpanResult>> MapSpansAsync(Document document, ImmutableArray<TextSpan> spans, CancellationToken cancellationToken)`
Original file line number	Diff line number	Diff line change
`@@ -657,7 +657,7 @@ internal override void ApplyMappedFileChanges(SolutionChanges solutionChanges)`
`657`	`657`
`658`	`658`	`var newDocument = projectChanges.NewProject.GetRequiredDocument(changedDocumentId);`
`659`	`659`	`var mappedTextChanges = await mappingService.GetMappedTextChangesAsync(`
`660`		`- oldDocument, newDocument, cancellationToken).ConfigureAwait(false);`
	`660`	`+ oldDocument, newDocument, cancellationToken).ConfigureAwait(true);`
`661`	`661`	`foreach (var (filePath, textChange) in mappedTextChanges)`
`662`	`662`	`{`
`663`	`663`	`filePathToMappedTextChanges.Add(filePath, (textChange, projectChanges.ProjectId));`
`@@ -672,15 +672,15 @@ internal override void ApplyMappedFileChanges(SolutionChanges solutionChanges)`
`672`	`672`	`// so the mapper has something to compare to.`
`673`	`673`	`foreach (var (docId, state) in solutionChanges.NewSolution.CompilationState.FrozenSourceGeneratedDocumentStates.States)`
`674`	`674`	`{`
`675`		`- var document = await solutionChanges.OldSolution.GetRequiredDocumentAsync(docId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false);`
	`675`	`+ var document = await solutionChanges.OldSolution.GetRequiredDocumentAsync(docId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(true);`
`676`	`676`	`Contract.ThrowIfFalse(document.IsRazorSourceGeneratedDocument());`
`677`	`677`	`}`
`678`	`678`
`679`	`679`	`foreach (var docId in solutionChanges.GetExplicitlyChangedSourceGeneratedDocuments())`
`680`	`680`	`{`
`681`	`681`	`var oldDocument = solutionChanges.OldSolution.GetRequiredSourceGeneratedDocumentForAlreadyGeneratedId(docId);`
`682`	`682`	`var newDocument = solutionChanges.NewSolution.GetRequiredSourceGeneratedDocumentForAlreadyGeneratedId(docId);`
`683`		`- var mappedTextChanges = await sourceGeneratedDocumentMappingService.GetMappedTextChangesAsync(oldDocument, newDocument, cancellationToken).ConfigureAwait(false);`
	`683`	`+ var mappedTextChanges = await sourceGeneratedDocumentMappingService.GetMappedTextChangesAsync(oldDocument, newDocument, cancellationToken).ConfigureAwait(true);`
`684`	`684`	`foreach (var (filePath, textChange) in mappedTextChanges)`
`685`	`685`	`{`
`686`	`686`	`filePathToMappedTextChanges.Add(filePath, (textChange, docId.ProjectId));`
Original file line number	Diff line number	Diff line change
`@@ -13,60 +13,57 @@ namespace Microsoft.CodeAnalysis;`
`13`	`13`
`14`	`14`	`public readonly struct SolutionChanges`
`15`	`15`	`{`
`16`		`- private readonly Solution _newSolution;`
`17`		`- private readonly Solution _oldSolution;`
`18`		`-`
`19`		`- internal Solution OldSolution => _oldSolution;`
`20`		`- internal Solution NewSolution => _newSolution;`
	`16`	`+ internal Solution OldSolution { get; }`
	`17`	`+ internal Solution NewSolution { get; }`
`21`	`18`
`22`	`19`	`internal SolutionChanges(Solution newSolution, Solution oldSolution)`
`23`	`20`	`{`
`24`		`- _newSolution = newSolution;`
`25`		`- _oldSolution = oldSolution;`
	`21`	`+ NewSolution = newSolution;`
	`22`	`+ OldSolution = oldSolution;`
`26`	`23`	`}`
`27`	`24`
`28`	`25`	`public IEnumerable<Project> GetAddedProjects()`
`29`	`26`	`{`
`30`		`- foreach (var id in _newSolution.ProjectIds)`
	`27`	`+ foreach (var id in NewSolution.ProjectIds)`
`31`	`28`	`{`
`32`		`- if (!_oldSolution.ContainsProject(id))`
	`29`	`+ if (!OldSolution.ContainsProject(id))`
`33`	`30`	`{`
`34`		`- yield return _newSolution.GetRequiredProject(id);`
	`31`	`+ yield return NewSolution.GetRequiredProject(id);`
`35`	`32`	`}`
`36`	`33`	`}`
`37`	`34`	`}`
`38`	`35`
`39`	`36`	`public IEnumerable<ProjectChanges> GetProjectChanges()`
`40`	`37`	`{`
`41`		`- var old = _oldSolution;`
	`38`	`+ var old = OldSolution;`
`42`	`39`
`43`	`40`	`// if the project states are different then there is a change.`
`44`		`- foreach (var id in _newSolution.ProjectIds)`
	`41`	`+ foreach (var id in NewSolution.ProjectIds)`
`45`	`42`	`{`
`46`		`- var newState = _newSolution.GetProjectState(id);`
	`43`	`+ var newState = NewSolution.GetProjectState(id);`
`47`	`44`	`var oldState = old.GetProjectState(id);`
`48`	`45`	`if (oldState != null && newState != null && newState != oldState)`
`49`	`46`	`{`
`50`		`- yield return _newSolution.GetRequiredProject(id).GetChanges(_oldSolution.GetRequiredProject(id));`
	`47`	`+ yield return NewSolution.GetRequiredProject(id).GetChanges(OldSolution.GetRequiredProject(id));`
`51`	`48`	`}`
`52`	`49`	`}`
`53`	`50`	`}`
`54`	`51`
`55`	`52`	`public IEnumerable<Project> GetRemovedProjects()`
`56`	`53`	`{`
`57`		`- foreach (var id in _oldSolution.ProjectIds)`
	`54`	`+ foreach (var id in OldSolution.ProjectIds)`
`58`	`55`	`{`
`59`		`- if (!_newSolution.ContainsProject(id))`
	`56`	`+ if (!NewSolution.ContainsProject(id))`
`60`	`57`	`{`
`61`		`- yield return _oldSolution.GetRequiredProject(id);`
	`58`	`+ yield return OldSolution.GetRequiredProject(id);`
`62`	`59`	`}`
`63`	`60`	`}`
`64`	`61`	`}`
`65`	`62`
`66`	`63`	`public IEnumerable<AnalyzerReference> GetAddedAnalyzerReferences()`
`67`	`64`	`{`
`68`		`- var oldAnalyzerReferences = new HashSet<AnalyzerReference>(_oldSolution.AnalyzerReferences);`
`69`		`- foreach (var analyzerReference in _newSolution.AnalyzerReferences)`
	`65`	`+ var oldAnalyzerReferences = new HashSet<AnalyzerReference>(OldSolution.AnalyzerReferences);`
	`66`	`+ foreach (var analyzerReference in NewSolution.AnalyzerReferences)`
`70`	`67`	`{`
`71`	`68`	`if (!oldAnalyzerReferences.Contains(analyzerReference))`
`72`	`69`	`{`
`@@ -77,8 +74,8 @@ public IEnumerable<AnalyzerReference> GetAddedAnalyzerReferences()`
`77`	`74`
`78`	`75`	`public IEnumerable<AnalyzerReference> GetRemovedAnalyzerReferences()`
`79`	`76`	`{`
`80`		`- var newAnalyzerReferences = new HashSet<AnalyzerReference>(_newSolution.AnalyzerReferences);`
`81`		`- foreach (var analyzerReference in _oldSolution.AnalyzerReferences)`
	`77`	`+ var newAnalyzerReferences = new HashSet<AnalyzerReference>(NewSolution.AnalyzerReferences);`
	`78`	`+ foreach (var analyzerReference in OldSolution.AnalyzerReferences)`
`82`	`79`	`{`
`83`	`80`	`if (!newAnalyzerReferences.Contains(analyzerReference))`
`84`	`81`	`{`
`@@ -97,18 +94,18 @@ public IEnumerable<AnalyzerReference> GetRemovedAnalyzerReferences()`
`97`	`94`	`/// </remarks>`
`98`	`95`	`internal IEnumerable<DocumentId> GetExplicitlyChangedSourceGeneratedDocuments()`
`99`	`96`	`{`
`100`		`- if (_newSolution.CompilationState.FrozenSourceGeneratedDocumentStates.IsEmpty)`
	`97`	`+ if (NewSolution.CompilationState.FrozenSourceGeneratedDocumentStates.IsEmpty)`
`101`	`98`	`return [];`
`102`	`99`
`103`	`100`	`using var _ = ArrayBuilder<SourceGeneratedDocumentState>.GetInstance(out var oldStateBuilder);`
`104`		`- foreach (var (id, _) in _newSolution.CompilationState.FrozenSourceGeneratedDocumentStates.States)`
	`101`	`+ foreach (var (id, _) in NewSolution.CompilationState.FrozenSourceGeneratedDocumentStates.States)`
`105`	`102`	`{`
`106`		`- var oldState = _oldSolution.CompilationState.TryGetSourceGeneratedDocumentStateForAlreadyGeneratedId(id);`
	`103`	`+ var oldState = OldSolution.CompilationState.TryGetSourceGeneratedDocumentStateForAlreadyGeneratedId(id);`
`107`	`104`	`oldStateBuilder.AddIfNotNull(oldState);`
`108`	`105`	`}`
`109`	`106`
`110`	`107`	`var oldStates = new TextDocumentStates<SourceGeneratedDocumentState>(oldStateBuilder);`
`111`		`- return _newSolution.CompilationState.FrozenSourceGeneratedDocumentStates.GetChangedStateIds(`
	`108`	`+ return NewSolution.CompilationState.FrozenSourceGeneratedDocumentStates.GetChangedStateIds(`
`112`	`109`	`oldStates,`
`113`	`110`	`ignoreUnchangedContent: true,`
`114`	`111`	`ignoreUnchangeableDocuments: false);`