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
18.71k stars 3.98k forks source link

Add Using refactoring is not available #25548

Open AmadeusW opened 6 years ago

AmadeusW commented 6 years ago

Version Used: Microsoft Visual Studio Community 2017 Version 15.6.2 VisualStudio.15.Release/15.6.2+27428.2005 C# Tools 2.7.0-beta3-62707-11. Commit Hash: 75dfc9b33ed624dff3985c7435c902c3c58c0e5c

Steps to Reproduce:

  1. Create a new console app
  2. Paste var x = new System.Diagnostics.BooleanSwitch("param1", "param2"); into main
  3. Hit Ctrl+.

Expected Behavior: Add Using refactoring is available via lightbulb \ paintbrush \ popsicle icon 😉

Actual Behavior:

No refactoring available image

lightbulb works otherwise. Just not to Add Using image

CyrusNajmabadi commented 6 years ago

What using do you expect to be added?

AmadeusW commented 6 years ago

I expect using System.Diagnostics and using SixLabors.ImageSharp.MetaData; to be added

jmarolf commented 6 years ago

@AmadeusW I see, so you want us to offer add usings so that you can simplify your code. This is not how the feature works today. If the user writes BooleanSwitch or ImageProperty we offer the add usings so that the code compiles. If you write System.Diagnostics.BooleanSwitch we assume you want to be explicit here.

I see no problem offering this as a refactoring, though I think we'd probably want to offer a code style option here as some people won't like a refactoring offering to change the code when its already how they like it.

I also don't know how we should handle cases where this refactoring is not possible (due to naming collisions). If we don't offer it in places where it would break there code it may look like the feature isn't working. Perhaps a complete version of this refactoring would also offer to add an alias.

CyrusNajmabadi commented 6 years ago

Agreed. This would be an interesting and nice feature to have. Effectively, simplify the name, then add the using to make the simplified name ok.

I also don't know how we should handle cases where this refactoring is not possible (due to naming collisions).

Currently, we don't care. We'll already add usings today that cause name conflicts. So far, it hasn't been enough of a problem to need us to do anything about it.

though I think we'd probably want to offer a code style option here as some people won't like a refactoring offering to change the code when its already how they like it.

Yeah, i am a little bit worried about things being too noisy. That said, it doesn't seem like it would be too bad. At worse, you have a person that fully qualifies a bunch of stuff. All that happens is they get the screwdriver in a bunch of places. My expectation is that, honestly, with all the features we have, users will almost always have the lightbulb/screwdriver. So it's just a constant presence, and adding more options doesn't really degrade anything.

AmadeusW commented 6 years ago

I was certain there is a refactoring that simplifies a full type name. It must be active only when the relevant using is already present in the code! In this case, that's my bad - I thought I stumbled across a regression, while this really is a feature request.

IMO, from the user point of view, it shouldn't matter whether using is present - I simply want Ctrl+. to refactor the code.

CyrusNajmabadi commented 6 years ago

IMO, from the user point of view, it shouldn't matter whether using is present - I simply want Ctrl+. to refactor the code.

Fair. Still a feature request :D And you're the first person to ever ask for it. Want to write it for us?

AmadeusW commented 6 years ago

I kind of do :) I won't commit to anything but I'll look into this in spare time

CyrusNajmabadi commented 6 years ago

Cool. If you do, i'd happily review it. Also, if you need pointers on where to start, we can def provide that.

MisinformedDNA commented 5 years ago

I never thought about this, but I like it. I'm always cleaning up my code by deleting the namespace qualifier, and then triggering the refactoring so it adds the using statement. This would reduce the keystrokes needed to accomplish this.