From e79be97e37654f8389f9d0d49bb27d39dd413577 Mon Sep 17 00:00:00 2001 From: David Barbet Date: Wed, 6 Apr 2022 19:43:22 -0700 Subject: [PATCH] Implement handling of rename intent (#59410) * Implement handling of rename intent * Use documentId instead of URI * Move record definition up top * feedback --- .../Intents/GenerateConstructorIntentTests.cs | 2 +- .../CSharpTest/Intents/IntentTestsBase.cs | 64 ++++--- .../CSharpTest/Intents/RenameIntentTests.cs | 160 ++++++++++++++++++ .../IntelliCode/Api/IIntentSourceProvider.cs | 12 +- .../IntelliCode/IntentProcessor.cs | 30 +++- .../Core/Intents/RenameIntentProvider.cs | 59 +++++++ ...etersFromMembersCodeRefactoringProvider.cs | 2 +- ...uctorFromMembersCodeRefactoringProvider.cs | 6 +- .../Core/Portable/Intents/IntentResult.cs | 9 +- .../Core/Portable/Intents/WellKnownIntents.cs | 1 + .../Extensions/ProtocolConversions.cs | 1 + 11 files changed, 314 insertions(+), 32 deletions(-) create mode 100644 src/EditorFeatures/CSharpTest/Intents/RenameIntentTests.cs create mode 100644 src/EditorFeatures/Core/Intents/RenameIntentProvider.cs diff --git a/src/EditorFeatures/CSharpTest/Intents/GenerateConstructorIntentTests.cs b/src/EditorFeatures/CSharpTest/Intents/GenerateConstructorIntentTests.cs index c0437e5ef325f..c436a7137e624 100644 --- a/src/EditorFeatures/CSharpTest/Intents/GenerateConstructorIntentTests.cs +++ b/src/EditorFeatures/CSharpTest/Intents/GenerateConstructorIntentTests.cs @@ -140,7 +140,7 @@ public C(int someInt) } }"; - await VerifyExpectedTextAsync(WellKnownIntents.GenerateConstructor, initialText, additionalDocuments, expectedText).ConfigureAwait(false); + await VerifyExpectedTextAsync(WellKnownIntents.GenerateConstructor, initialText, additionalDocuments, new[] { expectedText }).ConfigureAwait(false); } [Fact] diff --git a/src/EditorFeatures/CSharpTest/Intents/IntentTestsBase.cs b/src/EditorFeatures/CSharpTest/Intents/IntentTestsBase.cs index 5b1ce8e4d6268..503b898166e48 100644 --- a/src/EditorFeatures/CSharpTest/Intents/IntentTestsBase.cs +++ b/src/EditorFeatures/CSharpTest/Intents/IntentTestsBase.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using System.Threading; @@ -12,6 +14,8 @@ using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; using Microsoft.CodeAnalysis.ExternalAccess.IntelliCode.Api; using Microsoft.CodeAnalysis.Features.Intents; +using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Test.Utilities; using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.Text.Shared.Extensions; @@ -22,12 +26,25 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Intents { public class IntentTestsBase { - internal static Task VerifyExpectedTextAsync(string intentName, string markup, string expectedText, OptionsCollection? options = null, string? intentData = null) + internal static Task VerifyExpectedTextAsync( + string intentName, + string markup, + string expectedText, + OptionsCollection? options = null, + string? intentData = null, + string? priorText = null) { - return VerifyExpectedTextAsync(intentName, markup, new string[] { }, expectedText, options, intentData); + return VerifyExpectedTextAsync(intentName, markup, new string[] { }, new string[] { expectedText }, options, intentData, priorText); } - internal static async Task VerifyExpectedTextAsync(string intentName, string activeDocument, string[] additionalDocuments, string expectedText, OptionsCollection? options = null, string? intentData = null) + internal static async Task VerifyExpectedTextAsync( + string intentName, + string activeDocument, + string[] additionalDocuments, + string[] expectedTexts, + OptionsCollection? options = null, + string? intentData = null, + string? priorText = null) { var documentSet = additionalDocuments.Prepend(activeDocument).ToArray(); using var workspace = TestWorkspace.CreateCSharp(documentSet, exportProvider: EditorTestCompositions.EditorFeatures.ExportProviderFactory.CreateExportProvider()); @@ -41,19 +58,13 @@ internal static async Task VerifyExpectedTextAsync(string intentName, string act // The first document will be the active document. var document = workspace.Documents.Single(d => d.Name == "test1.cs"); var textBuffer = document.GetTextBuffer(); - var typedSpan = document.AnnotatedSpans["typed"].Single(); - // Get the current snapshot span and selection. - var currentSelectedSpan = document.SelectedSpans.FirstOrDefault(); - if (currentSelectedSpan.IsEmpty) - { - currentSelectedSpan = TextSpan.FromBounds(typedSpan.End, typedSpan.End); - } + // Get the text change to rewind the document to the correct pre-intent location. + var rewindTextChange = new TextChange(document.AnnotatedSpans["typed"].Single(), priorText ?? string.Empty); - var currentSnapshotSpan = new SnapshotSpan(textBuffer.CurrentSnapshot, currentSelectedSpan.ToSpan()); + // Get the current snapshot span to pass in. + var currentSnapshot = new SnapshotSpan(textBuffer.CurrentSnapshot, new Span(0, textBuffer.CurrentSnapshot.Length)); - // Determine the edits to rewind to the prior snapshot by removing the changes in the annotated span. - var rewindTextChange = new TextChange(typedSpan, ""); var priorSelection = TextSpan.FromBounds(rewindTextChange.Span.Start, rewindTextChange.Span.Start); if (document.AnnotatedSpans.ContainsKey("priorSelection")) { @@ -62,7 +73,7 @@ internal static async Task VerifyExpectedTextAsync(string intentName, string act var intentContext = new IntentRequestContext( intentName, - currentSnapshotSpan, + currentSnapshot, ImmutableArray.Create(rewindTextChange), priorSelection, intentData: intentData); @@ -71,15 +82,28 @@ internal static async Task VerifyExpectedTextAsync(string intentName, string act // For now, we're just taking the first result to match intellicode behavior. var result = results.First(); - using var edit = textBuffer.CreateEdit(); - foreach (var change in result.TextChanges) + var actualDocumentTexts = new List(); + foreach (var documentChange in result.DocumentChanges) { - edit.Replace(change.Span.ToSpan(), change.NewText); - } + // Get the document and open it. Since we're modifying the text buffer we don't care about linked documents. + var documentBuffer = workspace.GetTestDocument(documentChange.Key).GetTextBuffer(); + + using var edit = documentBuffer.CreateEdit(); + foreach (var change in documentChange.Value) + { + edit.Replace(change.Span.ToSpan(), change.NewText); + } - edit.Apply(); + edit.Apply(); - Assert.Equal(expectedText, textBuffer.CurrentSnapshot.GetText()); + actualDocumentTexts.Add(documentBuffer.CurrentSnapshot.GetText()); + } + + Assert.Equal(expectedTexts.Length, actualDocumentTexts.Count); + foreach (var expectedText in expectedTexts) + { + Assert.True(actualDocumentTexts.Contains(expectedText)); + } } } } diff --git a/src/EditorFeatures/CSharpTest/Intents/RenameIntentTests.cs b/src/EditorFeatures/CSharpTest/Intents/RenameIntentTests.cs new file mode 100644 index 0000000000000..3a915102f2d3b --- /dev/null +++ b/src/EditorFeatures/CSharpTest/Intents/RenameIntentTests.cs @@ -0,0 +1,160 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Linq; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Features.Intents; +using Microsoft.CodeAnalysis.Test.Utilities; +using Microsoft.CodeAnalysis.Text; +using Xunit; + +namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Intents; + +[UseExportProvider] +public class RenameIntentTests : IntentTestsBase +{ + [Fact] + public async Task TestRenameIntentAsync() + { + var initialText = +@"class C +{ + void M() + { + var thing = 1; + {|typed:something|}.ToString(); + } +}"; + var expectedText = +@"class C +{ + void M() + { + var something = 1; + something.ToString(); + } +}"; + + await VerifyExpectedRenameAsync(initialText, expectedText, "thing", "something").ConfigureAwait(false); + } + + [Fact] + public async Task TestRenameIntentAsync_Insert() + { + var initialText = +@"class C +{ + void M() + { + var thing = 1; + {|typed:some|}thing.ToString(); + } +}"; + var expectedText = +@"class C +{ + void M() + { + var something = 1; + something.ToString(); + } +}"; + + await VerifyExpectedRenameAsync(initialText, expectedText, string.Empty, "something").ConfigureAwait(false); + } + + [Fact] + public async Task TestRenameIntentAsync_Delete() + { + var initialText = +@"class C +{ + void M() + { + var something = 1; + {|typed:|}thing.ToString(); + } +}"; + var expectedText = +@"class C +{ + void M() + { + var thing = 1; + thing.ToString(); + } +}"; + + await VerifyExpectedRenameAsync(initialText, expectedText, "some", "thing").ConfigureAwait(false); + } + + [Fact] + public async Task TestRenameIntentAsync_MultipleFiles() + { + var initialText = +@"namespace M +{ + public class C + { + public static string {|typed:BetterString|} = string.Empty; + + void M() + { + var m = SomeString; + } + } +}"; + var additionalDocuments = new string[] + { +@"namespace M +{ + public class D + { + void M() + { + var m = C.SomeString; + } + } +}" + }; + + var expectedTexts = new string[] + { +@"namespace M +{ + public class C + { + public static string BetterString = string.Empty; + + void M() + { + var m = BetterString; + } + } +}", +@"namespace M +{ + public class D + { + void M() + { + var m = C.BetterString; + } + } +}" + }; + + await VerifyExpectedRenameAsync(initialText, additionalDocuments, expectedTexts, "SomeString", "BetterString").ConfigureAwait(false); + } + + private static Task VerifyExpectedRenameAsync(string initialText, string expectedText, string priorText, string newName) + { + return VerifyExpectedTextAsync(WellKnownIntents.Rename, initialText, expectedText, intentData: $"{{ \"newName\": \"{newName}\" }}", priorText: priorText); + } + + private static Task VerifyExpectedRenameAsync(string initialText, string[] additionalText, string[] expectedTexts, string priorText, string newName) + { + return VerifyExpectedTextAsync(WellKnownIntents.Rename, initialText, additionalText, expectedTexts, intentData: $"{{ \"newName\": \"{newName}\" }}", priorText: priorText); + } +} diff --git a/src/EditorFeatures/Core/ExternalAccess/IntelliCode/Api/IIntentSourceProvider.cs b/src/EditorFeatures/Core/ExternalAccess/IntelliCode/Api/IIntentSourceProvider.cs index 2734f46275bc0..465e595ad2804 100644 --- a/src/EditorFeatures/Core/ExternalAccess/IntelliCode/Api/IIntentSourceProvider.cs +++ b/src/EditorFeatures/Core/ExternalAccess/IntelliCode/Api/IIntentSourceProvider.cs @@ -77,9 +77,16 @@ internal readonly struct IntentSource /// /// The text changes that should be applied to the + /// TODO - Remove once intellicode switches over to reading instead. /// + [Obsolete("Use DocumentChanges instead")] public readonly ImmutableArray TextChanges { get; } + /// + /// The text changes that should be applied to each document. + /// + public readonly ImmutableDictionary> DocumentChanges; + /// /// Contains metadata that can be used to identify the kind of sub-action these edits /// apply to for the requested intent. Used for telemetry purposes only. @@ -87,11 +94,14 @@ internal readonly struct IntentSource /// public readonly string ActionName { get; } - public IntentSource(string title, ImmutableArray textChanges, string actionName) + public IntentSource(string title, ImmutableArray textChanges, string actionName, ImmutableDictionary> documentChanges) { +#pragma warning disable CS0618 // Type or member is obsolete TextChanges = textChanges; +#pragma warning restore CS0618 // Type or member is obsolete Title = title ?? throw new ArgumentNullException(nameof(title)); ActionName = actionName ?? throw new ArgumentNullException(nameof(actionName)); + DocumentChanges = documentChanges; } } } diff --git a/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs b/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs index d99b779598904..94b13eccf6795 100644 --- a/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs +++ b/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs @@ -13,6 +13,7 @@ using Microsoft.CodeAnalysis.Features.Intents; using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Internal.Log; +using Microsoft.CodeAnalysis.LanguageServer; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; @@ -93,14 +94,33 @@ public async Task> ComputeIntentsAsync(IntentReques CancellationToken cancellationToken) { var newSolution = processorResult.Solution; - // Merge linked file changes so all linked files have the same text changes. newSolution = await newSolution.WithMergedLinkedFileChangesAsync(originalDocument.Project.Solution, cancellationToken: cancellationToken).ConfigureAwait(false); - // For now we only support changes to the current document. Everything else is dropped. - var changedDocument = newSolution.GetRequiredDocument(currentDocument.Id); + using var _ = PooledDictionary>.GetInstance(out var results); + foreach (var changedDocumentId in processorResult.ChangedDocuments) + { + // Calculate the text changes by comparing the solution with intent applied to the current solution (not to be confused with the original solution, the one prior to intent detection). + var docChanges = await GetTextChangesForDocumentAsync(newSolution, currentDocument.Project.Solution, changedDocumentId, cancellationToken).ConfigureAwait(false); + if (docChanges != null) + { + results[changedDocumentId] = docChanges.Value; + } + } + + return new IntentSource(processorResult.Title, results[originalDocument.Id], processorResult.ActionName, results.ToImmutableDictionary()); + } + + private static async Task?> GetTextChangesForDocumentAsync( + Solution changedSolution, + Solution currentSolution, + DocumentId changedDocumentId, + CancellationToken cancellationToken) + { + var changedDocument = changedSolution.GetRequiredDocument(changedDocumentId); + var currentDocument = currentSolution.GetRequiredDocument(changedDocumentId); - var textDiffService = newSolution.Workspace.Services.GetRequiredService(); + var textDiffService = changedSolution.Workspace.Services.GetRequiredService(); // Compute changes against the current version of the document. var textDiffs = await textDiffService.GetTextChangesAsync(currentDocument, changedDocument, cancellationToken).ConfigureAwait(false); if (textDiffs.IsEmpty) @@ -108,7 +128,7 @@ public async Task> ComputeIntentsAsync(IntentReques return null; } - return new IntentSource(processorResult.Title, textDiffs, processorResult.ActionName); + return textDiffs; } } } diff --git a/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs b/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs new file mode 100644 index 0000000000000..670d23cda4da9 --- /dev/null +++ b/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs @@ -0,0 +1,59 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Immutable; +using System.Composition; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Editor; +using Microsoft.CodeAnalysis.Features.Intents; +using Microsoft.CodeAnalysis.Host.Mef; +using Microsoft.CodeAnalysis.Rename; +using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Text; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis.EditorFeatures.Intents; + +[IntentProvider(WellKnownIntents.Rename, LanguageNames.CSharp), Shared] +internal class RenameIntentProvider : IIntentProvider +{ + private record RenameIntentData(string NewName); + + [ImportingConstructor] + [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] + public RenameIntentProvider() + { + } + + public async Task> ComputeIntentAsync( + Document priorDocument, + TextSpan priorSelection, + Document currentDocument, + IntentDataProvider intentDataProvider, + CancellationToken cancellationToken) + { + var renameIntentData = intentDataProvider.GetIntentData(); + Contract.ThrowIfNull(renameIntentData); + + var renameService = priorDocument.GetRequiredLanguageService(); + var renameInfo = await renameService.GetRenameInfoAsync(priorDocument, priorSelection.Start, cancellationToken).ConfigureAwait(false); + if (!renameInfo.CanRename) + { + return ImmutableArray.Empty; + } + + var options = new SymbolRenameOptions( + RenameOverloads: false, + RenameInStrings: false, + RenameInComments: false, + RenameFile: false); + + var renameLocationSet = await renameInfo.FindRenameLocationsAsync(options, cancellationToken).ConfigureAwait(false); + var renameReplacementInfo = await renameLocationSet.GetReplacementsAsync(renameIntentData.NewName, options, cancellationToken).ConfigureAwait(false); + + return ImmutableArray.Create(new IntentProcessorResult(renameReplacementInfo.NewSolution, renameReplacementInfo.DocumentIds.ToImmutableArray(), EditorFeaturesResources.Rename, WellKnownIntents.Rename)); + } +} diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs index c31f1a999bcb1..c90a3914b9812 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs @@ -176,7 +176,7 @@ public async Task> ComputeIntentAsync(Docu { var changedSolution = await action.GetChangedSolutionInternalAsync(postProcessChanges: true, cancellationToken).ConfigureAwait(false); Contract.ThrowIfNull(changedSolution); - var intent = new IntentProcessorResult(changedSolution, action.Title, action.ActionName); + var intent = new IntentProcessorResult(changedSolution, ImmutableArray.Create(priorDocument.Id), action.Title, action.ActionName); results.Add(intent); } diff --git a/src/Features/Core/Portable/GenerateConstructorFromMembers/AbstractGenerateConstructorFromMembersCodeRefactoringProvider.cs b/src/Features/Core/Portable/GenerateConstructorFromMembers/AbstractGenerateConstructorFromMembersCodeRefactoringProvider.cs index 17a3333b2223b..cc2676406ce59 100644 --- a/src/Features/Core/Portable/GenerateConstructorFromMembers/AbstractGenerateConstructorFromMembersCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/GenerateConstructorFromMembers/AbstractGenerateConstructorFromMembersCodeRefactoringProvider.cs @@ -87,13 +87,13 @@ await ComputeRefactoringsAsync( using var resultsBuilder = ArrayBuilder.GetInstance(out var results); foreach (var action in actions) { - var intentResult = await GetIntentProcessorResultAsync(action, cancellationToken).ConfigureAwait(false); + var intentResult = await GetIntentProcessorResultAsync(action, priorDocument.Id, cancellationToken).ConfigureAwait(false); results.AddIfNotNull(intentResult); } return results.ToImmutable(); - static async Task GetIntentProcessorResultAsync(CodeAction codeAction, CancellationToken cancellationToken) + static async Task GetIntentProcessorResultAsync(CodeAction codeAction, DocumentId document, CancellationToken cancellationToken) { var operations = await GetCodeActionOperationsAsync(codeAction, cancellationToken).ConfigureAwait(false); @@ -106,7 +106,7 @@ await ComputeRefactoringsAsync( } var type = codeAction.GetType(); - return new IntentProcessorResult(applyChangesOperation.ChangedSolution, codeAction.Title, type.Name); + return new IntentProcessorResult(applyChangesOperation.ChangedSolution, ImmutableArray.Create(document), codeAction.Title, type.Name); } static async Task> GetCodeActionOperationsAsync( diff --git a/src/Features/Core/Portable/Intents/IntentResult.cs b/src/Features/Core/Portable/Intents/IntentResult.cs index 3f56c09397da5..b0bdf6f94587c 100644 --- a/src/Features/Core/Portable/Intents/IntentResult.cs +++ b/src/Features/Core/Portable/Intents/IntentResult.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Immutable; namespace Microsoft.CodeAnalysis.Features.Intents { @@ -16,6 +17,11 @@ internal struct IntentProcessorResult /// public readonly Solution Solution; + /// + /// The set of documents that have changed for this intent result. + /// + public readonly ImmutableArray ChangedDocuments; + /// /// The title associated with this intent result. /// @@ -27,9 +33,10 @@ internal struct IntentProcessorResult /// public readonly string ActionName; - public IntentProcessorResult(Solution solution, string title, string actionName) + public IntentProcessorResult(Solution solution, ImmutableArray changedDocuments, string title, string actionName) { Solution = solution; + ChangedDocuments = changedDocuments; Title = title ?? throw new ArgumentNullException(nameof(title)); ActionName = actionName ?? throw new ArgumentNullException(nameof(actionName)); } diff --git a/src/Features/Core/Portable/Intents/WellKnownIntents.cs b/src/Features/Core/Portable/Intents/WellKnownIntents.cs index 0438f295e49f2..b0e2b66a0a98a 100644 --- a/src/Features/Core/Portable/Intents/WellKnownIntents.cs +++ b/src/Features/Core/Portable/Intents/WellKnownIntents.cs @@ -11,5 +11,6 @@ internal static class WellKnownIntents { public const string GenerateConstructor = nameof(GenerateConstructor); public const string AddConstructorParameter = nameof(AddConstructorParameter); + public const string Rename = nameof(Rename); } } diff --git a/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs b/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs index 4b8bb4e256a50..d431e10b5f718 100644 --- a/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs +++ b/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.IO; using System.Linq; using System.Text; using System.Text.RegularExpressions;