Skip to content

Commit 395d298

Browse files
CyrusNajmabadiCyrusNajmabadi
authored
Remove sync over async blocking in Extract Method/Interface options code (#76510)
2 parents 32ec9b9 + 9d0e3eb commit 395d298

File tree

13 files changed

+175
-184
lines changed

13 files changed

+175
-184
lines changed

‎src/EditorFeatures/TestUtilities/ExtractInterface/TestExtractInterfaceOptions.cs‎

Lines changed: 44 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -8,63 +8,56 @@
88
using System.Collections.Generic;
99
using System.Composition;
1010
using System.Threading;
11-
using System.Threading.Tasks;
12-
using Microsoft.CodeAnalysis.CodeGeneration;
1311
using Microsoft.CodeAnalysis.ExtractInterface;
1412
using Microsoft.CodeAnalysis.Host.Mef;
1513
using Microsoft.CodeAnalysis.LanguageService;
1614
using Microsoft.CodeAnalysis.Notification;
1715

18-
namespace Microsoft.CodeAnalysis.Editor.UnitTests.ExtractInterface
16+
namespace Microsoft.CodeAnalysis.Editor.UnitTests.ExtractInterface;
17+
18+
[ExportWorkspaceService(typeof(IExtractInterfaceOptionsService), ServiceLayer.Test), Shared, PartNotDiscoverable]
19+
[method: ImportingConstructor]
20+
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
21+
internal sealed class TestExtractInterfaceOptionsService() : IExtractInterfaceOptionsService
1922
{
20-
[ExportWorkspaceService(typeof(IExtractInterfaceOptionsService), ServiceLayer.Test), Shared, PartNotDiscoverable]
21-
internal class TestExtractInterfaceOptionsService : IExtractInterfaceOptionsService
23+
public IEnumerable<ISymbol> AllExtractableMembers { get; private set; }
24+
public string DefaultInterfaceName { get; private set; }
25+
public List<string> ConflictingTypeNames { get; private set; }
26+
public string DefaultNamespace { get; private set; }
27+
public string GeneratedNameTypeParameterSuffix { get; set; }
28+
29+
public bool IsCancelled { get; set; }
30+
public string ChosenInterfaceName { get; set; }
31+
public string ChosenFileName { get; set; }
32+
public IEnumerable<ISymbol> ChosenMembers { get; set; }
33+
public bool SameFile { get; set; }
34+
35+
public ExtractInterfaceOptionsResult GetExtractInterfaceOptions(
36+
ISyntaxFactsService syntaxFactsService,
37+
INotificationService notificationService,
38+
List<ISymbol> extractableMembers,
39+
string defaultInterfaceName,
40+
List<string> conflictingTypeNames,
41+
string defaultNamespace,
42+
string generatedNameTypeParameterSuffix,
43+
string languageName,
44+
CancellationToken cancellationToken)
2245
{
23-
[ImportingConstructor]
24-
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
25-
public TestExtractInterfaceOptionsService()
26-
{
27-
}
28-
29-
public IEnumerable<ISymbol> AllExtractableMembers { get; private set; }
30-
public string DefaultInterfaceName { get; private set; }
31-
public List<string> ConflictingTypeNames { get; private set; }
32-
public string DefaultNamespace { get; private set; }
33-
public string GeneratedNameTypeParameterSuffix { get; set; }
34-
35-
public bool IsCancelled { get; set; }
36-
public string ChosenInterfaceName { get; set; }
37-
public string ChosenFileName { get; set; }
38-
public IEnumerable<ISymbol> ChosenMembers { get; set; }
39-
public bool SameFile { get; set; }
40-
41-
public Task<ExtractInterfaceOptionsResult> GetExtractInterfaceOptionsAsync(
42-
ISyntaxFactsService syntaxFactsService,
43-
INotificationService notificationService,
44-
List<ISymbol> extractableMembers,
45-
string defaultInterfaceName,
46-
List<string> conflictingTypeNames,
47-
string defaultNamespace,
48-
string generatedNameTypeParameterSuffix,
49-
string languageName,
50-
CancellationToken cancellationToken)
51-
{
52-
this.AllExtractableMembers = extractableMembers;
53-
this.DefaultInterfaceName = defaultInterfaceName;
54-
this.ConflictingTypeNames = conflictingTypeNames;
55-
this.DefaultNamespace = defaultNamespace;
56-
this.GeneratedNameTypeParameterSuffix = generatedNameTypeParameterSuffix;
57-
58-
var result = IsCancelled
59-
? ExtractInterfaceOptionsResult.Cancelled
60-
: new ExtractInterfaceOptionsResult(
61-
isCancelled: false,
62-
includedMembers: (ChosenMembers ?? AllExtractableMembers).AsImmutable(),
63-
interfaceName: ChosenInterfaceName ?? defaultInterfaceName,
64-
fileName: ChosenFileName ?? defaultInterfaceName,
65-
location: SameFile ? ExtractInterfaceOptionsResult.ExtractLocation.SameFile : ExtractInterfaceOptionsResult.ExtractLocation.NewFile);
66-
67-
return Task.FromResult(result);
68-
}
46+
this.AllExtractableMembers = extractableMembers;
47+
this.DefaultInterfaceName = defaultInterfaceName;
48+
this.ConflictingTypeNames = conflictingTypeNames;
49+
this.DefaultNamespace = defaultNamespace;
50+
this.GeneratedNameTypeParameterSuffix = generatedNameTypeParameterSuffix;
51+
52+
var result = IsCancelled
53+
? ExtractInterfaceOptionsResult.Cancelled
54+
: new ExtractInterfaceOptionsResult(
55+
isCancelled: false,
56+
includedMembers: (ChosenMembers ?? AllExtractableMembers).AsImmutable(),
57+
interfaceName: ChosenInterfaceName ?? defaultInterfaceName,
58+
fileName: ChosenFileName ?? defaultInterfaceName,
59+
location: SameFile ? ExtractInterfaceOptionsResult.ExtractLocation.SameFile : ExtractInterfaceOptionsResult.ExtractLocation.NewFile);
60+
61+
return result;
6962
}
7063
}

‎src/Features/CSharpTest/ExtractClass/ExtractClassTests.cs‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
using Microsoft.CodeAnalysis.CSharp.CodeStyle;
1616
using Microsoft.CodeAnalysis.Editor.UnitTests.CodeActions;
1717
using Microsoft.CodeAnalysis.ExtractClass;
18+
using Microsoft.CodeAnalysis.Formatting;
1819
using Microsoft.CodeAnalysis.PullMemberUp;
1920
using Microsoft.CodeAnalysis.Test.Utilities;
2021
using Microsoft.CodeAnalysis.Testing;
@@ -3086,7 +3087,7 @@ public void M() { }
30863087
private static IEnumerable<(string name, bool makeAbstract)> MakeSelection(params string[] memberNames)
30873088
=> memberNames.Select(m => (m, false));
30883089

3089-
private class TestExtractClassOptionsService : IExtractClassOptionsService
3090+
private sealed class TestExtractClassOptionsService : IExtractClassOptionsService
30903091
{
30913092
private readonly IEnumerable<(string name, bool makeAbstract)>? _dialogSelection;
30923093
private readonly bool _sameFile;
@@ -3102,7 +3103,12 @@ public TestExtractClassOptionsService(IEnumerable<(string name, bool makeAbstrac
31023103
public string FileName { get; set; } = "MyBase.cs";
31033104
public string BaseName { get; set; } = "MyBase";
31043105

3105-
public Task<ExtractClassOptions?> GetExtractClassOptionsAsync(Document document, INamedTypeSymbol originalSymbol, ImmutableArray<ISymbol> selectedMembers, CancellationToken cancellationToken)
3106+
public ExtractClassOptions? GetExtractClassOptions(
3107+
Document document,
3108+
INamedTypeSymbol originalSymbol,
3109+
ImmutableArray<ISymbol> selectedMembers,
3110+
SyntaxFormattingOptions formattingOptions,
3111+
CancellationToken cancellationToken)
31063112
{
31073113
var availableMembers = originalSymbol.GetMembers().Where(member => MemberAndDestinationValidator.IsMemberValid(member));
31083114

@@ -3126,13 +3132,12 @@ public TestExtractClassOptionsService(IEnumerable<(string name, bool makeAbstrac
31263132
selections = _dialogSelection.Select(selection => (member: availableMembers.Single(symbol => symbol.Name == selection.name), selection.makeAbstract));
31273133
}
31283134

3129-
var memberAnalysis = selections.Select(s =>
3135+
var memberAnalysis = selections.SelectAsArray(s =>
31303136
new ExtractClassMemberAnalysisResult(
31313137
s.member,
3132-
s.makeAbstract))
3133-
.ToImmutableArray();
3138+
s.makeAbstract));
31343139

3135-
return Task.FromResult<ExtractClassOptions?>(new ExtractClassOptions(FileName, BaseName, _sameFile, memberAnalysis));
3140+
return new ExtractClassOptions(FileName, BaseName, _sameFile, memberAnalysis);
31363141
}
31373142
}
31383143
}

‎src/Features/Core/Portable/ExtractClass/AbstractExtractClassRefactoringProvider.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Linq;
77
using System.Threading.Tasks;
88
using Microsoft.CodeAnalysis.CodeRefactorings;
9+
using Microsoft.CodeAnalysis.Formatting;
910
using Microsoft.CodeAnalysis.LanguageService;
1011
using Microsoft.CodeAnalysis.PullMemberUp;
1112
using Microsoft.CodeAnalysis.Shared.Extensions;
@@ -106,8 +107,9 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
106107
return (null, false);
107108
}
108109

110+
var formattingOptions = await document.GetSyntaxFormattingOptionsAsync(cancellationToken).ConfigureAwait(false);
109111
var action = new ExtractClassWithDialogCodeAction(
110-
document, memberSpan, optionsService, containingType, containingTypeDeclarationNode, selectedMembers);
112+
document, memberSpan, optionsService, containingType, containingTypeDeclarationNode, selectedMembers, formattingOptions);
111113

112114
return (action, false);
113115
}
@@ -133,8 +135,9 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
133135
return null;
134136
}
135137

138+
var formattingOptions = await document.GetSyntaxFormattingOptionsAsync(cancellationToken).ConfigureAwait(false);
136139
return new ExtractClassWithDialogCodeAction(
137-
document, span, optionsService, selectedType, selectedClassNode, selectedMembers: []);
140+
document, span, optionsService, selectedType, selectedClassNode, selectedMembers: [], formattingOptions);
138141
}
139142

140143
private static bool HasBaseType(INamedTypeSymbol containingType) => containingType.BaseType?.SpecialType != SpecialType.System_Object;

‎src/Features/Core/Portable/ExtractClass/ExtractClassWithDialogCodeAction.cs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using Microsoft.CodeAnalysis.CodeRefactorings.PullMemberUp;
1414
using Microsoft.CodeAnalysis.Editing;
1515
using Microsoft.CodeAnalysis.ExtractInterface;
16+
using Microsoft.CodeAnalysis.Formatting;
1617
using Microsoft.CodeAnalysis.LanguageService;
1718
using Microsoft.CodeAnalysis.PooledObjects;
1819
using Microsoft.CodeAnalysis.Shared.Extensions;
@@ -28,13 +29,15 @@ internal sealed class ExtractClassWithDialogCodeAction(
2829
IExtractClassOptionsService service,
2930
INamedTypeSymbol selectedType,
3031
SyntaxNode selectedTypeDeclarationNode,
31-
ImmutableArray<ISymbol> selectedMembers) : CodeActionWithOptions
32+
ImmutableArray<ISymbol> selectedMembers,
33+
SyntaxFormattingOptions formattingOptions) : CodeActionWithOptions
3234
{
3335
private readonly Document _document = document;
3436
private readonly ImmutableArray<ISymbol> _selectedMembers = selectedMembers;
3537
private readonly INamedTypeSymbol _selectedType = selectedType;
3638
private readonly SyntaxNode _selectedTypeDeclarationNode = selectedTypeDeclarationNode;
3739
private readonly IExtractClassOptionsService _service = service;
40+
private readonly SyntaxFormattingOptions _formattingOptions = formattingOptions;
3841

3942
// If the user brought up the lightbulb on a class itself, it's more likely that they want to extract a base
4043
// class. on a member however, we deprioritize this as there are likely more member-specific operations
@@ -52,8 +55,8 @@ protected sealed override CodeActionPriority ComputePriority()
5255
public override object? GetOptions(CancellationToken cancellationToken)
5356
{
5457
var extractClassService = _service ?? _document.Project.Solution.Services.GetRequiredService<IExtractClassOptionsService>();
55-
return extractClassService.GetExtractClassOptionsAsync(_document, _selectedType, _selectedMembers, cancellationToken)
56-
.WaitAndGetResult_CanCallOnBackground(cancellationToken);
58+
return extractClassService.GetExtractClassOptions(
59+
_document, _selectedType, _selectedMembers, _formattingOptions, cancellationToken);
5760
}
5861

5962
protected override async Task<IEnumerable<CodeActionOperation>> ComputeOperationsAsync(

‎src/Features/Core/Portable/ExtractClass/IExtractClassOptionsService.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,17 @@
44

55
using System.Collections.Immutable;
66
using System.Threading;
7-
using System.Threading.Tasks;
7+
using Microsoft.CodeAnalysis.Formatting;
88
using Microsoft.CodeAnalysis.Host;
99

1010
namespace Microsoft.CodeAnalysis.ExtractClass;
1111

1212
internal interface IExtractClassOptionsService : IWorkspaceService
1313
{
14-
Task<ExtractClassOptions?> GetExtractClassOptionsAsync(Document document, INamedTypeSymbol originalType, ImmutableArray<ISymbol> selectedMembers, CancellationToken cancellationToken);
14+
ExtractClassOptions? GetExtractClassOptions(
15+
Document document,
16+
INamedTypeSymbol originalType,
17+
ImmutableArray<ISymbol> selectedMembers,
18+
SyntaxFormattingOptions formattingOptions,
19+
CancellationToken cancellationToken);
1520
}

‎src/Features/Core/Portable/ExtractInterface/AbstractExtractInterfaceService.cs‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,21 +104,25 @@ public async Task<ExtractInterfaceTypeAnalysisResult> AnalyzeTypeAtPositionAsync
104104
return new ExtractInterfaceTypeAnalysisResult(errorMessage);
105105
}
106106

107-
return new ExtractInterfaceTypeAnalysisResult(document, typeNode, typeToExtractFrom, extractableMembers);
107+
var formattingOptions = await document.GetSyntaxFormattingOptionsAsync(cancellationToken).ConfigureAwait(false);
108+
return new ExtractInterfaceTypeAnalysisResult(document, typeNode, typeToExtractFrom, extractableMembers, formattingOptions);
108109
}
109110

110-
public async Task<ExtractInterfaceResult> ExtractInterfaceFromAnalyzedTypeAsync(ExtractInterfaceTypeAnalysisResult refactoringResult, CancellationToken cancellationToken)
111+
public async Task<ExtractInterfaceResult> ExtractInterfaceFromAnalyzedTypeAsync(
112+
ExtractInterfaceTypeAnalysisResult refactoringResult,
113+
CancellationToken cancellationToken)
111114
{
112115
var containingNamespaceDisplay = refactoringResult.TypeToExtractFrom.ContainingNamespace.IsGlobalNamespace
113116
? string.Empty
114117
: refactoringResult.TypeToExtractFrom.ContainingNamespace.ToDisplayString();
115118

116-
var extractInterfaceOptions = await GetExtractInterfaceOptionsAsync(
119+
var extractInterfaceOptions = GetExtractInterfaceOptions(
117120
refactoringResult.DocumentToExtractFrom,
118121
refactoringResult.TypeToExtractFrom,
119122
refactoringResult.ExtractableMembers,
120123
containingNamespaceDisplay,
121-
cancellationToken).ConfigureAwait(false);
124+
refactoringResult.FormattingOptions,
125+
cancellationToken);
122126

123127
if (extractInterfaceOptions.IsCancelled)
124128
{
@@ -249,23 +253,23 @@ private async Task<ExtractInterfaceResult> ExtractInterfaceToSameFileAsync(
249253
navigationDocumentId: refactoringResult.DocumentToExtractFrom.Id);
250254
}
251255

252-
internal static async Task<ExtractInterfaceOptionsResult> GetExtractInterfaceOptionsAsync(
256+
internal static ExtractInterfaceOptionsResult GetExtractInterfaceOptions(
253257
Document document,
254258
INamedTypeSymbol type,
255259
IEnumerable<ISymbol> extractableMembers,
256260
string containingNamespace,
261+
SyntaxFormattingOptions formattingOptions,
257262
CancellationToken cancellationToken)
258263
{
259264
var conflictingTypeNames = type.ContainingNamespace.GetAllTypes(cancellationToken).Select(t => t.Name);
260265
var candidateInterfaceName = type.TypeKind == TypeKind.Interface ? type.Name : "I" + type.Name;
261266
var defaultInterfaceName = NameGenerator.GenerateUniqueName(candidateInterfaceName, name => !conflictingTypeNames.Contains(name));
262-
var syntaxFactsService = document.GetLanguageService<ISyntaxFactsService>();
263-
var notificationService = document.Project.Solution.Services.GetService<INotificationService>();
264-
var formattingOptions = await document.GetSyntaxFormattingOptionsAsync(cancellationToken).ConfigureAwait(false);
267+
var syntaxFactsService = document.GetRequiredLanguageService<ISyntaxFactsService>();
268+
var notificationService = document.Project.Solution.Services.GetRequiredService<INotificationService>();
265269
var generatedNameTypeParameterSuffix = ExtractTypeHelpers.GetTypeParameterSuffix(document, formattingOptions, type, extractableMembers, cancellationToken);
266270

267271
var service = document.Project.Solution.Services.GetRequiredService<IExtractInterfaceOptionsService>();
268-
return await service.GetExtractInterfaceOptionsAsync(
272+
return service.GetExtractInterfaceOptions(
269273
syntaxFactsService,
270274
notificationService,
271275
extractableMembers.ToList(),
@@ -274,7 +278,7 @@ internal static async Task<ExtractInterfaceOptionsResult> GetExtractInterfaceOpt
274278
containingNamespace,
275279
generatedNameTypeParameterSuffix,
276280
document.Project.Language,
277-
cancellationToken).ConfigureAwait(false);
281+
cancellationToken);
278282
}
279283

280284
private static async Task<Solution> GetFormattedSolutionAsync(Solution unformattedSolution, IEnumerable<DocumentId> documentIds, CancellationToken cancellationToken)

‎src/Features/Core/Portable/ExtractInterface/ExtractInterfaceCodeAction.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,13 @@ public override object GetOptions(CancellationToken cancellationToken)
2222
? string.Empty
2323
: _typeAnalysisResult.TypeToExtractFrom.ContainingNamespace.ToDisplayString();
2424

25-
return AbstractExtractInterfaceService.GetExtractInterfaceOptionsAsync(
25+
return AbstractExtractInterfaceService.GetExtractInterfaceOptions(
2626
_typeAnalysisResult.DocumentToExtractFrom,
2727
_typeAnalysisResult.TypeToExtractFrom,
2828
_typeAnalysisResult.ExtractableMembers,
2929
containingNamespaceDisplay,
30-
cancellationToken).WaitAndGetResult_CanCallOnBackground(cancellationToken);
30+
_typeAnalysisResult.FormattingOptions,
31+
cancellationToken);
3132
}
3233

3334
protected override async Task<IEnumerable<CodeActionOperation>> ComputeOperationsAsync(

‎src/Features/Core/Portable/ExtractInterface/ExtractInterfaceTypeAnalysisResult.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#nullable disable
66

77
using System.Collections.Generic;
8+
using Microsoft.CodeAnalysis.Formatting;
89

910
namespace Microsoft.CodeAnalysis.ExtractInterface;
1011

@@ -15,19 +16,22 @@ internal sealed class ExtractInterfaceTypeAnalysisResult
1516
public readonly SyntaxNode TypeNode;
1617
public readonly INamedTypeSymbol TypeToExtractFrom;
1718
public readonly IEnumerable<ISymbol> ExtractableMembers;
19+
public readonly SyntaxFormattingOptions FormattingOptions;
1820
public readonly string ErrorMessage;
1921

2022
public ExtractInterfaceTypeAnalysisResult(
2123
Document documentToExtractFrom,
2224
SyntaxNode typeNode,
2325
INamedTypeSymbol typeToExtractFrom,
24-
IEnumerable<ISymbol> extractableMembers)
26+
IEnumerable<ISymbol> extractableMembers,
27+
SyntaxFormattingOptions formattingOptions)
2528
{
2629
CanExtractInterface = true;
2730
DocumentToExtractFrom = documentToExtractFrom;
2831
TypeNode = typeNode;
2932
TypeToExtractFrom = typeToExtractFrom;
3033
ExtractableMembers = extractableMembers;
34+
FormattingOptions = formattingOptions;
3135
}
3236

3337
public ExtractInterfaceTypeAnalysisResult(string errorMessage)

‎src/Features/Core/Portable/ExtractInterface/IExtractInterfaceOptionsService.cs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,8 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5-
#nullable disable
6-
75
using System.Collections.Generic;
86
using System.Threading;
9-
using System.Threading.Tasks;
10-
using Microsoft.CodeAnalysis.CodeGeneration;
117
using Microsoft.CodeAnalysis.Host;
128
using Microsoft.CodeAnalysis.LanguageService;
139
using Microsoft.CodeAnalysis.Notification;
@@ -16,7 +12,7 @@ namespace Microsoft.CodeAnalysis.ExtractInterface;
1612

1713
internal interface IExtractInterfaceOptionsService : IWorkspaceService
1814
{
19-
Task<ExtractInterfaceOptionsResult> GetExtractInterfaceOptionsAsync(
15+
ExtractInterfaceOptionsResult GetExtractInterfaceOptions(
2016
ISyntaxFactsService syntaxFactsService,
2117
INotificationService notificationService,
2218
List<ISymbol> extractableMembers,

0 commit comments

Comments
 (0)