Skip to content

Commit

Permalink
Prevent CodeActions commits during Inline Renames
Browse files Browse the repository at this point in the history
Fix dotnet#554: This fixes a crash that was caused by invoking a lightbulb
item that caused an Inline Rename session to launch when there's already
an inline rename session active. We now check for inline rename sessions
during lightbulb commit. If an inline rename session is active, we halt
the lightbulb commit and tell the user to complete their rename session.

This only prevents one (but probably the most frequent) manifestation of
a larger problem in which rename is trying to perform a series of
changes to the workspace that can be interrupted by some other feature
applying its changes to the workspace. This larger design problem is
tracked as dotnet#681.
  • Loading branch information
David Poeschl committed Feb 19, 2015
1 parent 8b4a0a0 commit b21230a
Show file tree
Hide file tree
Showing 11 changed files with 145 additions and 50 deletions.
9 changes: 9 additions & 0 deletions src/EditorFeatures/Core/EditorFeaturesResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/EditorFeatures/Core/EditorFeaturesResources.resx
Expand Up @@ -730,4 +730,7 @@ Do you want to proceed?</value>
<data name="FixAllOccurrences" xml:space="preserve">
<value>Fix all occurrences</value>
</data>
<data name="CannotApplyOperationWhileRenameSessionIsActive" xml:space="preserve">
<value>Cannot apply operation while a rename session is active.</value>
</data>
</root>
Expand Up @@ -11,6 +11,7 @@
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;
using Microsoft.CodeAnalysis.Notification;

namespace Microsoft.CodeAnalysis.Editor.Implementation.CodeActions
{
Expand Down Expand Up @@ -88,6 +89,14 @@ public SolutionPreviewResult GetPreviews(Workspace workspace, IEnumerable<CodeAc

public void Apply(Workspace workspace, Document fromDocument, IEnumerable<CodeActionOperation> operations, string title, CancellationToken cancellationToken)
{
if (_renameService.ActiveSession != null)
{
workspace.Services.GetService<INotificationService>()?.SendNotification(
EditorFeaturesResources.CannotApplyOperationWhileRenameSessionIsActive,
severity: NotificationSeverity.Error);
return;
}

#if DEBUG
var documentErrorLookup = new HashSet<DocumentId>();
foreach (var project in workspace.CurrentSolution.Projects)
Expand Down
Expand Up @@ -3,6 +3,7 @@
using System.Collections.Generic;
using System.Threading;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.Notification;

namespace Microsoft.CodeAnalysis.Editor
{
Expand Down
Expand Up @@ -14,6 +14,7 @@
using Microsoft.VisualStudio.Language.Intellisense;
using Microsoft.VisualStudio.Text;
using Roslyn.Utilities;
using Microsoft.CodeAnalysis.Notification;

namespace Microsoft.CodeAnalysis.Editor.Implementation.Suggestions
{
Expand Down
1 change: 1 addition & 0 deletions src/EditorFeatures/Test2/EditorServicesTest2.vbproj
Expand Up @@ -320,6 +320,7 @@
<Compile Include="Simplification\TypeInferenceSimplifierTests.vb" />
<Compile Include="Simplification\TypeNameSimplifierTest.vb" />
<Compile Include="Utilities\AssertEx.vb" />
<Compile Include="Utilities\TestNotificationService.vb" />
<Compile Include="Workspaces\SymbolDescriptionServiceTests.vb" />
<Compile Include="Workspaces\TryFindSourceDefinitionTests.vb" />
</ItemGroup>
Expand Down
85 changes: 85 additions & 0 deletions src/EditorFeatures/Test2/Rename/InlineRenameTests.vb
@@ -1,8 +1,14 @@
' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

Imports System.Threading
Imports Microsoft.CodeAnalysis.CodeActions
Imports Microsoft.CodeAnalysis.CodeRefactorings
Imports Microsoft.CodeAnalysis.CSharp.CodeRefactorings.IntroduceVariable
Imports Microsoft.CodeAnalysis.Editor.Host
Imports Microsoft.CodeAnalysis.Editor.UnitTests.RenameTracking
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Utilities
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
Imports Microsoft.CodeAnalysis.Notification
Imports Microsoft.CodeAnalysis.Options
Imports Microsoft.CodeAnalysis.Rename
Imports Microsoft.VisualStudio.Text
Expand Down Expand Up @@ -984,5 +990,84 @@ End Class
VerifyTagsAreCorrect(workspace, "maw")
End Using
End Sub

<Fact>
<Trait(Traits.Feature, Traits.Features.Rename)>
<Trait(Traits.Feature, Traits.Features.CodeActionsIntroduceVariable)>
<WorkItem(554, "https://github.com/dotnet/roslyn/issues/554")>
Public Sub CodeActionCannotCommitDuringInlineRename()
Using workspace = CreateWorkspaceWithWaiter(
<Workspace>
<Project Language="C#" CommonReferences="true" AssemblyName="CSProj">
<Document FilePath="C.cs">
class C
{
void M()
{
var z = {|introducelocal:5 + 5|};
var q = [|x$$|];
}

int [|x|];
}</Document>
</Project>
</Workspace>)

Dim session = StartSession(workspace)

' Type a bit in the file
Dim caretPosition = workspace.Documents.First(Function(d) d.CursorPosition.HasValue).CursorPosition.Value
Dim textBuffer = workspace.Documents.First().TextBuffer
textBuffer.Insert(caretPosition, "yz")
WaitForRename(workspace)

' Invoke a CodeAction
Dim introduceVariableRefactoringProvider = New IntroduceVariableCodeRefactoringProvider()
Dim actions = New List(Of CodeAction)
Dim context = New CodeRefactoringContext(
workspace.CurrentSolution.GetDocument(workspace.Documents.Single().Id),
workspace.Documents.Single().AnnotatedSpans()("introducelocal").Single(),
Sub(a) actions.Add(a),
CancellationToken.None)

workspace.Documents.Single().AnnotatedSpans.Clear()
introduceVariableRefactoringProvider.ComputeRefactoringsAsync(context).Wait()

Dim editHandler = workspace.ExportProvider.GetExportedValue(Of ICodeActionEditHandlerService)

Dim actualSeverity As NotificationSeverity = Nothing
Dim notificationService = DirectCast(workspace.Services.GetService(Of INotificationService)(), INotificationServiceCallback)
notificationService.NotificationCallback = Sub(message, title, severity) actualSeverity = severity

editHandler.Apply(
workspace,
workspace.CurrentSolution.GetDocument(workspace.Documents.Single().Id),
actions.First().GetOperationsAsync(CancellationToken.None).Result,
"unused",
CancellationToken.None)

' CodeAction should be rejected
Assert.Equal(NotificationSeverity.Error, actualSeverity)
Assert.Equal("
class C
{
void M()
{
var z = 5 + 5;
var q = xyz;
}

int xyz;
}",
textBuffer.CurrentSnapshot.GetText())

' Rename should still be active
VerifyTagsAreCorrect(workspace, "xyz")

textBuffer.Insert(caretPosition + 2, "q")
WaitForRename(workspace)
VerifyTagsAreCorrect(workspace, "xyzq")
End Using
End Sub
End Class
End Namespace
33 changes: 33 additions & 0 deletions src/EditorFeatures/Test2/Utilities/TestNotificationService.vb
@@ -0,0 +1,33 @@
' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

Imports Microsoft.CodeAnalysis.Notification

Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Utilities
Friend Class TestNotificationService
Implements INotificationService

Public MessageText As String
Public MessageTitle As String
Public MessageSeverity As NotificationSeverity

Public ConfirmBoxText As String
Public ConfirmBoxTitle As String
Public ConfirmBoxSeverity As NotificationSeverity

Public DesiredConfirmBoxResult As Boolean

Public Sub SendNotification(message As String, Optional title As String = Nothing, Optional severity As NotificationSeverity = NotificationSeverity.Warning) Implements INotificationService.SendNotification
MessageText = message
MessageTitle = title
MessageSeverity = severity
End Sub

Public Function ConfirmMessageBox(message As String, Optional title As String = Nothing, Optional severity As NotificationSeverity = NotificationSeverity.Warning) As Boolean Implements INotificationService.ConfirmMessageBox
ConfirmBoxText = message
ConfirmBoxTitle = title
ConfirmBoxSeverity = severity

Return DesiredConfirmBoxResult
End Function
End Class
End Namespace
Expand Up @@ -12,6 +12,7 @@ Imports Microsoft.CodeAnalysis.ChangeSignature
Imports Microsoft.CodeAnalysis.Shared.Extensions
Imports Roslyn.Test.Utilities
Imports Microsoft.VisualStudio.LanguageServices.Implementation.ChangeSignature
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Utilities

Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ChangeSignature
Public Class ReorderParametersViewModelTests
Expand Down Expand Up @@ -287,33 +288,5 @@ class MyClass
Return New ChangeSignatureViewModelTestState(viewModel, symbol.GetParameters())
End Using
End Function

Friend Class TestNotificationService
Implements INotificationService

Public MessageText As String
Public MessageTitle As String
Public MessageSeverity As NotificationSeverity

Public ConfirmBoxText As String
Public ConfirmBoxTitle As String
Public ConfirmBoxSeverity As NotificationSeverity

Public DesiredConfirmBoxResult As Boolean

Public Sub SendNotification(message As String, Optional title As String = Nothing, Optional severity As NotificationSeverity = NotificationSeverity.Warning) Implements INotificationService.SendNotification
MessageText = message
MessageTitle = title
MessageSeverity = severity
End Sub

Public Function ConfirmMessageBox(message As String, Optional title As String = Nothing, Optional severity As NotificationSeverity = NotificationSeverity.Warning) As Boolean Implements INotificationService.ConfirmMessageBox
ConfirmBoxText = message
ConfirmBoxTitle = title
ConfirmBoxSeverity = severity

Return DesiredConfirmBoxResult
End Function
End Class
End Class
End Namespace
Expand Up @@ -3,6 +3,7 @@
Imports System.Threading
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Extensions
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Utilities
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
Imports Microsoft.CodeAnalysis.LanguageServices
Imports Microsoft.CodeAnalysis.Notification
Expand Down Expand Up @@ -484,16 +485,5 @@ public class $$MyClass
fileExtension:=If(languageName = LanguageNames.CSharp, ".cs", ".vb"))
End Using
End Function

Friend Class TestNotificationService
Implements INotificationService

Public Sub SendNotification(message As String, Optional title As String = Nothing, Optional severity As NotificationSeverity = NotificationSeverity.Warning) Implements INotificationService.SendNotification
End Sub

Public Function ConfirmMessageBox(message As String, Optional title As String = Nothing, Optional severity As NotificationSeverity = NotificationSeverity.Warning) As Boolean Implements INotificationService.ConfirmMessageBox
Throw New NotImplementedException()
End Function
End Class
End Class
End Namespace
Expand Up @@ -4,6 +4,7 @@ Imports System.IO
Imports System.Threading
Imports Microsoft.CodeAnalysis
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Extensions
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Utilities
Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
Imports Microsoft.CodeAnalysis.GeneratedCodeRecognition
Imports Microsoft.CodeAnalysis.GenerateType
Expand Down Expand Up @@ -882,17 +883,6 @@ namespace A
End Function
End Class

Friend Class TestNotificationService
Implements INotificationService

Public Sub SendNotification(message As String, Optional title As String = Nothing, Optional severity As NotificationSeverity = NotificationSeverity.Warning) Implements INotificationService.SendNotification
End Sub

Public Function ConfirmMessageBox(message As String, Optional title As String = Nothing, Optional severity As NotificationSeverity = NotificationSeverity.Warning) As Boolean Implements INotificationService.ConfirmMessageBox
Throw New NotImplementedException()
End Function
End Class

Friend Class TestProjectManagementService
Implements IProjectManagementService

Expand Down

0 comments on commit b21230a

Please sign in to comment.