Code cleanup

I while back i wrote a post concerning what i called Informational Observations.  These were snippets of information that the compiler could give you to help you work in a code focused way.  For example, one observation we could make for you would be to tell you if you had a "using" statement that you weren't actually using.  We considered this to be an observation, as opposed to a warning, because during active code development one often writes speculative code to help enable some later bit of work and you don't won't to be bothered over it.  In the "using" case you might be including a set of usings because you know that in a few minutes you're going to need some type from the imported namespace and you won't want to have to leave where you're currently typing to go add it.

Initially the thought was that this would be a change to the compiler.  Along with "Errors" and "Warnings" we would add an "Information" class for messages.  However there was much debate about whether or not the compiler was the right place for such a thing.  There's a lot of feeling that there should be a core compiler whose job it is to just take code and compile it and nothing more.  Analysis should be done by seperate tools (like FxCop) since that's their main focus and they aggregate all those tests into one logical place instead of having some analysis done in tool Foo, some in tool Bar and some in tool Baz.  However, there's a problem with that right now.  In order for a tool like FxCop to determine that "usings" are unused it needs access to a level of semantic information that is currently unavailable to it.  FxCop operates solely on the IL level, and even if it could operate on a semantic graph it has no way to get access to it since our compiler doesn't expose it.

So we have a dillemma.  There are tools that are suited for doing this kind of work, but they need information from us.  We have the information to do it, but then it muddies what the roles of our tools are.  There are also interesting issues that come up wrt signal-to-noise ratios.  If we suddenly turn on this informational observation and you run this over your 5 million line codebase you're going to get a heck of a lot of hits and it could be extremely annoying if you don't care about it.  We could have it as a warning that's disabled by default, but that goes against our philosophy that all warnings should be enabled.   It's also a problem since users almost never change defaults.  So why add the feature if it's probably not going to end up getting used?

Finally, after discussing these "observations" a bit we realized that for the most part users wouldn't care much about what the actual observation was they'd just want to get the issue fixed.  It's not very useful to get 10,000 messages about unused usings if you then have to go fix them all up yourself.  So rather than actually giving the "observations" why not just give the tools to fix them for you?  In that regard i decided to go ahead and see what that would look like in this specific case.

Right now it looks like this when you right-click on a file:

As you can see there's a new menu underneath the now familiar "Refactor" submenu.   I think the names aer pretty self-explanatory but i'll go over them:

The first is "Simplify type names".  It will go through the entire file and for every location where you have a qualified type name (like System.Collections.Generic.IList<System.DateTime>) it will shorten it to the smallest possible type name given the usings in your current file (which in this case could result as: IList<DateTime>).  I picture the primary use for this to be when you paste in code or generate a whole bunch of code automatically (like through implement interface) and it contains a lot of fully qualified names because you don't have the appropriate usings in place.  Once you get that "using" in place (say with the "add-using" smart tag), then you can just run this and it will clean up all that code for you.

The second is "Remove unused usings".  It's also pretty basic.  It will go through the entire file and see which "usings" were used so that types could be bound successfully in your current file.  Any that ended up not being used are then removed.  Interestingly enough this has several potential consequences.  For example, if you have inactive preprocessor regions (i.e. #ifdef debug) then we won't be able to tell if your usings have any effect within those regions and we might end up removing them and breaking your code when you switch #defines.  There are also issues if your code is not in a buildable state.  If you have unparsable code then we might not realize that you're using a type inside that bad code and thus we won't realize that a "usings" is used.   I think in both cases I'd probably want to pop up a dialog saying "You have inactive preprocessor regions.  Removing usings could cause your code to no longer build in certain configurations.  Do you want to continue?   Yes/No/Don't ask me about this again"

The third is "Sort usings".  Pretty simple stuff.  We place extern aliases first sorted by the alias name, followed by using namespaces, followed by using aliases sorted by the alias name.  To sort the namespaces we go piece by piece over the namespace name and sort it lexicographically.  If the first name in the namespace is "System" then we check an option to see if we should be special-cased and placed in front of the other usings.  I'm also considering adding an option to place a newline between groups of usings based on how much they have in common.  So, you could say "group all usings together that share the first name in common".  This would group all usings that start with "System" together, and all usings that start with your own namespace name together with a newline bettween the two groups.   Sorting was also interesting because you need to consider how comments are dealt with in code and what should happen to them if you start moving code blocks around.  I think the solution found will be pretty intuitive and will work for pretty  much all cases.

Finally there's an option to perform all three organizations.  That was included because as i started using this feature on my own code I started noticing that i was performing them all at the same time and it was quite annoying to have to invoke each command manually.  So i was thinking about just getting rid of the first three options... but i felt they still had value and there would be users out there who would definitely want to do something like "removed unused usings" but would never run "simplify type names".

I doubt we would be able to ship this in Whidbey, but i hope that there will be some way to get these features to you in a timely manner.  Would this be something helpful to you?  Do the featuers sound useful in their current incarnation, or would you like to see them behave differently?


Funny fact: It took longer to create the icons for the menu items than to actually write the code for the features.  It's tough trying to convey meaning when you only have 16x16 pixels to work with and you're futzing around with MSPaint.  I'm used to having gobs of memory and near unlimited CPU at my disposal, so working with such a small canvas was quite ... constraining.