From 4e41fdccc1523e6a41df7f779a4d7004bba441bc Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 22 Apr 2020 19:00:11 -0700 Subject: [PATCH 01/21] Remote ConflictResolver calls to OOP --- .../Test2/Rename/RenameEngineResult.vb | 7 +- .../Rename/ConflictEngine/ConflictResolver.cs | 40 ++++++++ .../Core/Portable/Rename/ConflictResoution.cs | 37 ++++++++ .../Core/Portable/Rename/IRemoteRenamer.cs | 94 ++++++++++++++++++- .../Services/CodeAnalysisService_Renamer.cs | 42 +++++++++ .../Compiler/Core/Log/FunctionId.cs | 3 +- 6 files changed, 215 insertions(+), 8 deletions(-) diff --git a/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb b/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb index 96f07e96218a4..ef7430aabeb2c 100644 --- a/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb +++ b/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb @@ -185,12 +185,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename ElseIf isRenameWithinStringOrComment AndAlso newToken.FullSpan.Contains(newLocation) Then newText = newToken.ToFullString().Substring(newLocation.Start - newToken.FullSpan.Start, newLocation.Length) Else - Dim newNode = newToken.Parent - While (newNode IsNot Nothing AndAlso newNode.Span <> newLocation) - newNode = newNode.Parent - End While - - newText = newNode.ToString() + newText = newTree.GetText().ToString(newLocation) End If Assert.Equal(replacementText, newText) diff --git a/src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.cs b/src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.cs index 8416e1c182920..adeaf4314d467 100644 --- a/src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.cs +++ b/src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolver.cs @@ -13,7 +13,9 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.FindSymbols; +using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.LanguageServices; +using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Shared.Extensions; using Roslyn.Utilities; @@ -39,6 +41,44 @@ internal static async Task ResolveConflictsAsync( string replacementText, ImmutableHashSet nonConflictSymbols, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + + using (Logger.LogBlock(FunctionId.Renamer_FindRenameLocationsAsync, cancellationToken)) + { + var solution = renameLocationSet.Solution; + var client = await RemoteHostClient.TryGetClientAsync(solution.Workspace, cancellationToken).ConfigureAwait(false); + if (client != null) + { + var result = await client.TryRunRemoteAsync( + WellKnownServiceHubServices.CodeAnalysisService, + nameof(IRemoteRenamer.ResolveConflictsAsync), + solution, + new object[] + { + renameLocationSet.Dehydrate(solution, cancellationToken), + replacementText, + nonConflictSymbols?.Select(s => SerializableSymbolAndProjectId.Dehydrate(solution, s, cancellationToken)).ToArray(), + }, + callbackTarget: null, + cancellationToken).ConfigureAwait(false); + + if (result.HasValue) + { + return await result.Value.RehydrateAsync(solution, cancellationToken).ConfigureAwait(false); + } + } + } + + return await ResolveConflictsInCurrentProcessAsync( + renameLocationSet, replacementText, nonConflictSymbols, cancellationToken).ConfigureAwait(false); + } + + private static async Task ResolveConflictsInCurrentProcessAsync( + RenameLocations renameLocationSet, + string replacementText, + ImmutableHashSet nonConflictSymbols, + CancellationToken cancellationToken) { var resolution = await ResolveMutableConflictsAsync( renameLocationSet, replacementText, nonConflictSymbols, cancellationToken).ConfigureAwait(false); diff --git a/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs b/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs index 4a72fcf7a206b..78fb254132971 100644 --- a/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs +++ b/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs @@ -3,6 +3,9 @@ // See the LICENSE file in the project root for more information. using System.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Rename.ConflictEngine; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; @@ -66,6 +69,40 @@ public Solution NewSolution } } + public async Task DehydrateAsync(CancellationToken cancellationToken) + { + return new SerializableConflictResolution + { + ReplacementTextValid = ReplacementTextValid, + RenamedDocument = _renamedDocument, + DocumentIds = DocumentIds, + RelatedLocations = RelatedLocations.SelectAsArray(loc => SerializableRelatedLocation.Dehydrate(loc)), + DocumentToTextChanges = await GetDocumentToTextChangesAsync(cancellationToken).ConfigureAwait(false), + DocumentToModifiedSpansMap = _documentToModifiedSpansMap.SelectAsArray(kvp => (kvp.Key, kvp.Value)), + DocumentToComplexifiedSpansMap = _documentToComplexifiedSpansMap.SelectAsArray(kvp => (kvp.Key, kvp.Value.SelectAsArray(s => SerializableComplexifiedSpan.Dehydrate(s)))), + DocumentToRelatedLocationsMap = _documentToRelatedLocationsMap.SelectAsArray(kvp => (kvp.Key, kvp.Value.SelectAsArray(s => SerializableRelatedLocation.Dehydrate(s)))), + }; + } + + private async Task)>> GetDocumentToTextChangesAsync(CancellationToken cancellationToken) + { + using var _ = ArrayBuilder<(DocumentId, ImmutableArray)>.GetInstance(out var builder); + + var solutionChanges = _newSolutionWithoutRenamedDocument.GetChanges(OldSolution); + foreach (var projectChange in solutionChanges.GetProjectChanges()) + { + foreach (var docId in projectChange.GetChangedDocuments()) + { + var oldDoc = OldSolution.GetDocument(docId); + var newDoc = _newSolutionWithoutRenamedDocument.GetDocument(docId); + var textChanges = await newDoc.GetTextChangesAsync(oldDoc, cancellationToken).ConfigureAwait(false); + builder.Add((docId, textChanges.ToImmutableArray())); + } + } + + return builder.ToImmutable(); + } + public ImmutableArray<(TextSpan oldSpan, TextSpan newSpan)> GetComplexifiedSpans(DocumentId documentId) => _documentToComplexifiedSpansMap.TryGetValue(documentId, out var complexifiedSpans) ? complexifiedSpans.SelectAsArray(c => (c.OriginalSpan, c.NewSpan)) diff --git a/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs b/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs index 4c226cfe09e5e..08a6a517979c5 100644 --- a/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs +++ b/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs @@ -9,6 +9,7 @@ using Microsoft.CodeAnalysis.FindSymbols; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Remote; +using Microsoft.CodeAnalysis.Rename.ConflictEngine; using Microsoft.CodeAnalysis.Text; namespace Microsoft.CodeAnalysis.Rename @@ -16,7 +17,17 @@ namespace Microsoft.CodeAnalysis.Rename internal interface IRemoteRenamer { Task FindRenameLocationsAsync( - PinnedSolutionInfo solutionInfo, SerializableSymbolAndProjectId symbol, SerializableRenameOptionSet options, CancellationToken cancellationToken); + PinnedSolutionInfo solutionInfo, + SerializableSymbolAndProjectId symbol, + SerializableRenameOptionSet options, + CancellationToken cancellationToken); + + Task ResolveConflictsAsync( + PinnedSolutionInfo solutionInfo, + SerializableRenameLocations renameLocationSet, + string replacementText, + SerializableSymbolAndProjectId[] nonConflictSymbols, + CancellationToken cancellationToken); } internal struct SerializableRenameOptionSet @@ -208,4 +219,85 @@ internal class SerializableRenameLocations public SerializableRenameLocation[] StringsResult; public SerializableRenameLocation[] CommentsResult; } + + internal class SerializableComplexifiedSpan + { + public TextSpan OriginalSpan; + public TextSpan NewSpan; + public ImmutableArray<(TextSpan oldSpan, TextSpan newSpan)> ModifiedSubSpans; + + public ComplexifiedSpan Rehydrate() + => new ComplexifiedSpan(OriginalSpan, NewSpan, ModifiedSubSpans); + + public static SerializableComplexifiedSpan Dehydrate(ComplexifiedSpan span) + => new SerializableComplexifiedSpan + { + OriginalSpan = span.OriginalSpan, + NewSpan = span.NewSpan, + ModifiedSubSpans = span.ModifiedSubSpans, + }; + } + + internal class SerializableRelatedLocation + { + public TextSpan ConflictCheckSpan; + public RelatedLocationType Type; + public bool IsReference; + public DocumentId DocumentId; + public TextSpan ComplexifiedTargetSpan; + + public RelatedLocation Rehydrate() + => new RelatedLocation(ConflictCheckSpan, DocumentId, Type, IsReference, ComplexifiedTargetSpan); + + public static SerializableRelatedLocation Dehydrate(RelatedLocation location) + => new SerializableRelatedLocation + { + ConflictCheckSpan = location.ConflictCheckSpan, + Type = location.Type, + IsReference = location.IsReference, + DocumentId = location.DocumentId, + ComplexifiedTargetSpan = location.ComplexifiedTargetSpan, + }; + } + + internal class SerializableConflictResolution + { + // public readonly Solution NewSolution; + public bool ReplacementTextValid; + + public (DocumentId documentId, string newName) RenamedDocument; + + public ImmutableArray DocumentIds; + public ImmutableArray RelatedLocations; + public ImmutableArray<(DocumentId, ImmutableArray)> DocumentToTextChanges; + public ImmutableArray<(DocumentId, ImmutableArray<(TextSpan oldSpan, TextSpan newSpan)>)> DocumentToModifiedSpansMap; + public ImmutableArray<(DocumentId, ImmutableArray)> DocumentToComplexifiedSpansMap; + public ImmutableArray<(DocumentId, ImmutableArray)> DocumentToRelatedLocationsMap; + + public async Task RehydrateAsync(Solution oldSolution, CancellationToken cancellationToken) + { + return new ConflictResolution( + oldSolution, + await CreateNewSolutionAsync(oldSolution, cancellationToken).ConfigureAwait(false), + ReplacementTextValid, + RenamedDocument, + DocumentIds, + RelatedLocations.SelectAsArray(loc => loc.Rehydrate()), + DocumentToModifiedSpansMap.ToImmutableDictionary(t => t.Item1, t => t.Item2), + DocumentToComplexifiedSpansMap.ToImmutableDictionary(t => t.Item1, t => t.Item2.SelectAsArray(c => c.Rehydrate())), + DocumentToRelatedLocationsMap.ToImmutableDictionary(t => t.Item1, t => t.Item2.SelectAsArray(c => c.Rehydrate()))); + } + + private async Task CreateNewSolutionAsync(Solution oldSolution, CancellationToken cancellationToken) + { + var currentSolution = oldSolution; + foreach (var (docId, textChanges) in this.DocumentToTextChanges) + { + var text = await oldSolution.GetDocument(docId).GetTextAsync(cancellationToken).ConfigureAwait(false); + currentSolution = currentSolution.WithDocumentText(docId, text.WithChanges(textChanges)); + } + + return currentSolution; + } + } } diff --git a/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_Renamer.cs b/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_Renamer.cs index 84bd5d4499064..840119a13b556 100644 --- a/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_Renamer.cs +++ b/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_Renamer.cs @@ -2,9 +2,11 @@ // 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.Collections.Immutable; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Rename; +using Microsoft.CodeAnalysis.Rename.ConflictEngine; namespace Microsoft.CodeAnalysis.Remote { @@ -35,5 +37,45 @@ public Task FindRenameLocationsAsync( } }, cancellationToken); } + + public Task ResolveConflictsAsync( + PinnedSolutionInfo solutionInfo, + SerializableRenameLocations renameLocationSet, + string replacementText, + SerializableSymbolAndProjectId[] nonConflictSymbolIds, + CancellationToken cancellationToken) + { + return RunServiceAsync(async () => + { + using (UserOperationBooster.Boost()) + { + var solution = await GetSolutionAsync(solutionInfo, cancellationToken).ConfigureAwait(false); + var nonConflictSymbols = await GetNonConflictSymbolsAsync(solution, nonConflictSymbolIds, cancellationToken).ConfigureAwait(false); + + var result = await ConflictResolver.ResolveConflictsAsync( + await RenameLocations.RehydrateAsync(solution, renameLocationSet, cancellationToken).ConfigureAwait(false), + replacementText, + nonConflictSymbols, + cancellationToken).ConfigureAwait(false); + return await result.DehydrateAsync(cancellationToken).ConfigureAwait(false); + } + }, cancellationToken); + } + + private async Task> GetNonConflictSymbolsAsync(Solution solution, SerializableSymbolAndProjectId[] nonConflictSymbolIds, CancellationToken cancellationToken) + { + if (nonConflictSymbolIds == null) + return null; + + var builder = ImmutableHashSet.CreateBuilder(); + foreach (var id in nonConflictSymbolIds) + { + var symbol = await id.TryRehydrateAsync(solution, cancellationToken).ConfigureAwait(false); + if (symbol != null) + builder.Add(symbol); + } + + return builder.ToImmutable(); + } } } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs index 0857831316949..7188f843a9335 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs @@ -481,6 +481,7 @@ internal enum FunctionId ToolsOptions_GenerateEditorconfig = 385, Renamer_RenameSymbolAsync = 386, - Renamer_FindRenameLocationsAsync = 386, + Renamer_FindRenameLocationsAsync = 387, + Renamer_ResolveConflictsAsync = 388, } } From 0e0c2a1cfff455f5657cc7daf7e2ef3ca3491f12 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 22 Apr 2020 20:54:27 -0700 Subject: [PATCH 02/21] Marshal error messages across OOP as well --- .../Core/Portable/Rename/ConflictResoution.cs | 17 +---------- .../Core/Portable/Rename/IRemoteRenamer.cs | 28 ++++++++++++++++++- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs b/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs index 58194e1fc6d88..9742da95681e3 100644 --- a/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs +++ b/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs @@ -13,7 +13,7 @@ namespace Microsoft.CodeAnalysis.Rename { - internal readonly struct ConflictResolution + internal readonly partial struct ConflictResolution { public readonly string ErrorMessage; @@ -75,21 +75,6 @@ public Solution NewSolution } } - public async Task DehydrateAsync(CancellationToken cancellationToken) - { - return new SerializableConflictResolution - { - ReplacementTextValid = ReplacementTextValid, - RenamedDocument = _renamedDocument, - DocumentIds = DocumentIds, - RelatedLocations = RelatedLocations.SelectAsArray(loc => SerializableRelatedLocation.Dehydrate(loc)), - DocumentToTextChanges = await GetDocumentToTextChangesAsync(cancellationToken).ConfigureAwait(false), - DocumentToModifiedSpansMap = _documentToModifiedSpansMap.SelectAsArray(kvp => (kvp.Key, kvp.Value)), - DocumentToComplexifiedSpansMap = _documentToComplexifiedSpansMap.SelectAsArray(kvp => (kvp.Key, kvp.Value.SelectAsArray(s => SerializableComplexifiedSpan.Dehydrate(s)))), - DocumentToRelatedLocationsMap = _documentToRelatedLocationsMap.SelectAsArray(kvp => (kvp.Key, kvp.Value.SelectAsArray(s => SerializableRelatedLocation.Dehydrate(s)))), - }; - } - private async Task)>> GetDocumentToTextChangesAsync(CancellationToken cancellationToken) { using var _ = ArrayBuilder<(DocumentId, ImmutableArray)>.GetInstance(out var builder); diff --git a/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs b/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs index 08a6a517979c5..7d84d4dbedb2a 100644 --- a/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs +++ b/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs @@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Rename.ConflictEngine; using Microsoft.CodeAnalysis.Text; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Rename { @@ -262,7 +263,8 @@ public static SerializableRelatedLocation Dehydrate(RelatedLocation location) internal class SerializableConflictResolution { - // public readonly Solution NewSolution; + public string ErrorMessage; + public bool ReplacementTextValid; public (DocumentId documentId, string newName) RenamedDocument; @@ -276,6 +278,9 @@ internal class SerializableConflictResolution public async Task RehydrateAsync(Solution oldSolution, CancellationToken cancellationToken) { + if (ErrorMessage != null) + return new ConflictResolution(ErrorMessage); + return new ConflictResolution( oldSolution, await CreateNewSolutionAsync(oldSolution, cancellationToken).ConfigureAwait(false), @@ -300,4 +305,25 @@ private async Task CreateNewSolutionAsync(Solution oldSolution, Cancel return currentSolution; } } + + internal partial struct ConflictResolution + { + public async Task DehydrateAsync(CancellationToken cancellationToken) + { + if (ErrorMessage != null) + return new SerializableConflictResolution { ErrorMessage = ErrorMessage }; + + return new SerializableConflictResolution + { + ReplacementTextValid = ReplacementTextValid, + RenamedDocument = _renamedDocument, + DocumentIds = DocumentIds, + RelatedLocations = RelatedLocations.SelectAsArray(loc => SerializableRelatedLocation.Dehydrate(loc)), + DocumentToTextChanges = await GetDocumentToTextChangesAsync(cancellationToken).ConfigureAwait(false), + DocumentToModifiedSpansMap = _documentToModifiedSpansMap.SelectAsArray(kvp => (kvp.Key, kvp.Value)), + DocumentToComplexifiedSpansMap = _documentToComplexifiedSpansMap.SelectAsArray(kvp => (kvp.Key, kvp.Value.SelectAsArray(s => SerializableComplexifiedSpan.Dehydrate(s)))), + DocumentToRelatedLocationsMap = _documentToRelatedLocationsMap.SelectAsArray(kvp => (kvp.Key, kvp.Value.SelectAsArray(s => SerializableRelatedLocation.Dehydrate(s)))), + }; + } + } } From 8bce494436950ef21b63ae77e93eddb5affba960 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 22 Apr 2020 21:36:51 -0700 Subject: [PATCH 03/21] Use simple arrays. --- .../Core/Portable/Rename/ConflictResoution.cs | 4 ++-- .../Core/Portable/Rename/IRemoteRenamer.cs | 24 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs b/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs index 9742da95681e3..4331fb043f083 100644 --- a/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs +++ b/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs @@ -75,7 +75,7 @@ public Solution NewSolution } } - private async Task)>> GetDocumentToTextChangesAsync(CancellationToken cancellationToken) + private async Task<(DocumentId, ImmutableArray)[]> GetDocumentToTextChangesAsync(CancellationToken cancellationToken) { using var _ = ArrayBuilder<(DocumentId, ImmutableArray)>.GetInstance(out var builder); @@ -91,7 +91,7 @@ public Solution NewSolution } } - return builder.ToImmutable(); + return builder.ToArray(); } public ImmutableArray<(TextSpan oldSpan, TextSpan newSpan)> GetComplexifiedSpans(DocumentId documentId) diff --git a/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs b/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs index 7d84d4dbedb2a..b32a830bb1206 100644 --- a/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs +++ b/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs @@ -269,12 +269,12 @@ internal class SerializableConflictResolution public (DocumentId documentId, string newName) RenamedDocument; - public ImmutableArray DocumentIds; - public ImmutableArray RelatedLocations; - public ImmutableArray<(DocumentId, ImmutableArray)> DocumentToTextChanges; - public ImmutableArray<(DocumentId, ImmutableArray<(TextSpan oldSpan, TextSpan newSpan)>)> DocumentToModifiedSpansMap; - public ImmutableArray<(DocumentId, ImmutableArray)> DocumentToComplexifiedSpansMap; - public ImmutableArray<(DocumentId, ImmutableArray)> DocumentToRelatedLocationsMap; + public DocumentId[] DocumentIds; + public SerializableRelatedLocation[] RelatedLocations; + public (DocumentId, ImmutableArray)[] DocumentToTextChanges; + public (DocumentId, ImmutableArray<(TextSpan oldSpan, TextSpan newSpan)>)[] DocumentToModifiedSpansMap; + public (DocumentId, ImmutableArray)[] DocumentToComplexifiedSpansMap; + public (DocumentId, ImmutableArray)[] DocumentToRelatedLocationsMap; public async Task RehydrateAsync(Solution oldSolution, CancellationToken cancellationToken) { @@ -286,7 +286,7 @@ public async Task RehydrateAsync(Solution oldSolution, Cance await CreateNewSolutionAsync(oldSolution, cancellationToken).ConfigureAwait(false), ReplacementTextValid, RenamedDocument, - DocumentIds, + DocumentIds.ToImmutableArray(), RelatedLocations.SelectAsArray(loc => loc.Rehydrate()), DocumentToModifiedSpansMap.ToImmutableDictionary(t => t.Item1, t => t.Item2), DocumentToComplexifiedSpansMap.ToImmutableDictionary(t => t.Item1, t => t.Item2.SelectAsArray(c => c.Rehydrate())), @@ -317,12 +317,12 @@ public async Task DehydrateAsync(CancellationTok { ReplacementTextValid = ReplacementTextValid, RenamedDocument = _renamedDocument, - DocumentIds = DocumentIds, - RelatedLocations = RelatedLocations.SelectAsArray(loc => SerializableRelatedLocation.Dehydrate(loc)), + DocumentIds = DocumentIds.ToArray(), + RelatedLocations = RelatedLocations.Select(loc => SerializableRelatedLocation.Dehydrate(loc)).ToArray(), DocumentToTextChanges = await GetDocumentToTextChangesAsync(cancellationToken).ConfigureAwait(false), - DocumentToModifiedSpansMap = _documentToModifiedSpansMap.SelectAsArray(kvp => (kvp.Key, kvp.Value)), - DocumentToComplexifiedSpansMap = _documentToComplexifiedSpansMap.SelectAsArray(kvp => (kvp.Key, kvp.Value.SelectAsArray(s => SerializableComplexifiedSpan.Dehydrate(s)))), - DocumentToRelatedLocationsMap = _documentToRelatedLocationsMap.SelectAsArray(kvp => (kvp.Key, kvp.Value.SelectAsArray(s => SerializableRelatedLocation.Dehydrate(s)))), + DocumentToModifiedSpansMap = _documentToModifiedSpansMap.Select(kvp => (kvp.Key, kvp.Value)).ToArray(), + DocumentToComplexifiedSpansMap = _documentToComplexifiedSpansMap.Select(kvp => (kvp.Key, kvp.Value.SelectAsArray(s => SerializableComplexifiedSpan.Dehydrate(s)))).ToArray(), + DocumentToRelatedLocationsMap = _documentToRelatedLocationsMap.Select(kvp => (kvp.Key, kvp.Value.SelectAsArray(s => SerializableRelatedLocation.Dehydrate(s)))).ToArray(), }; } } From 33b95745d28e4a0e78b6307109ab2e1c3752bf5d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 23 Apr 2020 14:51:00 -0700 Subject: [PATCH 04/21] Have callers deal with ErrorMessage --- ...neRenameService.InlineRenameLocationSet.cs | 4 +-- .../Test2/Rename/RenameEngineResult.vb | 11 +++++-- .../Test2/Rename/RenameEngineTests.vb | 28 +++++++---------- .../AbstractEncapsulateFieldService.cs | 9 ++++-- .../AbstractUseAutoPropertyCodeFixProvider.cs | 15 +++++---- .../Core/Portable/Rename/Renamer.cs | 31 +++++-------------- 6 files changed, 45 insertions(+), 53 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.InlineRenameLocationSet.cs b/src/EditorFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.InlineRenameLocationSet.cs index df00d9850652d..2fe9b83fde906 100644 --- a/src/EditorFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.InlineRenameLocationSet.cs +++ b/src/EditorFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.InlineRenameLocationSet.cs @@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Rename; using Microsoft.CodeAnalysis.Rename.ConflictEngine; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename { @@ -43,8 +44,7 @@ public async Task GetReplacementsAsync(string repl var conflicts = await _renameLocationSet.ResolveConflictsAsync( _renameInfo.GetFinalSymbolName(replacementText), nonConflictSymbols: null, cancellationToken: cancellationToken).ConfigureAwait(false); - if (conflicts.ErrorMessage != null) - throw new ArgumentException(conflicts.ErrorMessage); + Contract.ThrowIfTrue(conflicts.ErrorMessage != null); return new InlineRenameReplacementInfo(conflicts); } diff --git a/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb b/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb index e2e8b84c75de2..ed5fc08cebe6f 100644 --- a/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb +++ b/src/EditorFeatures/Test2/Rename/RenameEngineResult.vb @@ -49,7 +49,8 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename workspaceXml As XElement, renameTo As String, host As TestHost, - Optional changedOptionSet As Dictionary(Of OptionKey, Object) = Nothing) As RenameEngineResult + Optional changedOptionSet As Dictionary(Of OptionKey, Object) = Nothing, + Optional expectFailure As Boolean = False) As RenameEngineResult Dim workspace = TestWorkspace.CreateWorkspace(workspaceXml) workspace.SetTestLogger(AddressOf helper.WriteLine) @@ -84,8 +85,12 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename workspace.CurrentSolution, symbol, optionSet, CancellationToken.None).Result Dim result = locations.ResolveConflictsAsync(renameTo, nonConflictSymbols:=Nothing, cancellationToken:=CancellationToken.None).GetAwaiter().GetResult() - If result.ErrorMessage IsNot Nothing Then - Throw New ArgumentException(result.ErrorMessage) + + If expectFailure Then + Assert.NotNull(result.ErrorMessage) + Return engineResult + Else + Assert.Null(result.ErrorMessage) End If engineResult = New RenameEngineResult(workspace, result, renameTo) diff --git a/src/EditorFeatures/Test2/Rename/RenameEngineTests.vb b/src/EditorFeatures/Test2/Rename/RenameEngineTests.vb index cf55b59e5b265..341368b763e70 100644 --- a/src/EditorFeatures/Test2/Rename/RenameEngineTests.vb +++ b/src/EditorFeatures/Test2/Rename/RenameEngineTests.vb @@ -3672,11 +3672,10 @@ Class C Public Sub Bug622086_CSRenameVarNotSupported(host As TestHost) - Assert.ThrowsAny(Of ArgumentException)(Sub() - Dim result = RenameEngineResult.Create(_outputHelper, - - - + + ] - - , host:=host, renameTo:="Int32") - End Sub) + +, host:=host, renameTo:="Int32", expectFailure:=True) End Sub @@ -3731,11 +3729,10 @@ namespace X Public Sub Bug622086_CSRenameDynamicNotSupported(host As TestHost) - Assert.ThrowsAny(Of ArgumentException)(Sub() - Dim result = RenameEngineResult.Create(_outputHelper, - - - + + ] - - , host:=host, renameTo:="Int32") - End Sub) + +, host:=host, renameTo:="Int32", expectFailure:=True) End Sub diff --git a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs index 3957e60ada41f..cd96b7fc48d20 100644 --- a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs +++ b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs @@ -283,9 +283,12 @@ private async Task RenameAsync( var initialLocations = await Renamer.FindRenameLocationsAsync( solution, field, optionSet: null, cancellationToken).ConfigureAwait(false); - return await Renamer.RenameAsync( - initialLocations.Filter(filter), finalName, - cancellationToken: cancellationToken).ConfigureAwait(false); + var resolution = await initialLocations.Filter(filter).ResolveConflictsAsync( + finalName, nonConflictSymbols: null, cancellationToken).ConfigureAwait(false); + + Contract.ThrowIfTrue(resolution.ErrorMessage != null); + + return resolution.NewSolution; } private bool IntersectsWithAny(Location location, ISet constructorLocations) diff --git a/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyCodeFixProvider.cs b/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyCodeFixProvider.cs index 92dcd680efce4..137674e780178 100644 --- a/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyCodeFixProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyCodeFixProvider.cs @@ -126,15 +126,18 @@ private async Task ProcessResultAsync(CodeFixContext context, Diagnost // To address this, we let rename know that there is no conflict if the new symbol it resolves to is the // same as the property we're trying to get the references pointing to. - var updatedSolution = await Renamer.RenameAsync( - fieldLocations.Filter( - location => !location.IntersectsWith(declaratorLocation) && - CanEditDocument(solution, location.SourceTree, linkedFiles, canEdit)), + var filteredLocations = fieldLocations.Filter( + location => !location.IntersectsWith(declaratorLocation) && + CanEditDocument(solution, location.SourceTree, linkedFiles, canEdit)); + + var resolution = await filteredLocations.ResolveConflictsAsync( propertySymbol.Name, nonConflictSymbols: ImmutableHashSet.Create(propertySymbol), cancellationToken).ConfigureAwait(false); - solution = updatedSolution; + Contract.ThrowIfTrue(resolution.ErrorMessage != null); + + solution = resolution.NewSolution; // Now find the field and property again post rename. fieldDocument = solution.GetDocument(fieldDocument.Id); @@ -219,7 +222,7 @@ private async Task ProcessResultAsync(CodeFixContext context, Diagnost newFieldTreeRoot = await FormatAsync(newFieldTreeRoot, fieldDocument, cancellationToken).ConfigureAwait(false); newPropertyTreeRoot = await FormatAsync(newPropertyTreeRoot, propertyDocument, cancellationToken).ConfigureAwait(false); - updatedSolution = solution.WithDocumentSyntaxRoot(fieldDocument.Id, newFieldTreeRoot); + var updatedSolution = solution.WithDocumentSyntaxRoot(fieldDocument.Id, newFieldTreeRoot); updatedSolution = updatedSolution.WithDocumentSyntaxRoot(propertyDocument.Id, newPropertyTreeRoot); return updatedSolution; diff --git a/src/Workspaces/Core/Portable/Rename/Renamer.cs b/src/Workspaces/Core/Portable/Rename/Renamer.cs index c784638d7512b..1cad0c1439281 100644 --- a/src/Workspaces/Core/Portable/Rename/Renamer.cs +++ b/src/Workspaces/Core/Portable/Rename/Renamer.cs @@ -13,7 +13,7 @@ namespace Microsoft.CodeAnalysis.Rename { public static class Renamer { - public static Task RenameSymbolAsync( + public static async Task RenameSymbolAsync( Solution solution, ISymbol symbol, string newName, OptionSet optionSet, CancellationToken cancellationToken = default) { if (solution == null) @@ -30,7 +30,11 @@ public static Task RenameSymbolAsync( optionSet ??= solution.Options; - return RenameSymbolAsync(solution, symbol, newName, optionSet, nonConflictSymbols: null, cancellationToken); + var resolution = await RenameSymbolAsync(solution, symbol, newName, optionSet, nonConflictSymbols: null, cancellationToken).ConfigureAwait(false); + if (resolution.ErrorMessage != null) + throw new ArgumentException(resolution.ErrorMessage); + + return resolution.NewSolution; } internal static Task FindRenameLocationsAsync( @@ -40,26 +44,7 @@ internal static Task FindRenameLocationsAsync( symbol, solution, RenameOptionSet.From(solution, optionSet), cancellationToken); } - internal static async Task RenameAsync( - RenameLocations locations, - string newName, - ImmutableHashSet nonConflictSymbols = null, - CancellationToken cancellationToken = default) - { - Contract.ThrowIfTrue(string.IsNullOrEmpty(newName)); - - cancellationToken.ThrowIfCancellationRequested(); - - var resolution = await locations.ResolveConflictsAsync( - newName, nonConflictSymbols, cancellationToken).ConfigureAwait(false); - - if (resolution.ErrorMessage != null) - throw new ArgumentException(resolution.ErrorMessage); - - return resolution.NewSolution; - } - - internal static async Task RenameSymbolAsync( + internal static async Task RenameSymbolAsync( Solution solution, ISymbol symbol, string newName, @@ -75,7 +60,7 @@ internal static async Task RenameSymbolAsync( cancellationToken.ThrowIfCancellationRequested(); var renameLocations = await FindRenameLocationsAsync(solution, symbol, optionSet, cancellationToken).ConfigureAwait(false); - return await RenameAsync(renameLocations, newName, nonConflictSymbols, cancellationToken).ConfigureAwait(false); + return await renameLocations.ResolveConflictsAsync(newName, nonConflictSymbols, cancellationToken).ConfigureAwait(false); } } } From f15f28c1ceb6eaf8d72364a8833c44443df4e108 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 23 Apr 2020 14:52:12 -0700 Subject: [PATCH 05/21] Rename file --- .../Rename/{ConflictResoution.cs => ConflictResolution.cs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/Workspaces/Core/Portable/Rename/{ConflictResoution.cs => ConflictResolution.cs} (100%) diff --git a/src/Workspaces/Core/Portable/Rename/ConflictResoution.cs b/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs similarity index 100% rename from src/Workspaces/Core/Portable/Rename/ConflictResoution.cs rename to src/Workspaces/Core/Portable/Rename/ConflictResolution.cs From d7ba99598acaabbc715f985e099e0ba5a797d3b1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 23 Apr 2020 14:53:57 -0700 Subject: [PATCH 06/21] Move code to constructor --- .../Portable/Rename/ConflictResolution.cs | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs b/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs index 4331fb043f083..d9dc5491ec204 100644 --- a/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs +++ b/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs @@ -22,6 +22,11 @@ internal readonly partial struct ConflictResolution public readonly Solution OldSolution; + /// + /// The final solution snapshot. Including any renamed documents. + /// + public readonly Solution NewSolution; + public readonly bool ReplacementTextValid; /// @@ -58,21 +63,10 @@ public ConflictResolution( _documentToModifiedSpansMap = documentToModifiedSpansMap; _documentToComplexifiedSpansMap = documentToComplexifiedSpansMap; _documentToRelatedLocationsMap = documentToRelatedLocationsMap; - } - - /// - /// The final solution snapshot - /// - public Solution NewSolution - { - get - { - var newSolution = _newSolutionWithoutRenamedDocument; - if (_renamedDocument.documentId != null) - newSolution = newSolution.WithDocumentName(_renamedDocument.documentId, _renamedDocument.newName); - return newSolution; - } + NewSolution = _renamedDocument.documentId == null + ? _newSolutionWithoutRenamedDocument + : _newSolutionWithoutRenamedDocument.WithDocumentName(_renamedDocument.documentId, _renamedDocument.newName); } private async Task<(DocumentId, ImmutableArray)[]> GetDocumentToTextChangesAsync(CancellationToken cancellationToken) From e1f15cd0913cdab208643c2a2c14f9b28016f55e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 11:28:21 -0700 Subject: [PATCH 07/21] Use tuple --- .../EncapsulateField/CSharpEncapsulateFieldService.cs | 10 +++++----- .../AbstractEncapsulateFieldService.cs | 6 ++---- .../VisualBasicEncapsulateFieldService.vb | 6 +++--- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/Features/CSharp/Portable/EncapsulateField/CSharpEncapsulateFieldService.cs b/src/Features/CSharp/Portable/EncapsulateField/CSharpEncapsulateFieldService.cs index 363b781a087e2..0c7e1fb59dc69 100644 --- a/src/Features/CSharp/Portable/EncapsulateField/CSharpEncapsulateFieldService.cs +++ b/src/Features/CSharp/Portable/EncapsulateField/CSharpEncapsulateFieldService.cs @@ -146,13 +146,13 @@ protected override async Task> GetFieldsAsync(Document private bool CanEncapsulate(FieldDeclarationSyntax field) => field.Parent is TypeDeclarationSyntax; - protected override Tuple GeneratePropertyAndFieldNames(IFieldSymbol field) + protected override (string fieldName, string propertyName) GenerateFieldAndPropertyNames(IFieldSymbol field) { // Special case: if the field is "new", we will preserve its original name and the new keyword. if (field.DeclaredAccessibility == Accessibility.Private || IsNew(field)) { // Create some capitalized version of the field name for the property - return Tuple.Create(field.Name, MakeUnique(GeneratePropertyName(field.Name), field.ContainingType)); + return (field.Name, MakeUnique(GeneratePropertyName(field.Name), field.ContainingType)); } else { @@ -162,7 +162,7 @@ protected override Tuple GeneratePropertyAndFieldNames(IFieldSym if (newPropertyName == field.Name) { // If we wind up with the field's old name, give the field the unique version of its current name. - return Tuple.Create(MakeUnique(GenerateFieldName(field.Name), field.ContainingType), newPropertyName); + return (MakeUnique(GenerateFieldName(field.Name), field.ContainingType), newPropertyName); } // Otherwise, ensure the property's name is unique. @@ -172,11 +172,11 @@ protected override Tuple GeneratePropertyAndFieldNames(IFieldSym // If converting the new property's name into a field name results in the old field name, we're done. if (newFieldName == field.Name) { - return Tuple.Create(newFieldName, newPropertyName); + return (newFieldName, newPropertyName); } // Otherwise, ensure the new field name is unique. - return Tuple.Create(MakeUnique(newFieldName, field.ContainingType), newPropertyName); + return (MakeUnique(newFieldName, field.ContainingType), newPropertyName); } } diff --git a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs index cd96b7fc48d20..e93579171b587 100644 --- a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs +++ b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs @@ -150,9 +150,7 @@ private async Task EncapsulateFieldResultAsync(Document document, TextSp private async Task EncapsulateFieldAsync(IFieldSymbol field, Document document, bool updateReferences, CancellationToken cancellationToken) { var originalField = field; - var finalNames = GeneratePropertyAndFieldNames(field); - var finalFieldName = finalNames.Item1; - var generatedPropertyName = finalNames.Item2; + var (finalFieldName, generatedPropertyName) = GenerateFieldAndPropertyNames(field); // Annotate the field declarations so we can find it after rename. var fieldDeclaration = field.DeclaringSyntaxReferences.First(); @@ -352,7 +350,7 @@ protected IPropertySymbol GenerateProperty( Formatter.Annotation.AddAnnotationToSymbol(propertySymbol)); } - protected abstract Tuple GeneratePropertyAndFieldNames(IFieldSymbol field); + protected abstract (string fieldName, string propertyName) GenerateFieldAndPropertyNames(IFieldSymbol field); protected Accessibility ComputeAccessibility(Accessibility accessibility, ITypeSymbol type) { diff --git a/src/Features/VisualBasic/Portable/EncapsulateField/VisualBasicEncapsulateFieldService.vb b/src/Features/VisualBasic/Portable/EncapsulateField/VisualBasicEncapsulateFieldService.vb index a18c713114664..0094a0a1204bf 100644 --- a/src/Features/VisualBasic/Portable/EncapsulateField/VisualBasicEncapsulateFieldService.vb +++ b/src/Features/VisualBasic/Portable/EncapsulateField/VisualBasicEncapsulateFieldService.vb @@ -100,19 +100,19 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.EncapsulateField End If End Function - Protected Overrides Function GeneratePropertyAndFieldNames(field As IFieldSymbol) As Tuple(Of String, String) + Protected Overrides Function GenerateFieldAndPropertyNames(field As IFieldSymbol) As (fieldName As String, propertyName As String) ' If the field is marked shadows, it will keep its name. If field.DeclaredAccessibility = Accessibility.Private OrElse IsShadows(field) Then Dim propertyName = GeneratePropertyName(field.Name) propertyName = MakeUnique(propertyName, field) - Return Tuple.Create(field.Name, propertyName) + Return (field.Name, propertyName) Else Dim propertyName = GeneratePropertyName(field.Name) Dim containingTypeMemberNames = field.ContainingType.GetAccessibleMembersInThisAndBaseTypes(Of ISymbol)(field.ContainingType).Select(Function(s) s.Name) propertyName = NameGenerator.GenerateUniqueName(propertyName, containingTypeMemberNames.Where(Function(m) m <> field.Name).ToSet(), StringComparer.OrdinalIgnoreCase) Dim newFieldName = MakeUnique("_" + Char.ToLower(propertyName(0)) + propertyName.Substring(1), field) - Return Tuple.Create(newFieldName, propertyName) + Return (newFieldName, propertyName) End If End Function From d237d7f3a3ef103eb0654122a56ed3b58522816d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 14:53:07 -0700 Subject: [PATCH 08/21] Add overload for rpc that can work on streams, not just object writers. --- .../ServiceHub/Shared/RemoteEndPoint.cs | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/Workspaces/Remote/ServiceHub/Shared/RemoteEndPoint.cs b/src/Workspaces/Remote/ServiceHub/Shared/RemoteEndPoint.cs index a0957abdee111..9b8dcc647d533 100644 --- a/src/Workspaces/Remote/ServiceHub/Shared/RemoteEndPoint.cs +++ b/src/Workspaces/Remote/ServiceHub/Shared/RemoteEndPoint.cs @@ -137,10 +137,12 @@ public async Task InvokeAsync(string targetName, IReadOnlyList ar } /// - /// Invokes a remote method with specified and - /// establishes a pipe through which the target method may transfer large binary data. - /// The name of the pipe is passed to the target method as an additional argument following the specified . - /// The target method is expected to use to write the data to the pipe stream. + /// Invokes a remote method with specified and + /// establishes a pipe through which the target method may transfer large binary data. The name of the pipe is + /// passed to the target method as an additional argument following the specified . + /// The target method is expected to use + /// + /// to write the data to the pipe stream. /// public async Task InvokeAsync(string targetName, IReadOnlyList arguments, Func> dataReader, CancellationToken cancellationToken) { @@ -188,7 +190,15 @@ public async Task InvokeAsync(string targetName, IReadOnlyList ar } } - public static async Task WriteDataToNamedPipeAsync(string pipeName, TData data, Func dataWriter, CancellationToken cancellationToken) + public static Task WriteDataToNamedPipeAsync(string pipeName, TData data, Func dataWriter, CancellationToken cancellationToken) + => WriteDataToNamedPipeAsync(pipeName, data, + async (s, d, c) => + { + using var objectWriter = new ObjectWriter(s, leaveOpen: true, c); + await dataWriter(objectWriter, d, c).ConfigureAwait(false); + }, cancellationToken); + + public static async Task WriteDataToNamedPipeAsync(string pipeName, TData data, Func dataWriter, CancellationToken cancellationToken) { const int BufferSize = 4 * 1024; @@ -213,11 +223,7 @@ public static async Task WriteDataToNamedPipeAsync(string pipeName, TData // Transfer ownership of the pipe to BufferedStream, it will dispose it: using var stream = new BufferedStream(pipe, BufferSize); - using (var objectWriter = new ObjectWriter(stream, leaveOpen: true, cancellationToken)) - { - await dataWriter(objectWriter, data, cancellationToken).ConfigureAwait(false); - } - + await dataWriter(stream, data, cancellationToken).ConfigureAwait(false); await stream.FlushAsync(cancellationToken).ConfigureAwait(false); } catch (Exception) when (cancellationToken.IsCancellationRequested) From d2c457401ccb7a3bb2aa63c6ad30038998613644 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 15:49:28 -0700 Subject: [PATCH 09/21] Enable OOP testing of encapsulte field --- .../EncapsulateField/EncapsulateFieldTests.cs | 270 ++++++++++-------- 1 file changed, 143 insertions(+), 127 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/CodeActions/EncapsulateField/EncapsulateFieldTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/EncapsulateField/EncapsulateFieldTests.cs index e07f73e6f8bcc..b87a6805a0800 100644 --- a/src/EditorFeatures/CSharpTest/CodeActions/EncapsulateField/EncapsulateFieldTests.cs +++ b/src/EditorFeatures/CSharpTest/CodeActions/EncapsulateField/EncapsulateFieldTests.cs @@ -11,11 +11,18 @@ using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions; using Microsoft.CodeAnalysis.EncapsulateField; using Microsoft.CodeAnalysis.Test.Utilities; +using Microsoft.CodeAnalysis.Test.Utilities.RemoteHost; using Roslyn.Test.Utilities; using Xunit; namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.CodeRefactorings.EncapsulateField { + public enum TestHost + { + InProcess, + OutOfProcess, + } + public class EncapsulateFieldTests : AbstractCSharpCodeActionTest { protected override CodeRefactoringProvider CreateCodeRefactoringProvider(Workspace workspace, TestParameters parameters) @@ -29,20 +36,21 @@ protected override CodeRefactoringProvider CreateCodeRefactoringProvider(Workspa }; internal Task TestAllOptionsOffAsync( - string initialMarkup, string expectedMarkup, + TestHost host, string initialMarkup, string expectedMarkup, ParseOptions parseOptions = null, CompilationOptions compilationOptions = null, int index = 0, OptionsCollection options = null) { options = options ?? new OptionsCollection(GetLanguage()); options.AddRange(AllOptionsOff); + options.Add(RemoteHostOptions.RemoteHostTest, host == TestHost.OutOfProcess); return TestAsync(initialMarkup, expectedMarkup, parseOptions, compilationOptions, index, options); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task PrivateFieldToPropertyIgnoringReferences() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task PrivateFieldToPropertyIgnoringReferences(TestHost host) { var text = @" class goo @@ -80,11 +88,11 @@ void baz() } } "; - await TestAllOptionsOffAsync(text, expected, index: 1); + await TestAllOptionsOffAsync(host, text, expected, index: 1); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task PrivateNullableFieldToPropertyIgnoringReferences() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task PrivateNullableFieldToPropertyIgnoringReferences(TestHost host) { var text = @"#nullable enable class goo @@ -122,11 +130,11 @@ void baz() } } "; - await TestAllOptionsOffAsync(text, expected, index: 1); + await TestAllOptionsOffAsync(host, text, expected, index: 1); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task PrivateFieldToPropertyUpdatingReferences() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task PrivateFieldToPropertyUpdatingReferences(TestHost host) { var text = @" class goo @@ -164,11 +172,11 @@ void baz() } } "; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task TestCodeStyle1() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task TestCodeStyle1(TestHost host) { var text = @" class goo @@ -211,11 +219,12 @@ await TestInRegularAndScriptAsync(text, expected, { { CSharpCodeStyleOptions.PreferExpressionBodiedProperties, ExpressionBodyPreference.WhenPossible, NotificationOption2.Silent }, { CSharpCodeStyleOptions.PreferExpressionBodiedAccessors, ExpressionBodyPreference.Never, NotificationOption2.Silent }, + { RemoteHostOptions.RemoteHostTest, host == TestHost.OutOfProcess } }); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task TestCodeStyle2() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task TestCodeStyle2(TestHost host) { var text = @" class goo @@ -243,11 +252,15 @@ void baz() } "; await TestInRegularAndScriptAsync(text, expected, - options: Option(CSharpCodeStyleOptions.PreferExpressionBodiedAccessors, CSharpCodeStyleOptions.WhenPossibleWithSilentEnforcement)); + options: new OptionsCollection(GetLanguage()) + { + { CSharpCodeStyleOptions.PreferExpressionBodiedAccessors, CSharpCodeStyleOptions.WhenPossibleWithSilentEnforcement }, + { RemoteHostOptions.RemoteHostTest, host == TestHost.OutOfProcess } + }); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task PublicFieldIntoPublicPropertyIgnoringReferences() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task PublicFieldIntoPublicPropertyIgnoringReferences(TestHost host) { var text = @" class goo @@ -285,11 +298,11 @@ void baz() } } "; - await TestAllOptionsOffAsync(text, expected, index: 1); + await TestAllOptionsOffAsync(host, text, expected, index: 1); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task PublicFieldIntoPublicPropertyUpdatingReferences() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task PublicFieldIntoPublicPropertyUpdatingReferences(TestHost host) { var text = @" class goo @@ -327,11 +340,11 @@ void baz() } } "; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task StaticPreserved() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task StaticPreserved(TestHost host) { var text = @"class Program { @@ -355,11 +368,11 @@ public static int Goo } } }"; - await TestAllOptionsOffAsync(text, expected); + await TestAllOptionsOffAsync(host, text, expected); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task UniqueNameGenerated() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task UniqueNameGenerated(TestHost host) { var text = @" class Program @@ -387,11 +400,11 @@ public int Goo1 } } }"; - await TestAllOptionsOffAsync(text, expected); + await TestAllOptionsOffAsync(host, text, expected); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task GenericField() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task GenericField(TestHost host) { var text = @" class C @@ -417,11 +430,11 @@ public T Goo } } }"; - await TestAllOptionsOffAsync(text, expected); + await TestAllOptionsOffAsync(host, text, expected); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task NewFieldNameIsUnique() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task NewFieldNameIsUnique(TestHost host) { var text = @" class goo @@ -449,11 +462,11 @@ public int X } } }"; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task RespectReadonly() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task RespectReadonly(TestHost host) { var text = @" class goo @@ -474,11 +487,11 @@ public int X } } }"; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task PreserveNewAndConsiderBaseMemberNames() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task PreserveNewAndConsiderBaseMemberNames(TestHost host) { var text = @" class c @@ -518,11 +531,11 @@ protected int Goo1 } } }"; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task EncapsulateMultiplePrivateFields() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task EncapsulateMultiplePrivateFields(TestHost host) { var text = @" class goo @@ -573,11 +586,11 @@ void bar() Y = 2; } }"; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task EncapsulateMultiplePrivateFields2() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task EncapsulateMultiplePrivateFields2(TestHost host) { var text = @" class goo @@ -630,11 +643,11 @@ void bar() Y = 2; } }"; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task EncapsulateSinglePublicFieldInMultipleVariableDeclarationAndUpdateReferences() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task EncapsulateSinglePublicFieldInMultipleVariableDeclarationAndUpdateReferences(TestHost host) { var text = @" class goo @@ -673,12 +686,12 @@ void bar() y = 2; } }"; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } [WorkItem(694057, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/694057")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task ConstFieldNoGetter() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task ConstFieldNoGetter(TestHost host) { var text = @" class Program @@ -701,12 +714,12 @@ public static int Bar } } "; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } [WorkItem(694276, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/694276")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task EncapsulateFieldNamedValue() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task EncapsulateFieldNamedValue(TestHost host) { var text = @" class Program @@ -729,12 +742,12 @@ public static int Bar } } "; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } [WorkItem(694276, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/694276")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task PublicFieldNamed__() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task PublicFieldNamed__(TestHost host) { var text = @" class Program @@ -762,12 +775,12 @@ public int __ } } "; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } [WorkItem(695046, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/695046")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task AvailableNotJustOnVariableName() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task AvailableNotJustOnVariableName(TestHost host) { var text = @" class Program @@ -776,12 +789,17 @@ class Program } "; - await TestActionCountAsync(text, 2); + await TestActionCountAsync(text, 2, GetRemoteHostOptions(host)); + } + + private TestParameters GetRemoteHostOptions(TestHost host) + { + return new TestParameters(options: Option(RemoteHostOptions.RemoteHostTest, host == TestHost.OutOfProcess)); } [WorkItem(705898, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/705898")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task CopyFieldAccessibility() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task CopyFieldAccessibility(TestHost host) { var text = @" class Program @@ -804,11 +822,11 @@ protected static int Bar } } "; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task UpdateReferencesCrossProject() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task UpdateReferencesCrossProject(TestHost host) { var text = @" @@ -872,12 +890,12 @@ void bar() "; - await TestAllOptionsOffAsync(text, expected, new CodeAnalysis.CSharp.CSharpParseOptions(), TestOptions.ReleaseExe); + await TestAllOptionsOffAsync(host, text, expected, new CodeAnalysis.CSharp.CSharpParseOptions(), TestOptions.ReleaseExe); } [WorkItem(713269, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/713269")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task PreserveUnsafe() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task PreserveUnsafe(TestHost host) { var text = @" class C @@ -905,12 +923,12 @@ public unsafe int* Goo } } "; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } [WorkItem(713240, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/713240")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task ConsiderReturnTypeAccessibility() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task ConsiderReturnTypeAccessibility(TestHost host) { var text = @" public class Program @@ -948,12 +966,12 @@ internal enum State WA } "; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } [WorkItem(713191, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/713191")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task DoNotReferToReadOnlyPropertyInConstructor() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task DoNotReferToReadOnlyPropertyInConstructor(TestHost host) { var text = @" class Program @@ -984,12 +1002,12 @@ public int Field } } }"; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } [WorkItem(713191, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/713191")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task DoNotReferToStaticReadOnlyPropertyInConstructor() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task DoNotReferToStaticReadOnlyPropertyInConstructor(TestHost host) { var text = @" class Program @@ -1020,12 +1038,12 @@ public static int Field } } }"; - await TestAllOptionsOffAsync(text, expected, index: 0); + await TestAllOptionsOffAsync(host, text, expected, index: 0); } [WorkItem(765959, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/765959")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task GenerateInTheCorrectPart() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task GenerateInTheCorrectPart(TestHost host) { var text = @" partial class Program {} @@ -1054,24 +1072,24 @@ public int X } } "; - await TestAllOptionsOffAsync(text, expected); + await TestAllOptionsOffAsync(host, text, expected); } [WorkItem(829178, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/829178")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task ErrorTolerance() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task ErrorTolerance(TestHost host) { var text = @"class Program { a b c [|b|] }"; - await TestActionCountAsync(text, count: 2); + await TestActionCountAsync(text, count: 2, GetRemoteHostOptions(host)); } [WorkItem(834072, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/834072")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task DuplicateFieldErrorTolerance() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task DuplicateFieldErrorTolerance(TestHost host) { var text = @" class Program @@ -1081,12 +1099,12 @@ class Program } "; - await TestActionCountAsync(text, count: 2); + await TestActionCountAsync(text, count: 2, GetRemoteHostOptions(host)); } [WorkItem(862517, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/862517")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task Trivia() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task Trivia(TestHost host) { var text = @" namespace ConsoleApplication1 @@ -1123,36 +1141,36 @@ public int MyInt } "; - await TestAllOptionsOffAsync(text, expected); + await TestAllOptionsOffAsync(host, text, expected); } [WorkItem(1096007, "https://github.com/dotnet/roslyn/issues/282")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task DoNotEncapsulateOutsideTypeDeclaration() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task DoNotEncapsulateOutsideTypeDeclaration(TestHost host) { await TestMissingInRegularAndScriptAsync( -@"var [|x|] = 1;"); +@"var [|x|] = 1;", GetRemoteHostOptions(host)); await TestMissingInRegularAndScriptAsync( @"namespace N { var [|x|] = 1; -}"); +}", GetRemoteHostOptions(host)); await TestMissingInRegularAndScriptAsync( @"enum E { [|x|] = 1; -}"); +}", GetRemoteHostOptions(host)); } [WorkItem(5524, "https://github.com/dotnet/roslyn/issues/5524")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task AlwaysUseEnglishUSCultureWhenFixingVariableNames_TurkishDottedI() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task AlwaysUseEnglishUSCultureWhenFixingVariableNames_TurkishDottedI(TestHost host) { using (new CultureContext(new CultureInfo("tr-TR", useUserOverride: false))) { - await TestAllOptionsOffAsync( + await TestAllOptionsOffAsync(host, @"class C { int [|iyi|]; @@ -1178,12 +1196,12 @@ public int Iyi } [WorkItem(5524, "https://github.com/dotnet/roslyn/issues/5524")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task AlwaysUseEnglishUSCultureWhenFixingVariableNames_TurkishUndottedI() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task AlwaysUseEnglishUSCultureWhenFixingVariableNames_TurkishUndottedI(TestHost host) { using (new CultureContext(new CultureInfo("tr-TR", useUserOverride: false))) { - await TestAllOptionsOffAsync( + await TestAllOptionsOffAsync(host, @"class C { int [|ırak|]; @@ -1209,12 +1227,12 @@ public int Irak } [WorkItem(5524, "https://github.com/dotnet/roslyn/issues/5524")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task AlwaysUseEnglishUSCultureWhenFixingVariableNames_Arabic() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task AlwaysUseEnglishUSCultureWhenFixingVariableNames_Arabic(TestHost host) { using (new CultureContext(new CultureInfo("ar-EG", useUserOverride: false))) { - await TestAllOptionsOffAsync( + await TestAllOptionsOffAsync(host, @"class C { int [|بيت|]; @@ -1240,12 +1258,12 @@ public int بيت1 } [WorkItem(5524, "https://github.com/dotnet/roslyn/issues/5524")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task AlwaysUseEnglishUSCultureWhenFixingVariableNames_Spanish() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task AlwaysUseEnglishUSCultureWhenFixingVariableNames_Spanish(TestHost host) { using (new CultureContext(new CultureInfo("es-ES", useUserOverride: false))) { - await TestAllOptionsOffAsync( + await TestAllOptionsOffAsync(host, @"class C { int [|árbol|]; @@ -1271,12 +1289,12 @@ public int Árbol } [WorkItem(5524, "https://github.com/dotnet/roslyn/issues/5524")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task AlwaysUseEnglishUSCultureWhenFixingVariableNames_Greek() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task AlwaysUseEnglishUSCultureWhenFixingVariableNames_Greek(TestHost host) { using (new CultureContext(new CultureInfo("el-GR", useUserOverride: false))) { - await TestAllOptionsOffAsync( + await TestAllOptionsOffAsync(host, @"class C { int [|σκύλος|]; @@ -1301,10 +1319,10 @@ public int Σκύλος } } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task TestEncapsulateEscapedIdentifier() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task TestEncapsulateEscapedIdentifier(TestHost host) { - await TestAllOptionsOffAsync(@" + await TestAllOptionsOffAsync(host, @" class C { int [|@class|]; @@ -1330,10 +1348,10 @@ public int Class "); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task TestEncapsulateEscapedIdentifierAndQualifiedAccess() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task TestEncapsulateEscapedIdentifierAndQualifiedAccess(TestHost host) { - await TestAllOptionsOffAsync(@" + await TestAllOptionsOffAsync(host, @" class C { int [|@class|]; @@ -1360,10 +1378,10 @@ public int Class } [WorkItem(7090, "https://github.com/dotnet/roslyn/issues/7090")] - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField)] - public async Task ApplyCurrentThisPrefixStyle() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField)] + public async Task ApplyCurrentThisPrefixStyle(TestHost host) { - await TestAllOptionsOffAsync( + await TestAllOptionsOffAsync(host, @"class C { int [|i|]; @@ -1387,8 +1405,8 @@ public int I }", options: Option(CodeStyleOptions2.QualifyFieldAccess, true, NotificationOption2.Error)); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)] - public async Task TestTuple() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)] + public async Task TestTuple(TestHost host) { var text = @" class C @@ -1426,12 +1444,11 @@ void M() } } "; - await TestAllOptionsOffAsync( - text, expected, index: 1); + await TestAllOptionsOffAsync(host, text, expected, index: 1); } - [Fact, Trait(Traits.Feature, Traits.Features.EncapsulateField), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)] - public async Task TupleWithNames() + [Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.EncapsulateField), Test.Utilities.CompilerTrait(Test.Utilities.CompilerFeature.Tuples)] + public async Task TupleWithNames(TestHost host) { var text = @" class C @@ -1469,8 +1486,7 @@ void M() } } "; - await TestAllOptionsOffAsync( - text, expected, index: 1); + await TestAllOptionsOffAsync(host, text, expected, index: 1); } } } From 2d98434030ac799b0c26ff673cecfb22e474e0f2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 15:52:06 -0700 Subject: [PATCH 10/21] Move type to file --- .../AbstractEncapsulateFieldService.cs | 40 +-------------- .../Core/Portable/EncapsulateField/Result.cs | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+), 39 deletions(-) create mode 100644 src/Features/Core/Portable/EncapsulateField/Result.cs diff --git a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs index e93579171b587..dfb53cbb4b0d8 100644 --- a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs +++ b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs @@ -24,7 +24,7 @@ namespace Microsoft.CodeAnalysis.EncapsulateField { - internal abstract class AbstractEncapsulateFieldService : ILanguageService + internal abstract partial class AbstractEncapsulateFieldService : ILanguageService { public async Task EncapsulateFieldAsync(Document document, TextSpan span, bool useDefaultBehavior, CancellationToken cancellationToken) { @@ -209,7 +209,6 @@ private async Task EncapsulateFieldAsync(IFieldSymbol field, Document do document = solution.GetDocument(document.Id); semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); - compilation = semanticModel.Compilation; var newRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var newDeclaration = newRoot.GetAnnotatedNodes(declarationAnnotation).First(); @@ -430,42 +429,5 @@ protected string GeneratePropertyName(string fieldName) protected abstract Task RewriteFieldNameAndAccessibilityAsync(string originalFieldName, bool makePrivate, Document document, SyntaxAnnotation declarationAnnotation, CancellationToken cancellationToken); protected abstract Task> GetFieldsAsync(Document document, TextSpan span, CancellationToken cancellationToken); - - internal class Result - { - public Result(Solution solutionWithProperty, string name, Glyph glyph) - { - Solution = solutionWithProperty; - Name = name; - Glyph = glyph; - } - - public Result(Solution solutionWithProperty, string name, Glyph glyph, List failedFieldSymbols) - : this(solutionWithProperty, name, glyph) - { - FailedFields = failedFieldSymbols.ToImmutableArrayOrEmpty(); - } - - public Result(Solution originalSolution, params IFieldSymbol[] fields) - : this(originalSolution, string.Empty, Glyph.Error) - { - FailedFields = fields.ToImmutableArrayOrEmpty(); - } - - public Solution Solution { get; } - public string Name { get; } - public Glyph Glyph { get; } - public ImmutableArray FailedFields { get; } - - public Result WithFailedFields(List failedFieldSymbols) - { - if (failedFieldSymbols.Count == 0) - { - return this; - } - - return new Result(Solution, Name, Glyph, failedFieldSymbols); - } - } } } diff --git a/src/Features/Core/Portable/EncapsulateField/Result.cs b/src/Features/Core/Portable/EncapsulateField/Result.cs new file mode 100644 index 0000000000000..b0f9c11450bad --- /dev/null +++ b/src/Features/Core/Portable/EncapsulateField/Result.cs @@ -0,0 +1,50 @@ +// 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.Collections.Generic; +using System.Collections.Immutable; +using Roslyn.Utilities; + +namespace Microsoft.CodeAnalysis.EncapsulateField +{ + internal abstract partial class AbstractEncapsulateFieldService + { + internal class Result + { + public Solution Solution { get; } + public string Name { get; } + public Glyph Glyph { get; } + public ImmutableArray FailedFields { get; } + + public Result(Solution solutionWithProperty, string name, Glyph glyph) + { + Solution = solutionWithProperty; + Name = name; + Glyph = glyph; + } + + public Result(Solution solutionWithProperty, string name, Glyph glyph, List failedFieldSymbols) + : this(solutionWithProperty, name, glyph) + { + FailedFields = failedFieldSymbols.ToImmutableArrayOrEmpty(); + } + + public Result(Solution originalSolution, params IFieldSymbol[] fields) + : this(originalSolution, string.Empty, Glyph.Error) + { + FailedFields = fields.ToImmutableArrayOrEmpty(); + } + + public Result WithFailedFields(List failedFieldSymbols) + { + if (failedFieldSymbols.Count == 0) + { + return this; + } + + return new Result(Solution, Name, Glyph, failedFieldSymbols); + } + } + } +} From af236d4c91d13c3418c1508565a4f1b41deda1ca Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 16:01:30 -0700 Subject: [PATCH 11/21] Make an option2 --- .../TestUtilities/InProcRemoteHostClientFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/TestUtilities/InProcRemoteHostClientFactory.cs b/src/EditorFeatures/TestUtilities/InProcRemoteHostClientFactory.cs index 3d9946a8a3c43..c75e777611e49 100644 --- a/src/EditorFeatures/TestUtilities/InProcRemoteHostClientFactory.cs +++ b/src/EditorFeatures/TestUtilities/InProcRemoteHostClientFactory.cs @@ -18,7 +18,7 @@ namespace Microsoft.CodeAnalysis.Test.Utilities.RemoteHost { internal static class RemoteHostOptions { - public static readonly Option RemoteHostTest = new Option( + public static readonly Option2 RemoteHostTest = new Option2( nameof(RemoteHostOptions), nameof(RemoteHostTest), defaultValue: false); } From b3098d280873b70642171fc4283d72ce6bfd7a18 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 16:10:39 -0700 Subject: [PATCH 12/21] Cleanup encapsulate field prior to OOPing it --- .../CSharpEncapsulateFieldService.cs | 8 ++-- .../AbstractEncapsulateFieldService.cs | 37 +++++-------------- .../Core/Portable/EncapsulateField/Result.cs | 20 +--------- .../VisualBasicEncapsulateFieldService.vb | 12 +++--- 4 files changed, 23 insertions(+), 54 deletions(-) diff --git a/src/Features/CSharp/Portable/EncapsulateField/CSharpEncapsulateFieldService.cs b/src/Features/CSharp/Portable/EncapsulateField/CSharpEncapsulateFieldService.cs index 0c7e1fb59dc69..ade5dbd13991d 100644 --- a/src/Features/CSharp/Portable/EncapsulateField/CSharpEncapsulateFieldService.cs +++ b/src/Features/CSharp/Portable/EncapsulateField/CSharpEncapsulateFieldService.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.Composition; using System.Linq; using System.Threading; @@ -115,7 +116,7 @@ protected async override Task RewriteFieldNameAndAccessibilityAsync( return root; } - protected override async Task> GetFieldsAsync(Document document, TextSpan span, CancellationToken cancellationToken) + protected override async Task> GetFieldsAsync(Document document, TextSpan span, CancellationToken cancellationToken) { var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); @@ -139,8 +140,9 @@ protected override async Task> GetFieldsAsync(Document } return declarators.Select(d => semanticModel.GetDeclaredSymbol(d, cancellationToken) as IFieldSymbol) - .WhereNotNull() - .Where(f => f.Name.Length != 0); + .WhereNotNull() + .Where(f => f.Name.Length != 0) + .ToImmutableArray(); } private bool CanEncapsulate(FieldDeclarationSyntax field) diff --git a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs index dfb53cbb4b0d8..4d68c5c4a0842 100644 --- a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs +++ b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs @@ -10,7 +10,6 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CodeGeneration; using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Formatting; @@ -26,24 +25,23 @@ namespace Microsoft.CodeAnalysis.EncapsulateField { internal abstract partial class AbstractEncapsulateFieldService : ILanguageService { + protected abstract Task RewriteFieldNameAndAccessibilityAsync(string originalFieldName, bool makePrivate, Document document, SyntaxAnnotation declarationAnnotation, CancellationToken cancellationToken); + protected abstract Task> GetFieldsAsync(Document document, TextSpan span, CancellationToken cancellationToken); + public async Task EncapsulateFieldAsync(Document document, TextSpan span, bool useDefaultBehavior, CancellationToken cancellationToken) { var fields = await GetFieldsAsync(document, span, cancellationToken).ConfigureAwait(false); - if (fields == null || !fields.Any()) - { + if (fields.IsDefaultOrEmpty) return null; - } return new EncapsulateFieldResult(c => EncapsulateFieldResultAsync(document, span, useDefaultBehavior, c)); } public async Task> GetEncapsulateFieldCodeActionsAsync(Document document, TextSpan span, CancellationToken cancellationToken) { - var fields = (await GetFieldsAsync(document, span, cancellationToken).ConfigureAwait(false)).ToImmutableArrayOrEmpty(); + var fields = await GetFieldsAsync(document, span, cancellationToken).ConfigureAwait(false); if (fields.Length == 0) - { return ImmutableArray.Empty; - } if (fields.Length == 1) { @@ -93,14 +91,14 @@ private ImmutableArray EncapsulateOneField(Document private async Task SingleEncapsulateFieldResultAsync(Document document, TextSpan span, int index, bool updateReferences, CancellationToken cancellationToken) { - var fields = (await GetFieldsAsync(document, span, cancellationToken).ConfigureAwait(false)).ToImmutableArrayOrEmpty(); + var fields = await GetFieldsAsync(document, span, cancellationToken).ConfigureAwait(false); Debug.Assert(fields.Length > index); var field = fields[index]; var result = await EncapsulateFieldAsync(field, document, updateReferences, cancellationToken).ConfigureAwait(false); if (result == null) { - return new Result(document.Project.Solution, field); + return new Result(document.Project.Solution); } return result; @@ -108,11 +106,8 @@ private async Task SingleEncapsulateFieldResultAsync(Document document, private async Task EncapsulateFieldResultAsync(Document document, TextSpan span, bool updateReferences, CancellationToken cancellationToken) { - // probably later we want to add field and reason why it failed. - var failedFieldSymbols = new List(); - var fields = await GetFieldsAsync(document, span, cancellationToken).ConfigureAwait(false); - Debug.Assert(fields.Any()); + Debug.Assert(fields.Length > 0); // For now, build up the multiple field case by encapsulating one at a time. Result result = null; @@ -123,28 +118,19 @@ private async Task EncapsulateFieldResultAsync(Document document, TextSp // We couldn't resolve this field. skip it if (!(field.GetSymbolKey().Resolve(compilation, cancellationToken: cancellationToken).Symbol is IFieldSymbol currentField)) - { - failedFieldSymbols.Add(field); continue; - } result = await EncapsulateFieldAsync(currentField, document, updateReferences, cancellationToken).ConfigureAwait(false); if (result == null) - { - failedFieldSymbols.Add(field); continue; - } document = result.Solution.GetDocument(document.Id); } if (result == null) - { - return new Result(document.Project.Solution, fields.ToArray()); - } + return new Result(document.Project.Solution); - // add failed field symbol info - return result.WithFailedFields(failedFieldSymbols); + return result; } private async Task EncapsulateFieldAsync(IFieldSymbol field, Document document, bool updateReferences, CancellationToken cancellationToken) @@ -426,8 +412,5 @@ protected string GeneratePropertyName(string fieldName) } private static readonly CultureInfo EnUSCultureInfo = new CultureInfo("en-US"); - - protected abstract Task RewriteFieldNameAndAccessibilityAsync(string originalFieldName, bool makePrivate, Document document, SyntaxAnnotation declarationAnnotation, CancellationToken cancellationToken); - protected abstract Task> GetFieldsAsync(Document document, TextSpan span, CancellationToken cancellationToken); } } diff --git a/src/Features/Core/Portable/EncapsulateField/Result.cs b/src/Features/Core/Portable/EncapsulateField/Result.cs index b0f9c11450bad..e11371446869e 100644 --- a/src/Features/Core/Portable/EncapsulateField/Result.cs +++ b/src/Features/Core/Portable/EncapsulateField/Result.cs @@ -15,7 +15,6 @@ internal class Result public Solution Solution { get; } public string Name { get; } public Glyph Glyph { get; } - public ImmutableArray FailedFields { get; } public Result(Solution solutionWithProperty, string name, Glyph glyph) { @@ -24,26 +23,9 @@ public Result(Solution solutionWithProperty, string name, Glyph glyph) Glyph = glyph; } - public Result(Solution solutionWithProperty, string name, Glyph glyph, List failedFieldSymbols) - : this(solutionWithProperty, name, glyph) - { - FailedFields = failedFieldSymbols.ToImmutableArrayOrEmpty(); - } - - public Result(Solution originalSolution, params IFieldSymbol[] fields) + public Result(Solution originalSolution) : this(originalSolution, string.Empty, Glyph.Error) { - FailedFields = fields.ToImmutableArrayOrEmpty(); - } - - public Result WithFailedFields(List failedFieldSymbols) - { - if (failedFieldSymbols.Count == 0) - { - return this; - } - - return new Result(Solution, Name, Glyph, failedFieldSymbols); } } } diff --git a/src/Features/VisualBasic/Portable/EncapsulateField/VisualBasicEncapsulateFieldService.vb b/src/Features/VisualBasic/Portable/EncapsulateField/VisualBasicEncapsulateFieldService.vb index 0094a0a1204bf..8dfccb64c2b8b 100644 --- a/src/Features/VisualBasic/Portable/EncapsulateField/VisualBasicEncapsulateFieldService.vb +++ b/src/Features/VisualBasic/Portable/EncapsulateField/VisualBasicEncapsulateFieldService.vb @@ -2,6 +2,7 @@ ' The .NET Foundation licenses this file to you under the MIT license. ' See the LICENSE file in the project root for more information. +Imports System.Collections.Immutable Imports System.Composition Imports System.Threading Imports Microsoft.CodeAnalysis.EncapsulateField @@ -65,7 +66,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.EncapsulateField Return root End Function - Protected Overrides Async Function GetFieldsAsync(document As Document, span As TextSpan, cancellationToken As CancellationToken) As Task(Of IEnumerable(Of IFieldSymbol)) + Protected Overrides Async Function GetFieldsAsync(document As Document, span As TextSpan, cancellationToken As CancellationToken) As Task(Of ImmutableArray(Of IFieldSymbol)) Dim root = Await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(False) Dim semanticModel = Await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(False) @@ -82,10 +83,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.EncapsulateField names = fields.SelectMany(Function(f) f.Declarators.SelectMany(Function(d) d.Names.Where(Function(n) n.Span.IntersectsWith(span)))) End If - Return names.Select(Function(n) semanticModel.GetDeclaredSymbol(n)) _ - .OfType(Of IFieldSymbol)() _ - .WhereNotNull() _ - .Where(Function(f) f.Name.Length > 0) + Return names.Select(Function(n) semanticModel.GetDeclaredSymbol(n)). + OfType(Of IFieldSymbol)(). + WhereNotNull(). + Where(Function(f) f.Name.Length > 0). + ToImmutableArray() End Function Private Function CanEncapsulate(field As FieldDeclarationSyntax) As Boolean From dd12bce120e411261a2d1661fc0b63cb712e956f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 16:42:00 -0700 Subject: [PATCH 13/21] Cleanup encapsulate field prior to OOPing it --- .../EncapsulateField/EncapsulateFieldTests.cs | 10 +- .../AbstractEncapsulateFieldCommandHandler.cs | 2 +- .../AbstractEncapsulateFieldService.cs | 95 +++++++------------ 3 files changed, 41 insertions(+), 66 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/CodeActions/EncapsulateField/EncapsulateFieldTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/EncapsulateField/EncapsulateFieldTests.cs index b87a6805a0800..81695b3d30d0c 100644 --- a/src/EditorFeatures/CSharpTest/CodeActions/EncapsulateField/EncapsulateFieldTests.cs +++ b/src/EditorFeatures/CSharpTest/CodeActions/EncapsulateField/EncapsulateFieldTests.cs @@ -20,7 +20,7 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.CodeRefactorings.Encaps public enum TestHost { InProcess, - OutOfProcess, + //OutOfProcess, } public class EncapsulateFieldTests : AbstractCSharpCodeActionTest @@ -43,7 +43,7 @@ internal Task TestAllOptionsOffAsync( { options = options ?? new OptionsCollection(GetLanguage()); options.AddRange(AllOptionsOff); - options.Add(RemoteHostOptions.RemoteHostTest, host == TestHost.OutOfProcess); + options.Add(RemoteHostOptions.RemoteHostTest, host != TestHost.InProcess); return TestAsync(initialMarkup, expectedMarkup, parseOptions, compilationOptions, index, options); @@ -219,7 +219,7 @@ await TestInRegularAndScriptAsync(text, expected, { { CSharpCodeStyleOptions.PreferExpressionBodiedProperties, ExpressionBodyPreference.WhenPossible, NotificationOption2.Silent }, { CSharpCodeStyleOptions.PreferExpressionBodiedAccessors, ExpressionBodyPreference.Never, NotificationOption2.Silent }, - { RemoteHostOptions.RemoteHostTest, host == TestHost.OutOfProcess } + { RemoteHostOptions.RemoteHostTest, host != TestHost.InProcess } }); } @@ -255,7 +255,7 @@ await TestInRegularAndScriptAsync(text, expected, options: new OptionsCollection(GetLanguage()) { { CSharpCodeStyleOptions.PreferExpressionBodiedAccessors, CSharpCodeStyleOptions.WhenPossibleWithSilentEnforcement }, - { RemoteHostOptions.RemoteHostTest, host == TestHost.OutOfProcess } + { RemoteHostOptions.RemoteHostTest, host != TestHost.InProcess } }); } @@ -794,7 +794,7 @@ class Program private TestParameters GetRemoteHostOptions(TestHost host) { - return new TestParameters(options: Option(RemoteHostOptions.RemoteHostTest, host == TestHost.OutOfProcess)); + return new TestParameters(options: Option(RemoteHostOptions.RemoteHostTest, host != TestHost.InProcess)); } [WorkItem(705898, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/705898")] diff --git a/src/EditorFeatures/Core/Implementation/EncapsulateField/AbstractEncapsulateFieldCommandHandler.cs b/src/EditorFeatures/Core/Implementation/EncapsulateField/AbstractEncapsulateFieldCommandHandler.cs index a5b8aeff8605b..43179f4532593 100644 --- a/src/EditorFeatures/Core/Implementation/EncapsulateField/AbstractEncapsulateFieldCommandHandler.cs +++ b/src/EditorFeatures/Core/Implementation/EncapsulateField/AbstractEncapsulateFieldCommandHandler.cs @@ -64,7 +64,7 @@ private bool Execute(EncapsulateFieldCommandArgs args, IUIThreadOperationScope w var service = document.GetLanguageService(); - var result = service.EncapsulateFieldAsync(document, spans.First().Span.ToTextSpan(), true, cancellationToken).WaitAndGetResult(cancellationToken); + var result = service.EncapsulateFieldsInSpanAsync(document, spans.First().Span.ToTextSpan(), true, cancellationToken).WaitAndGetResult(cancellationToken); // We are about to show a modal UI dialog so we should take over the command execution // wait context. That means the command system won't attempt to show its own wait dialog diff --git a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs index 4d68c5c4a0842..47a097c573464 100644 --- a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs +++ b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; -using System.Diagnostics; using System.Globalization; using System.Linq; using System.Threading; @@ -28,13 +27,14 @@ internal abstract partial class AbstractEncapsulateFieldService : ILanguageServi protected abstract Task RewriteFieldNameAndAccessibilityAsync(string originalFieldName, bool makePrivate, Document document, SyntaxAnnotation declarationAnnotation, CancellationToken cancellationToken); protected abstract Task> GetFieldsAsync(Document document, TextSpan span, CancellationToken cancellationToken); - public async Task EncapsulateFieldAsync(Document document, TextSpan span, bool useDefaultBehavior, CancellationToken cancellationToken) + public async Task EncapsulateFieldsInSpanAsync(Document document, TextSpan span, bool useDefaultBehavior, CancellationToken cancellationToken) { var fields = await GetFieldsAsync(document, span, cancellationToken).ConfigureAwait(false); if (fields.IsDefaultOrEmpty) return null; - return new EncapsulateFieldResult(c => EncapsulateFieldResultAsync(document, span, useDefaultBehavior, c)); + return new EncapsulateFieldResult( + c => EncapsulateFieldsAsync(document, fields, useDefaultBehavior, c)); } public async Task> GetEncapsulateFieldCodeActionsAsync(Document document, TextSpan span, CancellationToken cancellationToken) @@ -46,73 +46,50 @@ public async Task> GetEncapsulateFiel if (fields.Length == 1) { // there is only one field - return EncapsulateOneField(document, span, fields[0], index: 0); + return EncapsulateOneField(document, fields[0]); } - else - { - // there are multiple fields. - var current = ArrayBuilder.GetInstance(); - if (span.IsEmpty) - { - // if there is no selection, get action for each field + all of them. - for (var i = 0; i < fields.Length; i++) - { - current.AddRange(EncapsulateOneField(document, span, fields[i], i)); - } - } + // there are multiple fields. + using var _ = ArrayBuilder.GetInstance(out var builder); - current.AddRange(EncapsulateAllFields(document, span)); - return current.ToImmutableAndFree(); + if (span.IsEmpty) + { + // if there is no selection, get action for each field + all of them. + foreach (var field in fields) + builder.AddRange(EncapsulateOneField(document, field)); } + + builder.AddRange(EncapsulateAllFields(document, fields)); + return builder.ToImmutable(); } - private IEnumerable EncapsulateAllFields(Document document, TextSpan span) + private ImmutableArray EncapsulateAllFields(Document document, ImmutableArray fields) { - var action1Text = FeaturesResources.Encapsulate_fields_and_use_property; - var action2Text = FeaturesResources.Encapsulate_fields_but_still_use_field; - - return new[] - { - new EncapsulateFieldCodeAction(new EncapsulateFieldResult(c => EncapsulateFieldResultAsync(document, span, true, c)), action1Text), - new EncapsulateFieldCodeAction(new EncapsulateFieldResult(c => EncapsulateFieldResultAsync(document, span, false, c)), action2Text) - }; + return ImmutableArray.Create( + new EncapsulateFieldCodeAction(new EncapsulateFieldResult(c => EncapsulateFieldsAsync(document, fields, updateReferences: true, c)), FeaturesResources.Encapsulate_fields_and_use_property), + new EncapsulateFieldCodeAction(new EncapsulateFieldResult(c => EncapsulateFieldsAsync(document, fields, updateReferences: false, c)), FeaturesResources.Encapsulate_fields_but_still_use_field)); } - private ImmutableArray EncapsulateOneField(Document document, TextSpan span, IFieldSymbol field, int index) + private ImmutableArray EncapsulateOneField(Document document, IFieldSymbol field) { var action1Text = string.Format(FeaturesResources.Encapsulate_field_colon_0_and_use_property, field.Name); var action2Text = string.Format(FeaturesResources.Encapsulate_field_colon_0_but_still_use_field, field.Name); + var fields = ImmutableArray.Create(field); return ImmutableArray.Create( - new EncapsulateFieldCodeAction(new EncapsulateFieldResult(c => SingleEncapsulateFieldResultAsync(document, span, index, true, c)), action1Text), - new EncapsulateFieldCodeAction(new EncapsulateFieldResult(c => SingleEncapsulateFieldResultAsync(document, span, index, false, c)), action2Text)); + new EncapsulateFieldCodeAction(new EncapsulateFieldResult(c => EncapsulateFieldsAsync(document, fields, updateReferences: true, c)), action1Text), + new EncapsulateFieldCodeAction(new EncapsulateFieldResult(c => EncapsulateFieldsAsync(document, fields, updateReferences: false, c)), action2Text)); } - private async Task SingleEncapsulateFieldResultAsync(Document document, TextSpan span, int index, bool updateReferences, CancellationToken cancellationToken) + private async Task EncapsulateFieldsAsync(Document document, ImmutableArray fields, bool updateReferences, CancellationToken cancellationToken) { - var fields = await GetFieldsAsync(document, span, cancellationToken).ConfigureAwait(false); - Debug.Assert(fields.Length > index); - - var field = fields[index]; - var result = await EncapsulateFieldAsync(field, document, updateReferences, cancellationToken).ConfigureAwait(false); - if (result == null) - { - return new Result(document.Project.Solution); - } - - return result; - } - - private async Task EncapsulateFieldResultAsync(Document document, TextSpan span, bool updateReferences, CancellationToken cancellationToken) - { - var fields = await GetFieldsAsync(document, span, cancellationToken).ConfigureAwait(false); - Debug.Assert(fields.Length > 0); + Contract.ThrowIfTrue(fields.Length == 0); // For now, build up the multiple field case by encapsulating one at a time. - Result result = null; + var currentSolution = document.Project.Solution; foreach (var field in fields) { + document = currentSolution.GetDocument(document.Id); var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); var compilation = semanticModel.Compilation; @@ -120,20 +97,20 @@ private async Task EncapsulateFieldResultAsync(Document document, TextSp if (!(field.GetSymbolKey().Resolve(compilation, cancellationToken: cancellationToken).Symbol is IFieldSymbol currentField)) continue; - result = await EncapsulateFieldAsync(currentField, document, updateReferences, cancellationToken).ConfigureAwait(false); - if (result == null) + var nextSolution = await EncapsulateFieldAsync(document, currentField, updateReferences, cancellationToken).ConfigureAwait(false); + if (nextSolution == null) continue; - document = result.Solution.GetDocument(document.Id); + currentSolution = nextSolution; } - if (result == null) - return new Result(document.Project.Solution); - - return result; + var firstField = fields[0]; + return new Result(currentSolution, firstField.ToDisplayString(), firstField.GetGlyph()); } - private async Task EncapsulateFieldAsync(IFieldSymbol field, Document document, bool updateReferences, CancellationToken cancellationToken) + private async Task EncapsulateFieldAsync( + Document document, IFieldSymbol field, + bool updateReferences, CancellationToken cancellationToken) { var originalField = field; var (finalFieldName, generatedPropertyName) = GenerateFieldAndPropertyNames(field); @@ -170,9 +147,7 @@ private async Task EncapsulateFieldAsync(IFieldSymbol field, Document do // We couldn't resolve field after annotating its declaration. Bail if (field == null) - { return null; - } var solutionNeedingProperty = await UpdateReferencesAsync( updateReferences, solution, document, field, finalFieldName, generatedPropertyName, cancellationToken).ConfigureAwait(false); @@ -212,7 +187,7 @@ private async Task EncapsulateFieldAsync(IFieldSymbol field, Document do var solutionWithProperty = await AddPropertyAsync( document, document.Project.Solution, field, generatedProperty, cancellationToken).ConfigureAwait(false); - return new Result(solutionWithProperty, originalField.ToDisplayString(), originalField.GetGlyph()); + return solutionWithProperty; } private async Task UpdateReferencesAsync( From 4fd0f9ff3bc4331490d84c084d13d4595b5dfb59 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 16:59:18 -0700 Subject: [PATCH 14/21] Cleanup encapsulate field prior to OOPing it --- .../AbstractEncapsulateFieldCommandHandler.cs | 4 +- .../AbstractEncapsulateFieldService.cs | 50 ++++++++++++------- .../EncapsulateFieldCodeAction.cs | 27 ---------- .../EncapsulateFieldResult.cs | 27 ++++------ .../Core/Portable/EncapsulateField/Result.cs | 32 ------------ 5 files changed, 44 insertions(+), 96 deletions(-) delete mode 100644 src/Features/Core/Portable/EncapsulateField/EncapsulateFieldCodeAction.cs delete mode 100644 src/Features/Core/Portable/EncapsulateField/Result.cs diff --git a/src/EditorFeatures/Core/Implementation/EncapsulateField/AbstractEncapsulateFieldCommandHandler.cs b/src/EditorFeatures/Core/Implementation/EncapsulateField/AbstractEncapsulateFieldCommandHandler.cs index 43179f4532593..b4ca9e36c1b8b 100644 --- a/src/EditorFeatures/Core/Implementation/EncapsulateField/AbstractEncapsulateFieldCommandHandler.cs +++ b/src/EditorFeatures/Core/Implementation/EncapsulateField/AbstractEncapsulateFieldCommandHandler.cs @@ -91,8 +91,8 @@ private bool Execute(EncapsulateFieldCommandArgs args, IUIThreadOperationScope w string.Format(EditorFeaturesResources.Preview_Changes_0, EditorFeaturesResources.Encapsulate_Field), "vs.csharp.refactoring.preview", EditorFeaturesResources.Encapsulate_Field_colon, - result.GetNameAsync(cancellationToken).WaitAndGetResult(cancellationToken), - result.GetGlyphAsync(cancellationToken).WaitAndGetResult(cancellationToken), + result.Name, + result.Glyph, finalSolution, document.Project.Solution); } diff --git a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs index 47a097c573464..ce3107024ba14 100644 --- a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs +++ b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs @@ -9,6 +9,7 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeGeneration; using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Formatting; @@ -33,15 +34,18 @@ public async Task EncapsulateFieldsInSpanAsync(Document if (fields.IsDefaultOrEmpty) return null; + var firstField = fields[0]; return new EncapsulateFieldResult( + firstField.ToDisplayString(), + firstField.GetGlyph(), c => EncapsulateFieldsAsync(document, fields, useDefaultBehavior, c)); } - public async Task> GetEncapsulateFieldCodeActionsAsync(Document document, TextSpan span, CancellationToken cancellationToken) + public async Task> GetEncapsulateFieldCodeActionsAsync(Document document, TextSpan span, CancellationToken cancellationToken) { var fields = await GetFieldsAsync(document, span, cancellationToken).ConfigureAwait(false); if (fields.Length == 0) - return ImmutableArray.Empty; + return ImmutableArray.Empty; if (fields.Length == 1) { @@ -50,7 +54,7 @@ public async Task> GetEncapsulateFiel } // there are multiple fields. - using var _ = ArrayBuilder.GetInstance(out var builder); + using var _ = ArrayBuilder.GetInstance(out var builder); if (span.IsEmpty) { @@ -63,25 +67,30 @@ public async Task> GetEncapsulateFiel return builder.ToImmutable(); } - private ImmutableArray EncapsulateAllFields(Document document, ImmutableArray fields) + private ImmutableArray EncapsulateAllFields(Document document, ImmutableArray fields) { - return ImmutableArray.Create( - new EncapsulateFieldCodeAction(new EncapsulateFieldResult(c => EncapsulateFieldsAsync(document, fields, updateReferences: true, c)), FeaturesResources.Encapsulate_fields_and_use_property), - new EncapsulateFieldCodeAction(new EncapsulateFieldResult(c => EncapsulateFieldsAsync(document, fields, updateReferences: false, c)), FeaturesResources.Encapsulate_fields_but_still_use_field)); + return ImmutableArray.Create( + new MyCodeAction( + FeaturesResources.Encapsulate_fields_and_use_property, + c => EncapsulateFieldsAsync(document, fields, updateReferences: true, c)), + new MyCodeAction( + FeaturesResources.Encapsulate_fields_but_still_use_field, + c => EncapsulateFieldsAsync(document, fields, updateReferences: false, c))); } - private ImmutableArray EncapsulateOneField(Document document, IFieldSymbol field) + private ImmutableArray EncapsulateOneField(Document document, IFieldSymbol field) { - var action1Text = string.Format(FeaturesResources.Encapsulate_field_colon_0_and_use_property, field.Name); - var action2Text = string.Format(FeaturesResources.Encapsulate_field_colon_0_but_still_use_field, field.Name); - var fields = ImmutableArray.Create(field); - return ImmutableArray.Create( - new EncapsulateFieldCodeAction(new EncapsulateFieldResult(c => EncapsulateFieldsAsync(document, fields, updateReferences: true, c)), action1Text), - new EncapsulateFieldCodeAction(new EncapsulateFieldResult(c => EncapsulateFieldsAsync(document, fields, updateReferences: false, c)), action2Text)); + return ImmutableArray.Create( + new MyCodeAction( + string.Format(FeaturesResources.Encapsulate_field_colon_0_and_use_property, field.Name), + c => EncapsulateFieldsAsync(document, fields, updateReferences: true, c)), + new MyCodeAction( + string.Format(FeaturesResources.Encapsulate_field_colon_0_but_still_use_field, field.Name), + c => EncapsulateFieldsAsync(document, fields, updateReferences: false, c))); } - private async Task EncapsulateFieldsAsync(Document document, ImmutableArray fields, bool updateReferences, CancellationToken cancellationToken) + private async Task EncapsulateFieldsAsync(Document document, ImmutableArray fields, bool updateReferences, CancellationToken cancellationToken) { Contract.ThrowIfTrue(fields.Length == 0); @@ -104,8 +113,7 @@ private async Task EncapsulateFieldsAsync(Document document, ImmutableAr currentSolution = nextSolution; } - var firstField = fields[0]; - return new Result(currentSolution, firstField.ToDisplayString(), firstField.GetGlyph()); + return currentSolution; } private async Task EncapsulateFieldAsync( @@ -387,5 +395,13 @@ protected string GeneratePropertyName(string fieldName) } private static readonly CultureInfo EnUSCultureInfo = new CultureInfo("en-US"); + + private class MyCodeAction : CodeAction.SolutionChangeAction + { + public MyCodeAction(string title, Func> createChangedSolution) + : base(title, createChangedSolution) + { + } + } } } diff --git a/src/Features/Core/Portable/EncapsulateField/EncapsulateFieldCodeAction.cs b/src/Features/Core/Portable/EncapsulateField/EncapsulateFieldCodeAction.cs deleted file mode 100644 index 532ce34440ba1..0000000000000 --- a/src/Features/Core/Portable/EncapsulateField/EncapsulateFieldCodeAction.cs +++ /dev/null @@ -1,27 +0,0 @@ -// 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.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.CodeActions; - -namespace Microsoft.CodeAnalysis.EncapsulateField -{ - internal class EncapsulateFieldCodeAction : CodeAction - { - private readonly EncapsulateFieldResult _result; - private readonly string _title; - - public EncapsulateFieldCodeAction(EncapsulateFieldResult result, string title) - { - _result = result; - _title = title; - } - - public override string Title => _title; - - protected override Task GetChangedSolutionAsync(CancellationToken cancellationToken) - => _result.GetSolutionAsync(cancellationToken); - } -} diff --git a/src/Features/Core/Portable/EncapsulateField/EncapsulateFieldResult.cs b/src/Features/Core/Portable/EncapsulateField/EncapsulateFieldResult.cs index 05cab607d0db9..8b83f2fb00a3e 100644 --- a/src/Features/Core/Portable/EncapsulateField/EncapsulateFieldResult.cs +++ b/src/Features/Core/Portable/EncapsulateField/EncapsulateFieldResult.cs @@ -11,27 +11,18 @@ namespace Microsoft.CodeAnalysis.EncapsulateField { internal class EncapsulateFieldResult { - private readonly AsyncLazy _resultGetter; + public readonly string Name; + public readonly Glyph Glyph; + private readonly AsyncLazy _resultGetter; - public EncapsulateFieldResult(Func> resultGetter) - => _resultGetter = new AsyncLazy(c => resultGetter(c), cacheResult: true); - - public async Task GetNameAsync(CancellationToken cancellationToken) - { - var result = await _resultGetter.GetValueAsync(cancellationToken).ConfigureAwait(false); - return result.Name; - } - - public async Task GetGlyphAsync(CancellationToken cancellationToken) + public EncapsulateFieldResult(string name, Glyph glyph, Func> getSolutionAsync) { - var result = await _resultGetter.GetValueAsync(cancellationToken).ConfigureAwait(false); - return result.Glyph; + Name = name; + Glyph = glyph; + _resultGetter = new AsyncLazy(getSolutionAsync, cacheResult: true); } - public async Task GetSolutionAsync(CancellationToken cancellationToken) - { - var result = await _resultGetter.GetValueAsync(cancellationToken).ConfigureAwait(false); - return result.Solution; - } + public Task GetSolutionAsync(CancellationToken cancellationToken) + => _resultGetter.GetValueAsync(cancellationToken); } } diff --git a/src/Features/Core/Portable/EncapsulateField/Result.cs b/src/Features/Core/Portable/EncapsulateField/Result.cs deleted file mode 100644 index e11371446869e..0000000000000 --- a/src/Features/Core/Portable/EncapsulateField/Result.cs +++ /dev/null @@ -1,32 +0,0 @@ -// 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.Collections.Generic; -using System.Collections.Immutable; -using Roslyn.Utilities; - -namespace Microsoft.CodeAnalysis.EncapsulateField -{ - internal abstract partial class AbstractEncapsulateFieldService - { - internal class Result - { - public Solution Solution { get; } - public string Name { get; } - public Glyph Glyph { get; } - - public Result(Solution solutionWithProperty, string name, Glyph glyph) - { - Solution = solutionWithProperty; - Name = name; - Glyph = glyph; - } - - public Result(Solution originalSolution) - : this(originalSolution, string.Empty, Glyph.Error) - { - } - } - } -} From 9d30c3e22389ccbca4a426a59f15eb7fd7516df0 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 17:28:47 -0700 Subject: [PATCH 15/21] Move encapsulte field OOP --- .../EncapsulateField/EncapsulateFieldTests.cs | 2 +- .../AbstractEncapsulateFieldService.cs | 41 +++++++++++++- .../IRemoteEncapsulateFieldService.cs | 22 ++++++++ .../Core/Portable/Remote/RemoteUtilities.cs | 52 ++++++++++++++++++ .../Portable/Rename/ConflictResolution.cs | 22 -------- .../Core/Portable/Rename/IRemoteRenamer.cs | 21 ++----- .../CodeAnalysisService_EncapsulateField.cs | 55 +++++++++++++++++++ .../Compiler/Core/Log/FunctionId.cs | 2 + 8 files changed, 178 insertions(+), 39 deletions(-) create mode 100644 src/Features/Core/Portable/EncapsulateField/IRemoteEncapsulateFieldService.cs create mode 100644 src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs create mode 100644 src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_EncapsulateField.cs diff --git a/src/EditorFeatures/CSharpTest/CodeActions/EncapsulateField/EncapsulateFieldTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/EncapsulateField/EncapsulateFieldTests.cs index 81695b3d30d0c..1482bf97d86bd 100644 --- a/src/EditorFeatures/CSharpTest/CodeActions/EncapsulateField/EncapsulateFieldTests.cs +++ b/src/EditorFeatures/CSharpTest/CodeActions/EncapsulateField/EncapsulateFieldTests.cs @@ -20,7 +20,7 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.CodeRefactorings.Encaps public enum TestHost { InProcess, - //OutOfProcess, + OutOfProcess, } public class EncapsulateFieldTests : AbstractCSharpCodeActionTest diff --git a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs index ce3107024ba14..84b9ef35febe0 100644 --- a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs +++ b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs @@ -14,7 +14,9 @@ using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Host; +using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Rename; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Simplification; @@ -90,7 +92,44 @@ private ImmutableArray EncapsulateOneField(Document document, IField c => EncapsulateFieldsAsync(document, fields, updateReferences: false, c))); } - private async Task EncapsulateFieldsAsync(Document document, ImmutableArray fields, bool updateReferences, CancellationToken cancellationToken) + public async Task EncapsulateFieldsAsync( + Document document, ImmutableArray fields, + bool updateReferences, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + + using (Logger.LogBlock(FunctionId.Renamer_FindRenameLocationsAsync, cancellationToken)) + { + var solution = document.Project.Solution; + var client = await RemoteHostClient.TryGetClientAsync(solution.Workspace, cancellationToken).ConfigureAwait(false); + if (client != null) + { + var result = await client.TryRunRemoteAsync)>>( + WellKnownServiceHubServices.CodeAnalysisService, + nameof(IRemoteEncapsulateFieldService.EncapsulateFieldsAsync), + solution, + new object[] + { + document.Id, + fields.Select(f => SymbolKey.CreateString(f, cancellationToken)).ToArray(), + updateReferences, + }, + callbackTarget: null, + cancellationToken).ConfigureAwait(false); + + if (result.HasValue) + { + return await RemoteUtilities.UpdateSolutionAsync( + solution, result.Value, cancellationToken).ConfigureAwait(false); + } + } + } + + return await EncapsulateFieldsInCurrentProcessAsync( + document, fields, updateReferences, cancellationToken).ConfigureAwait(false); + } + + private async Task EncapsulateFieldsInCurrentProcessAsync(Document document, ImmutableArray fields, bool updateReferences, CancellationToken cancellationToken) { Contract.ThrowIfTrue(fields.Length == 0); diff --git a/src/Features/Core/Portable/EncapsulateField/IRemoteEncapsulateFieldService.cs b/src/Features/Core/Portable/EncapsulateField/IRemoteEncapsulateFieldService.cs new file mode 100644 index 0000000000000..b95355d8bb893 --- /dev/null +++ b/src/Features/Core/Portable/EncapsulateField/IRemoteEncapsulateFieldService.cs @@ -0,0 +1,22 @@ +// 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.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Remote; +using Microsoft.CodeAnalysis.Text; + +namespace Microsoft.CodeAnalysis.EncapsulateField +{ + internal interface IRemoteEncapsulateFieldService + { + Task textChanges)>> EncapsulateFieldsAsync( + PinnedSolutionInfo solutionInfo, + DocumentId documentId, + ImmutableArray fieldSymbolKeys, + bool updateReferences, + CancellationToken cancellationToken); + } +} diff --git a/src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs b/src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs new file mode 100644 index 0000000000000..b5681a903dc02 --- /dev/null +++ b/src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs @@ -0,0 +1,52 @@ +// 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.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Text; + +namespace Microsoft.CodeAnalysis.Remote +{ + using DocumentTextChanges = ImmutableArray<(DocumentId, ImmutableArray)>; + + internal static class RemoteUtilities + { + public static async Task GetDocumentTextChangesAsync( + Solution oldSolution, + Solution newSolution, + CancellationToken cancellationToken) + { + using var _ = ArrayBuilder<(DocumentId, ImmutableArray)>.GetInstance(out var builder); + + var solutionChanges = newSolution.GetChanges(oldSolution); + foreach (var projectChange in solutionChanges.GetProjectChanges()) + { + foreach (var docId in projectChange.GetChangedDocuments()) + { + var oldDoc = oldSolution.GetDocument(docId); + var newDoc = newSolution.GetDocument(docId); + var textChanges = await newDoc.GetTextChangesAsync(oldDoc, cancellationToken).ConfigureAwait(false); + builder.Add((docId, textChanges.ToImmutableArray())); + } + } + + return builder.ToImmutable(); + } + + public static async Task UpdateSolutionAsync( + Solution oldSolution, DocumentTextChanges documentTextChanges, CancellationToken cancellationToken) + { + var currentSolution = oldSolution; + foreach (var (docId, textChanges) in documentTextChanges) + { + var text = await oldSolution.GetDocument(docId).GetTextAsync(cancellationToken).ConfigureAwait(false); + currentSolution = currentSolution.WithDocumentText(docId, text.WithChanges(textChanges)); + } + + return currentSolution; + } + } +} diff --git a/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs b/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs index d9dc5491ec204..b73182d59d65f 100644 --- a/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs +++ b/src/Workspaces/Core/Portable/Rename/ConflictResolution.cs @@ -3,9 +3,6 @@ // See the LICENSE file in the project root for more information. using System.Collections.Immutable; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Rename.ConflictEngine; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; @@ -69,25 +66,6 @@ public ConflictResolution( : _newSolutionWithoutRenamedDocument.WithDocumentName(_renamedDocument.documentId, _renamedDocument.newName); } - private async Task<(DocumentId, ImmutableArray)[]> GetDocumentToTextChangesAsync(CancellationToken cancellationToken) - { - using var _ = ArrayBuilder<(DocumentId, ImmutableArray)>.GetInstance(out var builder); - - var solutionChanges = _newSolutionWithoutRenamedDocument.GetChanges(OldSolution); - foreach (var projectChange in solutionChanges.GetProjectChanges()) - { - foreach (var docId in projectChange.GetChangedDocuments()) - { - var oldDoc = OldSolution.GetDocument(docId); - var newDoc = _newSolutionWithoutRenamedDocument.GetDocument(docId); - var textChanges = await newDoc.GetTextChangesAsync(oldDoc, cancellationToken).ConfigureAwait(false); - builder.Add((docId, textChanges.ToImmutableArray())); - } - } - - return builder.ToArray(); - } - public ImmutableArray<(TextSpan oldSpan, TextSpan newSpan)> GetComplexifiedSpans(DocumentId documentId) => _documentToComplexifiedSpansMap.TryGetValue(documentId, out var complexifiedSpans) ? complexifiedSpans.SelectAsArray(c => (c.OriginalSpan, c.NewSpan)) diff --git a/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs b/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs index b32a830bb1206..ca6acb518201b 100644 --- a/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs +++ b/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs @@ -271,7 +271,7 @@ internal class SerializableConflictResolution public DocumentId[] DocumentIds; public SerializableRelatedLocation[] RelatedLocations; - public (DocumentId, ImmutableArray)[] DocumentToTextChanges; + public ImmutableArray<(DocumentId, ImmutableArray)> DocumentTextChanges; public (DocumentId, ImmutableArray<(TextSpan oldSpan, TextSpan newSpan)>)[] DocumentToModifiedSpansMap; public (DocumentId, ImmutableArray)[] DocumentToComplexifiedSpansMap; public (DocumentId, ImmutableArray)[] DocumentToRelatedLocationsMap; @@ -281,9 +281,11 @@ public async Task RehydrateAsync(Solution oldSolution, Cance if (ErrorMessage != null) return new ConflictResolution(ErrorMessage); + var newSolutionWithoutRenamedDocument = await RemoteUtilities.UpdateSolutionAsync( + oldSolution, DocumentTextChanges, cancellationToken).ConfigureAwait(false); return new ConflictResolution( oldSolution, - await CreateNewSolutionAsync(oldSolution, cancellationToken).ConfigureAwait(false), + newSolutionWithoutRenamedDocument, ReplacementTextValid, RenamedDocument, DocumentIds.ToImmutableArray(), @@ -292,18 +294,6 @@ await CreateNewSolutionAsync(oldSolution, cancellationToken).ConfigureAwait(fals DocumentToComplexifiedSpansMap.ToImmutableDictionary(t => t.Item1, t => t.Item2.SelectAsArray(c => c.Rehydrate())), DocumentToRelatedLocationsMap.ToImmutableDictionary(t => t.Item1, t => t.Item2.SelectAsArray(c => c.Rehydrate()))); } - - private async Task CreateNewSolutionAsync(Solution oldSolution, CancellationToken cancellationToken) - { - var currentSolution = oldSolution; - foreach (var (docId, textChanges) in this.DocumentToTextChanges) - { - var text = await oldSolution.GetDocument(docId).GetTextAsync(cancellationToken).ConfigureAwait(false); - currentSolution = currentSolution.WithDocumentText(docId, text.WithChanges(textChanges)); - } - - return currentSolution; - } } internal partial struct ConflictResolution @@ -313,13 +303,14 @@ public async Task DehydrateAsync(CancellationTok if (ErrorMessage != null) return new SerializableConflictResolution { ErrorMessage = ErrorMessage }; + var documentTextChanges = await RemoteUtilities.GetDocumentTextChangesAsync(OldSolution, _newSolutionWithoutRenamedDocument, cancellationToken).ConfigureAwait(false); return new SerializableConflictResolution { ReplacementTextValid = ReplacementTextValid, RenamedDocument = _renamedDocument, DocumentIds = DocumentIds.ToArray(), RelatedLocations = RelatedLocations.Select(loc => SerializableRelatedLocation.Dehydrate(loc)).ToArray(), - DocumentToTextChanges = await GetDocumentToTextChangesAsync(cancellationToken).ConfigureAwait(false), + DocumentTextChanges = documentTextChanges, DocumentToModifiedSpansMap = _documentToModifiedSpansMap.Select(kvp => (kvp.Key, kvp.Value)).ToArray(), DocumentToComplexifiedSpansMap = _documentToComplexifiedSpansMap.Select(kvp => (kvp.Key, kvp.Value.SelectAsArray(s => SerializableComplexifiedSpan.Dehydrate(s)))).ToArray(), DocumentToRelatedLocationsMap = _documentToRelatedLocationsMap.Select(kvp => (kvp.Key, kvp.Value.SelectAsArray(s => SerializableRelatedLocation.Dehydrate(s)))).ToArray(), diff --git a/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_EncapsulateField.cs b/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_EncapsulateField.cs new file mode 100644 index 0000000000000..969317a295a3c --- /dev/null +++ b/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_EncapsulateField.cs @@ -0,0 +1,55 @@ +// 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.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.EncapsulateField; +using Microsoft.CodeAnalysis.PooledObjects; +using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Text; + +namespace Microsoft.CodeAnalysis.Remote +{ + using DocumentTextChanges = ImmutableArray<(DocumentId documentId, ImmutableArray textChanges)>; + + internal partial class CodeAnalysisService : IRemoteEncapsulateFieldService + { + public Task EncapsulateFieldsAsync( + PinnedSolutionInfo solutionInfo, + DocumentId documentId, + ImmutableArray fieldSymbolKeys, + bool updateReferences, + CancellationToken cancellationToken) + { + return RunServiceAsync(async () => + { + using (UserOperationBooster.Boost()) + { + var solution = await GetSolutionAsync(solutionInfo, cancellationToken).ConfigureAwait(false); + var document = solution.GetRequiredDocument(documentId); + + using var _ = ArrayBuilder.GetInstance(out var fields); + var compilation = await document.Project.GetCompilationAsync(cancellationToken).ConfigureAwait(false); + + foreach (var key in fieldSymbolKeys) + { + var resolved = SymbolKey.ResolveString(key, compilation, cancellationToken: cancellationToken).GetAnySymbol() as IFieldSymbol; + if (resolved == null) + return DocumentTextChanges.Empty; + + fields.Add(resolved); + } + + var service = document.GetLanguageService(); + + var newSolution = await service.EncapsulateFieldsAsync( + document, fields.ToImmutable(), updateReferences, cancellationToken).ConfigureAwait(false); + return await RemoteUtilities.GetDocumentTextChangesAsync( + solution, newSolution, cancellationToken).ConfigureAwait(false); + } + }, cancellationToken); + } + } +} diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs index 1911686315d19..6126cb4748689 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Log/FunctionId.cs @@ -485,5 +485,7 @@ internal enum FunctionId Renamer_ResolveConflictsAsync = 388, ChangeSignature_Data = 389, + + AbstractEncapsulateFieldService_EncapsulateFieldsAsync = 390, } } From 218d1bfe18955a37e22d2d89dcee17338452897c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 17:39:13 -0700 Subject: [PATCH 16/21] Run VB tests OOP --- .../EncapsulateField/EncapsulateFieldTests.vb | 147 ++++++++++-------- 1 file changed, 83 insertions(+), 64 deletions(-) diff --git a/src/EditorFeatures/VisualBasicTest/CodeActions/EncapsulateField/EncapsulateFieldTests.vb b/src/EditorFeatures/VisualBasicTest/CodeActions/EncapsulateField/EncapsulateFieldTests.vb index 52ef69b0dfbd4..1d11500b49952 100644 --- a/src/EditorFeatures/VisualBasicTest/CodeActions/EncapsulateField/EncapsulateFieldTests.vb +++ b/src/EditorFeatures/VisualBasicTest/CodeActions/EncapsulateField/EncapsulateFieldTests.vb @@ -4,9 +4,16 @@ Imports Microsoft.CodeAnalysis.CodeRefactorings Imports Microsoft.CodeAnalysis.CodeStyle +Imports Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions Imports Microsoft.CodeAnalysis.EncapsulateField +Imports Microsoft.CodeAnalysis.Test.Utilities.RemoteHost Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.CodeRefactorings.EncapsulateField + Public Enum TestHost + InProcess + OutOfProcess + End Enum + Public Class EncapsulateFieldTests Inherits AbstractVisualBasicCodeActionTest @@ -14,8 +21,14 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.CodeRefactorings.E Return New EncapsulateFieldRefactoringProvider() End Function - - Public Async Function TestEncapsulatePrivateFieldAndUpdateReferences() As Task + Private Function GetHostOptions(host As TestHost) As OptionsCollection + Return New OptionsCollection(GetLanguage()) From { + {RemoteHostOptions.RemoteHostTest, host <> TestHost.InProcess} + } + End Function + + + Public Async Function TestEncapsulatePrivateFieldAndUpdateReferences(host As TestHost) As Task Dim text = Class C Private ReadOnly x[||] As Integer @@ -48,12 +61,11 @@ Class C End Sub End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) - + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestEncapsulateDimField() As Task + + Public Async Function TestEncapsulateDimField(host As TestHost) As Task Dim text = Class C Dim x[||] As Integer @@ -81,12 +93,12 @@ Class C End Sub End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestEncapsulateGenericField() As Task + + Public Async Function TestEncapsulateGenericField(host As TestHost) As Task Dim text = Class C(Of T) Dim x[||] As T @@ -114,12 +126,12 @@ Class C(Of T) End Sub End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestEncapsulatePublicFieldIgnoringReferences() As Task + + Public Async Function TestEncapsulatePublicFieldIgnoringReferences(host As TestHost) As Task Dim text = Class C Public [|x|] As Integer @@ -147,11 +159,11 @@ Class C End Sub End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected, index:=1) + Await TestInRegularAndScriptAsync(text, expected, index:=1, options:=GetHostOptions(host)) End Function - - Public Async Function TestEncapsulatePublicFieldUpdatingReferences() As Task + + Public Async Function TestEncapsulatePublicFieldUpdatingReferences(host As TestHost) As Task Dim text = Class C Public [|x|] As Integer @@ -179,11 +191,11 @@ Class C End Sub End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestEncapsulateMultiplePrivateFieldsWithReferences() As Task + + Public Async Function TestEncapsulateMultiplePrivateFieldsWithReferences(host As TestHost) As Task Dim text = Class C Private [|x, y|] As Integer @@ -222,11 +234,11 @@ Class C End Sub End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestEncapsulateMultiplePublicFieldsWithReferences() As Task + + Public Async Function TestEncapsulateMultiplePublicFieldsWithReferences(host As TestHost) As Task Dim text = Class C [|Public x As String @@ -267,11 +279,11 @@ Class C End Sub End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected, index:=1) + Await TestInRegularAndScriptAsync(text, expected, index:=1, options:=GetHostOptions(host)) End Function - - Public Async Function TestNoSetterForConstField() As Task + + Public Async Function TestNoSetterForConstField(host As TestHost) As Task Dim text = Class Program Private Const [|goo|] As Integer = 3 @@ -288,12 +300,12 @@ Class Program End Property End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestEncapsulateEscapedIdentifier() As Task + + Public Async Function TestEncapsulateEscapedIdentifier(host As TestHost) As Task Dim text = Class C Private [|[Class]|] As String @@ -313,12 +325,12 @@ Class C End Property End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestEncapsulateEscapedIdentifierWithQualifiedAccess() As Task + + Public Async Function TestEncapsulateEscapedIdentifierWithQualifiedAccess(host As TestHost) As Task Dim text = Class C Private [|[Class]|] As String @@ -338,12 +350,16 @@ Class C End Property End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected, options:=[Option](CodeStyleOptions2.QualifyFieldAccess, True, NotificationOption2.Error)) - + Await TestInRegularAndScriptAsync( + text, expected, + options:=New OptionsCollection(GetLanguage()) From { + {CodeStyleOptions2.QualifyFieldAccess, True, NotificationOption2.Error}, + {RemoteHostOptions.RemoteHostTest, host <> TestHost.InProcess} + }) End Function - - Public Async Function TestEncapsulateFieldNamedValue() As Task + + Public Async Function TestEncapsulateFieldNamedValue(host As TestHost) As Task Dim text = Class C Private [|value|] As Integer = 3 @@ -363,12 +379,12 @@ Class C End Property End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestEncapsulateFieldName__() As Task + + Public Async Function TestEncapsulateFieldName__(host As TestHost) As Task Dim text = Class D Public [|__|] As Integer @@ -390,12 +406,12 @@ Class D End Class .ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestPreserveTrivia() As Task + + Public Async Function TestPreserveTrivia(host As TestHost) As Task Dim text = Class AA Private name As String : Public [|dsds|] As Integer @@ -417,12 +433,12 @@ Class AA End Class .ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestNewPropertyNameIsUnique() As Task + + Public Async Function TestNewPropertyNameIsUnique(host As TestHost) As Task Dim text = Class AA Private [|name|] As String @@ -460,23 +476,23 @@ Class AA End Class .ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestAvailableNotJustOnVariableName() As Task + + Public Async Function TestAvailableNotJustOnVariableName(host As TestHost) As Task Dim text = Class C Private [||] ReadOnly x As Integer End Class.ConvertTestSourceTag() - Await TestActionCountAsync(text, 2) + Await TestActionCountAsync(text, 2, New TestParameters(options:=GetHostOptions(host))) End Function - - Public Async Function TestCopyAccessibility() As Task + + Public Async Function TestCopyAccessibility(host As TestHost) As Task Dim text = Class C Protected [|x|] As Integer @@ -496,12 +512,12 @@ Class C End Property End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestBackingFieldStartsWithUnderscore() As Task + + Public Async Function TestBackingFieldStartsWithUnderscore(host As TestHost) As Task Dim text = Public Class Class1 Public [|Name|] As String @@ -530,11 +546,11 @@ Public Class Class1 End Class .ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestEncapsulateShadowingField() As Task + + Public Async Function TestEncapsulateShadowingField(host As TestHost) As Task Dim text = Class C Protected _goo As Integer @@ -584,16 +600,16 @@ Class D End Property End Class.ConvertTestSourceTag() - Await TestInRegularAndScriptAsync(text, expected) + Await TestInRegularAndScriptAsync(text, expected, options:=GetHostOptions(host)) End Function - - Public Async Function TestDoNotEncapsulateOutsideTypeDeclaration() As Task + + Public Async Function TestDoNotEncapsulateOutsideTypeDeclaration(host As TestHost) As Task Dim globalField = Dim [|x|] = 1 .ConvertTestSourceTag() - Await TestMissingInRegularAndScriptAsync(globalField) + Await TestMissingInRegularAndScriptAsync(globalField, New TestParameters(options:=GetHostOptions(host))) Dim namespaceField = @@ -601,20 +617,20 @@ Namespace N Dim [|x|] = 1 End Namespace .ConvertTestSourceTag() - Await TestMissingInRegularAndScriptAsync(namespaceField) + Await TestMissingInRegularAndScriptAsync(namespaceField, New TestParameters(options:=GetHostOptions(host))) Dim enumField = Enum E [|x|] = 1 End Enum .ConvertTestSourceTag() - Await TestMissingInRegularAndScriptAsync(enumField) + Await TestMissingInRegularAndScriptAsync(enumField, New TestParameters(options:=GetHostOptions(host))) End Function - - Public Async Function ApplyCurrentMePrefixStyle() As Task + + Public Async Function ApplyCurrentMePrefixStyle(host As TestHost) As Task Await TestInRegularAndScriptAsync(" Class C Dim [|i|] As Integer @@ -633,7 +649,10 @@ Class C End Property End Class ", -options:=[Option](CodeStyleOptions2.QualifyFieldAccess, True, NotificationOption2.Error)) +options:=New OptionsCollection(GetLanguage()) From { + {CodeStyleOptions2.QualifyFieldAccess, True, NotificationOption2.Error}, + {RemoteHostOptions.RemoteHostTest, host <> TestHost.InProcess} +}) End Function End Class End Namespace From 98143ca9fbabc62f82f260374f36af3e0276253b Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Fri, 24 Apr 2020 19:18:45 -0700 Subject: [PATCH 17/21] Update src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs Co-Authored-By: Andrew Hall --- .../EncapsulateField/AbstractEncapsulateFieldService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs index 84b9ef35febe0..3962f5ce5e73b 100644 --- a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs +++ b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs @@ -46,7 +46,7 @@ public async Task EncapsulateFieldsInSpanAsync(Document public async Task> GetEncapsulateFieldCodeActionsAsync(Document document, TextSpan span, CancellationToken cancellationToken) { var fields = await GetFieldsAsync(document, span, cancellationToken).ConfigureAwait(false); - if (fields.Length == 0) + if (fields.IsDefaultOrEmpty) return ImmutableArray.Empty; if (fields.Length == 1) From d29fb04eacc42f66702d97983d415be1f0d5c1f0 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 19:19:57 -0700 Subject: [PATCH 18/21] Rename --- .../Portable/EncapsulateField/EncapsulateFieldResult.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Features/Core/Portable/EncapsulateField/EncapsulateFieldResult.cs b/src/Features/Core/Portable/EncapsulateField/EncapsulateFieldResult.cs index 8b83f2fb00a3e..cac88ec66aee4 100644 --- a/src/Features/Core/Portable/EncapsulateField/EncapsulateFieldResult.cs +++ b/src/Features/Core/Portable/EncapsulateField/EncapsulateFieldResult.cs @@ -13,16 +13,16 @@ internal class EncapsulateFieldResult { public readonly string Name; public readonly Glyph Glyph; - private readonly AsyncLazy _resultGetter; + private readonly AsyncLazy _lazySolution; public EncapsulateFieldResult(string name, Glyph glyph, Func> getSolutionAsync) { Name = name; Glyph = glyph; - _resultGetter = new AsyncLazy(getSolutionAsync, cacheResult: true); + _lazySolution = new AsyncLazy(getSolutionAsync, cacheResult: true); } public Task GetSolutionAsync(CancellationToken cancellationToken) - => _resultGetter.GetValueAsync(cancellationToken); + => _lazySolution.GetValueAsync(cancellationToken); } } From 2c410b955df786e2ba468d2faba802be5b14451e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 19:24:27 -0700 Subject: [PATCH 19/21] Add comment --- src/Workspaces/Core/Portable/Rename/Renamer.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Workspaces/Core/Portable/Rename/Renamer.cs b/src/Workspaces/Core/Portable/Rename/Renamer.cs index 1cad0c1439281..fe7c1a4bdab1d 100644 --- a/src/Workspaces/Core/Portable/Rename/Renamer.cs +++ b/src/Workspaces/Core/Portable/Rename/Renamer.cs @@ -31,6 +31,9 @@ public static async Task RenameSymbolAsync( optionSet ??= solution.Options; var resolution = await RenameSymbolAsync(solution, symbol, newName, optionSet, nonConflictSymbols: null, cancellationToken).ConfigureAwait(false); + + // This is a public entrypoint. So if rename failed to resolve conflicts, we report that back to caller as + // an exception. if (resolution.ErrorMessage != null) throw new ArgumentException(resolution.ErrorMessage); From 39c154b6ffda8e85d3ca24bf9961aee3f3efb7ea Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 19:36:47 -0700 Subject: [PATCH 20/21] Add docs --- src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs b/src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs index b5681a903dc02..9efbf620bcc07 100644 --- a/src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs +++ b/src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs @@ -14,6 +14,11 @@ namespace Microsoft.CodeAnalysis.Remote internal static class RemoteUtilities { + /// + /// Given two solution snapshots ( and ), determines + /// the set of document text changes necessary to convert to . + /// public static async Task GetDocumentTextChangesAsync( Solution oldSolution, Solution newSolution, @@ -36,6 +41,10 @@ public static async Task GetDocumentTextChangesAsync( return builder.ToImmutable(); } + /// + /// Applies the result of to to produce + /// a solution textually equivalent to the newSolution passed to . + /// public static async Task UpdateSolutionAsync( Solution oldSolution, DocumentTextChanges documentTextChanges, CancellationToken cancellationToken) { From e04b8e026f7abd668ff4ff56e46ba29190476b46 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 24 Apr 2020 20:37:57 -0700 Subject: [PATCH 21/21] use arrays --- .../AbstractEncapsulateFieldService.cs | 2 +- .../IRemoteEncapsulateFieldService.cs | 2 +- .../Core/Portable/Remote/RemoteUtilities.cs | 14 ++++++-------- .../Core/Portable/Rename/IRemoteRenamer.cs | 6 +++++- .../CodeAnalysisService_EncapsulateField.cs | 7 +++---- 5 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs index 3962f5ce5e73b..677ce209c9685 100644 --- a/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs +++ b/src/Features/Core/Portable/EncapsulateField/AbstractEncapsulateFieldService.cs @@ -104,7 +104,7 @@ public async Task EncapsulateFieldsAsync( var client = await RemoteHostClient.TryGetClientAsync(solution.Workspace, cancellationToken).ConfigureAwait(false); if (client != null) { - var result = await client.TryRunRemoteAsync)>>( + var result = await client.TryRunRemoteAsync<(DocumentId, TextChange[])[]>( WellKnownServiceHubServices.CodeAnalysisService, nameof(IRemoteEncapsulateFieldService.EncapsulateFieldsAsync), solution, diff --git a/src/Features/Core/Portable/EncapsulateField/IRemoteEncapsulateFieldService.cs b/src/Features/Core/Portable/EncapsulateField/IRemoteEncapsulateFieldService.cs index b95355d8bb893..e1edfef47f965 100644 --- a/src/Features/Core/Portable/EncapsulateField/IRemoteEncapsulateFieldService.cs +++ b/src/Features/Core/Portable/EncapsulateField/IRemoteEncapsulateFieldService.cs @@ -12,7 +12,7 @@ namespace Microsoft.CodeAnalysis.EncapsulateField { internal interface IRemoteEncapsulateFieldService { - Task textChanges)>> EncapsulateFieldsAsync( + Task<(DocumentId, TextChange[])[]> EncapsulateFieldsAsync( PinnedSolutionInfo solutionInfo, DocumentId documentId, ImmutableArray fieldSymbolKeys, diff --git a/src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs b/src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs index 9efbf620bcc07..6397763589adc 100644 --- a/src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs +++ b/src/Workspaces/Core/Portable/Remote/RemoteUtilities.cs @@ -2,7 +2,7 @@ // 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.Collections.Immutable; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.PooledObjects; @@ -10,8 +10,6 @@ namespace Microsoft.CodeAnalysis.Remote { - using DocumentTextChanges = ImmutableArray<(DocumentId, ImmutableArray)>; - internal static class RemoteUtilities { /// @@ -19,12 +17,12 @@ internal static class RemoteUtilities /// the set of document text changes necessary to convert to . /// - public static async Task GetDocumentTextChangesAsync( + public static async Task<(DocumentId, TextChange[])[]> GetDocumentTextChangesAsync( Solution oldSolution, Solution newSolution, CancellationToken cancellationToken) { - using var _ = ArrayBuilder<(DocumentId, ImmutableArray)>.GetInstance(out var builder); + using var _ = ArrayBuilder<(DocumentId, TextChange[])>.GetInstance(out var builder); var solutionChanges = newSolution.GetChanges(oldSolution); foreach (var projectChange in solutionChanges.GetProjectChanges()) @@ -34,11 +32,11 @@ public static async Task GetDocumentTextChangesAsync( var oldDoc = oldSolution.GetDocument(docId); var newDoc = newSolution.GetDocument(docId); var textChanges = await newDoc.GetTextChangesAsync(oldDoc, cancellationToken).ConfigureAwait(false); - builder.Add((docId, textChanges.ToImmutableArray())); + builder.Add((docId, textChanges.ToArray())); } } - return builder.ToImmutable(); + return builder.ToArray(); } /// @@ -46,7 +44,7 @@ public static async Task GetDocumentTextChangesAsync( /// a solution textually equivalent to the newSolution passed to . /// public static async Task UpdateSolutionAsync( - Solution oldSolution, DocumentTextChanges documentTextChanges, CancellationToken cancellationToken) + Solution oldSolution, (DocumentId, TextChange[])[] documentTextChanges, CancellationToken cancellationToken) { var currentSolution = oldSolution; foreach (var (docId, textChanges) in documentTextChanges) diff --git a/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs b/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs index ca6acb518201b..36ba0fd70f0c2 100644 --- a/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs +++ b/src/Workspaces/Core/Portable/Rename/IRemoteRenamer.cs @@ -269,9 +269,13 @@ internal class SerializableConflictResolution public (DocumentId documentId, string newName) RenamedDocument; + // Note: arrays are used (instead of ImmutableArray) as jsonrpc can't marshal null values to/from those types. + // + // We also flatten dictionaries into key/value tuples because jsonrpc only supports dictionaries with string keys. + public DocumentId[] DocumentIds; public SerializableRelatedLocation[] RelatedLocations; - public ImmutableArray<(DocumentId, ImmutableArray)> DocumentTextChanges; + public (DocumentId, TextChange[])[] DocumentTextChanges; public (DocumentId, ImmutableArray<(TextSpan oldSpan, TextSpan newSpan)>)[] DocumentToModifiedSpansMap; public (DocumentId, ImmutableArray)[] DocumentToComplexifiedSpansMap; public (DocumentId, ImmutableArray)[] DocumentToRelatedLocationsMap; diff --git a/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_EncapsulateField.cs b/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_EncapsulateField.cs index 969317a295a3c..d309993ea20de 100644 --- a/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_EncapsulateField.cs +++ b/src/Workspaces/Remote/ServiceHub/Services/CodeAnalysisService_EncapsulateField.cs @@ -2,6 +2,7 @@ // 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.Threading; using System.Threading.Tasks; @@ -12,11 +13,9 @@ namespace Microsoft.CodeAnalysis.Remote { - using DocumentTextChanges = ImmutableArray<(DocumentId documentId, ImmutableArray textChanges)>; - internal partial class CodeAnalysisService : IRemoteEncapsulateFieldService { - public Task EncapsulateFieldsAsync( + public Task<(DocumentId, TextChange[])[]> EncapsulateFieldsAsync( PinnedSolutionInfo solutionInfo, DocumentId documentId, ImmutableArray fieldSymbolKeys, @@ -37,7 +36,7 @@ public Task EncapsulateFieldsAsync( { var resolved = SymbolKey.ResolveString(key, compilation, cancellationToken: cancellationToken).GetAnySymbol() as IFieldSymbol; if (resolved == null) - return DocumentTextChanges.Empty; + return Array.Empty<(DocumentId, TextChange[])>(); fields.Add(resolved); }