Checking out the Source Analysis for C#

I’ve just spent about 30 minutes checking out the recently announced Microsoft Source Analysis for C#. I definitely love the idea, but I’m not sure I’ll be using it just yet. Here are some of my impressions (again, I haven’t spent a lot of time on it yet)…

All using directives must be placed inside of the namespace

Hmm, but VS itself violates that rule on every class we create…!

Fig01

The property must not be placed on a single line. The opening and closing curly brackets must each be placed on their own line

Huh? But that an auto-implemented property. What would we want to split that into multiple lines?!

Fig02

Variable names must not start with ‘m_’

Hmpf, that’s the standard with have here at the company…

Fig03

A closing curly bracket must not be preceded by a blank line

Ok, this one I like.

Fig04

Tabs are not allowed. Use spaces instead

Huh? Thanks, but no, thanks. I’ve seen a lot of people complaining online about this one too. What’s the problem with using tabs??

Fig05

The call … must begin with ‘this.’…

Ah, I like that one. This is something we’ve made as part of our guideline here, but couldn’t use FxCop to trap for that, since the ‘this.’ doesn’t make into the IL.

Now, the problem here is that, again, the code that VS generates violates this rule all over the place, such as on the example below:

Fig06

I’ll continue to use this tool to see if it’s going to stick with me. I’ve tried it on a very small project, and got 246 violations; the vast majority where produced by code generated by VS. On a big project, I’d probably end up with a LOT more violations, so I’m not sure I can live with that. I believe there is a way to disable some of the rules, but haven’t looked for it just yet…

Anyway, I do appreciate Microsoft releasing tools like this one. Hopefully, more and more developers will start paying more attention to how they write code.

  1. #1 by Serge on May 28, 2008 - 3:37 pm

    Hi Claudio,Thank you for the great review. I too advocate for developers to adopting good coding style.I wonder if you like our CodeIt.Right tool – http://submain.com/codeit.right – it is more efficient as it can automatically correct violations and code smells for you.Please feel free to contact me if you have any questions or are interested in a review copy.

  2. #2 by Serge on May 28, 2008 - 3:52 pm

    Holy crap!
     
    Sorry about the duplicate comments … don\’t know how they came up. I only submitted once. Apparently Live Spaces have issues with Firefox… Any way you could delete the duplicate comments? I don\’t seem to find a way to delete my own.
     
    Sorry about the inconvenience again…

  3. #3 by Unknown on May 30, 2008 - 2:27 am

    "What\’s the problem with using tabs??"You can change how many spaces wide a tab looks in the IDE.  That\’s the problem.  are they 2 spaces, 4, 8, or what?  Visual Studio defaults them to 4, but meanwhile, most diff tools display them at 8.As soon as tabs mix with spaces, then formatting goes to hell.That said, that\’s the only one of these rules that I agree with.

  4. #4 by Don Awalt on May 30, 2008 - 8:11 am

    Be wary of this add-on. When I added it to my Windows Server 2008 dev system for VS 2008, when I went into Project properties a couple of days later, I received an error message "COM object that has been separated from its underlying RCW cannot be used" and nothing showed up on any of the Properties pages! I un-installed this add-on and the problem went away.Not worth the hassle, besides I too got a bunch of really un-useful reports. Very hard to find the gems in all the fluff.

  5. #5 by Jane on July 8, 2008 - 2:54 am

    In the Detailed Settings pane you can choose to switch off "Analyze designer files" and "Analyze generated files" which I found help remove the number of warnings. 

  6. #6 by Alfred on July 8, 2008 - 2:36 pm

    As a matter of fact, "this" ALWAYS makes its way into IL.
    private string foo;

    public string Foo
    {
    get
    {
    return foo;
    }
    set
    {
    this.foo = value;
    }
    }
     
    gets translated to
    method public hidebysig specialname instance string get_Foo() cil managed{    .maxstack 1    .locals init (
    [0] string CS$1$0000)    L_0000: nop     L_0001: ldarg.0     L_0002: ldfld string ItauTests.Program::foo    L_0007: stloc.0     L_0008: br.s L_000a    L_000a: ldloc.0     L_000b: ret }

    .method public hidebysig specialname instance void set_Foo(string \’value\’) cil managed
    {
    .maxstack 8
    L_0000: nop
    L_0001: ldarg.0
    L_0002: ldarg.1
    L_0003: stfld string ItauTests.Program::foo
    L_0008: ret
    }

    arg.0 is "this"If you use .NET Reflector you\’ll get:public string Foo
    {
    get
    {
    return this.foo;
    }
    set
    {
    this.foo = value;
    }
    }
     

  7. #7 by Lex on July 28, 2008 - 9:33 pm

    It is rational to use spaces because not everyone uses the same tab settings in his/her IDE. In my case, when I navigate source code files on CodePlex.com, tabs are misaligned (CodePlex web page uses more spaces than 4).
     
    Also I find it a must to rurn off some rules (persoanlly I hate "this", and ReSharper hates it, too) on legacy projects. Otherwise, over 10,000 warnings may be provided by StyleCop.

  8. #8 by neslihan on July 30, 2008 - 1:48 pm

    very good . Thanks

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

%d bloggers like this: