Rename Razor source generated documents in all scenarios, and map edits #79604
Conversation
The only locations that will be added for processing are either not generated, or Razor files
davidwengier
changed the title
| /// We check for the presence of the razor SG by full name | ||
| /// so we have to make sure this is the right name in the right namespace. | ||
| /// </summary> | ||
| internal sealed class RazorSourceGenerator(Action<GeneratorExecutionContext> execute) : ISourceGenerator |
There was a problem hiding this comment.
Copied this from #79510
| _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.
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 GetRequiredDocumentAsync had to go async, then it was for Razor which is what we want anyway.
| var changes = _baseSolution.GetChanges(newSolution); | ||
| var changes = newSolution.GetChanges(_baseSolution); |
There was a problem hiding this comment.
These parameters were backwards, and it was a huge pain to find! For regular documents it turns out not to matter.
| new A().{|renamed:M|}(); | ||
| new A().M(); |
There was a problem hiding this comment.
This is not a Razor generated document, just a normal one, so these spans are no longer found by rename. Efficiency!
src/EditorFeatures/Core/InlineRename/AbstractEditorInlineRenameService.cs
Show resolved
Hide resolved
| if (!documentId.IsSourceGenerated) | ||
| { | ||
| finalSolution = finalSolution.WithDocumentName(documentId, newName); | ||
| } |
There was a problem hiding this comment.
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.
Source generated documents don't support modifying document attributes, which this call does. Just like WithDocumentText and WithSyntaxRoot weren't supported until I added it a few months ago.
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.
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.
src/Tools/ExternalAccess/Razor/Features/RazorSourceGeneratedDocumentSpanMappingService.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/Core/Def/ProjectSystem/VisualStudioWorkspaceImpl.cs
Outdated
Show resolved
Hide resolved
src/Workspaces/Core/Portable/Workspace/Solution/SolutionChanges.cs
Outdated
Show resolved
Hide resolved
Follow up to #79604 Part of dotnet/razor#9519 This is the follow up to the above PR to support rename in LSP, but thought I'd make the API slightly stronger while I was here since we haven't consumed it on Razor yet. I also misunderstood the span mapping API, and it needing to return an array of the same length as the input.
Fixes #12054 Part of #9519 Needs ~dotnet/roslyn#79604 and~ dotnet/roslyn#79677 to insert before this will build This PR implements the workspace version of our span mapping service, since the old one is hooked up via a document service, which we can't do in cohosting.
Part of dotnet/razor#9519
Half of the fix for dotnet/razor#12054, along with the Razor side at dotnet/razor#12055
Previously I allowed rename to work on any source generated document, but made it only opt in for when the rename request came from Razor. Now that we have a little more time for development, it's time to finish the feature off properly. This PR:
There will still need to be more PRs to make this work in VS Code, and to hook up the span mapping aspect of the service, but I didn't want this PR to get too big.