Roslyn: Rename object/variable names no longer available on the context menu
_This issue has been moved from a ticket on Developer Community._
[regression] [worked-in:All versions prior to Version 16.6.0]
Renaming an object or variable name in Visual Studio Version 16.6.0 no longer provides the 'rename' option on the context menu. It's no longer possible to effectively and efficiently rename objects as a bulk operation.
Original Comments
David Bentley on 5/21/2020, 08:16 PM:
Just updated to 16.6.0 and having the same issue. Was driving me nuts thinking it was just me. As a side note, [Ctrl + R], [Ctrl + R] allows to rename, but you have to use it BEFORE you rename and not after. It is just not as efficient or familiar.
Visual Studio Feedback System on 5/22/2020, 01:53 AM:
We have directed your feedback to the appropriate engineering team for further evaluation. The team will review the feedback and notify you about the next steps.
dave on 5/22/2020, 06:06 AM:
Yes. I don't know if that [CTRL+R]^2 functionality has always been there, but it's un-intuitive and not nearly as efficient. Especially with the extra step of needing to click 'Apply' on a dialog that is on the other side of the screen.
There was nothing wrong with the functionality in prior versions, rename the object, right+click and select Rename. It worked, it was effective, it wasn't broken.
That needs to be put back. Right now that missing functionality is simply a bug.
David Bentley on 5/22/2020, 06:56 AM:
Looks like this was around in 16.5.0 and was claimed to be fixed (but clearly has not):
https://github.com/dotnet/roslyn/issues/42703
Visual Studio Feedback System on 5/27/2020, 09:55 AM:
Thank you for sharing your feedback! Our teams prioritize action on product issues with broad customer impact. See details at: https://docs.microsoft.com/en-us/visualstudio/ide/report-a-problem?view=vs-2019#faq. In case you need answers to common questions or need assisted support, be sure to use https://visualstudio.microsoft.com/vs/support/. We鈥檒l keep you posted on any updates to this feedback.
Original Solutions
(no solutions)
vsfeedback
All 31 comments
Similar to the issue reported https://github.com/dotnet/roslyn/issues/42703 and also reported internally in Roslyn discussion forum. We need to fix the lightbulb that shows up within the text editor to show applicable refactorings, currently it only shows code fixes for analyzers and rename tracking is no longer an analyzer/code fix.
mavasani
on 27 May 2020
Tagging @CyrusNajmabadi @jinujoseph @AmadeusW @olegtk
It seems Roslyn does not control whether refactorings are shown in the lightbulb that shows in the text editor under the cursor. This is controlled by the editor/lightbulb code.
Invoke light bulb from the margin
Roslyn is asked for suggested actions with categories "CODEFIX" and "REFACTORING":
Invoke light bulb within the text editor under the cursor
Roslyn is asked for suggested actions with category "CODEFIX":
We need the editor team to change the latter to also include "REFACTORING" to fix this issue.
mavasani
on 27 May 2020
I think it was by design as far back as to Dev14. The intent was I believe that if you hover over a squiggle you indent to fix that particular issue. Why change it now?
olegtk
on 27 May 2020
Also if Roslyn choses to, it can ignore requestedActionCategories in this call and return whatever actions you think are appropriate (I think there are precedents of that already).
olegtk
on 27 May 2020
The core issue was that rename tracking was incorrectly implemented as an analyzer/code fix, and hence always showed up in the latter case. This was fixed by @CyrusNajmabadi to be written as a refactoring, and now no longer shows up.
Isn't the intent for the span based trigger to show all actions relevant on that token span, while the one in the margin shows everything relevant for the entire line span?
mavasani
on 27 May 2020
Also if Roslyn choses to, it can ignore requestedActionCategories in this call and return whatever actions you think are appropriate (I think there are precedents of that already).
Ah, that sounds reasonable. @CyrusNajmabadi @jinujoseph I think we should treat our refactorings with HighPriority to be equivalent to Code fixes for this scenario and just use "CODEFIX" as its category. I'll work on a fix on the same lines. On second thought, we should probably return all applicable refactorings for the span as we initially discussed.
mavasani
on 27 May 2020
No, the intent is to show fixes for a specific issue indicated by the squiggle. Even the link in the QuickInfo says "Show potential fixes". But providers are free to return any relevant suggested actions anyway.
olegtk
on 27 May 2020
No, the intent is to show fixes for a specific issue indicated by the squiggle. Even the link in the QuickInfo says "Show potential fixes".
Ah, so it seems like users got into a bad habit of expecting rename tracking refactoring to show up in "Show potential fixes", and see this as a regression now.
mavasani
on 27 May 2020
I actually dont' see any of the value of this behavior. Indeed, i view it as the opposite. I don't distinguish "refactorings" versus "fixes". THey're all just things i want to do with the code. When my cursor is on something, i often want to refactor it. It may be a lower priority than potentially fixing squiggled issues, but its' no less valid to show.
I've never really understood why this change was made. Back when refactorings were smart-tags, they appeared where your cursor was, which made lots of sense and was easy to use. The concept that we have two different light-bulbs that do different things based on if they're on the left of a line, or inside the line, just doesn't really make much sense to me.
CyrusNajmabadi
on 27 May 2020
That's all true in general for the LightBulb. But LightBulb on a squiggle is considered a more focused scenario - see a squiggle, hover, read error description, click on LightBulb, get is fixed. So it's scoped down to that particular squiggle span only, we don't want to pollute it with suggestions unrelated to that particular squiggle (even if more important). Same for refactorings - there can be refactorings everywhere, but if I want to get one squiggle to go away I only want to see what can get it fixed.
This was original thinking back in Dev14, perhaps it's time to revisit it. For example we assumed that no squiggle will be shown for something that requires refactoring to get fixed.
olegtk
on 27 May 2020
No, the intent is to show fixes for a specific issue indicated by the squiggle. Even the link in the QuickInfo says "Show potential fixes".
Ah, so it seems like users got into a bad habit of expecting rename tracking refactoring to show up in "Show potential fixes", and see this as a regression now.
Please do not call this a "bad habit." The ability to mouse hover over and quickly fix AND rename something might not have been your original intent, but it is a massive time saving function.
pvpxan
on 31 May 2020
Please do not call this a "bad habit."
Agreed. The charitable reading here is that it's 'bad' in that we didn't realize this would break when the change was made. This will be fixed. Thanks!
CyrusNajmabadi
on 31 May 2020
Yes, agree with Cyrus. @CyrusNajmabadi @olegtk @jinujoseph do we want to pursue a fix in the platform layer OR Roslyn? Either fix is not too complicated, it is more of a design decision - do we want _applicable_ refactorings for a token to show up for the lightbulb under the cursor? I agree with @CyrusNajmabadi's stance, but it is reasonable to make a fix in Roslyn if the platform team disagrees or feels this is a risky change.
mavasani
on 31 May 2020
in this case i think roslny should do it. we effectively treat this like a hybriad refactoring/analzyer. It's a refactoring due to it's nature of what it's doing. But it's an analyzer because we're paying attention to what the user is doing and effectively popping up our own squiggle (the rename box) around the user's text.
Because we have our own 'virtual squiggle', we really should treat this specially IMO. Thoughts @mavasani ?
CyrusNajmabadi
on 31 May 2020
@CyrusNajmabadi Thanks. If we are doing it in Roslyn, my next question is should we just special case refactorings with HighPriority and show them up for the light bulb under the cursor OR should we allow every refactoring that is applicable for the span to show up? I think we should do the latter as it makes the lightbulb consistent with expectations - _show me all applicable quick actions at this position/token, I don't really care if it is a fix or a refactoring_. However, if we are worried about polluting the light bulb and do just the former, that also seems fine.
mavasani
on 31 May 2020
let's try the 'high priority' approach for now to see how that works out. It would make sense given how we specialize high-pri in other ways :)
CyrusNajmabadi
on 31 May 2020
It was decided in the design meeting to show all applicable Roslyn refactorings for the lightbulb under the caret. We can iterate over this to best meet the user expectations.
mavasani
on 2 Jun 2020
to show all applicable Roslyn refactorings
To clarify this: this means all refactorings that specify an applicability range as part of their result.
CyrusNajmabadi
on 2 Jun 2020
To clarify this: this means all refactorings that specify an applicability range as part of their result.
Yes, thanks for adding that!
mavasani
on 2 Jun 2020
To clarify this: this means all refactorings that specify an applicability range as part of their result.
Yes, thanks for adding that!
Thanks also... assuming this includes rename refactoring. :)
pvpxan
on 3 Jun 2020
Thanks also... assuming this includes rename refactoring. :)
Absolutely!
mavasani
on 3 Jun 2020
Is this planned for 16.6.x or 16.7.x?
pvpxan
on 3 Jun 2020
Is this planned for 16.6.x or 16.7.x?
16.7 Preview 3
mavasani
on 3 Jun 2020
@pvpxan Thanks very much for your feedback. Thsi will be fixed in 16.7.
CyrusNajmabadi
on 4 Jun 2020
@olegtk Is there a keyboard shortcut key to invoke the lightbulb menu under the caret?
mavasani
on 4 Jun 2020
Or, an "editor command" we can invoke for this?
CyrusNajmabadi
on 4 Jun 2020
Yes, View.QuickActionsForPosition, Ctrl+\ Ctrl+.
olegtk
on 4 Jun 2020
Perfect, thanks!
CyrusNajmabadi
on 4 Jun 2020
@olegtk @sharwell Any suggestions on how we can enhance the Roslyn integration test framework to have an integration test here?
Currently, it does the following to show the light bulb in the margin:
https://github.com/dotnet/roslyn/blob/b99a48b62648a2b1e24ec833a48cac997e6f94b8/src/VisualStudio/IntegrationTest/TestUtilities/InProcess/TextViewWindow_InProc.cs#L67-L80
I presume we need to use a different command ID to invoke View.QuickActionsForPosition, but I am not sure what that ID should be.
mavasani
on 9 Jun 2020
command set: VSConstants.CMDSETID.StandardCommandSet14_guid
command id: VSConstants.VSStd14CmdID.ShowQuickFixesForPosition
olegtk
on 9 Jun 2020
Thanks @olegtk. However, that still does not seem to be bring up the lightbulb under the caret and expands the lightbulb in the margin. At this point, I am not sure if this is some issue in our integration test framework or not. @sharwell I am going to assign https://github.com/dotnet/roslyn/issues/44855 to you for assistance in writing this integration test.
mavasani
on 9 Jun 2020
Related issues
MadsTorgersen
路
170Comments
davidroth
路
158Comments
alrz
路
125Comments
MadsTorgersen
路
542Comments
stephentoub
路
167Comments
Most helpful comment
Agreed. The charitable reading here is that it's 'bad' in that we didn't realize this would break when the change was made. This will be fixed. Thanks!