Skip to content

Commit bddccde

Browse files
CyrusNajmabadiCyrusNajmabadi
authored
Remove sync over async blocking in sig change (#76519)
2 parents 395d298 + 38de961 commit bddccde

File tree

9 files changed

+65
-73
lines changed

9 files changed

+65
-73
lines changed

‎src/EditorFeatures/TestUtilities/ChangeSignature/TestChangeSignatureOptionsService.cs‎

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,34 +11,29 @@
1111
using Microsoft.CodeAnalysis.Test.Utilities.ChangeSignature;
1212
using Roslyn.Utilities;
1313

14-
namespace Microsoft.CodeAnalysis.Editor.UnitTests.ChangeSignature
15-
{
16-
[ExportWorkspaceService(typeof(IChangeSignatureOptionsService), ServiceLayer.Test), Shared, PartNotDiscoverable]
17-
internal class TestChangeSignatureOptionsService : IChangeSignatureOptionsService
18-
{
19-
public AddedParameterOrExistingIndex[]? UpdatedSignature = null;
14+
namespace Microsoft.CodeAnalysis.Editor.UnitTests.ChangeSignature;
2015

21-
[ImportingConstructor]
22-
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
23-
public TestChangeSignatureOptionsService()
24-
{
25-
}
16+
[ExportWorkspaceService(typeof(IChangeSignatureOptionsService), ServiceLayer.Test), Shared, PartNotDiscoverable]
17+
[method: ImportingConstructor]
18+
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
19+
internal sealed class TestChangeSignatureOptionsService() : IChangeSignatureOptionsService
20+
{
21+
public AddedParameterOrExistingIndex[]? UpdatedSignature = null;
2622

27-
ChangeSignatureOptionsResult IChangeSignatureOptionsService.GetChangeSignatureOptions(
28-
Document document,
29-
int positionForTypeBinding,
30-
ISymbol symbol,
31-
ParameterConfiguration parameters)
32-
{
33-
var list = parameters.ToListOfParameters();
34-
var updateParameters = UpdatedSignature != null
35-
? UpdatedSignature.Select(item => item.IsExisting ? list[item.OldIndex ?? -1] : item.GetAddedParameter(document)).ToImmutableArray()
36-
: new ImmutableArray<Parameter>();
37-
return new ChangeSignatureOptionsResult(new SignatureChange(
38-
parameters,
39-
UpdatedSignature == null
40-
? parameters
41-
: ParameterConfiguration.Create(updateParameters, parameters.ThisParameter != null, selectedIndex: 0)), previewChanges: false);
42-
}
23+
ChangeSignatureOptionsResult IChangeSignatureOptionsService.GetChangeSignatureOptions(
24+
SemanticDocument document,
25+
int positionForTypeBinding,
26+
ISymbol symbol,
27+
ParameterConfiguration parameters)
28+
{
29+
var list = parameters.ToListOfParameters();
30+
var updateParameters = UpdatedSignature != null
31+
? UpdatedSignature.Select(item => item.IsExisting ? list[item.OldIndex ?? -1] : item.GetAddedParameter(document.Document)).ToImmutableArray()
32+
: new ImmutableArray<Parameter>();
33+
return new ChangeSignatureOptionsResult(new SignatureChange(
34+
parameters,
35+
UpdatedSignature == null
36+
? parameters
37+
: ParameterConfiguration.Create(updateParameters, parameters.ThisParameter != null, selectedIndex: 0)), previewChanges: false);
4338
}
4439
}

‎src/Features/Core/Portable/ChangeSignature/AbstractChangeSignatureService.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,9 @@ internal async Task<ChangeSignatureAnalyzedContext> GetChangeSignatureContextAsy
182182
GetParameters(symbol).Select(p => new ExistingParameter(p)).ToImmutableArray<Parameter>(),
183183
symbol.IsExtensionMethod(), selectedIndex);
184184

185+
var semanticDocument = await SemanticDocument.CreateAsync(declarationDocument, cancellationToken).ConfigureAwait(false);
185186
return new ChangeSignatureAnalysisSucceededContext(
186-
declarationDocument, positionForTypeBinding, symbol, parameterConfiguration);
187+
semanticDocument, positionForTypeBinding, symbol, parameterConfiguration);
187188
}
188189

189190
internal async Task<ChangeSignatureResult> ChangeSignatureWithContextAsync(ChangeSignatureAnalyzedContext context, ChangeSignatureOptionsResult? options, CancellationToken cancellationToken)

‎src/Features/Core/Portable/ChangeSignature/ChangeSignatureAnalyzedContext.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,16 @@
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-
using Microsoft.CodeAnalysis.CodeCleanup;
6-
75
namespace Microsoft.CodeAnalysis.ChangeSignature;
86

97
internal abstract class ChangeSignatureAnalyzedContext
108
{
119
}
1210

1311
internal sealed class ChangeSignatureAnalysisSucceededContext(
14-
Document document, int positionForTypeBinding, ISymbol symbol, ParameterConfiguration parameterConfiguration) : ChangeSignatureAnalyzedContext
12+
SemanticDocument document, int positionForTypeBinding, ISymbol symbol, ParameterConfiguration parameterConfiguration) : ChangeSignatureAnalyzedContext
1513
{
16-
public readonly Document Document = document;
14+
public readonly SemanticDocument Document = document;
1715
public readonly ISymbol Symbol = symbol;
1816
public readonly ParameterConfiguration ParameterConfiguration = parameterConfiguration;
1917
public readonly int PositionForTypeBinding = positionForTypeBinding;

‎src/Features/Core/Portable/ChangeSignature/IChangeSignatureOptionsService.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ internal interface IChangeSignatureOptionsService : IWorkspaceService
1919
/// <param name="parameters">existing parameters of the symbol</param>
2020
/// <returns></returns>
2121
ChangeSignatureOptionsResult? GetChangeSignatureOptions(
22-
Document document,
22+
SemanticDocument document,
2323
int positionForTypeBinding,
2424
ISymbol symbol,
2525
ParameterConfiguration parameters);

‎src/VisualStudio/Core/Def/ChangeSignature/AddParameterDialogViewModel.cs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,26 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System.Diagnostics.CodeAnalysis;
6-
using System.Threading;
76
using System.Windows;
87
using Microsoft.CodeAnalysis;
98
using Microsoft.CodeAnalysis.LanguageService;
109
using Microsoft.CodeAnalysis.Notification;
1110
using Microsoft.CodeAnalysis.Shared.Extensions;
1211
using Microsoft.VisualStudio.LanguageServices.Implementation.Utilities;
13-
using Roslyn.Utilities;
1412

1513
namespace Microsoft.VisualStudio.LanguageServices.Implementation.ChangeSignature;
1614

17-
internal class AddParameterDialogViewModel : AbstractNotifyPropertyChanged
15+
internal sealed class AddParameterDialogViewModel : AbstractNotifyPropertyChanged
1816
{
1917
private readonly INotificationService? _notificationService;
2018

21-
public readonly Document Document;
19+
public readonly SemanticDocument Document;
2220
public readonly int PositionForTypeBinding;
2321

24-
private readonly SemanticModel _semanticModel;
25-
26-
public AddParameterDialogViewModel(Document document, int positionForTypeBinding)
22+
public AddParameterDialogViewModel(
23+
SemanticDocument document, int positionForTypeBinding)
2724
{
2825
_notificationService = document.Project.Solution.Services.GetService<INotificationService>();
29-
_semanticModel = document.GetRequiredSemanticModelAsync(CancellationToken.None).AsTask().WaitAndGetResult_CanCallOnBackground(CancellationToken.None);
3026

3127
TypeIsEmptyImage = Visibility.Visible;
3228
TypeBindsImage = Visibility.Collapsed;
@@ -200,7 +196,7 @@ private void SetCurrentTypeTextAndUpdateBindingStatus(string typeName)
200196
TypeIsEmptyImage = Visibility.Collapsed;
201197

202198
var languageService = Document.GetRequiredLanguageService<IChangeSignatureViewModelFactoryService>();
203-
TypeSymbol = _semanticModel.GetSpeculativeTypeInfo(PositionForTypeBinding, languageService.GetTypeNode(typeName), SpeculativeBindingOption.BindAsTypeOrNamespace).Type;
199+
TypeSymbol = Document.SemanticModel.GetSpeculativeTypeInfo(PositionForTypeBinding, languageService.GetTypeNode(typeName), SpeculativeBindingOption.BindAsTypeOrNamespace).Type;
204200

205201
if (TypeSymbol is { SpecialType: SpecialType.System_Void })
206202
{

‎src/VisualStudio/Core/Def/ChangeSignature/ChangeSignatureDialogViewModel.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,19 +43,19 @@ internal partial class ChangeSignatureDialogViewModel : AbstractNotifyPropertyCh
4343
/// <summary>
4444
/// The document where the symbol we are changing signature is defined.
4545
/// </summary>
46-
private readonly Document _document;
46+
private readonly SemanticDocument _document;
4747
private readonly int _positionForTypeBinding;
4848

4949
internal ChangeSignatureDialogViewModel(
50+
SemanticDocument document,
5051
ParameterConfiguration parameters,
5152
ISymbol symbol,
52-
Document document,
5353
int positionForTypeBinding,
5454
IClassificationFormatMap classificationFormatMap,
5555
ClassificationTypeMap classificationTypeMap)
5656
{
57-
_originalParameterConfiguration = parameters;
5857
_document = document;
58+
_originalParameterConfiguration = parameters;
5959
_positionForTypeBinding = positionForTypeBinding;
6060
_classificationFormatMap = classificationFormatMap;
6161
_classificationTypeMap = classificationTypeMap;

‎src/VisualStudio/Core/Def/ChangeSignature/VisualStudioChangeSignatureOptionsService.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,17 @@ internal sealed class VisualStudioChangeSignatureOptionsService(
2626
private readonly IThreadingContext _threadingContext = threadingContext;
2727

2828
public ChangeSignatureOptionsResult? GetChangeSignatureOptions(
29-
Document document,
29+
SemanticDocument document,
3030
int positionForTypeBinding,
3131
ISymbol symbol,
3232
ParameterConfiguration parameters)
3333
{
3434
_threadingContext.ThrowIfNotOnUIThread();
3535

3636
var viewModel = new ChangeSignatureDialogViewModel(
37+
document,
3738
parameters,
3839
symbol,
39-
document,
4040
positionForTypeBinding,
4141
_classificationFormatMap,
4242
_classificationTypeMap);

‎src/VisualStudio/Core/Test/ChangeSignature/AddParameterViewModelTests.vb‎

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
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+
Imports System.Threading
56
Imports System.Windows
67
Imports Microsoft.CodeAnalysis
78
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Extensions
8-
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
99
Imports Microsoft.CodeAnalysis.Test.Utilities
1010
Imports Microsoft.VisualStudio.LanguageServices.Implementation.ChangeSignature
1111
Imports Roslyn.Test.Utilities
@@ -15,14 +15,14 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ChangeSignature
1515
Public Class AddParameterViewModelTests
1616

1717
<WpfFact>
18-
Public Sub AddParameter_SubmittingRequiresTypeAndNameAndCallsiteValue()
18+
Public Async Function AddParameter_SubmittingRequiresTypeAndNameAndCallsiteValue() As Task
1919
Dim markup = <Text><![CDATA[
2020
class MyClass
2121
{
2222
public void M($$) { }
2323
}"]]></Text>
2424

25-
Dim viewModelTestState = GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
25+
Dim viewModelTestState = Await GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
2626
Dim viewModel = viewModelTestState.ViewModel
2727

2828
VerifyOpeningState(viewModel)
@@ -43,17 +43,17 @@ class MyClass
4343

4444
viewModel.CallSiteValue = "7"
4545
Assert.True(viewModel.TrySubmit())
46-
End Sub
46+
End Function
4747

4848
<WpfFact>
49-
Public Sub AddParameter_TypeNameTextBoxInteractions()
49+
Public Async Function AddParameter_TypeNameTextBoxInteractions() As Task
5050
Dim markup = <Text><![CDATA[
5151
class MyClass<T>
5252
{
5353
public void M($$) { }
5454
}"]]></Text>
5555

56-
Dim viewModelTestState = GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
56+
Dim viewModelTestState = Await GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
5757
Dim viewModel = viewModelTestState.ViewModel
5858

5959
VerifyOpeningState(viewModel)
@@ -127,13 +127,13 @@ class MyClass<T>
127127
monitor.Detach()
128128

129129
AssertTypeBindingIconAndTextIs(viewModel, NameOf(viewModel.TypeIsEmptyImage), ServicesVSResources.Please_enter_a_type_name)
130-
End Sub
130+
End Function
131131

132132
<WpfTheory>
133133
<InlineData("int")>
134134
<InlineData("MyClass")>
135135
<InlineData("NS1.NS2.DifferentClass")>
136-
Public Sub AddParameter_NoExistingParameters_TypeBinds(typeName As String)
136+
Public Async Function AddParameter_NoExistingParameters_TypeBinds(typeName As String) As Task
137137
Dim markup = <Text><![CDATA[
138138
namespace NS1
139139
{
@@ -151,7 +151,7 @@ class MyClass
151151
}
152152
}"]]></Text>
153153

154-
Dim viewModelTestState = GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
154+
Dim viewModelTestState = Await GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
155155
Dim viewModel = viewModelTestState.ViewModel
156156

157157
VerifyOpeningState(viewModel)
@@ -174,17 +174,17 @@ class MyClass
174174
viewModel.CallSiteValue = "0"
175175

176176
Assert.True(viewModel.TrySubmit())
177-
End Sub
177+
End Function
178178

179179
<WpfFact>
180-
Public Sub AddParameter_CannotBeBothRequiredAndOmit()
180+
Public Async Function AddParameter_CannotBeBothRequiredAndOmit() As Task
181181
Dim markup = <Text><![CDATA[
182182
class MyClass<T>
183183
{
184184
public void M($$) { }
185185
}"]]></Text>
186186

187-
Dim viewModelTestState = GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
187+
Dim viewModelTestState = Await GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
188188
Dim viewModel = viewModelTestState.ViewModel
189189

190190
VerifyOpeningState(viewModel)
@@ -204,13 +204,13 @@ class MyClass<T>
204204

205205
Assert.True(viewModel.IsCallsiteRegularValue)
206206
Assert.False(viewModel.IsCallsiteOmitted)
207-
End Sub
207+
End Function
208208

209209
<WpfTheory>
210210
<InlineData("int")>
211211
<InlineData("MyClass")>
212212
<InlineData("NS1.NS2.DifferentClass")>
213-
Public Sub AddParameter_ExistingParameters_TypeBinds(typeName As String)
213+
Public Async Function AddParameter_ExistingParameters_TypeBinds(typeName As String) As Task
214214
Dim markup = <Text><![CDATA[
215215
namespace NS1
216216
{
@@ -228,7 +228,7 @@ class MyClass
228228
}
229229
}"]]></Text>
230230

231-
Dim viewModelTestState = GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
231+
Dim viewModelTestState = Await GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
232232
Dim viewModel = viewModelTestState.ViewModel
233233

234234
VerifyOpeningState(viewModel)
@@ -251,7 +251,7 @@ class MyClass
251251
viewModel.CallSiteValue = "0"
252252

253253
Assert.True(viewModel.TrySubmit())
254-
End Sub
254+
End Function
255255

256256
Private Shared Sub AssertTypeBindingIconAndTextIs(viewModel As AddParameterDialogViewModel, currentIcon As String, expectedMessage As String)
257257
Assert.True(viewModel.TypeIsEmptyImage = If(NameOf(viewModel.TypeIsEmptyImage) = currentIcon, Visibility.Visible, Visibility.Collapsed))
@@ -285,9 +285,9 @@ class MyClass
285285
Assert.Equal(ServicesVSResources.A_type_and_name_must_be_provided, message)
286286
End Sub
287287

288-
Private Shared Function GetViewModelTestStateAsync(
288+
Private Shared Async Function GetViewModelTestStateAsync(
289289
markup As XElement,
290-
languageName As String) As AddParameterViewModelTestState
290+
languageName As String) As Task(Of AddParameterViewModelTestState)
291291

292292
Dim workspaceXml =
293293
<Workspace>
@@ -303,20 +303,21 @@ class MyClass
303303
Assert.True(False, "Missing caret location in document.")
304304
End If
305305

306-
Dim viewModel = New AddParameterDialogViewModel(workspaceDoc, doc.CursorPosition.Value)
306+
Dim document = Await SemanticDocument.CreateAsync(workspaceDoc, CancellationToken.None)
307+
Dim viewModel = New AddParameterDialogViewModel(document, doc.CursorPosition.Value)
307308
Return New AddParameterViewModelTestState(viewModel)
308309
End Using
309310
End Function
310311

311312
<WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/44958")>
312-
Public Sub AddParameter_SubmittingTypeWithModifiersIsInvalid()
313+
Public Async Function AddParameter_SubmittingTypeWithModifiersIsInvalid() As Task
313314
Dim markup = <Text><![CDATA[
314315
class MyClass
315316
{
316317
public void M($$) { }
317318
}"]]></Text>
318319

319-
Dim viewModelTestState = GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
320+
Dim viewModelTestState = Await GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
320321
Dim viewModel = viewModelTestState.ViewModel
321322

322323
VerifyOpeningState(viewModel)
@@ -346,17 +347,17 @@ class MyClass
346347
viewModel.VerbatimTypeName = "params int[]"
347348
Assert.False(viewModel.CanSubmit(message))
348349
Assert.Equal(ServicesVSResources.Parameter_type_contains_invalid_characters, message)
349-
End Sub
350+
End Function
350351

351352
<WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/44959")>
352-
Public Sub AddParameter_CannotSubmitVoidParameterType()
353+
Public Async Function AddParameter_CannotSubmitVoidParameterType() As Task
353354
Dim markup = <Text><![CDATA[
354355
class MyClass
355356
{
356357
public void M($$) { }
357358
}"]]></Text>
358359

359-
Dim viewModelTestState = GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
360+
Dim viewModelTestState = Await GetViewModelTestStateAsync(markup, LanguageNames.CSharp)
360361
Dim viewModel = viewModelTestState.ViewModel
361362

362363
VerifyOpeningState(viewModel)
@@ -371,6 +372,6 @@ class MyClass
371372
viewModel.VerbatimTypeName = "System.Void"
372373
Assert.False(viewModel.CanSubmit(message))
373374
Assert.Equal(ServicesVSResources.SystemVoid_is_not_a_valid_type_for_a_parameter, message)
374-
End Sub
375+
End Function
375376
End Class
376377
End Namespace

‎src/VisualStudio/Core/Test/ChangeSignature/ChangeSignatureViewModelTests.vb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,10 +467,11 @@ class Goo
467467
Dim token = Await tree.GetTouchingWordAsync(doc.CursorPosition.Value, workspaceDoc.Project.Services.GetService(Of ISyntaxFactsService)(), CancellationToken.None)
468468
Dim symbol = (Await workspaceDoc.GetSemanticModelAsync()).GetDeclaredSymbol(token.Parent)
469469

470+
Dim document = Await SemanticDocument.CreateAsync(workspaceDoc, CancellationToken.None)
470471
Dim viewModel = New ChangeSignatureDialogViewModel(
472+
document,
471473
ParameterConfiguration.Create(symbol.GetParameters().Select(Function(p) DirectCast(New ExistingParameter(p), Parameter)).ToImmutableArray(), symbol.IsExtensionMethod(), selectedIndex:=0),
472474
symbol,
473-
workspaceDoc,
474475
positionForTypeBinding:=0,
475476
workspace.ExportProvider.GetExportedValue(Of IClassificationFormatMapService)().GetClassificationFormatMap("text"),
476477
workspace.ExportProvider.GetExportedValue(Of ClassificationTypeMap)())

0 commit comments

Comments
 (0)