search

dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.13k stars 4.04k forks source link

Inline Rename currentState is null #72017

Open KirillOsenkov opened 9 months ago

KirillOsenkov commented 9 months ago

I got myself into a state where currentState is null here: https://github.com/dotnet/roslyn/blob/2296cb59d33c0beeb1501dcb1c9b49bd26d025c4/src/EditorFeatures/Core/InlineRename/AbstractInlineRenameUndoManager.cs#L79

Every keystroke in rename throws a NullReferenceException. Persists after I close the rename dialog and open a new session using F2.

@ryzngard

ryzngard commented 9 months ago

Interesting, that means

https://github.com/dotnet/roslyn/blob/2296cb59d33c0beeb1501dcb1c9b49bd26d025c4/src/EditorFeatures/Core/InlineRename/AbstractInlineRenameUndoManager.cs#L118

was never called...

@Cosifne do you know of any changes here? We could fix the NRE, but I fear other things would be broken if we never initialize

KirillOsenkov commented 2 months ago

Ah, I know what is happening.

  1. Open Visual Studio, create a new .cs file without a project
  2. invoke Inline Rename in that file, close the file
  3. open any project
  4. invoke Inline Rename in any .cs file that is part of the project
  5. type new name and press Enter

There are two workspaces in VS: VisualStudioWorkspaceImpl and MiscellaneousFilesWorkspace. Each of them gets a new copy of InlineRenameUndoManager: https://github.com/dotnet/roslyn/blob/fc368834c6087507a5df36c49a3b7f072aec39ae/src/VisualStudio/Core/Def/InlineRename/InlineRenameUndoManager.cs#L56

By invoking inline rename in two different workspaces you're creating two copies of the InlineRenameUndoManager, that compete with each other. currentState will only be set in one of them.

I think we should reuse the same InlineRenameUndoManager for all workspaces, so keep a singleton in a field instead of returning a new instance from CreateService for each workspace.

@ryzngard Please fix, should be easy ;) Much obliged!

KirillOsenkov commented 2 months ago

Same problem here too: https://github.com/dotnet/roslyn/blob/fc368834c6087507a5df36c49a3b7f072aec39ae/src/EditorFeatures/Core/InlineRename/UndoManagerServiceFactory.cs#L30

KirillOsenkov commented 2 months ago

This seems to have fixed it:

image