Code cleanup

Article
12/04/2004

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.

Comments

Anonymous
December 04, 2004
This looks like a pretty cool feature! Especially the "Simplify type names" would be something that I would be using a lot.

I hope it makes it for Whidbey or as a power tool...
Anonymous
December 04, 2004
The only issue I have with lots of warnings (using VS 2003) is that actual errors can be hard to find among the 20 to 30 Obsolete warnings in the project.

I like Simplify and Sort a lot. I actualy spend time sorting and grouping the usings.

For Remove, you might want to consider just putting them in comments so we don't have to re-type them if we need them again.
For #if DEBUG | RELEASE : before showing a dialog, maybe make a second pass with them swapped, only showing the dailog with non-standard defines.

When I'm pasting code in, it's often from MSDN samples. It is devoid of using statements and does not have full names like System.Collections.Generic.IList.
How about a "Suggest usings" that does a quick search for matching class names in the project?
Anonymous
December 04, 2004
Frederik: Could you give me examples of how you would be using it and why you would be using it a lot? It will help when explaining to others if I can site specific examples of how customers like you would find it valuable. Thanks!
Anonymous
December 04, 2004
Andrew: In 2k5 we made it pretty simple to add a new using to a file. Just type something that needs the using and we'll offer to add it automatically. i.e. if you type 'List<int>' we'll offer to add "using System.Collections.Generic". So i don't think that commenting out "usings" would be that helpful.

The DEBUG | RELEASE idea is possible and i'll look into it. (although it feels icky to special case like that).

---

BTW, about the warnings, that's why we wanted a new "Information" classification. That way you could hide all warnings in the task list and jsut see the information tidbits.
Anonymous
December 04, 2004
These are great features. I'd love to get my hands on these in Whidbey or as soon as possible after that. I think the best would be "Remove unused usings". I've actually been wanting this for a while - especially useful in simplifying code that's gone through heavy refactoring.

I'll vote for it!

I'm glad to hear about the suggest usings that Andrew had asked about.
Anonymous
December 04, 2004
I think this is a generally useful addition. My current editor supports organizing (expanding, collapsing, sorting, removing unused) type imports and I get a lot of use out of it. Since it's so easy to just do the right thing in most cases, I agree it is best to just offer to fix it rather than providing code critics telling you about the problem. Something missing from your description that I currently use is the ability to do organize all files with one click. In VS this would probably mean all files in the project; in my editor it means all open files.
Anonymous
December 04, 2004
Wow, these are great!

Please try to get these into Whidbey, or as Frederik said, a power tool.
Anonymous
December 04, 2004
The comment has been removed
Anonymous
December 04, 2004
The ReSharper from JetBrains gives you the delete/simplify option today, in VS2003.
http://www.jetbrains.com/resharper/

Another code cleanup idea is to wrap methods with the same prefix in regions.

Ariel
Anonymous
December 04, 2004
Resharper is slow and ugly.
Anonymous
December 04, 2004
Wow, neat feature.

I do, however, have a request: That either Ctrl+Z will undo all changes made by a click on an option or that there would be a dialog showing the changes about to be made (with a 'never show this dialog again' checkbox) or both (preferrably both).

And another thing - If I only use System.Collections.Generic.IList<System.DateTime> once in my code, but use stuff from System a lot, I can understand why you would add a 'using System;' clause, but why would I want to burden my 'global namespace' with everything in the System.Collections.Generic if I only refer to something in it once?
Anonymous
December 04, 2004
While, this is a nice feature, I would much prefer to have incremental compilation for C#.

I think Eclipse does a fantastic job here, and VS is clearly lacking. I do realize that there are perf considerations once the code base becomes huge, but these can be addressed through some advanced options.

It would have been awesome if incremental compilation made it to Whidbey :(; I just hope that it appears in Orcas.
Anonymous
December 04, 2004
The comment has been removed
Anonymous
December 04, 2004
Senkwe: MSPaint was only to generate the icon for the menu option. These are actual screenshots and the features work as advertised (without testing and verification that they work in all cases of course).
Anonymous
December 04, 2004
Omer: "I do, however, have a request: That either Ctrl+Z will undo all changes made by a click on an option or that there would be a dialog showing the changes about to be made (with a 'never show this dialog again' checkbox) or both (preferrably both)."

Ctrl-Z will always undo the last action you performed, and it does behave as expected in this case.

We could utilize the "refactoring preview changes" dialog in this case, but i was shooting for a very lightweight feature here.

"And another thing - If I only use System.Collections.Generic.IList<System.DateTime> once in my code, but use stuff from System a lot, I can understand why you would add a 'using System;' clause, but why would I want to burden my 'global namespace' with everything in the System.Collections.Generic if I only refer to something in it once?"

You won't be burdening yourself. We don't automatically add "usings" for you, you must explicitly do it yourself. So if there is no "using System.Collections.Generic" then you'll be fine.
Anonymous
December 04, 2004
The comment has been removed
Anonymous
December 04, 2004
Harry:
" While, this is a nice feature, I would much prefer to have incremental compilation for C#."

Me2. But unfortunately incremental compile is something that would take us weeks (if not months) to do, whereas this feature took me 2 hours :)

"I think Eclipse does a fantastic job here, and VS is clearly lacking. I do realize that there are perf considerations once the code base becomes huge, but these can be addressed through some advanced options."

Agreed. We are well aware of the benefits of incremental compile and are seriously considering it against all the other work we'd like to do.

"It would have been awesome if incremental compilation made it to Whidbey :(; I just hope that it appears in Orcas."

I hope it does too :)
Anonymous
December 04, 2004
Nicholas: "Something missing from your description that I currently use is the ability to do organize all files with one click. In VS this would probably mean all files in the project; in my editor it means all open files."

Yes, this is true. However, these will just be standard commands (like "Edit.RemoveUnusedUsings") so it would be trivial to write a macro that did something like:

foreach (Project p in dte.Solution) {
....foreach (File f in p.Files) {
........f.ExecuteCommand("Edit.RemoveUnusedUsings");
....}
}

(i'm not sure of the syntax, but it would be something like that).
Anonymous
December 04, 2004
Ariel: "Another code cleanup idea is to wrap methods with the same prefix in regions."

I don't really like the sound of that. it seems kinda limiting to just do it based on method prefix. I think i'd prefer an extensible model here where you have more flexibility to specify what code goes in what regions.
Anonymous
December 04, 2004
Cyrus, cool ideas. I do have one suggestion though... perhaps you could make the 'Sort' feature a little more general. Perhaps it could sort the currently highlighted block of code instead of just being used for Using statements. I often spend (waste?) time copying and pasting my WinForms event procedures to get them listed in alphabetical order. Being able to simply highligh the code (while colasped to definitions) and clicking 'Sort' from a context menu to do the same job would be way cool.

Typically, my classes look something like this:

public class Whatever {

/Private field variables go here/

/Constructor(s) go here/

/Private properties go here/

/Public properties go here/

/Private methods go here/

/Public methods go here.

/Event procedures go here/

/Display and/or Finalize go here/
}

Being able to sort each 'section' of code using this new Sort feature would save me a lot of time.

Thats my $0.02 worth.
Anonymous
December 04, 2004
Len: I absolutely agree and this was something we were considering for whidbey but didn't have time for.

It's basically just a tree sort and having access to the parse tree and semantic information and having the former be mutable would make it pretty trivial to do exactly what you want. Hopefully we'll be able to get that in for Orcas.
Anonymous
December 04, 2004
Len: I absolutely agree and this was something we were considering for whidbey but didn't have time for.

It's basically just a tree sort and having access to the parse tree and semantic information and having the former be mutable would make it pretty trivial to do exactly what you want. Hopefully we'll be able to get that in for Orcas.

At that point all you'll need is to implement an IComparable<Node> object that will do the custom sorting that you want.

Of course we would want to provide some intuitive UI so you could do a lot of the setup from the IDE. But in the end if you wanted maximum flexibility then that would be available to you.
Anonymous
December 05, 2004
Would this be something helpful to you? (Yes, I could definitely see using these features)

Do the featuers sound useful in their current incarnation, or would you like to see them behave differently?

(Sounds good at this point. Most likely can't give useful feedback until I get to use the features in a beta).

Thanks!
Anonymous
December 05, 2004
The comment has been removed
Anonymous
December 05, 2004
The comment has been removed
Anonymous
December 05, 2004
Out of interest what would the behaviour be when simplifying typenames when naming conflicts occur, such as:

System.Web.UI.WebControls.Calendar c1;
System.Globalization.Calendar c2;

Would one be simplified, would both be left unchanged?
Anonymous
December 05, 2004
Alex: "Simplify Type names" will only do so when the new names means exactly the same thing as the old name.

So in your case it depends on the "usings" you have. I'll walk through the cases:

a) You have no usings. Then no change will be made
b) you have a "using System.Globalization". Then the first type name will stay the same, the second will be simplified to "Calendar"
c) You have both "using System.Globalization" and "using System.Web.UI.WebControls". Then no change will be made because it would change the meaning of your code since "Calendar" would be ambiguous.

Does that make sense?
Anonymous
December 05, 2004
I guess I'm wondering about the overal usefulness of this feature. What is the penalty for having unused usings? If this is such an issue that we need a special tool to remove them, then why do the default project wizards in VS.NET include references to certain assemblies like System.Xml or System.Data that I may never use? My understanding is that assembly references that were in the references list, but never actually used weren't included in the manefest, so no harm no foul. Doesn't this fall in the same category?

I guess I would think there are other issues that are much more worth your time than this.

By the way, don't ever, ever do incremental compilation for C#... That's one of the things that I hate about VB.NET. I think that if you were to observe C-style programmers (vs. VB-style) that you would find that the majority of them write their brand new code ahead of the compiler... calling functions and classes that don't exist yet, and find incremental compliation to be more of a burden then a help.
Anonymous
December 05, 2004
Cyrus I really like this post and these Ideas I really hope you can get it in. I use resharper specifically for this, Yeah shortcuts and refactoring are also a big bonus but cleaning up the using are outstanding.
Anonymous
December 05, 2004
Very cool ideas. +1 for releasing it as a power tool if it can 't make it into the release.
Anonymous
December 05, 2004
Nick: There is no penalty except for added conceptual confusion. Some people ask themselves "why is this using here, do i need it?". Other's prefer code to be as tidy as possible and want to remove as much extraneous information as possible.

The default projects include certain things like System.Collections.Generic because we find that people are always using collections and thus it helps them since the types from those usings will now be in the completion lists.

"I guess I would think there are other issues that are much more worth your time than this. "

Of course. But i did this on my own time jsut for the fun of it.

"By the way, don't ever, ever do incremental compilation for C#... That's one of the things that I hate about VB.NET. I think that if you were to observe C-style programmers (vs. VB-style) that you would find that the majority of them write their brand new code ahead of the compiler... calling functions and classes that don't exist yet, and find incremental compliation to be more of a burden then a help. "

I don't see how increment compilation hurts you here. Could you expand on this? What's wrong with the compiler saying "you're calling a non-existent method, would you like me to generate it for you?"
Anonymous
December 06, 2004
Cyrus: I prefer keeping type names short. I prefer to see "SqlConnection" in my code over "System.Data.SqlClient.SqlConnection", for example. However, it happens to my that I do use the fully qualified name of a type, only to find out later that I use that specific type or more types from the same namespaces quite often in the same class. That's where the Simplify Type Names comes in handy.

Another use scenario is when you copy and paste code - example code, for example, might be using full type names a lot. I did spend time simplifying type names before.

Whidbey also has the code snippets tool. I tend to use full type names in snippets, because, well, they can end up anywhere in my code and I don't want to bother about the 'using' statements at that time. The Simplify Type Names comes in handy here as well.

Does that help you?
Anonymous
December 06, 2004
Frederik: Thanks! That helps a lot.

---

"Whidbey also has the code snippets tool. I tend to use full type names in snippets, because, well, they can end up anywhere in my code and I don't want to bother about the 'using' statements at that time. The Simplify Type Names comes in handy here as well. "

We addressed that problem in whidbey. There is a SimpleTypeName function that you can use with snippets. It's how we make sure that we'll dump the best name for global::System.NotImplementedException into your code.

Check out any of the refactoring snippets and you'll see how to do this.
Anonymous
December 15, 2004
I would love to see this in Whidbey or as part of the TweakC# tool that you mentioned earlier.

How would we use it? Today I'm oftening tidying up using statements where the code has been updated, so the namespace is no longer in use - but the using statements remain.

In addition no one of my other co-workers are pedantic enough to care about the order using statements.
Anonymous
September 02, 2005
u should give us more info
Anonymous
November 22, 2005
In VS 2005 Relase version, i have found "Sort usings" in "Tools->Customize" in "Edit" category. But when i add the icon in the standard toolbar, it is always disabled ! Have you finally activate these tools ?
Anonymous
February 09, 2006
It's definitely a shame to see that the feature was only half added to the IDE :( A small feature but a very welcome one it would have been!
Anonymous
January 21, 2009
PingBack from http://www.keyongtech.com/2320265-sort-usings-command-is-always
Anonymous
June 02, 2009
PingBack from http://indoorgrillsrecipes.info/story.php?id=13114

Partager via

Code cleanup

Comments

Ressources supplémentaires