From 6962897bc6781d2f29338ae0fd61ad4718fa293d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 7 Mar 2020 13:12:29 -0800 Subject: [PATCH] Remove intermediate translation from state->diagnostic->codeaction in rename --- .../RenameTrackingCodeRefactoringProvider.cs | 41 +++------------- ...nameTrackingTaggerProvider.StateMachine.cs | 45 +++++++++--------- .../RenameTrackingTaggerProvider.cs | 47 +++++-------------- .../RenameTrackingTaggerProviderTests.cs | 28 +++++------ .../RenameTracking/RenameTrackingTestState.cs | 38 +++++++-------- 5 files changed, 73 insertions(+), 126 deletions(-) diff --git a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingCodeRefactoringProvider.cs b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingCodeRefactoringProvider.cs index 31f8827a547..20a44d8fe0d 100644 --- a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingCodeRefactoringProvider.cs +++ b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingCodeRefactoringProvider.cs @@ -21,15 +21,6 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.RenameTracking Name = nameof(RenameTrackingCodeRefactoringProvider)), Shared] internal class RenameTrackingCodeRefactoringProvider : CodeRefactoringProvider { - public const string DiagnosticId = "RenameTracking"; - public static DiagnosticDescriptor DiagnosticDescriptor = new DiagnosticDescriptor( - DiagnosticId, title: "", messageFormat: "", category: "", - defaultSeverity: DiagnosticSeverity.Hidden, isEnabledByDefault: true, - customTags: DiagnosticCustomTags.Microsoft.Append(WellKnownDiagnosticTags.NotConfigurable)); - - public const string RenameFromPropertyKey = "RenameFrom"; - public const string RenameToPropertyKey = "RenameTo"; - private readonly ITextUndoHistoryRegistry _undoHistoryRegistry; private readonly IEnumerable _refactorNotifyServices; @@ -42,37 +33,17 @@ internal class RenameTrackingCodeRefactoringProvider : CodeRefactoringProvider _refactorNotifyServices = refactorNotifyServices; } - // Internal for testing purposes - internal async Task TryGetDiagnosticAsync(Document document, CancellationToken cancellationToken) - { - var syntaxTree = await document.GetRequiredSyntaxTreeAsync(cancellationToken).ConfigureAwait(false); - return RenameTrackingTaggerProvider.TryGetDiagnostic( - syntaxTree, DiagnosticDescriptor, cancellationToken); - } - - public override async Task ComputeRefactoringsAsync(CodeRefactoringContext context) + public override Task ComputeRefactoringsAsync(CodeRefactoringContext context) { var (document, span, cancellationToken) = context; - // Ensure rename can still be invoked in this document. We reanalyze the document for - // diagnostics when rename tracking is manually dismissed, but the existence of our - // diagnostic may still be cached, so we have to double check before actually providing - // any fixes. - if (!RenameTrackingTaggerProvider.CanInvokeRename(document)) - return; - - var diagnostic = await TryGetDiagnosticAsync(document, cancellationToken).ConfigureAwait(false); - if (diagnostic == null) - return; + var action = RenameTrackingTaggerProvider.TryGetCodeAction( + document, span, _refactorNotifyServices, _undoHistoryRegistry, cancellationToken); - // user needs to be on the same line as the diagnostic location. - var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); - if (!text.AreOnSameLine(span.Start, diagnostic.Location.SourceSpan.Start)) - return; + if (action != null) + context.RegisterRefactoring(action); - var action = RenameTrackingTaggerProvider.CreateCodeAction( - document, diagnostic, _refactorNotifyServices, _undoHistoryRegistry); - context.RegisterRefactoring(action); + return Task.CompletedTask; } } } diff --git a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.StateMachine.cs b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.StateMachine.cs index 618db439d3d..eb4b611c290 100644 --- a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.StateMachine.cs +++ b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.StateMachine.cs @@ -5,8 +5,10 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Threading; +using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editor.Shared.Extensions; using Microsoft.CodeAnalysis.Editor.Shared.Options; @@ -18,6 +20,7 @@ using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.Text; +using Microsoft.VisualStudio.Text.Operations; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Editor.Implementation.RenameTracking @@ -263,21 +266,25 @@ internal int StoreCurrentTrackingSessionAndGenerateId() return index; } - public bool CanInvokeRename(out TrackingSession trackingSession, bool isSmartTagCheck = false, bool waitForResult = false, CancellationToken cancellationToken = default) + public bool CanInvokeRename( + [NotNullWhen(true)] out TrackingSession trackingSession, + bool isSmartTagCheck = false, bool waitForResult = false, CancellationToken cancellationToken = default) { // This needs to be able to run on a background thread for the diagnostic. trackingSession = this.TrackingSession; if (trackingSession == null) - { return false; - } return TryGetSyntaxFactsService(out var syntaxFactsService) && TryGetLanguageHeuristicsService(out var languageHeuristicsService) && trackingSession.CanInvokeRename(syntaxFactsService, languageHeuristicsService, isSmartTagCheck, waitForResult, cancellationToken); } - internal Diagnostic TryGetDiagnostic(SyntaxTree tree, DiagnosticDescriptor diagnosticDescriptor, CancellationToken cancellationToken) + internal CodeAction TryGetCodeAction( + Document document, SourceText text, TextSpan userSpan, + IEnumerable refactorNotifyServices, + ITextUndoHistoryRegistry undoHistoryRegistry, + CancellationToken cancellationToken) { try { @@ -288,26 +295,22 @@ internal Diagnostic TryGetDiagnostic(SyntaxTree tree, DiagnosticDescriptor diagn // method. If it does, we may give an incorrect response, but the diagnostics // engine will know that the document changed and not display the lightbulb anyway. - if (Buffer.AsTextContainer().CurrentText != tree.GetText(cancellationToken)) - { - return null; - } - - if (CanInvokeRename(out var trackingSession, waitForResult: true, cancellationToken: cancellationToken)) + if (Buffer.AsTextContainer().CurrentText == text && + CanInvokeRename(out var trackingSession, waitForResult: true, cancellationToken: cancellationToken)) { var snapshotSpan = trackingSession.TrackingSpan.GetSpan(Buffer.CurrentSnapshot); - var textSpan = snapshotSpan.Span.ToTextSpan(); - - var builder = ImmutableDictionary.CreateBuilder(); - builder.Add(RenameTrackingCodeRefactoringProvider.RenameFromPropertyKey, trackingSession.OriginalName); - builder.Add(RenameTrackingCodeRefactoringProvider.RenameToPropertyKey, snapshotSpan.GetText()); - var properties = builder.ToImmutable(); - - var diagnostic = Diagnostic.Create(diagnosticDescriptor, - tree.GetLocation(textSpan), - properties); - return diagnostic; + // user needs to be on the same line as the diagnostic location. + if (text.AreOnSameLine(userSpan.Start, snapshotSpan.Start)) + { + var title = string.Format( + EditorFeaturesResources.Rename_0_to_1, + trackingSession.OriginalName, + snapshotSpan.GetText()); + + return new RenameTrackingCodeAction( + document, title, refactorNotifyServices, undoHistoryRegistry); + } } return null; diff --git a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.cs b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.cs index 6a8d4988425..67f674c55dd 100644 --- a/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.cs +++ b/src/EditorFeatures/Core/Implementation/RenameTracking/RenameTrackingTaggerProvider.cs @@ -112,18 +112,23 @@ internal static bool ResetRenameTrackingStateWorker(Workspace workspace, Documen return false; } - internal static Diagnostic TryGetDiagnostic(SyntaxTree tree, DiagnosticDescriptor diagnosticDescriptor, CancellationToken cancellationToken) + public static CodeAction TryGetCodeAction( + Document document, TextSpan textSpan, + IEnumerable refactorNotifyServices, + ITextUndoHistoryRegistry undoHistoryRegistry, + CancellationToken cancellationToken) { try { - // This can run on a background thread. - if (tree != null && - tree.TryGetText(out var text)) + if (document != null && document.TryGetText(out var text)) { var textBuffer = text.Container.TryGetTextBuffer(); - if (textBuffer != null && textBuffer.Properties.TryGetProperty(typeof(StateMachine), out StateMachine stateMachine)) + if (textBuffer != null && + textBuffer.Properties.TryGetProperty(typeof(StateMachine), out StateMachine stateMachine) && + stateMachine.CanInvokeRename(out _)) { - return stateMachine.TryGetDiagnostic(tree, diagnosticDescriptor, cancellationToken); + return stateMachine.TryGetCodeAction( + document, text, textSpan, refactorNotifyServices, undoHistoryRegistry, cancellationToken); } } @@ -135,22 +140,6 @@ internal static Diagnostic TryGetDiagnostic(SyntaxTree tree, DiagnosticDescripto } } - internal static CodeAction CreateCodeAction( - Document document, - Diagnostic diagnostic, - IEnumerable refactorNotifyServices, - ITextUndoHistoryRegistry undoHistoryRegistry) - { - // This can run on a background thread. - - var message = string.Format( - EditorFeaturesResources.Rename_0_to_1, - diagnostic.Properties[RenameTrackingCodeRefactoringProvider.RenameFromPropertyKey], - diagnostic.Properties[RenameTrackingCodeRefactoringProvider.RenameToPropertyKey]); - - return new RenameTrackingCodeAction(document, message, refactorNotifyServices, undoHistoryRegistry); - } - internal static bool IsRenamableIdentifier(Task isRenamableIdentifierTask, bool waitForResult, CancellationToken cancellationToken) { if (isRenamableIdentifierTask.Status == TaskStatus.RanToCompletion && isRenamableIdentifierTask.Result != TriggerIdentifierKind.NotRenamable) @@ -184,19 +173,5 @@ internal static bool WaitForIsRenamableIdentifier(Task is return false; } } - - internal static bool CanInvokeRename(Document document) - { - ITextBuffer textBuffer; - if (document == null || !document.TryGetText(out var text)) - { - return false; - } - - textBuffer = text.Container.TryGetTextBuffer(); - return textBuffer != null && - textBuffer.Properties.TryGetProperty(typeof(StateMachine), out StateMachine stateMachine) && - stateMachine.CanInvokeRename(out _); - } } } diff --git a/src/EditorFeatures/Test/RenameTracking/RenameTrackingTaggerProviderTests.cs b/src/EditorFeatures/Test/RenameTracking/RenameTrackingTaggerProviderTests.cs index 9b500b36c5d..7ac96940b0a 100644 --- a/src/EditorFeatures/Test/RenameTracking/RenameTrackingTaggerProviderTests.cs +++ b/src/EditorFeatures/Test/RenameTracking/RenameTrackingTaggerProviderTests.cs @@ -970,12 +970,12 @@ class C$$ state.EditorOperations.InsertText("at"); await state.AssertTag("C", "Cat"); - Assert.NotNull(await state.TryGetDocumentDiagnosticAsync()); + Assert.NotNull(await state.TryGetCodeActionAsync()); state.SendEscape(); await state.AssertNoTag(); - Assert.Null(await state.TryGetDocumentDiagnosticAsync()); + Assert.Null(await state.TryGetCodeActionAsync()); } [WpfFact] @@ -1139,15 +1139,15 @@ void M() state.EditorOperations.InsertText("va"); await state.AssertTag("C", "va"); - Assert.NotNull(await state.TryGetDocumentDiagnosticAsync()); + Assert.NotNull(await state.TryGetCodeActionAsync()); state.EditorOperations.InsertText("r"); await state.AssertNoTag(); - Assert.Null(await state.TryGetDocumentDiagnosticAsync()); + Assert.Null(await state.TryGetCodeActionAsync()); state.EditorOperations.InsertText("p"); await state.AssertTag("C", "varp"); - Assert.NotNull(await state.TryGetDocumentDiagnosticAsync()); + Assert.NotNull(await state.TryGetCodeActionAsync()); } [WpfFact] @@ -1166,7 +1166,7 @@ void M() using var state = RenameTrackingTestState.Create(code, LanguageNames.CSharp); state.EditorOperations.Backspace(); await state.AssertNoTag(); - Assert.Null(await state.TryGetDocumentDiagnosticAsync()); + Assert.Null(await state.TryGetCodeActionAsync()); } [WpfFact] @@ -1185,7 +1185,7 @@ End Sub state.EditorOperations.InsertText("var"); await state.AssertTag("C", "var"); - Assert.NotNull(await state.TryGetDocumentDiagnosticAsync()); + Assert.NotNull(await state.TryGetCodeActionAsync()); } [WpfFact] @@ -1206,15 +1206,15 @@ void M() state.EditorOperations.InsertText("dynami"); await state.AssertTag("C", "dynami"); - Assert.NotNull(await state.TryGetDocumentDiagnosticAsync()); + Assert.NotNull(await state.TryGetCodeActionAsync()); state.EditorOperations.InsertText("c"); await state.AssertNoTag(); - Assert.Null(await state.TryGetDocumentDiagnosticAsync()); + Assert.Null(await state.TryGetCodeActionAsync()); state.EditorOperations.InsertText("s"); await state.AssertTag("C", "dynamics"); - Assert.NotNull(await state.TryGetDocumentDiagnosticAsync()); + Assert.NotNull(await state.TryGetCodeActionAsync()); } [WpfFact] @@ -1234,7 +1234,7 @@ void M() state.EditorOperations.Backspace(); state.EditorOperations.Backspace(); await state.AssertNoTag(); - Assert.Null(await state.TryGetDocumentDiagnosticAsync()); + Assert.Null(await state.TryGetCodeActionAsync()); } [WpfFact] @@ -1253,7 +1253,7 @@ End Class state.EditorOperations.Backspace(); state.EditorOperations.Backspace(); await state.AssertNoTag(); - Assert.Null(await state.TryGetDocumentDiagnosticAsync()); + Assert.Null(await state.TryGetCodeActionAsync()); } [WpfFact] @@ -1274,7 +1274,7 @@ void M() state.EditorOperations.Backspace(); state.EditorOperations.Backspace(); await state.AssertNoTag(); - Assert.Null(await state.TryGetDocumentDiagnosticAsync()); + Assert.Null(await state.TryGetCodeActionAsync()); } [WpfFact] @@ -1293,7 +1293,7 @@ End Class state.EditorOperations.Backspace(); state.EditorOperations.Backspace(); await state.AssertNoTag(); - Assert.Null(await state.TryGetDocumentDiagnosticAsync()); + Assert.Null(await state.TryGetCodeActionAsync()); } [WpfFact] diff --git a/src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs b/src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs index 11c4b91906e..2acf2e33107 100644 --- a/src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs +++ b/src/EditorFeatures/Test/RenameTracking/RenameTrackingTestState.cs @@ -13,6 +13,7 @@ using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editor.CSharp.RenameTracking; using Microsoft.CodeAnalysis.Editor.Implementation.RenameTracking; +using Microsoft.CodeAnalysis.Editor.Shared.Extensions; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.CodeAnalysis.Editor.UnitTests.Utilities; using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; @@ -20,6 +21,7 @@ using Microsoft.CodeAnalysis.Notification; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.CodeAnalysis.Shared.Utilities; +using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.Text.Shared.Extensions; using Microsoft.CodeAnalysis.UnitTests.Diagnostics; using Microsoft.VisualStudio.Composition; @@ -169,10 +171,18 @@ public async Task AssertNoTag() Assert.Equal(0, tags.Count()); } - public Task TryGetDocumentDiagnosticAsync(Document document = null) + /// If the current caret position will be used. + public async Task TryGetCodeActionAsync(TextSpan? textSpan = null) { - document ??= this.Workspace.CurrentSolution.GetDocument(_hostDocument.Id); - return _codeRefactoringProvider.TryGetDiagnosticAsync(document, CancellationToken.None); + var span = textSpan ?? new TextSpan(_view.Caret.Position.BufferPosition, 0); + + var document = this.Workspace.CurrentSolution.GetDocument(_hostDocument.Id); + + var actions = new List(); + var context = new CodeRefactoringContext( + document, span, actions.Add, CancellationToken.None); + await _codeRefactoringProvider.ComputeRefactoringsAsync(context); + return actions.SingleOrDefault(); } public async Task AssertTag( @@ -188,26 +198,14 @@ public Task TryGetDocumentDiagnosticAsync(Document document = null) var tag = tags.Single(); - var document = this.Workspace.CurrentSolution.GetDocument(_hostDocument.Id); - var diagnostic = await TryGetDocumentDiagnosticAsync(document); - - // There should be a single rename tracking diagnostic - Assert.NotNull(diagnostic); - Assert.Equal(RenameTrackingCodeRefactoringProvider.DiagnosticId, diagnostic.Id); - - var actions = new List(); - var context = new CodeRefactoringContext( - document, diagnostic.Location.SourceSpan, actions.Add, CancellationToken.None); - await _codeRefactoringProvider.ComputeRefactoringsAsync(context); - - // There should only be one code action - Assert.Equal(1, actions.Count); - - Assert.Equal(string.Format(EditorFeaturesResources.Rename_0_to_1, expectedFromName, expectedToName), actions[0].Title); + // There should only be one code action for the tag + var codeAction = await TryGetCodeActionAsync(tag.Span.Span.ToTextSpan()); + Assert.NotNull(codeAction); + Assert.Equal(string.Format(EditorFeaturesResources.Rename_0_to_1, expectedFromName, expectedToName), codeAction.Title); if (invokeAction) { - var operations = (await actions[0].GetOperationsAsync(CancellationToken.None)).ToArray(); + var operations = (await codeAction.GetOperationsAsync(CancellationToken.None)).ToArray(); Assert.Equal(1, operations.Length); operations[0].TryApply(this.Workspace, new ProgressTracker(), CancellationToken.None); -- GitLab