FxCop: How the Cop beat me down - A Review

Ok, so I have been talking about FxCop for a while, and during all my trails I really liked the tests it performed. Today I thought I would put FxCop 1.23 through our master sources of a product we are about to release. OMG.

Lets just say my C style of writing is not appreciated by the FxCop rules. At the start this morning, I had 588 violations reported by FxCop. Not a pretty sight.

But then I started reading their "Rules Details" to understand why it was flagged.

I am really impressed with the detail of the rules. In most cases I found the "Additional Information" quite useful, although I will admit the documentation does more to show examples on how to actually resolve the issues.

A majority of the violations failed in a single rule library. As I marshal a LOT of C style structs to P/Invoke native Win32 components as part of my kernelmode compatibility stuff, I guess this should be expected. I found that around 40% - 50% were errors because I would use underlines and capital letters (ie: SECURITY_DESCRIPTOR) which FxCop didn't like. I know I could name it to be more .NET friendly, but I like keeping the consistancy for the structs across the C# and C boundaries of my master sources tree. As such, I "excluded" quite a bit of violations to pass the test. These weren't actually false-positives... they were right. I just believe my style is more right for my needs.

I could have simply turned off that rule check completely... but it did find some internal structs which I could and did change. Something for you to consider: It is better NOT to supress a complete test. Don't be lazy.... analyze every violation and make an informed decision. I personally liked how you could attach a note to each violation you would exclude, allowing other developers to udnerstand WHY I excluded the violation from the tests.

FxCop found two security violations in my code which were focused on the fact I was using a virtual override to WndProc for two forms without setting the LinkDemand. I didn't even know WndProc had a Demand set to it... so it was good to see that FxCop caught it. I spent a great deal of time reviewing this in an attempt to figure out the LinkDemand status of the Control.WndProc... only to have someone point out to me an example where it sets the Demand state, which means I just had to add "[System.Security.Permissions.PermissionSet(System.Security.Permissions.SecurityAction.Demand, Name="FullTrust")]" to pass the test.

The rest of the violations I simply went and fixed. Most were .NET style issues which I am still trying to master/accept (ie:using a property instead of a method) and a lot of localization issues with CultureInfo/IFormatProvider.

After 2 and a half hours, all tests passed. 588 violations were reduced to 0... with about 200 in the exclude list. I will take some time to think about the impact of the naming conventions, and then go tackle the exclude list at a later date. Some things really should be renamed, but are left in so they match the C code in the kernel. (Example... .NET doesn't like a function name of OpenFIFOQue. I can pass that test if I change it to OpenFifoQue. Not a major change, but left in for clarity for anyone who is grepping through the sources to match up code against either side of the kernel)

Now, with such a great tool I did find some things I think could be improved upon. Most of it has to do with the user interface. There may be work arounds for many of this, I just don't know about them yet:

1. FxCop locks the assembly, which means I cannot simply run Visual Studio and FxCop together. A recent blog entry on their site says you can, but I couldn't get it to work. If I make a change and recompile the code, during a post-build event to copy the executable it fails. I tried closing the project, but keep FxCop open.. and even then for some reason the UI has it locked. I have to completely close FxCop down each time to properly recompile the code. It would be nice to keep both open, and then run "Analyze" after each rebuild.
   
2. Since I was constantly stopping and starting FxCop I was getting perturbed that it wouldn't remember the last project it was working with, and reload it. It would be nice if they had an option to "Load last Project on Start" or something like that.
   
3. I couldn't find a way to cmd line the FxCop tests directly in Visual Studio for post-build events. It would be kind of kewl if as a post-build event it could run FxCop and barf the violations to the output window.
   
4. When reviewing a violation, it has a field called "Source". I couldn't figure out how to get it to point to the offending line. I am guessing it has to do with some sort of debugging condition in the assembly, but I couldn't figure out how to make it show up.
   

When you think about it... those aren't big issues. Mostly comfort / style issues which are easy to overlook.

All and all I found FxCop a great tool to use. When I have some time I think I will learn how to write my own security rules, and then put them into FxCop for testing on a regualr basis. If I can figure out how to automate FxCop during postbuild events... I will even make it a daily build task.

Thanks to the FxCop team for their great work. Nice tool.

Posted by SilverStr at February 10, 2004 12:15 PM | TrackBack