Which are the "must follow" FxCop rules for any C# developer?

Question

I'm planning to start using FxCop in one of our ongoing project. But, when i tried it with selecting all available rules, it looks like I have to make lots of changes in my code. Being a "team member" I can't right away start making these changes, like naming convention change etc. anyway i would like to start using FxCop with a minimal rule set and would gradually increase the rule set as we goes on. Can you suggest me some must have FxCop rules which i should start following. Or do you suggest any better approach?

Note: Most of my code is in C#.

Answer 1

The minimal fxcop and also code analysis (if using VS2010 Premium or Ultimate) is the following: http://msdn.microsoft.com/en-us/library/dd264893.aspx

Answer 2

An alternative to FxCop would be to use the tool NDepend that lets write Code Rules over C# LINQ Queries (namely CQLinq). Disclaimer: I am one of the developers of the tool

More than 200 code rules are proposed by default. Customizing existing rules or creating your own rules is straightforward thanks to the well-known C# LINQ syntax.

NDepend overlaps with FxCop on some code rules, but proposes plenty of unique code rules. Here are a few rules that I would classify as must-follow:

• Avoid decreasing code coverage by tests of types
• Avoid making complex methods even more complex (Source CC)
• Avoid transforming an immutable type into a mutable one
• Overrides of Method() should call base.Method()
• Avoid the Singleton pattern
• Types with disposable instance fields must be disposable
• Disposable types with unmanaged resources should declare finalizer
• Avoid namespaces mutually dependent
• Avoid namespaces dependency cycles
• UI layer shouldn't use directly DB types
• API Breaking Changes: Methods
• Complex methods partially covered by tests should be 100% covered
• Potentially dead Types
• Structures should be immutable
• Avoid naming types and namespaces with the same identifier

Notice that Rules can be verified live in Visual Studio and at Build Process time, in a generated HTML+javascript report.

Answer 3

Some of the rules avoid us bugs or leaks:

• Do not catch general exception types (May be the best rule for us. According to the case, it can be easy or difficult to enforce)
• Test for NaN correctly (easy to enforce)
• Disposable fields should be disposed (quite easy to enforce)
• Dispose should call base dispose (quite easy to enforce)
• Disposable types should declare finalizer (quite easy to enforce)

Some help us have a better design, but be careful, they may lead you to big refactoring when central API is impacted. We like

• Collection properties should be readonly (difficult to enforce in our case)
• Do not expose generic list
• member should not expose certain conrete types
• Review usuned parameters (Improves easily your API)

Someone on our project tried the performance rules with no improvement. (Actually, these rules are about micro-optimizing, which gives no result if no bottleneck identification shows microoptimizing is needed). I would suggest not starting with these ones.

Answer 4

Turn on one rule at a time. Fix or exclude any warnings it reports, then start on the next one.

Answer 5

In our process, we enabled all the rules and then we have to justify any suppressions as part of our review process. Often, it's just not possible to fix the error in time-efficient manner with regards to deadlines or its an error raised in error (this sometimes occurs - especially if your architecture handles plug-ins via reflection).

We also wrote a custom rule for globalization to replace an existing one because we didn't want to globalize the strings passed to exceptions.

In general, I'd say it's best to try and adhere to all rules. In my current home project, I have four build configurations - one set that specify the CODE_ANALYSIS define and one set that don't. That way, I can see all the messages I have suppressed just by building a non-CODE_ANALYSIS configuration. This means that suppressed messages can be periodically reviewed and potentially addressed or removed as required.

What I'd like to do in the long-run is have a build step that analyzes the SuppressMessage attributes against the actual errors and highlights those suppressions that are no longer required, but that's not currently possible with my setup.

Answer 6

I fully agree with Sklivvz. But for existing projects, you may clean up FxCop violations category by category.

From time to time, Gendarme accepts new rules that are quite useful. So you may use Gendarme besides.

Answer 7

We're a web shop so we drop the following rules:

• Anything with Interop (we don't support COM integration unless a client pays for it!)
• Key signing (web apps shouldn't need high security prilages)

Occationally we'll drop the rule about using higher frameworks in dependancies as some of our CMS's are still .NET 2.0, but that doesn't mean the DAL/ Business Layers can't be .NET 3.5 as long as you're not trying to return an IQueryable (or anything .NET 3, 3.5).

Answer 8

On our most important code:

• Treat warnings as errors (level 4)
• FxCop must pass 100% (no ignores generally allowed)
• Gendarme used as a guideline (sometimes it conflicts with FxCop)

Believe it or not, FxCop teaches you a hell of a lot on how to write better code... great tool! So for us, all rules are equally important.

Answer 9

In my opinion, do the following:

For any new project, follow all FxCop rules. You may want to disable some of them, since not everything will make sense for your project. For an existing project, follow the rules from these categories as a minimum set:

• Globalization
• Interoperability
• Security
• Performance
• Portability

Since these are typically only few rule violations in an existing project, compared to the other categories, but may improve the quality of your application. When these rules are clear, try to fix the following categories:

• Design
• Usage

Since these will make it easier for you to spot bugs that have to do with the violations, but you will have a large amount of violations in existing code.

Always sort the violations by level/fix category and start with the critical ones. Skip the warnings for now.

In case you didn't know, there's also StyleCop available from Microsoft, checking your code on the source level. Be sure to enable MSBuild integration during installation.

Answer 10

The design and security rules are a good place to start.