Friday 26 September 2008

Why FxCop is essential

As part of my recent work configuring an automated build server I was asked to ensure that FxCop was run as part of the process. I have had, in previous jobs, experience of FxCop in it's integrated form (as the code analysis tool in Visual Studio Team System). But I have to say it was something that was never used that often and I have to admit most of us ignored what it was saying.

We decided to aim to reduce the FxCop errors in our build report to zero. It seemed a little harsh at first, but I think this is going to seriously help our coding standards.

First off (for all you doubters out there), FxCop is not always right. There are occasions that it is unhappy about something that cannot be changed. For example, we use NUnit for our unit tests and FxCop loves to complain that many of the test methods should be static methods (as they don't reference anything in the class). However, if you convert these to static methods NUnit fails.

Luckily FxCop provides a means to ignore methods that are breaking rules. This is how you do it:
  1. In Visual Studio, open up the .NET project properties and select the Build tab. In the conditional compilation symbols box add CODE_ANALYSIS.
  2. Add using System.Diagnostics.CodeAnalysis; to the code file.
  3. Above the method that is giving the error add a [SuppressMessage] attribute, referencing the FxCop rule assembly, The rule Id/Name and a Justification (this is optional but I strongly recommend this is used so that future developers will know why this isn't being checked). Below is an example from one of my NUnit test files to avoid the previously mentioned static problem:
[SuppressMessage("Microsoft.Performance", "CA1822:MarkMembersAsStatic",
Justification = "FXcop doesn't realise that this Nunit test method cannot be static")]

You can also turn off particular rules from the FxCop configuration tool, or as a command line argument (e.g. /ruleid:-Microsoft.Design#CA1014 The minus before the rule name indicates that this rule should not be used).

This suppression is useful, but don't just suppress every warning and error. This will lose any benefit of running FxCop!

So, what sort of genuine errors has FxCop found? Many are small things like picking up incorrect casing in names of methods, parameters and other class members. These may seem insignificant to the project as a whole but they are key to future developers understanding the project.

Some of the more interesting errors that FxCop has returned have involved Globalisation issues. I have to admit that this is something that is often missed in project development. For example a recent FxCop report noticed that I created a DataTable without specifying a Locale... so what you might say. However, if this isn't set to CultureInfo.InvariantCulture it could effect any sorting performed on this DataTable by users in different locations!

In conclusion, we should be using code analysis tools like FxCop. We may think that our code is perfect, but as the analysis proves, it often is not.

No comments: