How stringent should I be with Code Analysis compliance in Visual Studio?

Question

After playing with Code Analysis for a small project I am working on, I am wondering just how severe I should be when resolving code to be analytically compliant.

I know I can suppress warnings for this, but to me, suppressing a warning to some extent is a Cop-out (no pun intended..."FXCop").

Example warning:

Do not raise exceptions in unexpected locations 'CustomObject.Equals(object)' creates an exception of type 'ArgumentException'. Exceptions should not be raised in this type of method. If this exception instance might be raised, change this method's logic so it no longer raises an exception.

Reason for throwing this...

CustomObject.Equals(object) might try and compare CustomObject to FooBarObject...which aren't even of the same type, so in this instance, should I throw an exception, or just return false?

---

In general, should I be really anal (for want of a better word) in making my code absolutely compliant, or will I come across situations where warning suppression will become necessary?

Answer 1

FxCop warnings are just warnings, they don't flag invalid code. That's the job of the compiler. The rules FxCop uses were collected from years of experience writing .NET code. They represent "best practices" and in general are there to remind you of unintended consequences and the more obscure parts of .NET programming, like CAS.

Always refer back to the documentation to see why the rule exists. For CA1065 you'll see:

An Equals method should return true or false instead of throwing an exception. For example, if Equals is passed two mismatched types it should just return false instead of throwing an ArgumentException.

Which exactly matches your usage, you'll have no trouble adopting the advice. Unfortunately it is a bit short on the exact reason the rule was created. Which really doesn't go beyond the "don't throw in unexpected places" guidance. The unintended consequence here is that another programmer that uses your class won't realize that a try/catch would be needed if he doesn't want the code to fail. Feel free to put a Debug.Assert() in your Equals method. There are plenty of cases where you'll want to ignore the advice, CA2000 is particularly prone to false warnings for example. Apply the [SuppressMessage] attribute if necessary to not have to look at it again.