Skip to content

Commit e158050

Browse files
CyrusNajmabadiCyrusNajmabadi
committed
Remove blocking sync calls in rename
1 parent 395d298 commit e158050

File tree

6 files changed

+67
-72
lines changed

6 files changed

+67
-72
lines changed

‎src/EditorFeatures/Core/RenameTracking/RenameTrackingCodeRefactoringProvider.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@ public RenameTrackingCodeRefactoringProvider(
3434

3535
public override Task ComputeRefactoringsAsync(CodeRefactoringContext context)
3636
{
37-
var (document, span, cancellationToken) = context;
37+
var (document, span, _) = context;
3838

3939
var (action, renameSpan) = RenameTrackingTaggerProvider.TryGetCodeAction(
40-
document, span, _refactorNotifyServices, _undoHistoryRegistry, cancellationToken);
40+
document, span, _refactorNotifyServices, _undoHistoryRegistry);
4141

4242
if (action != null)
4343
context.RegisterRefactoring(action, renameSpan);

‎src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,8 @@ protected override Task<ImmutableArray<CodeActionOperation>> ComputeOperationsAs
5959
IProgress<CodeAnalysisProgress> progress, CancellationToken cancellationToken)
6060
{
6161
// Invoked directly without previewing.
62-
if (_renameTrackingCommitter == null)
63-
{
64-
if (!TryInitializeRenameTrackingCommitter(cancellationToken))
65-
{
66-
return SpecializedTasks.EmptyImmutableArray<CodeActionOperation>();
67-
}
68-
}
62+
if (_renameTrackingCommitter == null && !TryInitializeRenameTrackingCommitter())
63+
return SpecializedTasks.EmptyImmutableArray<CodeActionOperation>();
6964

7065
var committerOperation = new RenameTrackingCommitterOperation(_renameTrackingCommitter, _threadingContext);
7166
return Task.FromResult(ImmutableArray.Create<CodeActionOperation>(committerOperation));
@@ -74,7 +69,7 @@ protected override Task<ImmutableArray<CodeActionOperation>> ComputeOperationsAs
7469
protected override async Task<IEnumerable<CodeActionOperation>> ComputePreviewOperationsAsync(CancellationToken cancellationToken)
7570
{
7671
if (!_globalOptions.GetOption(RenameTrackingOptionsStorage.RenameTrackingPreview, _document.Project.Language) ||
77-
!TryInitializeRenameTrackingCommitter(cancellationToken))
72+
!TryInitializeRenameTrackingCommitter())
7873
{
7974
return await SpecializedTasks.EmptyEnumerable<CodeActionOperation>().ConfigureAwait(false);
8075
}
@@ -84,14 +79,14 @@ protected override async Task<IEnumerable<CodeActionOperation>> ComputePreviewOp
8479
return [new ApplyChangesOperation(solutionSet.RenamedSolution)];
8580
}
8681

87-
private bool TryInitializeRenameTrackingCommitter(CancellationToken cancellationToken)
82+
private bool TryInitializeRenameTrackingCommitter()
8883
{
8984
if (_document.TryGetText(out var text))
9085
{
9186
var textBuffer = text.Container.GetTextBuffer();
9287
if (textBuffer.Properties.TryGetProperty(typeof(StateMachine), out StateMachine stateMachine))
9388
{
94-
if (!stateMachine.CanInvokeRename(out _, cancellationToken: cancellationToken))
89+
if (!stateMachine.CanInvokeRename(out _))
9590
{
9691
// The rename tracking could be dismissed while a codefix is still cached
9792
// in the lightbulb. If this happens, do not perform the rename requested

‎src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.StateMachine.cs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ private void Buffer_Changed(object sender, TextContentChangedEventArgs e)
129129
public void UpdateTrackingSessionIfRenamable()
130130
{
131131
ThreadingContext.ThrowIfNotOnUIThread();
132-
if (this.TrackingSession.IsDefinitelyRenamableIdentifier())
132+
if (this.TrackingSession.IsDefinitelyRenamableIdentifierFastCheck())
133133
{
134134
this.TrackingSession.CheckNewIdentifier(this, Buffer.CurrentSnapshot);
135135
TrackingSessionUpdated();
@@ -214,7 +214,7 @@ public bool ClearTrackingSession()
214214
previousTrackingSession.Cancel();
215215

216216
// If there may have been a tag showing, then actually clear the tags.
217-
if (previousTrackingSession.IsDefinitelyRenamableIdentifier())
217+
if (previousTrackingSession.IsDefinitelyRenamableIdentifierFastCheck())
218218
{
219219
TrackingSessionCleared(previousTrackingSession.TrackingSpan);
220220
}
@@ -229,7 +229,7 @@ public bool ClearVisibleTrackingSession()
229229
{
230230
ThreadingContext.ThrowIfNotOnUIThread();
231231

232-
if (this.TrackingSession != null && this.TrackingSession.IsDefinitelyRenamableIdentifier())
232+
if (this.TrackingSession != null && this.TrackingSession.IsDefinitelyRenamableIdentifierFastCheck())
233233
{
234234
var document = Buffer.CurrentSnapshot.GetOpenDocumentInCurrentContextWithChanges();
235235
if (document != null)
@@ -271,7 +271,7 @@ internal int StoreCurrentTrackingSessionAndGenerateId()
271271

272272
public bool CanInvokeRename(
273273
[NotNullWhen(true)] out TrackingSession trackingSession,
274-
bool isSmartTagCheck = false, bool waitForResult = false, CancellationToken cancellationToken = default)
274+
bool isSmartTagCheck = false)
275275
{
276276
// This needs to be able to run on a background thread for the diagnostic.
277277

@@ -280,14 +280,13 @@ public bool CanInvokeRename(
280280
return false;
281281

282282
return TryGetSyntaxFactsService(out var syntaxFactsService) && TryGetLanguageHeuristicsService(out var languageHeuristicsService) &&
283-
trackingSession.CanInvokeRename(syntaxFactsService, languageHeuristicsService, isSmartTagCheck, waitForResult, cancellationToken);
283+
trackingSession.CanInvokeRename(syntaxFactsService, languageHeuristicsService, isSmartTagCheck);
284284
}
285285

286286
internal (CodeAction action, TextSpan renameSpan) TryGetCodeAction(
287287
Document document, SourceText text, TextSpan userSpan,
288288
IEnumerable<IRefactorNotifyService> refactorNotifyServices,
289-
ITextUndoHistoryRegistry undoHistoryRegistry,
290-
CancellationToken cancellationToken)
289+
ITextUndoHistoryRegistry undoHistoryRegistry)
291290
{
292291
try
293292
{
@@ -299,7 +298,7 @@ public bool CanInvokeRename(
299298
// engine will know that the document changed and not display the lightbulb anyway.
300299

301300
if (Buffer.AsTextContainer().CurrentText == text &&
302-
CanInvokeRename(out var trackingSession, waitForResult: true, cancellationToken: cancellationToken))
301+
CanInvokeRename(out var trackingSession))
303302
{
304303
var snapshotSpan = trackingSession.TrackingSpan.GetSpan(Buffer.CurrentSnapshot);
305304

@@ -318,7 +317,7 @@ public bool CanInvokeRename(
318317

319318
return default;
320319
}
321-
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, cancellationToken))
320+
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
322321
{
323322
throw ExceptionUtilities.Unreachable();
324323
}

‎src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.TrackingSession.cs‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,10 @@ async Task<bool> DetermineIfNewIdentifierBindsAsync(Task<TriggerIdentifierKind>
145145
}
146146
}
147147

148-
internal bool IsDefinitelyRenamableIdentifier()
148+
internal bool IsDefinitelyRenamableIdentifierFastCheck()
149149
{
150150
// This needs to be able to run on a background thread for the CodeFix
151-
return IsRenamableIdentifier(_isRenamableIdentifierTask, waitForResult: false, cancellationToken: CancellationToken.None);
151+
return IsRenamableIdentifierFastCheck(_isRenamableIdentifierTask, out _);
152152
}
153153

154154
public void Cancel()
@@ -259,13 +259,11 @@ private TriggerIdentifierKind DetermineIfRenamableSymbol(ISymbol symbol, Documen
259259
internal bool CanInvokeRename(
260260
ISyntaxFactsService syntaxFactsService,
261261
IRenameTrackingLanguageHeuristicsService languageHeuristicsService,
262-
bool isSmartTagCheck,
263-
bool waitForResult,
264-
CancellationToken cancellationToken)
262+
bool isSmartTagCheck)
265263
{
266-
if (IsRenamableIdentifier(_isRenamableIdentifierTask, waitForResult, cancellationToken))
264+
if (IsRenamableIdentifierFastCheck(_isRenamableIdentifierTask, out var triggerIdentifierKind))
267265
{
268-
var isRenamingDeclaration = _isRenamableIdentifierTask.Result == TriggerIdentifierKind.RenamableDeclaration;
266+
var isRenamingDeclaration = triggerIdentifierKind == TriggerIdentifierKind.RenamableDeclaration;
269267
var newName = TrackingSpan.GetText(TrackingSpan.TextBuffer.CurrentSnapshot);
270268
var comparison = isRenamingDeclaration || syntaxFactsService.IsCaseSensitive ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase;
271269

‎src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.cs‎

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
using Microsoft.CodeAnalysis.ErrorReporting;
1717
using Microsoft.CodeAnalysis.Options;
1818
using Microsoft.CodeAnalysis.Shared.TestHooks;
19+
using Microsoft.CodeAnalysis.SQLite.Interop;
1920
using Microsoft.CodeAnalysis.Text;
2021
using Microsoft.VisualStudio.Text;
2122
using Microsoft.VisualStudio.Text.Editor;
@@ -103,9 +104,8 @@ internal static bool ResetRenameTrackingStateWorker(Workspace workspace, Documen
103104

104105
public static (CodeAction action, TextSpan renameSpan) TryGetCodeAction(
105106
Document document, TextSpan textSpan,
106-
IEnumerable<IRefactorNotifyService> refactorNotifyServices,
107-
ITextUndoHistoryRegistry undoHistoryRegistry,
108-
CancellationToken cancellationToken)
107+
IEnumerable<IRefactorNotifyService> refactorNotifyServices,
108+
ITextUndoHistoryRegistry undoHistoryRegistry)
109109
{
110110
try
111111
{
@@ -114,52 +114,55 @@ public static (CodeAction action, TextSpan renameSpan) TryGetCodeAction(
114114
var textBuffer = text.Container.TryGetTextBuffer();
115115
if (textBuffer != null &&
116116
textBuffer.Properties.TryGetProperty(typeof(StateMachine), out StateMachine stateMachine) &&
117-
stateMachine.CanInvokeRename(out _, cancellationToken: cancellationToken))
117+
stateMachine.CanInvokeRename(out _))
118118
{
119119
return stateMachine.TryGetCodeAction(
120-
document, text, textSpan, refactorNotifyServices, undoHistoryRegistry, cancellationToken);
120+
document, text, textSpan, refactorNotifyServices, undoHistoryRegistry);
121121
}
122122
}
123123

124124
return default;
125125
}
126-
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, cancellationToken, ErrorSeverity.General))
126+
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, ErrorSeverity.General))
127127
{
128128
throw ExceptionUtilities.Unreachable();
129129
}
130130
}
131131

132-
internal static bool IsRenamableIdentifier(Task<TriggerIdentifierKind> isRenamableIdentifierTask, bool waitForResult, CancellationToken cancellationToken)
132+
internal static bool IsRenamableIdentifierFastCheck(
133+
Task<TriggerIdentifierKind> isRenamableIdentifierTask, out TriggerIdentifierKind identifierKind)
133134
{
134-
if (isRenamableIdentifierTask.Status == TaskStatus.RanToCompletion && isRenamableIdentifierTask.Result != TriggerIdentifierKind.NotRenamable)
135+
if (isRenamableIdentifierTask.Status == TaskStatus.RanToCompletion)
135136
{
136-
return true;
137-
}
138-
else if (isRenamableIdentifierTask.Status == TaskStatus.Canceled)
139-
{
140-
return false;
141-
}
142-
else if (waitForResult)
143-
{
144-
return WaitForIsRenamableIdentifier(isRenamableIdentifierTask, cancellationToken);
145-
}
146-
else
147-
{
148-
return false;
137+
var kind = isRenamableIdentifierTask.Result;
138+
if (kind != TriggerIdentifierKind.NotRenamable)
139+
{
140+
identifierKind = kind;
141+
return true;
142+
}
149143
}
144+
145+
identifierKind = default;
146+
return false;
150147
}
151148

152-
internal static bool WaitForIsRenamableIdentifier(Task<TriggerIdentifierKind> isRenamableIdentifierTask, CancellationToken cancellationToken)
149+
internal static async Task<bool> IsRenamableIdentifier_ForTestingPurposesOnlyAsync(Task<TriggerIdentifierKind> isRenamableIdentifierTask)
153150
{
154-
try
155-
{
156-
return isRenamableIdentifierTask.WaitAndGetResult_CanCallOnBackground(cancellationToken) != TriggerIdentifierKind.NotRenamable;
157-
}
158-
catch (OperationCanceledException e) when (e.CancellationToken != cancellationToken || cancellationToken == CancellationToken.None)
159-
{
160-
// We passed in a different cancellationToken, so if there's a race and
161-
// isRenamableIdentifierTask was cancelled, we'll get a OperationCanceledException
162-
return false;
163-
}
151+
await isRenamableIdentifierTask.ConfigureAwait(false);
152+
return IsRenamableIdentifierFastCheck(isRenamableIdentifierTask, out _);
164153
}
154+
155+
//internal static bool WaitForIsRenamableIdentifier_ForTestingPurposesOnly(Task<TriggerIdentifierKind> isRenamableIdentifierTask, CancellationToken cancellationToken)
156+
//{
157+
// try
158+
// {
159+
// return isRenamableIdentifierTask.WaitAndGetResult_CanCallOnBackground(cancellationToken) != TriggerIdentifierKind.NotRenamable;
160+
// }
161+
// catch (OperationCanceledException e) when (e.CancellationToken != cancellationToken || cancellationToken == CancellationToken.None)
162+
// {
163+
// // We passed in a different cancellationToken, so if there's a race and
164+
// // isRenamableIdentifierTask was cancelled, we'll get a OperationCanceledException
165+
// return false;
166+
// }
167+
//}
165168
}

‎src/EditorFeatures/Test/RenameTracking/RenameTrackingTaggerProviderTests.cs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -872,33 +872,33 @@ End Enum
872872
}
873873

874874
[WpfFact, WorkItem("http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/1028072")]
875-
public void RenameTrackingDoesNotThrowAggregateException()
875+
public async Task RenameTrackingDoesNotThrowAggregateException()
876876
{
877-
var waitForResult = false;
878877
var notRenamable = Task.FromResult(RenameTrackingTaggerProvider.TriggerIdentifierKind.NotRenamable);
879-
Assert.False(RenameTrackingTaggerProvider.IsRenamableIdentifier(notRenamable, waitForResult, CancellationToken.None));
878+
Assert.False(RenameTrackingTaggerProvider.IsRenamableIdentifierFastCheck(notRenamable, out _));
880879

881880
var source = new TaskCompletionSource<RenameTrackingTaggerProvider.TriggerIdentifierKind>();
882-
Assert.False(RenameTrackingTaggerProvider.IsRenamableIdentifier(source.Task, waitForResult, CancellationToken.None));
881+
Assert.False(RenameTrackingTaggerProvider.IsRenamableIdentifierFastCheck(source.Task, out _));
883882
source.TrySetResult(RenameTrackingTaggerProvider.TriggerIdentifierKind.RenamableReference);
884-
Assert.True(RenameTrackingTaggerProvider.IsRenamableIdentifier(source.Task, waitForResult, CancellationToken.None));
883+
Assert.True(RenameTrackingTaggerProvider.IsRenamableIdentifierFastCheck(source.Task, out _));
885884

886885
source = new TaskCompletionSource<RenameTrackingTaggerProvider.TriggerIdentifierKind>();
887886
source.TrySetCanceled();
888-
Assert.False(RenameTrackingTaggerProvider.IsRenamableIdentifier(source.Task, waitForResult, CancellationToken.None));
889-
Assert.False(RenameTrackingTaggerProvider.WaitForIsRenamableIdentifier(source.Task, CancellationToken.None));
887+
Assert.False(RenameTrackingTaggerProvider.IsRenamableIdentifierFastCheck(source.Task, out _));
888+
Assert.False(await RenameTrackingTaggerProvider.IsRenamableIdentifier_ForTestingPurposesOnlyAsync(source.Task));
890889

891890
source = new TaskCompletionSource<RenameTrackingTaggerProvider.TriggerIdentifierKind>();
892891
source.TrySetException(new OperationCanceledException());
893-
Assert.False(RenameTrackingTaggerProvider.IsRenamableIdentifier(source.Task, waitForResult, CancellationToken.None));
894-
Assert.False(RenameTrackingTaggerProvider.WaitForIsRenamableIdentifier(source.Task, CancellationToken.None));
895-
Assert.False(RenameTrackingTaggerProvider.WaitForIsRenamableIdentifier(source.Task, new CancellationTokenSource().Token));
892+
Assert.False(RenameTrackingTaggerProvider.IsRenamableIdentifierFastCheck(source.Task, out _));
893+
Assert.False(await RenameTrackingTaggerProvider.IsRenamableIdentifier_ForTestingPurposesOnlyAsync(source.Task));
894+
Assert.False(await RenameTrackingTaggerProvider.IsRenamableIdentifier_ForTestingPurposesOnlyAsync(source.Task));
896895

897896
source = new TaskCompletionSource<RenameTrackingTaggerProvider.TriggerIdentifierKind>();
898-
Assert.Throws<OperationCanceledException>(() => RenameTrackingTaggerProvider.WaitForIsRenamableIdentifier(source.Task, new CancellationToken(canceled: true)));
897+
source.TrySetCanceled();
898+
await Assert.ThrowsAsync<OperationCanceledException>(() => RenameTrackingTaggerProvider.IsRenamableIdentifier_ForTestingPurposesOnlyAsync(source.Task));
899899
var thrownException = new Exception();
900900
source.TrySetException(thrownException);
901-
var caughtException = Assert.Throws<Exception>(() => RenameTrackingTaggerProvider.WaitForIsRenamableIdentifier(source.Task, CancellationToken.None));
901+
var caughtException = await Assert.ThrowsAsync<Exception>(() => RenameTrackingTaggerProvider.IsRenamableIdentifier_ForTestingPurposesOnlyAsync(source.Task));
902902
Assert.Same(thrownException, caughtException);
903903
}
904904

0 commit comments

Comments
 (0)