-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Rename Razor source generated documents in all scenarios, and map edits #79604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5aa1fe3
68d381e
2f75719
3c3ca9d
b67d1ad
4e73777
6903eab
8bac290
9bcda6e
67a2661
33fde18
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,35 +7,50 @@ | |
using System.Linq; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.Host; | ||
using Microsoft.CodeAnalysis.Rename; | ||
using Microsoft.CodeAnalysis.Shared.Extensions; | ||
using Roslyn.Utilities; | ||
|
||
namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename; | ||
|
||
internal abstract partial class AbstractEditorInlineRenameService | ||
{ | ||
private sealed class InlineRenameLocationSet : IInlineRenameLocationSet | ||
{ | ||
public static async Task<InlineRenameLocationSet> CreateAsync( | ||
SymbolInlineRenameInfo renameInfo, | ||
LightweightRenameLocations renameLocationSet, | ||
CancellationToken cancellationToken) | ||
{ | ||
var solution = renameLocationSet.Solution; | ||
var validLocations = renameLocationSet.Locations.Where(RenameLocation.ShouldRename); | ||
var locations = await validLocations.SelectAsArrayAsync(static (loc, solution, ct) => ConvertLocationAsync(solution, loc, ct), solution, cancellationToken).ConfigureAwait(false); | ||
|
||
return new InlineRenameLocationSet(renameInfo, renameLocationSet, locations); | ||
} | ||
|
||
private readonly LightweightRenameLocations _renameLocationSet; | ||
private readonly SymbolInlineRenameInfo _renameInfo; | ||
|
||
public IList<InlineRenameLocation> Locations { get; } | ||
|
||
public InlineRenameLocationSet( | ||
private InlineRenameLocationSet( | ||
SymbolInlineRenameInfo renameInfo, | ||
LightweightRenameLocations renameLocationSet) | ||
LightweightRenameLocations renameLocationSet, | ||
ImmutableArray<InlineRenameLocation> locations) | ||
{ | ||
_renameInfo = renameInfo; | ||
_renameLocationSet = renameLocationSet; | ||
this.Locations = renameLocationSet.Locations.Where(RenameLocation.ShouldRename) | ||
.Select(ConvertLocation) | ||
.ToImmutableArray(); | ||
this.Locations = locations; | ||
} | ||
|
||
private InlineRenameLocation ConvertLocation(RenameLocation location) | ||
private static async ValueTask<InlineRenameLocation> ConvertLocationAsync(Solution solution, RenameLocation location, CancellationToken cancellationToken) | ||
{ | ||
return new InlineRenameLocation( | ||
_renameLocationSet.Solution.GetRequiredDocument(location.DocumentId), location.Location.SourceSpan); | ||
var document = await solution.GetRequiredDocumentAsync(location.DocumentId, includeSourceGenerated: true, cancellationToken).ConfigureAwait(false); | ||
|
||
Contract.ThrowIfTrue(location.DocumentId.IsSourceGenerated && !document.IsRazorSourceGeneratedDocument()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a little odd that these contracts come after the possibly-expensive method, but sadly thats the way it has to be. They do prove that at least if the |
||
return new InlineRenameLocation(document, location.Location.SourceSpan); | ||
} | ||
|
||
public async Task<IInlineRenameReplacementInfo> GetReplacementsAsync( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -859,7 +859,7 @@ private async Task CommitCoreAsync(IUIThreadOperationContext operationContext, b | |
async Task<ImmutableArray<(DocumentId documentId, string newName, SyntaxNode newRoot, SourceText newText)>> CalculateFinalDocumentChangesAsync( | ||
Solution newSolution, CancellationToken cancellationToken) | ||
{ | ||
var changes = _baseSolution.GetChanges(newSolution); | ||
var changes = newSolution.GetChanges(_baseSolution); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These parameters were backwards, and it was a huge pain to find! For regular documents it turns out not to matter. |
||
var changedDocumentIDs = changes.GetProjectChanges().SelectManyAsArray(c => c.GetChangedDocuments()); | ||
|
||
using var _ = PooledObjects.ArrayBuilder<(DocumentId documentId, string newName, SyntaxNode newRoot, SourceText newText)>.GetInstance(out var result); | ||
|
@@ -877,6 +877,12 @@ private async Task CommitCoreAsync(IUIThreadOperationContext operationContext, b | |
: (documentId, newDocument.Name, newRoot: null, await newDocument.GetTextAsync(cancellationToken).ConfigureAwait(false))); | ||
} | ||
|
||
foreach (var documentId in changes.GetExplicitlyChangedSourceGeneratedDocuments()) | ||
{ | ||
var newDocument = newSolution.GetRequiredSourceGeneratedDocumentForAlreadyGeneratedId(documentId); | ||
result.Add((documentId, newDocument.Name, newRoot: null, await newDocument.GetTextAsync(cancellationToken).ConfigureAwait(false))); | ||
} | ||
|
||
return result.ToImmutableAndClear(); | ||
} | ||
} | ||
|
@@ -892,16 +898,7 @@ private async Task CommitCoreAsync(IUIThreadOperationContext operationContext, b | |
if (!RenameInfo.TryOnBeforeGlobalSymbolRenamed(Workspace, documentChanges.SelectAsArray(t => t.documentId), this.ReplacementText)) | ||
return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid); | ||
|
||
// Grab the workspace's current solution, and make the document changes to it we computed. | ||
var finalSolution = Workspace.CurrentSolution; | ||
foreach (var (documentId, newName, newRoot, newText) in documentChanges) | ||
{ | ||
finalSolution = newRoot != null | ||
? finalSolution.WithDocumentSyntaxRoot(documentId, newRoot) | ||
: finalSolution.WithDocumentText(documentId, newText); | ||
|
||
finalSolution = finalSolution.WithDocumentName(documentId, newName); | ||
} | ||
var finalSolution = _threadingContext.JoinableTaskFactory.Run(() => GetFinalSolutionAsync(documentChanges)); | ||
|
||
// Now actually go and apply the changes to the workspace. We expect this to succeed as we're on the UI | ||
// thread, and nothing else should have been able to make a change to workspace since we we grabbed its | ||
|
@@ -934,6 +931,35 @@ private async Task CommitCoreAsync(IUIThreadOperationContext operationContext, b | |
} | ||
} | ||
|
||
private async Task<Solution> GetFinalSolutionAsync(ImmutableArray<(DocumentId, string, SyntaxNode, SourceText)> documentChanges) | ||
{ | ||
// Grab the workspace's current solution, and make the document changes to it we computed. | ||
var finalSolution = Workspace.CurrentSolution; | ||
foreach (var (documentId, newName, newRoot, newText) in documentChanges) | ||
{ | ||
if (documentId.IsSourceGenerated) | ||
{ | ||
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); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you even have a case where you had an SG doc, and it gota new name? is this just being paranoid (which i'm fine with). might be good to doc. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Source generated documents don't support modifying document attributes, which this call does. Just like It's a bit of an annoying chicken and egg - it would be nice if this call could no-op if the attributes haven't actually changed, regardless of document types, but it means adding support for source generated documents in order to get far enough to know whether that is what is happening. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Docced. I doubt there is a scenario were a SG doc would want to get renamed here, and restricting it to Razor makes it entirely impossible - Roslyn would have to know to rename a seemingly arbitrary additional document to cause a change of Razor SG doc name. |
||
} | ||
|
||
return finalSolution; | ||
} | ||
|
||
internal bool TryGetContainingEditableSpan(SnapshotPoint point, out SnapshotSpan editableSpan) | ||
{ | ||
editableSpan = default; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied this from #79510