-
Notifications
You must be signed in to change notification settings - Fork 236
Rename a .razor file when Roslyn renames the component type name #12606
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
8979aaf
b3da18c
d4cd907
3a09c79
2a1c5a5
be528a2
312881b
c312c4a
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 |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.ComponentModel.Composition; | ||
| using System.Diagnostics; | ||
| using System.IO; | ||
| using Microsoft.AspNetCore.Razor.Utilities; | ||
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
| using Microsoft.CodeAnalysis.ExternalAccess.Razor; | ||
| using Microsoft.CodeAnalysis.Razor.Logging; | ||
| using Microsoft.CodeAnalysis.Razor.Workspaces; | ||
| using Microsoft.NET.Sdk.Razor.SourceGenerators; | ||
|
|
||
| namespace Microsoft.VisualStudio.Razor.Rename; | ||
|
|
||
| [Export(typeof(IRazorRefactorNotifyService))] | ||
| [method: ImportingConstructor] | ||
| internal sealed class RazorRefactorNotifyService( | ||
| ILoggerFactory loggerFactory) : IRazorRefactorNotifyService | ||
| { | ||
| private readonly IFileSystem _fileSystem = new FileSystem(); | ||
| private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<RazorRefactorNotifyService>(); | ||
|
|
||
| public bool TryOnAfterGlobalSymbolRenamed(CodeAnalysis.Workspace workspace, IEnumerable<DocumentId> changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) | ||
| { | ||
| return OnAfterGlobalSymbolRenamed(symbol, newName, throwOnFailure, _fileSystem); | ||
| } | ||
|
|
||
| private bool OnAfterGlobalSymbolRenamed(ISymbol symbol, string newName, bool throwOnFailure, IFileSystem fileSystem) | ||
| { | ||
| // If the user is renaming a Razor component, we need to rename the .razor file or things will break for them, | ||
| // however this method gets called for every symbol rename in Roslyn, so chances are low that it's a Razor component. | ||
| // We have a few heuristics we can use to quickly work out if we care about the symbol, and go from there. | ||
|
|
||
| ClassDeclarationSyntax? classDecl = null; | ||
| foreach (var reference in symbol.OriginalDefinition.DeclaringSyntaxReferences) | ||
| { | ||
| var syntaxTree = reference.SyntaxTree; | ||
|
|
||
| // First, we can check the file path of the syntax tree. Razor generated files have a very specific path format | ||
| if (syntaxTree.FilePath.IndexOf(typeof(RazorSourceGenerator).FullName) == -1 || | ||
| !syntaxTree.FilePath.EndsWith("_razor.g.cs")) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| // Now, we try to get the class declaration from the syntax tree. This method checks specifically for the | ||
| // structure that Razor generates, so it acts as an extra check that this is actually a Razor file, but | ||
| // it doesn't have anything to do with what is actually being renamed. | ||
| if (!syntaxTree.GetRoot().TryGetClassDeclaration(out var thisClassDecl)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| // We're pretty sure by now that the rename is of a symbol in a Razor file, but it might not be the component | ||
| // itself. Let's check. | ||
| if (reference.Span != thisClassDecl.Span) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| classDecl = thisClassDecl; | ||
| break; | ||
| } | ||
|
|
||
| // If we didn't find a class declaration that matches the symbol reference, its not a Razor file, or not the component | ||
| if (classDecl is null) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| // Now for the actual renaming, which is potentially the dodgiest bit. We need to figure out the original .razor file | ||
| // name, but we have no idea which document we're dealing with, nor which project, and there is no API we can use from | ||
| // Roslyn that can give us that info from the symbol. Additionally the changedDocumentIds parameter won't have it, | ||
| // because edits to generated files are filtered out before we get called, and even if it did, we wouldn't know which | ||
| // one was the right one. | ||
| // So we can do one final check, which is that all Razor generated documents begin with a pragma checksum that | ||
| // contains the Razor file name, and not only does that give us final validation, it also lets us know which file | ||
| // to rename. To do this "properly" would mean jumping over to OOP, and probably running all generators in all | ||
| // projects, and then looking at host outputs etc. In other words, it would be very slow and inefficient. This is | ||
| // quick. | ||
|
|
||
| if (classDecl.Parent is null || | ||
| classDecl.Parent.GetLeadingTrivia() is not [{ } firstTrivia, ..] || | ||
| !firstTrivia.IsKind(CodeAnalysis.CSharp.SyntaxKind.PragmaChecksumDirectiveTrivia) || | ||
| firstTrivia.ToString().Split(' ') is not ["#pragma", "checksum", { } quotedRazorFileName, ..]) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| // Let's make sure the pragma actually contained a Razor file name, just in case the compiler changes | ||
| if (quotedRazorFileName.Trim('"') is not { } razorFileName || | ||
| !FileUtilities.IsRazorComponentFilePath(razorFileName, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| if (!fileSystem.FileExists(razorFileName)) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| Debug.Assert(Path.GetExtension(razorFileName) == ".razor"); | ||
| var newFileName = Path.Combine(Path.GetDirectoryName(razorFileName), newName + ".razor"); | ||
|
|
||
| // If the new file name already exists, then the rename can continue, it will just be moving from ComponentA to | ||
| // ComponentB, but ComponentA remains. Hopefully this is what the user intended :) | ||
| if (fileSystem.FileExists(newFileName)) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| // Roslyn has no facility to rename an additional file, so there is no real benefit to do anything but | ||
| // rename the file on disk, and let all of the other systems handle it. | ||
| fileSystem.Move(razorFileName, newFileName); | ||
|
|
||
| // Now try to rename the associated files too | ||
| if (fileSystem.FileExists($"{razorFileName}.cs")) | ||
| { | ||
| fileSystem.Move($"{razorFileName}.cs", $"{newFileName}.cs"); | ||
| } | ||
|
|
||
| if (fileSystem.FileExists($"{razorFileName}.css")) | ||
| { | ||
| fileSystem.Move($"{razorFileName}.css", $"{newFileName}.css"); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Failed to rename Razor component file during symbol rename."); | ||
| if (throwOnFailure) | ||
| { | ||
| throw; | ||
| } | ||
|
|
||
| // If we've tried to actually rename the file, and it didn't work, then we can be okay to block the rename operation | ||
| // because otherwise the user will just get lots of broken references. Chances are its too late to block them anyway | ||
| // but here we are. | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| public bool TryOnBeforeGlobalSymbolRenamed(CodeAnalysis.Workspace workspace, IEnumerable<DocumentId> changedDocumentIDs, ISymbol symbol, string newName, bool throwOnFailure) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| internal TestAccessor GetTestAccessor() => new(this); | ||
|
|
||
| internal readonly struct TestAccessor(RazorRefactorNotifyService instance) | ||
| { | ||
| public bool OnAfterGlobalSymbolRenamed(ISymbol symbol, string newName, bool throwOnFailure, IFileSystem fileSystem) | ||
| => instance.OnAfterGlobalSymbolRenamed(symbol, newName, throwOnFailure, fileSystem); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,6 @@ | |
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.CSharp; | ||
| using Microsoft.CodeAnalysis.ExternalAccess.Razor; | ||
| using Microsoft.CodeAnalysis.ExternalAccess.Razor.Cohost.Handlers; | ||
| using Microsoft.CodeAnalysis.Razor; | ||
| using Microsoft.CodeAnalysis.Razor.CohostingShared; | ||
| using Microsoft.CodeAnalysis.Razor.DocumentMapping; | ||
|
|
@@ -24,6 +23,8 @@ | |
|
|
||
| namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost; | ||
|
|
||
| using Rename = Microsoft.CodeAnalysis.ExternalAccess.Razor.Cohost.Handlers.Rename; | ||
|
Contributor
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. Out of curiosity, which change made the using changes in this file necessary?
Member
Author
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. Something in Roslyn changed, I can't remember the exact thing that conflicted, but the Roslyn version bump alone introduced the issue. |
||
|
|
||
| public class CohostRoslynRenameTest(ITestOutputHelper testOutputHelper) : CohostEndpointTestBase(testOutputHelper) | ||
| { | ||
| [Theory] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.AspNetCore.Razor; | ||
| using Microsoft.AspNetCore.Razor.Test.Common; | ||
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.Rename; | ||
| using Microsoft.CodeAnalysis.Text; | ||
| using Microsoft.VisualStudio.Razor.LanguageClient.Cohost; | ||
| using Microsoft.VisualStudio.Razor.Rename; | ||
| using Xunit; | ||
| using Xunit.Abstractions; | ||
|
|
||
| namespace Microsoft.VisualStudio.LanguageServices.Razor.Test.Cohost; | ||
|
|
||
| public class RazorRefactorNotifyServiceTest(ITestOutputHelper testOutputHelper) : CohostEndpointTestBase(testOutputHelper) | ||
| { | ||
| [Fact] | ||
| public async Task Component() | ||
| { | ||
| var movedFiles = await GetRefactorRenamesAsync( | ||
| razorContents: """ | ||
| <div> | ||
| </div> | ||
| """, | ||
| additionalFiles: [ | ||
| (FilePath("File.cs"), """ | ||
| using SomeProject; | ||
|
|
||
| nameof(Comp$$onent).ToString(); | ||
| """)], | ||
| newName: "DifferentName"); | ||
|
|
||
| var move = Assert.Single(movedFiles); | ||
| Assert.Equal(FilePath("Component.razor"), move.source); | ||
| Assert.Equal(FilePath("DifferentName.razor"), move.destination); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task NotComponent() | ||
| { | ||
| var movedFiles = await GetRefactorRenamesAsync( | ||
| razorContents: """ | ||
| <div> | ||
| </div> | ||
|
|
||
| @code { | ||
| public class NotAComponent | ||
| { | ||
| } | ||
| } | ||
| """, | ||
| additionalFiles: [ | ||
| (FilePath("File.cs"), """ | ||
| using SomeProject; | ||
|
|
||
| nameof(Component.NotAComp$$onent).ToString(); | ||
| """)], | ||
| newName: "DifferentName"); | ||
|
|
||
| Assert.Empty(movedFiles); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task Component_WithCodeBehind() | ||
| { | ||
| var movedFiles = await GetRefactorRenamesAsync( | ||
| razorContents: """ | ||
| <div> | ||
| </div> | ||
| """, | ||
| additionalFiles: [ | ||
| (FilePath("File.cs"), """ | ||
| using SomeProject; | ||
|
|
||
| nameof(Comp$$onent).ToString(); | ||
| """), | ||
| (FilePath("Component.razor.cs"), """ | ||
| namespace SomeProject; | ||
|
|
||
| public partial class Component | ||
| { | ||
| } | ||
| """)], | ||
| newName: "DifferentName"); | ||
|
|
||
| Assert.Collection(movedFiles, | ||
| m => | ||
| { | ||
| Assert.Equal(FilePath("Component.razor"), m.source); | ||
| Assert.Equal(FilePath("DifferentName.razor"), m.destination); | ||
| }, | ||
| m => | ||
| { | ||
| Assert.Equal(FilePath("Component.razor.cs"), m.source); | ||
| Assert.Equal(FilePath("DifferentName.razor.cs"), m.destination); | ||
| }); | ||
| } | ||
|
|
||
| private async Task<List<(string source, string destination)>> GetRefactorRenamesAsync(string razorContents, string newName, params (string fileName, TestCode contents)[] additionalFiles) | ||
| { | ||
| var additionalContent = additionalFiles.Select(f => (f.fileName, f.contents.Text)).ToArray(); | ||
| var razorDocument = CreateProjectAndRazorDocument(razorContents, documentFilePath: FilePath("Component.razor"), additionalFiles: additionalContent); | ||
| var project = razorDocument.Project; | ||
| var csharpDocument = project.Documents.First(); | ||
|
|
||
| var compilation = await project.GetCompilationAsync(DisposalToken); | ||
|
|
||
| var csharpPosition = additionalFiles.Single(d => d.contents.Positions.Length == 1).contents.Position; | ||
| var node = await GetSyntaxNodeAsync(csharpDocument, csharpPosition); | ||
| var symbol = FindSymbolToRename(compilation.AssumeNotNull(), node); | ||
|
|
||
| var solution = await Renamer.RenameSymbolAsync(project.Solution, symbol, new SymbolRenameOptions(), newName, DisposalToken); | ||
|
|
||
| Assert.True(LocalWorkspace.TryApplyChanges(solution)); | ||
|
|
||
| var expectedChanges = (additionalContent ?? []).Concat([(razorDocument.FilePath!, razorContents)]); | ||
| var fileSystem = new TestFileSystem([.. expectedChanges]); | ||
| var service = new RazorRefactorNotifyService(LoggerFactory); | ||
| Assert.True(service.GetTestAccessor().OnAfterGlobalSymbolRenamed(symbol, newName, throwOnFailure: true, fileSystem)); | ||
| return fileSystem.MovedFiles; | ||
| } | ||
|
|
||
| private ISymbol FindSymbolToRename(Compilation compilation, SyntaxNode node) | ||
| { | ||
| var semanticModel = compilation.GetSemanticModel(node.SyntaxTree); | ||
| var symbol = semanticModel.GetDeclaredSymbol(node, DisposalToken); | ||
| if (symbol is null) | ||
| { | ||
| symbol = semanticModel.GetSymbolInfo(node, DisposalToken).Symbol; | ||
| } | ||
|
|
||
| Assert.NotNull(symbol); | ||
| return symbol; | ||
| } | ||
|
|
||
| private async Task<SyntaxNode> GetSyntaxNodeAsync(Document document, int position) | ||
| { | ||
| var sourceText = await document.GetTextAsync(DisposalToken); | ||
| var csharpPosition = sourceText.GetLinePosition(position); | ||
|
|
||
| var span = sourceText.GetTextSpan(csharpPosition, csharpPosition); | ||
| var tree = await document.GetSyntaxTreeAsync(DisposalToken); | ||
| var root = await tree.AssumeNotNull().GetRootAsync(DisposalToken); | ||
|
|
||
| return root.FindNode(span, getInnermostNodeForTie: true); | ||
| } | ||
| } |
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.
Worth verifying there are two more entries in the trivia?
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.
From my quick debug, there are 3 more, which makes me wonder if this would just be being a little too fussy about what the exact code gen should be. I think what does make sense though, now that I think about it, is validating that
quotedRazorFileNameis actually a .razor file name.In other words, I don't think we should necessarily care here if the compiler starts emitting more leading trivia (eg, suppressing some new C# compiler warning), or if the .NET 9 compiler emits 3 bits of trivia and the .NET 10 compiler emits 4. If we find a pragma we can use, we should use it IMO.