Merge pull request #35907 from sharwell/retry-lightbulb

Update SuggestedAction to track successful application

src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActions/SuggestedAction.cs

@@ -34,6 +34,8 @@ internal abstract partial class SuggestedAction : ForegroundThreadAffinitizedObj
         protected readonly object Provider;
         internal readonly CodeAction CodeAction;
 
+        private bool _isApplied;
+
         private ICodeActionEditHandlerService EditHandler => SourceProvider.EditHandler;
 
         internal SuggestedAction(
@@ -163,9 +165,11 @@ protected virtual void InnerInvoke(IProgressTracker progressTracker, Cancellatio
                     FunctionId.CodeFixes_ApplyChanges, KeyValueLogMessage.Create(LogType.UserAction, m => CreateLogProperties(m)), cancellationToken))
                 {
                     // Note: we want to block the UI thread here so the user cannot modify anything while the codefix applies
-                    EditHandler.ApplyAsync(Workspace, getFromDocument(),
+                    var applicationTask = EditHandler.ApplyAsync(Workspace, getFromDocument(),
                         operations.ToImmutableArray(), CodeAction.Title,
-                        progressTracker, cancellationToken).Wait(cancellationToken);
+                        progressTracker, cancellationToken);
+                    applicationTask.Wait(cancellationToken);
+                    _isApplied = applicationTask.Result;
                 }
             }
         }
@@ -308,5 +312,18 @@ public override int GetHashCode()
         }
 
         #endregion
+
+        internal TestAccessor GetTestAccessor()
+            => new TestAccessor(this);
+
+        internal readonly struct TestAccessor
+        {
+            private readonly SuggestedAction _suggestedAction;
+
+            public TestAccessor(SuggestedAction suggestedAction)
+                => _suggestedAction = suggestedAction;
+
+            public ref bool IsApplied => ref _suggestedAction._isApplied;
+        }
     }
 }

src/EditorFeatures/Core/Implementation/CodeActions/CodeActionEditHandlerService.cs

@@ -94,7 +94,7 @@ internal class CodeActionEditHandlerService : ForegroundThreadAffinitizedObject,
             return currentResult;
         }
 
-        public async Task ApplyAsync(
+        public async Task<bool> ApplyAsync(
             Workspace workspace, Document fromDocument,
             ImmutableArray<CodeActionOperation> operations,
             string title, IProgressTracker progressTracker,
@@ -104,7 +104,7 @@ internal class CodeActionEditHandlerService : ForegroundThreadAffinitizedObject,
 
             if (operations.IsDefaultOrEmpty)
             {
-                return;
+                return _renameService.ActiveSession is null;
             }
 
             if (_renameService.ActiveSession != null)
@@ -112,7 +112,7 @@ internal class CodeActionEditHandlerService : ForegroundThreadAffinitizedObject,
                 workspace.Services.GetService<INotificationService>()?.SendNotification(
                     EditorFeaturesResources.Cannot_apply_operation_while_a_rename_session_is_active,
                     severity: NotificationSeverity.Error);
-                return;
+                return false;
             }
 
 #if DEBUG && false
@@ -133,6 +133,8 @@ internal class CodeActionEditHandlerService : ForegroundThreadAffinitizedObject,
 
             var oldSolution = workspace.CurrentSolution;
 
+            bool applied;
+
             // Determine if we're making a simple text edit to a single file or not.
             // If we're not, then we need to make a linked global undo to wrap the 
             // application of these operations.  This way we should be able to undo 
@@ -151,7 +153,7 @@ internal class CodeActionEditHandlerService : ForegroundThreadAffinitizedObject,
 
                 using (workspace.Services.GetService<ISourceTextUndoService>().RegisterUndoTransaction(text, title))
                 {
-                    operations.Single().Apply(workspace, cancellationToken);
+                    applied = operations.Single().TryApply(workspace, progressTracker, cancellationToken);
                 }
             }
             else
@@ -168,7 +170,7 @@ internal class CodeActionEditHandlerService : ForegroundThreadAffinitizedObject,
                         transaction.AddDocument(fromDocument.Id);
                     }
 
-                    ProcessOperations(
+                    applied = ProcessOperations(
                         workspace, operations, progressTracker,
                         cancellationToken);
 
@@ -178,6 +180,7 @@ internal class CodeActionEditHandlerService : ForegroundThreadAffinitizedObject,
 
             var updatedSolution = operations.OfType<ApplyChangesOperation>().FirstOrDefault()?.ChangedSolution ?? oldSolution;
             TryNavigateToLocationOrStartRenameSession(workspace, oldSolution, updatedSolution, cancellationToken);
+            return applied;
         }
 
         private TextDocument TryGetSingleChangedText(
@@ -257,10 +260,13 @@ internal class CodeActionEditHandlerService : ForegroundThreadAffinitizedObject,
             }
         }
 
-        private static void ProcessOperations(
+        /// <returns><see langword="true"/> if all expected <paramref name="operations"/> are applied successfully;
+        /// otherwise, <see langword="false"/>.</returns>
+        private static bool ProcessOperations(
             Workspace workspace, ImmutableArray<CodeActionOperation> operations,
             IProgressTracker progressTracker, CancellationToken cancellationToken)
         {
+            var applied = true;
             var seenApplyChanges = false;
             foreach (var operation in operations)
             {
@@ -275,8 +281,10 @@ internal class CodeActionEditHandlerService : ForegroundThreadAffinitizedObject,
                     seenApplyChanges = true;
                 }
 
-                operation.TryApply(workspace, progressTracker, cancellationToken);
+                applied &= operation.TryApply(workspace, progressTracker, cancellationToken);
             }
+
+            return applied;
         }
 
         private void TryNavigateToLocationOrStartRenameSession(Workspace workspace, Solution oldSolution, Solution newSolution, CancellationToken cancellationToken)

src/EditorFeatures/Core/Implementation/CodeRefactorings/ICodeActionEditHandlerService.cs

@@ -16,7 +16,7 @@ internal interface ICodeActionEditHandlerService
         SolutionPreviewResult GetPreviews(
             Workspace workspace, ImmutableArray<CodeActionOperation> operations, CancellationToken cancellationToken);
 
-        Task ApplyAsync(
+        Task<bool> ApplyAsync(
             Workspace workspace, Document fromDocument,
             ImmutableArray<CodeActionOperation> operations,
             string title, IProgressTracker progressTracker,

src/VisualStudio/IntegrationTest/TestUtilities/InProcess/TextViewWindow_InProc.cs

@@ -329,7 +329,7 @@ private async Task<IEnumerable<ISuggestedAction>> GetLightBulbActionsAsync(ILigh
             return await SelectActionsAsync(actionSets);
         }
 
-        public void ApplyLightBulbAction(string actionName, FixAllScope? fixAllScope, bool blockUntilComplete)
+        public bool ApplyLightBulbAction(string actionName, FixAllScope? fixAllScope, bool blockUntilComplete)
         {
             var lightBulbAction = GetLightBulbApplicationAction(actionName, fixAllScope, blockUntilComplete);
             var task = ThreadHelper.JoinableTaskFactory.RunAsync(async () =>
@@ -337,16 +337,20 @@ public void ApplyLightBulbAction(string actionName, FixAllScope? fixAllScope, bo
                 await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
 
                 var activeTextView = GetActiveTextView();
-                await lightBulbAction(activeTextView);
+                return await lightBulbAction(activeTextView);
             });
 
             if (blockUntilComplete)
             {
-                task.Join();
+                var result = task.Join();
+                DismissLightBulbSession();
+                return result;
             }
+
+            return true;
         }
 
-        private Func<IWpfTextView, Task> GetLightBulbApplicationAction(string actionName, FixAllScope? fixAllScope, bool willBlockUntilComplete)
+        private Func<IWpfTextView, Task<bool>> GetLightBulbApplicationAction(string actionName, FixAllScope? fixAllScope, bool willBlockUntilComplete)
         {
             return async view =>
             {
@@ -396,7 +400,7 @@ public void ApplyLightBulbAction(string actionName, FixAllScope? fixAllScope, bo
 
                     if (string.IsNullOrEmpty(actionName))
                     {
-                        return;
+                        return false;
                     }
 
                     // Dismiss the lightbulb session as we not invoking the original code fix.
@@ -404,6 +408,8 @@ public void ApplyLightBulbAction(string actionName, FixAllScope? fixAllScope, bo
                 }
 
                 action.Invoke(CancellationToken.None);
+                return !(action is SuggestedAction suggestedAction)
+                    || suggestedAction.GetTestAccessor().IsApplied;
             };
         }
 

src/VisualStudio/IntegrationTest/TestUtilities/OutOfProcess/TextViewWindow_OutOfProc.Verifier.cs

@@ -3,6 +3,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading;
 using Microsoft.CodeAnalysis.CodeFixes;
 using Microsoft.CodeAnalysis.Shared.TestHooks;
 using Xunit;
@@ -30,12 +31,28 @@ public Verifier(TTextViewWindow textViewWindow, VisualStudioInstance instance)
                 FixAllScope? fixAllScope = null,
                 bool blockUntilComplete = true)
             {
+                using var cancellationTokenSource = new CancellationTokenSource(Helper.HangMitigatingTimeout);
+
                 var expectedItems = new[] { expectedItem };
-                CodeActions(expectedItems, applyFix ? expectedItem : null, verifyNotShowing,
-                    ensureExpectedItemsAreOrdered, fixAllScope, blockUntilComplete);
+
+                bool? applied;
+                do
+                {
+                    cancellationTokenSource.Token.ThrowIfCancellationRequested();
+
+                    applied = CodeActions(expectedItems, applyFix ? expectedItem : null, verifyNotShowing,
+                        ensureExpectedItemsAreOrdered, fixAllScope, blockUntilComplete);
+                } while (applied is false);
             }
 
-            public void CodeActions(
+            /// <returns>
+            /// <list type="bullet">
+            /// <item><description><see langword="true"/> if <paramref name="applyFix"/> is specified and the fix is successfully applied</description></item>
+            /// <item><description><see langword="false"/> if <paramref name="applyFix"/> is specified but the fix is not successfully applied</description></item>
+            /// <item><description><see langword="null"/> if <paramref name="applyFix"/> is false, so there is no fix to apply</description></item>
+            /// </list>
+            /// </returns>
+            public bool? CodeActions(
                 IEnumerable<string> expectedItems,
                 string applyFix = null,
                 bool verifyNotShowing = false,
@@ -49,7 +66,7 @@ public Verifier(TTextViewWindow textViewWindow, VisualStudioInstance instance)
                 if (verifyNotShowing)
                 {
                     CodeActionsNotShowing();
-                    return;
+                    return null;
                 }
 
                 var actions = _textViewWindow.GetLightBulbActions();
@@ -72,14 +89,18 @@ public Verifier(TTextViewWindow textViewWindow, VisualStudioInstance instance)
 
                 if (!string.IsNullOrEmpty(applyFix) || fixAllScope.HasValue)
                 {
-                    _textViewWindow.ApplyLightBulbAction(applyFix, fixAllScope, blockUntilComplete);
+                    var result = _textViewWindow.ApplyLightBulbAction(applyFix, fixAllScope, blockUntilComplete);
 
                     if (blockUntilComplete)
                     {
                         // wait for action to complete
                         _instance.Workspace.WaitForAsyncOperations(Helper.HangMitigatingTimeout, FeatureAttribute.LightBulb);
                     }
+
+                    return result;
                 }
+
+                return null;
             }
 
             public void CodeActionsNotShowing()

src/VisualStudio/IntegrationTest/TestUtilities/OutOfProcess/TextViewWindow_OutOfProc.cs

@@ -78,7 +78,7 @@ public void DismissLightBulbSession()
         public string[] GetLightBulbActions()
             => _textViewWindowInProc.GetLightBulbActions();
 
-        public void ApplyLightBulbAction(string action, FixAllScope? fixAllScope, bool blockUntilComplete = true)
+        public bool ApplyLightBulbAction(string action, FixAllScope? fixAllScope, bool blockUntilComplete = true)
             => _textViewWindowInProc.ApplyLightBulbAction(action, fixAllScope, blockUntilComplete);
 
         public void InvokeCompletionList()

src/Workspaces/Core/Portable/CodeActions/Operations/ApplyChangesOperation.cs

@@ -41,8 +41,7 @@ public override void Apply(Workspace workspace, CancellationToken cancellationTo
         internal override bool TryApply(
             Workspace workspace, IProgressTracker progressTracker, CancellationToken cancellationToken)
         {
-            workspace.TryApplyChanges(ChangedSolution, progressTracker);
-            return true;
+            return workspace.TryApplyChanges(ChangedSolution, progressTracker);
         }
     }
 }