Upgrading Code — Tackling Code Analysis in Visual Studio 2010 (Part 1)

Please note that this is all base on a non-RTM build of Visual Studio 2010, so you may get different mileage out of the final product.

Work has started on creating a version of the MSBuild Extension Pack targeted at .net 4.0 which comes with Visual Studio 2010. There are two big changes when it comes to VS2010, Rule Sets & new rules.

Version of the Extension Pack compiles with zero Code Analysis violations in Visual Studio 2008. There are suppressions in the code, but a lot of effort has gone into understanding what has been flagged and writing compliant code. The branch is based off the code base, so I was eager to see what the Code Analysis built into VS2010 would report.

When the projects were upgraded to VS2010 projects, they each received a “Migrated rules for [ProjectName].ruleset” file. This makes sense if you have crafted a subset of rules to use, but we use them all on the Extension Pack, and want to take advantage of the new rules; so all these files were deleted and each project configured to use the ‘Microsoft All Rules’ rule set.

That’s the easy part. Now lets build and see how bad it is….

1367 errors. Ok, so I was expecting a few errors, but not that many! Either we’ve found a way better way to write code, or there are some over zealous new rules…

A quick scan of the results shows that there are a lot of

  • CA2204: Literals should be spelled correctly (a new rule)
  • CA1303: Do not pass literals as localized parameters (a new rule)

For CA2204, it’s picking up just about every parameter used in messages. We could add a CA dictionary, but I don’t think there will be much benefit. Let’s suppress the messages initially and look into disabling the rule or implementing the dictionary at a later stage.

For CA1303, it’s a Globalization issue. Given more time and resource Globalization might make it up the priority list, but right now its pretty near the bottom. This rule will be disabled, so let’s edit the ‘Microsoft All Rules’ rule set and create a shared MSBuild.ExtensionPack.ruleset file. This file has the CA1303 rule removed and all rules set to error rather than warn. All projects are updated to point to the new rule set file.

Rescanning the code brings us to 87 errors. So we have cleared the ‘noise’! We are a fair way from zero still with the 87 falling into the following rules:

  • CA1062: Validate arguments of public methods (a new rule, 4)
  • CA1305: Specify IFormatProvider (3)
  • CA2000: Dispose objects before losing scope (a new rule 63)
  • CA2100: Review SQL queries for security vulnerabilities (a new rule 2)
  • CA2202: Do not dispose objects multiple times (a new rule 15)

So if VS 2008 had CA1305, why didn’t it pick up the errors? Well the error detected is not that IFormatProvider has not been specified, but that the wrong provider has been specified. So that’s good news. Not only do we have new rules, but the old rules have been improved too.

The ‘CA1062: Validate arguments of public methods’ and ‘CA1305: Specify IFormatProvider’ errors are easy enough, but the other three are worth looking at in more detail in Part 2.

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 )

Google photo

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

Twitter picture

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

Facebook photo

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

Connecting to %s