Skip to content

Commit 42f217f

Browse files
CyrusNajmabadiCyrusNajmabadi
authored
Remove blocking sync calls in rename (#76521)
2 parents bddccde + b0c4f83 commit 42f217f

File tree

6 files changed

+45
-80
lines changed

6 files changed

+45
-80
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: 11 additions & 10 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,15 @@ 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(
287-
Document document, SourceText text, TextSpan userSpan,
287+
Document document,
288+
SourceText text,
289+
TextSpan userSpan,
288290
IEnumerable<IRefactorNotifyService> refactorNotifyServices,
289-
ITextUndoHistoryRegistry undoHistoryRegistry,
290-
CancellationToken cancellationToken)
291+
ITextUndoHistoryRegistry undoHistoryRegistry)
291292
{
292293
try
293294
{
@@ -299,7 +300,7 @@ public bool CanInvokeRename(
299300
// engine will know that the document changed and not display the lightbulb anyway.
300301

301302
if (Buffer.AsTextContainer().CurrentText == text &&
302-
CanInvokeRename(out var trackingSession, waitForResult: true, cancellationToken: cancellationToken))
303+
CanInvokeRename(out var trackingSession))
303304
{
304305
var snapshotSpan = trackingSession.TrackingSpan.GetSpan(Buffer.CurrentSnapshot);
305306

@@ -318,7 +319,7 @@ public bool CanInvokeRename(
318319

319320
return default;
320321
}
321-
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, cancellationToken))
322+
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e))
322323
{
323324
throw ExceptionUtilities.Unreachable();
324325
}

‎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: 17 additions & 35 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,63 +104,44 @@ 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
{
112112
if (document != null && document.TryGetText(out var text))
113113
{
114114
var textBuffer = text.Container.TryGetTextBuffer();
115115
if (textBuffer != null &&
116-
textBuffer.Properties.TryGetProperty(typeof(StateMachine), out StateMachine stateMachine) &&
117-
stateMachine.CanInvokeRename(out _, cancellationToken: cancellationToken))
116+
textBuffer.Properties.TryGetProperty(typeof(StateMachine), out StateMachine stateMachine))
118117
{
119118
return stateMachine.TryGetCodeAction(
120-
document, text, textSpan, refactorNotifyServices, undoHistoryRegistry, cancellationToken);
119+
document, text, textSpan, refactorNotifyServices, undoHistoryRegistry);
121120
}
122121
}
123122

124123
return default;
125124
}
126-
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, cancellationToken, ErrorSeverity.General))
125+
catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, ErrorSeverity.General))
127126
{
128127
throw ExceptionUtilities.Unreachable();
129128
}
130129
}
131130

132-
internal static bool IsRenamableIdentifier(Task<TriggerIdentifierKind> isRenamableIdentifierTask, bool waitForResult, CancellationToken cancellationToken)
131+
internal static bool IsRenamableIdentifierFastCheck(
132+
Task<TriggerIdentifierKind> isRenamableIdentifierTask, out TriggerIdentifierKind identifierKind)
133133
{
134-
if (isRenamableIdentifierTask.Status == TaskStatus.RanToCompletion && isRenamableIdentifierTask.Result != TriggerIdentifierKind.NotRenamable)
134+
if (isRenamableIdentifierTask.Status == TaskStatus.RanToCompletion)
135135
{
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;
136+
var kind = isRenamableIdentifierTask.Result;
137+
if (kind != TriggerIdentifierKind.NotRenamable)
138+
{
139+
identifierKind = kind;
140+
return true;
141+
}
149142
}
150-
}
151143

152-
internal static bool WaitForIsRenamableIdentifier(Task<TriggerIdentifierKind> isRenamableIdentifierTask, CancellationToken cancellationToken)
153-
{
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-
}
144+
identifierKind = default;
145+
return false;
164146
}
165147
}

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

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -874,32 +874,21 @@ End Enum
874874
[WpfFact, WorkItem("http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/1028072")]
875875
public void 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 _));
890888

891889
source = new TaskCompletionSource<RenameTrackingTaggerProvider.TriggerIdentifierKind>();
892890
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));
896-
897-
source = new TaskCompletionSource<RenameTrackingTaggerProvider.TriggerIdentifierKind>();
898-
Assert.Throws<OperationCanceledException>(() => RenameTrackingTaggerProvider.WaitForIsRenamableIdentifier(source.Task, new CancellationToken(canceled: true)));
899-
var thrownException = new Exception();
900-
source.TrySetException(thrownException);
901-
var caughtException = Assert.Throws<Exception>(() => RenameTrackingTaggerProvider.WaitForIsRenamableIdentifier(source.Task, CancellationToken.None));
902-
Assert.Same(thrownException, caughtException);
891+
Assert.False(RenameTrackingTaggerProvider.IsRenamableIdentifierFastCheck(source.Task, out _));
903892
}
904893

905894
[WpfFact]

0 commit comments

Comments
 (0)