Simplify error handling and notifications in rename.

src/EditorFeatures/Core/EditorFeaturesResources.resx

@@ -1070,4 +1070,7 @@ Do you want to proceed?</value>
     <value>{0} (Esc to cancel)</value>
     <comment>Placeholder represents an operating that is happening.  Parenthesized expression indicates how you can cancel it.</comment>
   </data>
+  <data name="Rename_operation_could_not_complete_due_to_external_change_to_workspace" xml:space="preserve">
+    <value>Rename operation could not complete due to external change to workspace</value>
+  </data>
 </root>

src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs

@@ -788,24 +788,31 @@ private void CommitCore(IUIThreadOperationContext operationContext, bool preview
 
                 DismissUIAndRollbackEditsAndEndRenameSession(
                     RenameLogMessage.UserActionOutcome.Committed, previewChanges,
-                    () => ApplyRename(newSolution, operationContext));
+                    () =>
+                    {
+                        var error = TryApplyRename(newSolution);
+
+                        if (error is not null)
+                        {
+                            operationContext.TakeOwnership();
+                            var notificationService = _workspace.Services.GetService<INotificationService>();
+                            notificationService.SendNotification(
+                                error.Value.message, EditorFeaturesResources.Rename_Symbol, error.Value.severity);
+                        }
+                    });
             }
         }
 
-        private void ApplyRename(Solution newSolution, IUIThreadOperationContext operationContext)
+        /// <summary>
+        /// Returns non-null error message if renaming fails.
+        /// </summary>
+        private (NotificationSeverity severity, string message)? TryApplyRename(Solution newSolution)
         {
             var changes = _baseSolution.GetChanges(newSolution);
             var changedDocumentIDs = changes.GetProjectChanges().SelectMany(c => c.GetChangedDocuments()).ToList();
 
             if (!_renameInfo.TryOnBeforeGlobalSymbolRenamed(_workspace, changedDocumentIDs, this.ReplacementText))
-            {
-                var notificationService = _workspace.Services.GetService<INotificationService>();
-                notificationService.SendNotification(
-                    EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid,
-                    EditorFeaturesResources.Rename_Symbol,
-                    NotificationSeverity.Error);
-                return;
-            }
+                return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid);
 
             using var undoTransaction = _workspace.OpenGlobalUndoTransaction(EditorFeaturesResources.Inline_Rename);
             var finalSolution = newSolution.Workspace.CurrentSolution;
@@ -837,7 +844,10 @@ private void ApplyRename(Solution newSolution, IUIThreadOperationContext operati
                 finalSolution = finalSolution.WithDocumentName(id, newDocument.Name);
             }
 
-            if (_workspace.TryApplyChanges(finalSolution))
+            if (!_workspace.TryApplyChanges(finalSolution))
+                return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace);
+
+            try
             {
                 // Since rename can apply file changes as well, and those file
                 // changes can generate new document ids, include added documents
@@ -846,20 +856,19 @@ private void ApplyRename(Solution newSolution, IUIThreadOperationContext operati
                 var finalChanges = _workspace.CurrentSolution.GetChanges(_baseSolution);
 
                 var finalChangedIds = finalChanges
-                        .GetProjectChanges()
-                        .SelectMany(c => c.GetChangedDocuments().Concat(c.GetAddedDocuments()))
-                        .ToList();
+                    .GetProjectChanges()
+                    .SelectMany(c => c.GetChangedDocuments().Concat(c.GetAddedDocuments()))
+                    .ToList();
 
                 if (!_renameInfo.TryOnAfterGlobalSymbolRenamed(_workspace, finalChangedIds, this.ReplacementText))
-                {
-                    var notificationService = _workspace.Services.GetService<INotificationService>();
-                    operationContext.TakeOwnership();
-                    notificationService.SendNotification(
-                        EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated,
-                        EditorFeaturesResources.Rename_Symbol,
-                        NotificationSeverity.Information);
-                }
+                    return (NotificationSeverity.Information, EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated);
 
+                return null;
+            }
+            finally
+            {
+                // If we successfully update the workspace then make sure the undo transaction is committed and is
+                // always able to undo anything any other external listener did.
                 undoTransaction.Commit();
             }
         }

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

@@ -110,7 +110,15 @@ public RenameTrackingCommitterOperation(RenameTrackingCommitter committer)
                     => _committer = committer;
 
                 public override void Apply(Workspace workspace, CancellationToken cancellationToken)
-                    => _committer.Commit(cancellationToken);
+                {
+                    var error = _committer.TryCommit(cancellationToken);
+                    if (error != null)
+                    {
+                        var notificationService = workspace.Services.GetService<INotificationService>();
+                        notificationService.SendNotification(
+                            error.Value.message, EditorFeaturesResources.Rename_Symbol, error.Value.severity);
+                    }
+                }
             }
         }
     }

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

@@ -49,16 +49,24 @@ public RenameTrackingCommitter(
                 _renameSymbolResultGetter = new AsyncLazy<RenameTrackingSolutionSet>(c => RenameSymbolWorkerAsync(c), cacheResult: true);
             }
 
-            public void Commit(CancellationToken cancellationToken)
+            /// <summary>
+            /// Returns non-null error message if renaming fails.
+            /// </summary>
+            public (NotificationSeverity severity, string message)? TryCommit(CancellationToken cancellationToken)
             {
                 _stateMachine.ThreadingContext.ThrowIfNotOnUIThread();
 
-                ApplyChangesToWorkspace(cancellationToken);
-
-                // Clear the state machine so that future updates to the same token work,
-                // and any text changes caused by this update are not interpreted as 
-                // potential renames
-                _stateMachine.ClearTrackingSession();
+                try
+                {
+                    return TryApplyChangesToWorkspace(cancellationToken);
+                }
+                finally
+                {
+                    // Clear the state machine so that future updates to the same token work,
+                    // and any text changes caused by this update are not interpreted as 
+                    // potential renames
+                    _stateMachine.ClearTrackingSession();
+                }
             }
 
             public async Task<RenameTrackingSolutionSet> RenameSymbolAsync(CancellationToken cancellationToken)
@@ -82,7 +90,10 @@ private async Task<RenameTrackingSolutionSet> RenameSymbolWorkerAsync(Cancellati
                 return new RenameTrackingSolutionSet(symbol, solutionWithOriginalName, renamedSolution);
             }
 
-            private void ApplyChangesToWorkspace(CancellationToken cancellationToken)
+            /// <summary>
+            /// Returns non-null error message if renaming fails.
+            /// </summary>
+            private (NotificationSeverity, string)? TryApplyChangesToWorkspace(CancellationToken cancellationToken)
             {
                 _stateMachine.ThreadingContext.ThrowIfNotOnUIThread();
 
@@ -120,48 +131,46 @@ private void ApplyChangesToWorkspace(CancellationToken cancellationToken)
                 // update the state machine after undo/redo actions. 
 
                 var changedDocuments = renameTrackingSolutionSet.RenamedSolution.GetChangedDocuments(renameTrackingSolutionSet.OriginalSolution);
-
-                // When this action is undone (the user has undone twice), restore the state
-                // machine to so that they can continue their original rename tracking session.
-
-                var trackingSessionId = _stateMachine.StoreCurrentTrackingSessionAndGenerateId();
-                UpdateWorkspaceForResetOfTypedIdentifier(workspace, renameTrackingSolutionSet.OriginalSolution, trackingSessionId);
-
-                // Now that the solution is back in its original state, notify third parties about
-                // the coming rename operation.
-                if (!_refactorNotifyServices.TryOnBeforeGlobalSymbolRenamed(workspace, changedDocuments, renameTrackingSolutionSet.Symbol, newName, throwOnFailure: false))
+                try
                 {
-                    var notificationService = workspace.Services.GetService<INotificationService>();
-                    notificationService.SendNotification(
-                        EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid,
-                        EditorFeaturesResources.Rename_Symbol,
-                        NotificationSeverity.Error);
-
-                    return;
+                    // When this action is undone (the user has undone twice), restore the state
+                    // machine to so that they can continue their original rename tracking session.
+
+                    var trackingSessionId = _stateMachine.StoreCurrentTrackingSessionAndGenerateId();
+                    var result = TryUpdateWorkspaceForResetOfTypedIdentifier(workspace, renameTrackingSolutionSet.OriginalSolution, trackingSessionId);
+                    if (result is not null)
+                        return result;
+
+                    // Now that the solution is back in its original state, notify third parties about
+                    // the coming rename operation.
+                    if (!_refactorNotifyServices.TryOnBeforeGlobalSymbolRenamed(workspace, changedDocuments, renameTrackingSolutionSet.Symbol, newName, throwOnFailure: false))
+                        return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid);
+
+                    // move all changes to final solution based on the workspace's current solution, since the current solution
+                    // got updated when we reset it above.
+                    var finalSolution = workspace.CurrentSolution;
+                    foreach (var docId in changedDocuments)
+                    {
+                        // because changes have already been made to the workspace (UpdateWorkspaceForResetOfTypedIdentifier() above),
+                        // these calls can't be cancelled and must be allowed to complete.
+                        var root = renameTrackingSolutionSet.RenamedSolution.GetDocument(docId).GetSyntaxRootSynchronously(CancellationToken.None);
+                        finalSolution = finalSolution.WithDocumentSyntaxRoot(docId, root);
+                    }
+
+                    // Undo/redo on this action must always clear the state machine
+                    return TryUpdateWorkspaceForGlobalIdentifierRename(
+                        workspace,
+                        finalSolution,
+                        _displayText,
+                        changedDocuments,
+                        renameTrackingSolutionSet.Symbol,
+                        newName,
+                        trackingSessionId);
                 }
-
-                // move all changes to final solution based on the workspace's current solution, since the current solution
-                // got updated when we reset it above.
-                var finalSolution = workspace.CurrentSolution;
-                foreach (var docId in changedDocuments)
+                finally
                 {
-                    // because changes have already been made to the workspace (UpdateWorkspaceForResetOfTypedIdentifier() above),
-                    // these calls can't be cancelled and must be allowed to complete.
-                    var root = renameTrackingSolutionSet.RenamedSolution.GetDocument(docId).GetSyntaxRootSynchronously(CancellationToken.None);
-                    finalSolution = finalSolution.WithDocumentSyntaxRoot(docId, root);
+                    RenameTrackingDismisser.DismissRenameTracking(workspace, changedDocuments);
                 }
-
-                // Undo/redo on this action must always clear the state machine
-                UpdateWorkspaceForGlobalIdentifierRename(
-                    workspace,
-                    finalSolution,
-                    _displayText,
-                    changedDocuments,
-                    renameTrackingSolutionSet.Symbol,
-                    newName,
-                    trackingSessionId);
-
-                RenameTrackingDismisser.DismissRenameTracking(workspace, changedDocuments);
             }
 
             private Solution CreateSolutionWithOriginalName(Document document, CancellationToken cancellationToken)
@@ -203,7 +212,10 @@ private async Task<ISymbol> TryGetSymbolAsync(Solution solutionWithOriginalName,
                 return tokenRenameInfo.HasSymbols ? tokenRenameInfo.Symbols.First() : null;
             }
 
-            private void UpdateWorkspaceForResetOfTypedIdentifier(Workspace workspace, Solution newSolution, int trackingSessionId)
+            /// <summary>
+            /// Returns non-null error message if renaming fails.
+            /// </summary>
+            private (NotificationSeverity, string)? TryUpdateWorkspaceForResetOfTypedIdentifier(Workspace workspace, Solution newSolution, int trackingSessionId)
             {
                 _stateMachine.ThreadingContext.ThrowIfNotOnUIThread();
 
@@ -217,18 +229,24 @@ private void UpdateWorkspaceForResetOfTypedIdentifier(Workspace workspace, Solut
                 localUndoTransaction.AddUndo(undoPrimitiveBefore);
 
                 if (!workspace.TryApplyChanges(newSolution))
-                {
-                    Contract.Fail("Rename Tracking could not update solution.");
-                }
+                    return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace);
+
+                // If we successfully update the workspace then make sure the undo transaction is committed and is
+                // always able to undo anything any other external listener did.
 
                 // Never resume tracking session on redo
                 var undoPrimitiveAfter = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false);
                 localUndoTransaction.AddUndo(undoPrimitiveAfter);
 
                 localUndoTransaction.Complete();
+
+                return null;
             }
 
-            private void UpdateWorkspaceForGlobalIdentifierRename(
+            /// <summary>
+            /// Returns non-null error message if renaming fails.
+            /// </summary>
+            private (NotificationSeverity, string)? TryUpdateWorkspaceForGlobalIdentifierRename(
                 Workspace workspace,
                 Solution newSolution,
                 string undoName,
@@ -251,25 +269,27 @@ private void UpdateWorkspaceForGlobalIdentifierRename(
                 localUndoTransaction.AddUndo(undoPrimitiveBefore);
 
                 if (!workspace.TryApplyChanges(newSolution))
-                {
-                    Contract.Fail("Rename Tracking could not update solution.");
-                }
+                    return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace);
 
-                if (!_refactorNotifyServices.TryOnAfterGlobalSymbolRenamed(workspace, changedDocuments, symbol, newName, throwOnFailure: false))
+                try
                 {
-                    var notificationService = workspace.Services.GetService<INotificationService>();
-                    notificationService.SendNotification(
-                        EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated,
-                        EditorFeaturesResources.Rename_Symbol,
-                        NotificationSeverity.Information);
+                    if (!_refactorNotifyServices.TryOnAfterGlobalSymbolRenamed(workspace, changedDocuments, symbol, newName, throwOnFailure: false))
+                        return (NotificationSeverity.Information, EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated);
+
+                    return null;
                 }
+                finally
+                {
+                    // If we successfully update the workspace then make sure the undo transaction is committed and is
+                    // always able to undo anything any other external listener did.
 
-                // Never resume tracking session on redo
-                var undoPrimitiveAfter = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false);
-                localUndoTransaction.AddUndo(undoPrimitiveAfter);
+                    // Never resume tracking session on redo
+                    var undoPrimitiveAfter = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false);
+                    localUndoTransaction.AddUndo(undoPrimitiveAfter);
 
-                localUndoTransaction.Complete();
-                workspaceUndoTransaction.Commit();
+                    localUndoTransaction.Complete();
+                    workspaceUndoTransaction.Commit();
+                }
             }
         }
     }