From 16c64bf6f6657276c5d5fea855f5da12c282fef2 Mon Sep 17 00:00:00 2001 From: David Barbet Date: Tue, 8 Feb 2022 16:00:24 -0800 Subject: [PATCH 1/4] Implement handling of rename intent --- .../Intents/GenerateConstructorIntentTests.cs | 2 +- .../CSharpTest/Intents/IntentTestsBase.cs | 66 +++++--- .../CSharpTest/Intents/RenameIntentTests.cs | 160 ++++++++++++++++++ .../IntelliCode/Api/IIntentSourceProvider.cs | 12 +- .../IntelliCode/IntentProcessor.cs | 29 +++- .../Core/Intents/RenameIntentProvider.cs | 65 +++++++ ...etersFromMembersCodeRefactoringProvider.cs | 2 +- ...uctorFromMembersCodeRefactoringProvider.cs | 6 +- .../Core/Portable/Intents/IntentResult.cs | 9 +- .../Core/Portable/Intents/WellKnownIntents.cs | 1 + .../Extensions/ProtocolConversions.cs | 14 +- 11 files changed, 332 insertions(+), 34 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 ac9403f582bbc..e6b1e316369ae 100644 --- a/src/EditorFeatures/CSharpTest/Intents/GenerateConstructorIntentTests.cs +++ b/src/EditorFeatures/CSharpTest/Intents/GenerateConstructorIntentTests.cs @@ -86,7 +86,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 e7d6f5af75de2..0ce21b0f20430 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,7 @@ using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; using Microsoft.CodeAnalysis.ExternalAccess.IntelliCode.Api; using Microsoft.CodeAnalysis.Features.Intents; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Test.Utilities; using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.Text.Shared.Extensions; @@ -22,12 +25,25 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Intents { public class IntentTestsBase { - internal static Task VerifyExpectedTextAsync(string intentName, string markup, string expectedText, OptionsCollection? options = null) + internal static Task VerifyExpectedTextAsync( + string intentName, + string markup, + string expectedText, + OptionsCollection? options = null, + string? serializedData = null, + string? priorText = null) { - return VerifyExpectedTextAsync(intentName, markup, new string[] { }, expectedText, options); + return VerifyExpectedTextAsync(intentName, markup, new string[] { }, new string[] { expectedText }, options, serializedData, priorText); } - internal static async Task VerifyExpectedTextAsync(string intentName, string activeDocument, string[] additionalDocuments, string expectedText, OptionsCollection? options = null) + internal static async Task VerifyExpectedTextAsync( + string intentName, + string activeDocument, + string[] additionalDocuments, + string[] expectedTexts, + OptionsCollection? options = null, + string? serializedData = null, + string? priorText = null) { var documentSet = additionalDocuments.Prepend(activeDocument).ToArray(); using var workspace = TestWorkspace.CreateCSharp(documentSet, exportProvider: EditorTestCompositions.EditorFeatures.ExportProviderFactory.CreateExportProvider()); @@ -41,19 +57,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,24 +72,38 @@ internal static async Task VerifyExpectedTextAsync(string intentName, string act var intentContext = new IntentRequestContext( intentName, - currentSnapshotSpan, + currentSnapshot, ImmutableArray.Create(rewindTextChange), priorSelection, - intentData: null); + serializedData); var results = await intentSource.ComputeIntentsAsync(intentContext, CancellationToken.None).ConfigureAwait(false); // 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 documentToChange = workspace.CurrentSolution.GetDocumentIdsWithFilePath(documentChange.Key.OriginalString).First(); + var documentBuffer = workspace.GetTestDocument(documentToChange).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..abf7017193549 --- /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, serializedData: $"{{ \"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, serializedData: $"{{ \"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..d70083c0f8d8a 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 276fd632b39cd..9aaee612ce34a 100644 --- a/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs +++ b/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs @@ -12,6 +12,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; @@ -91,14 +92,32 @@ 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); + var results = new Dictionary>(); + 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) + { + var (uri, textChanges) = docChanges.Value; + results[uri] = textChanges; + } + } + + return new IntentSource(processorResult.Title, results[originalDocument.GetURI()], processorResult.ActionName, results.ToImmutableDictionary()); + } + + private static async Task<(Uri, ImmutableArray)?> GetTextChangesForDocumentAsync(Solution changedSolution, Solution currentSolution, DocumentId changedDocumentId, CancellationToken cancellationToken) + { + var changedDocument = changedSolution.GetRequiredDocument(changedDocumentId); + var currentDocument = currentSolution.GetRequiredDocument(changedDocumentId); + + Contract.ThrowIfNull(changedDocument.FilePath, $"{changedDocumentId} missing file path"); - 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) @@ -106,7 +125,7 @@ public async Task> ComputeIntentsAsync(IntentReques return null; } - return new IntentSource(processorResult.Title, textDiffs, processorResult.ActionName); + return (changedDocument.GetURI(), textDiffs); } } } diff --git a/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs b/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs new file mode 100644 index 0000000000000..c93776f5af884 --- /dev/null +++ b/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs @@ -0,0 +1,65 @@ +// 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.Text; +using Newtonsoft.Json; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis.EditorFeatures.Intents; + +[IntentProvider(WellKnownIntents.Rename, LanguageNames.CSharp), Shared] +internal class RenameIntentProvider : IIntentProvider +{ + [ImportingConstructor] + [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] + public RenameIntentProvider() + { + } + + public async Task> ComputeIntentAsync(Document priorDocument, TextSpan priorSelection, Document currentDocument, string? serializedIntentData, CancellationToken cancellationToken) + { + Contract.ThrowIfNull(serializedIntentData); + + var renameService = priorDocument.Project.LanguageServices.GetRequiredService(); + var renameInfo = await renameService.GetRenameInfoAsync(priorDocument, priorSelection.Start, cancellationToken).ConfigureAwait(false); + if (!renameInfo.CanRename) + { + return ImmutableArray.Empty; + } + + var renameIntentData = JsonConvert.DeserializeObject(serializedIntentData); + Contract.ThrowIfNull(renameIntentData); + + 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)); + } + + private class RenameIntentData + { + public string NewName { get; } + + [JsonConstructor] + public RenameIntentData([JsonProperty("newName", Required = Required.Always)] string newName) + { + NewName = newName; + } + } +} diff --git a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs index c95ac538f132f..2abd813043bf0 100644 --- a/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/AddConstructorParametersFromMembers/AddConstructorParametersFromMembersCodeRefactoringProvider.cs @@ -174,7 +174,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 e509c9adc0309..eaba781f0c308 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 04d3db8f76706..3bf89a362095e 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; @@ -146,7 +147,18 @@ public static Uri GetUriFromFilePath(string filePath) if (filePath is null) throw new ArgumentNullException(nameof(filePath)); - return new Uri(filePath, UriKind.Absolute); + if (Path.IsPathRooted(filePath)) + { + return new Uri(filePath, UriKind.Absolute); + } + else if (Uri.TryCreate(filePath, UriKind.Relative, out var uri)) + { + return uri; + } + else + { + throw new ArgumentException($"Failed to create URI for {filePath}"); + } } public static Uri? TryGetUriFromFilePath(string? filePath, RequestContext? context = null) From 6fdefcd076cbce5ee004cb1003a523afff23712c Mon Sep 17 00:00:00 2001 From: David Barbet Date: Wed, 6 Apr 2022 15:08:36 -0700 Subject: [PATCH 2/4] Use documentId instead of URI --- .../CSharpTest/Intents/IntentTestsBase.cs | 4 ++-- .../IntelliCode/Api/IIntentSourceProvider.cs | 4 ++-- .../ExternalAccess/IntelliCode/IntentProcessor.cs | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Intents/IntentTestsBase.cs b/src/EditorFeatures/CSharpTest/Intents/IntentTestsBase.cs index 54a6d6db3e962..503b898166e48 100644 --- a/src/EditorFeatures/CSharpTest/Intents/IntentTestsBase.cs +++ b/src/EditorFeatures/CSharpTest/Intents/IntentTestsBase.cs @@ -15,6 +15,7 @@ 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; @@ -85,8 +86,7 @@ internal static async Task VerifyExpectedTextAsync( foreach (var documentChange in result.DocumentChanges) { // Get the document and open it. Since we're modifying the text buffer we don't care about linked documents. - var documentToChange = workspace.CurrentSolution.GetDocumentIdsWithFilePath(documentChange.Key.OriginalString).First(); - var documentBuffer = workspace.GetTestDocument(documentToChange).GetTextBuffer(); + var documentBuffer = workspace.GetTestDocument(documentChange.Key).GetTextBuffer(); using var edit = documentBuffer.CreateEdit(); foreach (var change in documentChange.Value) diff --git a/src/EditorFeatures/Core/ExternalAccess/IntelliCode/Api/IIntentSourceProvider.cs b/src/EditorFeatures/Core/ExternalAccess/IntelliCode/Api/IIntentSourceProvider.cs index d70083c0f8d8a..465e595ad2804 100644 --- a/src/EditorFeatures/Core/ExternalAccess/IntelliCode/Api/IIntentSourceProvider.cs +++ b/src/EditorFeatures/Core/ExternalAccess/IntelliCode/Api/IIntentSourceProvider.cs @@ -85,7 +85,7 @@ internal readonly struct IntentSource /// /// The text changes that should be applied to each document. /// - public readonly ImmutableDictionary> DocumentChanges; + public readonly ImmutableDictionary> DocumentChanges; /// /// Contains metadata that can be used to identify the kind of sub-action these edits @@ -94,7 +94,7 @@ internal readonly struct IntentSource /// public readonly string ActionName { get; } - public IntentSource(string title, ImmutableArray textChanges, string actionName, ImmutableDictionary> documentChanges) + public IntentSource(string title, ImmutableArray textChanges, string actionName, ImmutableDictionary> documentChanges) { #pragma warning disable CS0618 // Type or member is obsolete TextChanges = textChanges; diff --git a/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs b/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs index 111dcaa1b9f21..28d7b3d96425e 100644 --- a/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs +++ b/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs @@ -97,22 +97,22 @@ public async Task> ComputeIntentsAsync(IntentReques // Merge linked file changes so all linked files have the same text changes. newSolution = await newSolution.WithMergedLinkedFileChangesAsync(originalDocument.Project.Solution, cancellationToken: cancellationToken).ConfigureAwait(false); - var results = new Dictionary>(); + var results = new Dictionary>(); 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) { - var (uri, textChanges) = docChanges.Value; - results[uri] = textChanges; + var (documentId, textChanges) = docChanges.Value; + results[documentId] = textChanges; } } - return new IntentSource(processorResult.Title, results[originalDocument.GetURI()], processorResult.ActionName, results.ToImmutableDictionary()); + return new IntentSource(processorResult.Title, results[originalDocument.Id], processorResult.ActionName, results.ToImmutableDictionary()); } - private static async Task<(Uri, ImmutableArray)?> GetTextChangesForDocumentAsync(Solution changedSolution, Solution currentSolution, DocumentId changedDocumentId, CancellationToken cancellationToken) + private static async Task<(DocumentId, ImmutableArray)?> GetTextChangesForDocumentAsync(Solution changedSolution, Solution currentSolution, DocumentId changedDocumentId, CancellationToken cancellationToken) { var changedDocument = changedSolution.GetRequiredDocument(changedDocumentId); var currentDocument = currentSolution.GetRequiredDocument(changedDocumentId); @@ -127,7 +127,7 @@ public async Task> ComputeIntentsAsync(IntentReques return null; } - return (changedDocument.GetURI(), textDiffs); + return (changedDocumentId, textDiffs); } } } From afbdc58c536887d3ef3a62dbd3b93a74690fe471 Mon Sep 17 00:00:00 2001 From: David Barbet Date: Wed, 6 Apr 2022 15:20:34 -0700 Subject: [PATCH 3/4] Move record definition up top --- src/EditorFeatures/Core/Intents/RenameIntentProvider.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs b/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs index 71068838f50b4..b33de31fe5817 100644 --- a/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs +++ b/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs @@ -19,6 +19,8 @@ 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() @@ -50,6 +52,4 @@ public async Task> ComputeIntentAsync(Docu return ImmutableArray.Create(new IntentProcessorResult(renameReplacementInfo.NewSolution, renameReplacementInfo.DocumentIds.ToImmutableArray(), EditorFeaturesResources.Rename, WellKnownIntents.Rename)); } - - private record RenameIntentData(string NewName); } From 1dd735d9d7f58236142a69dfde323b887d41f262 Mon Sep 17 00:00:00 2001 From: David Barbet Date: Wed, 6 Apr 2022 18:02:40 -0700 Subject: [PATCH 4/4] feedback --- .../ExternalAccess/IntelliCode/IntentProcessor.cs | 15 ++++++++------- .../Core/Intents/RenameIntentProvider.cs | 12 ++++++++---- .../Protocol/Extensions/ProtocolConversions.cs | 13 +------------ 3 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs b/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs index 28d7b3d96425e..94b13eccf6795 100644 --- a/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs +++ b/src/EditorFeatures/Core/ExternalAccess/IntelliCode/IntentProcessor.cs @@ -97,28 +97,29 @@ public async Task> ComputeIntentsAsync(IntentReques // Merge linked file changes so all linked files have the same text changes. newSolution = await newSolution.WithMergedLinkedFileChangesAsync(originalDocument.Project.Solution, cancellationToken: cancellationToken).ConfigureAwait(false); - var results = new Dictionary>(); + 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) { - var (documentId, textChanges) = docChanges.Value; - results[documentId] = textChanges; + results[changedDocumentId] = docChanges.Value; } } return new IntentSource(processorResult.Title, results[originalDocument.Id], processorResult.ActionName, results.ToImmutableDictionary()); } - private static async Task<(DocumentId, ImmutableArray)?> GetTextChangesForDocumentAsync(Solution changedSolution, Solution currentSolution, DocumentId changedDocumentId, CancellationToken cancellationToken) + private static async Task?> GetTextChangesForDocumentAsync( + Solution changedSolution, + Solution currentSolution, + DocumentId changedDocumentId, + CancellationToken cancellationToken) { var changedDocument = changedSolution.GetRequiredDocument(changedDocumentId); var currentDocument = currentSolution.GetRequiredDocument(changedDocumentId); - Contract.ThrowIfNull(changedDocument.FilePath, $"{changedDocumentId} missing file path"); - 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); @@ -127,7 +128,7 @@ public async Task> ComputeIntentsAsync(IntentReques return null; } - return (changedDocumentId, textDiffs); + return textDiffs; } } } diff --git a/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs b/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs index b33de31fe5817..670d23cda4da9 100644 --- a/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs +++ b/src/EditorFeatures/Core/Intents/RenameIntentProvider.cs @@ -11,6 +11,7 @@ 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; @@ -27,20 +28,23 @@ public RenameIntentProvider() { } - public async Task> ComputeIntentAsync(Document priorDocument, TextSpan priorSelection, Document currentDocument, IntentDataProvider intentDataProvider, CancellationToken cancellationToken) + public async Task> ComputeIntentAsync( + Document priorDocument, + TextSpan priorSelection, + Document currentDocument, + IntentDataProvider intentDataProvider, + CancellationToken cancellationToken) { var renameIntentData = intentDataProvider.GetIntentData(); Contract.ThrowIfNull(renameIntentData); - var renameService = priorDocument.Project.LanguageServices.GetRequiredService(); + var renameService = priorDocument.GetRequiredLanguageService(); var renameInfo = await renameService.GetRenameInfoAsync(priorDocument, priorSelection.Start, cancellationToken).ConfigureAwait(false); if (!renameInfo.CanRename) { return ImmutableArray.Empty; } - Contract.ThrowIfNull(renameIntentData); - var options = new SymbolRenameOptions( RenameOverloads: false, RenameInStrings: false, diff --git a/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs b/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs index 7fb3c02dc20d2..d431e10b5f718 100644 --- a/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs +++ b/src/Features/LanguageServer/Protocol/Extensions/ProtocolConversions.cs @@ -148,18 +148,7 @@ public static Uri GetUriFromFilePath(string filePath) if (filePath is null) throw new ArgumentNullException(nameof(filePath)); - if (Path.IsPathRooted(filePath)) - { - return new Uri(filePath, UriKind.Absolute); - } - else if (Uri.TryCreate(filePath, UriKind.Relative, out var uri)) - { - return uri; - } - else - { - throw new ArgumentException($"Failed to create URI for {filePath}"); - } + return new Uri(filePath, UriKind.Absolute); } public static Uri GetUriFromPartialFilePath(string? filePath)