April 2007 - Posts

If you've decided to try to tackle an FxCop violation backlog, one of the first issues you're going to face is deciding which rules to activate when. Here are some general guidelines...


Starting out

When you first begin the backlog clean-up process, you're going to need to introduce the FxCop tool to your team (assuming, of course, that you're not already using FxCop for new projects). In order to focus on mastering the tool and the cleanup process before diving into "difficult" rules, you'll want to pick a few rules to activate that meet the following criteria:

  1. The rule itself should be easy to understand with minimal background explanation.
  2. Violations of the rule should be easy to fix.
  3. Your code should contain very few violations of the rule.

My favourite introductory rule is DoNotConcatenateStringsInsideLoops. Unfortunately, it's slated to disappear in Orcas, so you might want to get a start on that backlog cleanup soon if you want to use it. ;) For finding other candidate "early" rules, the best approach would generally be to count your violations per rule, then look for any rules with a violation count of between approximately 5 and 20 that also meet the "easy to understand" and "easy to fix" criteria.


If it ain't broke, it don't need fixin'...

Another thing you'll want to do fairly early in the process is activate rules for which you have no existing violations. Even though this lock down might seem very important with respect to preventing additional backlog problems, it should probably wait until you're a few weeks into the cleanup process, and the team is starting to get reasonably comfortable with the whole thing. Don't worry too much about this delay causing new rule breaking -- if you've gone this long without breaking those rules, chances are pretty slim that you'll break them in the next three weeks or so. ;)

Simple as it may seem, there are still a few things to consider as you select "no violation" rules to activate:

  1. Some of these rules will have no violations because your team has been actively avoiding some problem. Take this opportunity to highlight that they've been doing the right thing and confirm what that "right thing" is since newer team members may not be aware of it.
  2. Some of the rules will have no violations because your project doesn't touch on the area covered by the rule. For example, if you don't interact with native APIs, there are several rules around p/invoke calls that you won't have broken. Given that your team is unlikely to start breaking these "irrelevant" rules, you can safely activate them without too much explanation. However, you should at least mention that you are activating the rule, give a quick description of its significance, and explain why it's not likely to be violated in the target code base. This approach will ensure that the team is aware of the rule should it eventually become more relevant due to changes in the project scope.
  3. Some rules (but not very many) will have no violations almost by chance alone and will require more in-depth explanation than you may wish to allot this early in your process. You might want to consider delaying activating such rules until you have a more appropriate opportunity to provide more in-depth training on the rule.

Mix and match

As you start activating rules, you'll soon realize that some rule violations are a lot quicker and easier to fix than others. For example, fixing a violation of RethrowToPreserveStackDetails usually takes seconds, while there are a few rules for which a fix may take hours. It's important that your immediate fix backlog contain a mix of problems, both with respect to difficulty and fix duration. The quick and easy fixes will provide small tasks that any developer will be able to perform, and they're nice little "fillers" for those five minute blocks when you're waiting on something like a pre-commit test run for another task. They'll also help keep the momentum going when you've got a backlog of "big" problems to address. If you only have big problems in the backlog at any given time, the overall fix rate will be slow, and that's just plain depressing for everyone.

Some fixes will require considerable knowledge of a particular area of your product, and one approach to handling these would be have developers fix such problems in their own areas of responsibility. However, you may also want to view such fixes as an opportunity for a bit of cross-training. Either way, you should make sure that your immediate backlog doesn't only contain violations against a limited subset of your product. Any given developer on your team should be able to draw problems from the backlog pool at any given time.


Slow down, you crazy child

Now and then, your immediate fix backlog is going to grow just a bit too big. This usually happens because you've activated a rule with a large number of violations and/or for which violations are expensive to fix. In order to prevent the backlog from growing even more, you're going to have to stop activating new rules for a while. There are basically two things you can do to avoid losing momentum because of this hiatus. The first is pretty trivial, and simply involves having developers fix violations during the time that would normally be allotted to introducing new rules. If you're concerned that the fix rate has dropped off because developers have fallen out of the habit of working on the backlog, then having everyone work on fixing problems at the same time like this can be one way of "rebooting" the process. However, if developers are already spending the time they ought to be on the backlog, a fix session like this isn't necessarily a very effective use of anyone's time.

The other "delaying tactic" is to treat the increased between-rule activation delay as an opportunity to provide more in-depth training, particularly for rules where a lot of background information may be required. It can help quite a bit to divide such rules into themes, with background training on any given theme spanning several weeks. A few obvious themes are:

  • finalization and disposition,
  • exceptions, and
  • code access security.

Custom rules

As you tackle your backlog, you'll probably start to identify some less than desirable usage patterns that aren't covered by existing rules. While it can be quite tempting to immediately activate a new custom rule based on such a discovery, don't forget that you have plenty of built-in rules that aren't yet active either. Your custom rule should go into the same backlog "pot" as the built-in rules, and it should be activated when most appropriate to your backlog process, which may or may not be when you first discover the problem that the rule covers.


How far, how fast?

Obviously, the rate at which you will be able to activate rules will depend quite a bit on the size of the backlog and rate at which your team will be able to fix violations. However, you should also expect the activation rate to drop off over time as you start to activate rules that are difficult to fix and rules with large numbers of violations. Given that you may also start adding custom rules, the proportion of rules activated may drop even though you're not inactivating any rules. Nothing exactly surprising in any of that, but it does mean that you shouldn't use the rate of rule activation as an important indicator of your backlog clearing progress.


Posted by calinoiu | 3 comment(s)
Filed under:

Surprise! (not the good kind)

If you use FxCop or Visual Studio Static Analysis and haven't yet started playing with Orcas, you may be in for a bit of an unpleasant surprise. While the code analysis team is doing all sorts of interesting things for Orcas, one somewhat less desirable change you probably haven't heard about yet is removal of the control flow engine and, consequently, the following rules which depend upon it:

Category Rule
Design ValidateArgumentsOfPublicMethods
Globalization DoNotPassLiteralsAsLocalizedParameters
Performance AvoidUnnecessaryStringCreation
DoNotCallPropertiesThatCloneValuesInLoops
DoNotConcatenateStringsInsideLoops
Reliability DisposeObjectsBeforeLosingScope
Security ReviewSqlQueriesForSecurityVulnerabilities
Usage AttributeStringLiteralsShouldParseCorrectly
DisposeMethodsShouldCallBaseClassDispose
DoNotDisposeObjectsMultipleTimes
LiteralsShouldBeSpelledCorrectly
ProvideCorrectArgumentsToFormattingMethods

The reasons given for removing the engine are basically that it's slow, buggy, and difficult to maintain. Well, we've all had to deal with that sort of problem before, but most of us haven't have the luxury of removing the nasty, old implementation without providing a replacement. I can certainly understand that the code analysis team might be under pressure to rapidly address performance problems introduced by an engine that slows any use of their product by a factor of 2 or more, but this strikes me as a case of throwing the baby out with the bath water.

The one piece of good news is that there seem to be plans to eventually replace the engine. However, we supposedly won't be seeing the new implementation until at least the post-Orcas release of Visual Studio, which very likely means waiting two years or more. A couple of things to consider:

  1. How big a backlog of violations against the missing control flow rules do you think your team will likely create in two years?
  2. Given that the code analysis team apparently considers it acceptable to remove this feature set without providing an immediate replacement, how confident are you in their ability to keep development of the replacement at a high enough priority level that it will actually ship in the post-Orcas release of Visual Studio?

On again, off again...

Let's assume for the moment that the performance hit caused by the control flow engine really does prevent some folks from using FxCop. Even if it doesn't, performance improvements are probably pretty high on many folks' wish lists for FxCop, and it would seem likely that the code analysis team has to put up with a lot of complaining on that front. In addition, there are known bugs in the control flow engine and rules, and those problems may cause both some minor inconvenience as well as false confidence that rules are being applied where they're not. Are these really sufficient justifications for eliminating the control flow engine entirely in Orcas?

Rather than removing the engine, I would much rather see it become an optional, disabled-by-default component. This scenario would remove the potential performance obstacle for new users while still allowing existing customers to at least keep up the same level of screening that was previously applied. If there is concern on the part of the code analysis team that users aren't aware of the buggy, partial screening against control flow rules, the feature that allows enabling of the control flow engine could include whatever warnings they deem necessary.

Given that Orcas is already in beta, it seems likely that re-introducing the control flow engine might be too big a change to support. If so, another possibility might be to make it available only in the stand-alone FxCop product. Obviously, this would be a little less convenient for users of Visual Studio Code Analysis. However, it would at least allow such users to make their own performance/convenience/quality trade-off decisions rather than having a potentially unsuitable decision imposed upon them by the complete absence of the engine.

I've already submitted a suggestion regarding re-introduction of the control flow engine as an optional component on the Microsoft Connect site. If you're concerned about the removal of the engine, perhaps it's time to consider voicing your opinion there as well...

Posted by calinoiu | 3 comment(s)
Filed under:

A few months ago, I gave a presentation on using FxCop at the Montreal Visual Studio Users Group. The material was divided into two main topics: (a) the mechanics of using FxCop and (b) integrating FxCop use into a development process. During the first part of the talk, some members of the peanut gallery kept piping up with questions about what one can do to handle the huge number of FxCop rule violations that an existing code base will have when one first runs FxCop against it. Lucky for me, most of the second part of the talk covered exactly that problem, and I managed to finish the evening without having any rotten produce lobbed in my direction. However, their questions did confirm my suspicion that dealing with the backlog problem is a topic of potential interest to folks, so it's one I might as well spend a bit of time writing about...


No backlog?  No problem.

Before tackling the hard part of the backlog problem, let's look at the easy part. If you're starting a new project, the easiest way to handle the backlog issue is to never develop a backlog in the first place. The way to do this is to start running FxCop with all rules activated from the moment you first set up your project(s) and to fix problems (or flag true exclusions) before each and every check-in.

This may seem like a lot of work and, to be honest, it will almost certainly add noticeable overhead at the very beginning, but the additional effort required will probably taper off more quickly than you might expect. Within a week or two, most individual developers should be over the learning curve "hump" with respect to the FxCop tool itself. What might surprise you is that it probably won't take much longer for most developers to become familiar with the rules that touch their own work and to stop introducing new violations of those rules. This will, of course, vary depending on the spectrum of tasks that any given developer performs, but most developers in anything but the very smallest shops are likely working on fairly similar tasks from one week to the next.

Another issue on the "no backlog" side of things is that you might need to occasionally create temporary exclusions (e.g.: when you've added the first class to a namespace, but the others that would eventually prevent Design.AvoidNamespacesWithFewTypes from firing don't yet exist), and allowing these to accumulate could also create a backlog problem. The way to prevent this is to avoid them where possible and clean them up as soon as possible when their creation can't be avoided. I would recommend creating a project work item for removing any given temporary exclusion immediately before adding the exclusion itself. A final check that they have all been removed should be part of a pre-release checklist, and the release in question should be the earliest you can isolate (e.g.: internal release to QA).


And now for the hard stuff...

If, like many folks with a medium-to-large existing code base, you've run FxCop and been overwhelmed at the thousands or tens of thousands of violations that it spits out, what can you possibly do? The most common reaction seems to be essentially giving up on the idea of using FxCop for that product. That's unfortunate, partly because the existing problems will go unaddressed until they manifest as user-noticeable bugs, but also because the product will continue to accumulate new problems as development progresses.

Another path is to disregard the existing backlog and follow the "no backlog" approach described above for new development. Since the FxCop team has produced a tool to help facilitate this approach, I presume that it is a relatively common choice. Unfortunately, while it does avoid the problem of accumulating new violations, it doesn't address the existing backlog particularly well.

So... Can one do something to both prevent new problems and clean up the old problems? At FinRad, we started a process in late September to do just that. One of the things that was immediately obvious was that "hiding" the existing backlog wasn't likely to be terribly helpful if our goal was to eventually clean it all up. Part of the problem is that folks would probably become rapidly inured to the large volume of existing exclusions. Another, potentially more important issue, is that a big backlog is just plain too depressing.

Rather than activating all rules at the beginning and excluding all existing rule violations (even temporarily), we decided to adopt a process whereby rules are activated one-by-one. Once a rule is activated, its existing violations are added to the immediate backlog and nobody is allowed to introduce new violations. In addition, we decided to provide training on the newly activated rules rather than expecting individual developers to do all the rule reason and fix approaches research on their own. The basic outline of this process is:

  1. Every week, a new set of rules to activate is selected.
  2. We have a one hour training session on the new rules. This training includes:
    • any background information that is required to understand the rules,
    • the specific problems that the rules are meant to address,
    • recommended approaches to fixing rule violations, and
    • reasons for which it is acceptable to create exclusions for violations.
    In addition, progress statistics are presented at each training session so that everyone is aware of where we stand with respect to the existing backlog.
  3. Immediately after the training session, the new rules are activated, and existing violations are added to the backlog by creating "TODO" exclusions.
  4. As of the activation of a rule, no new violations are accepted.
  5. Each developer is supposed to spend two hours per week fixing rule violations from the backlog.

In practice, this exact process isn't followed to the letter every week. Besides the obvious impossibility of grabbing three hours of each developer's time every single week, there are also all sorts of picky details around which rules can/should be activated when, and what one should do to keep the momentum going when the size of the backlog jumps because of the activation of a single rule. There's also the question of what sort of hit we're taking with respect to new violations against the rules that haven't yet been activated. However, I think I'll keep all of that as meat for further blog posts for now...

Posted by calinoiu | 11 comment(s)
Filed under:

Since I posted an FxCop rule sample over at bordecal.mvps.org, it's rapidly become the most popular content on the site. Not something I expected but, given that I have trouble coming up with blogging topics and there seems to be some interest in FxCop, I figured I might as well spend a lovely Saturday morning writing about it... (Actually, I'm pretty much just killing time while waiting for a tire change, so please feel free to keep that "get a life" comment to yourself. <g>)

There's all sorts of stuff I could (and will try to) write about FxCop use, but one thing that jumped out at me this week was quite a few searches for "no FxCop rules could be found" landing on my rule sample page, so I guess I'll start with that...

There are several reasons why FxCop might have trouble finding rules in a custom rule assembly:

  1. There are no rule classes.

    If you haven't added any rule classes to your assembly, it shouldn't be too much of a problem to figure out why FxCop can't find them. ;)

    Another problem might be that your rule classes don't implement the necessary rule interfaces. If you've done the usual thing and created your rules as subclasses of Microsoft.FxCop.Sdk.Introspection.BaseIntrospectionRule, then you've got the interface problem covered. However, if you're rolling your own rules from scratch, you're probably going to have to do quite a bit more work... (Which I've never done, so I have no idea what's the minimum required. My first guess would be IRule, but that seems rather useless since there are no problem collections exposed until one gets to IControlFlowRule or IIntrospectionRule.)

  2. You've forgotten to add an embedded rules resource file.

    This is probably the most common problem for beginners. If you're not sure what the rule resource file should contain, grab yourself a copy of Reflector, and take a look at the Microsoft.FxCop.Rules.Design.DesignRules.xml embedded resource in the DesignRules.dll file that ships with FxCop or Visual Studio Static Analysis.

    Also, remember that this file must be set to be an embedded resource. If the file doesn't end up in your compiled assembly, FxCop won't be able to find it.

  3. You've incorrectly identified the embedded rules resource file in which the data for your rule class can be found.

    Once you've created your embedded rules resource file, this may be the next problem you hit. If you're inheriting from BaseIntrospectionRule, you must pass the full resource file name (minus the .xml extension) when invoking the protected BaseIntrospectionRule constructor. Since all the custom rules in a given assembly will typically use the same rules resource file, one would usually create a base class from which they would all inherit. The rule base class might look something like this:

    namespace Bordecal.FxCop.Rules
    {
    	internal abstract class IntrospectionRule : BaseIntrospectionRule
    	{
    		protected IntrospectionRule(string name)
    			: base(name, "Bordecal.FxCop.Rules.Rules", typeof(IntrospectionRule).Assembly)
    		{
    		}
    	}
    }
    

    This avoids requiring all rules to provide the resource file name, but it doesn't help with figuring out what the file name ought to be in the first place.

    The bad news is that inferring the resource file name can be a bit tricky. It will depend on your project's default namespace, as well as on where the rule file resides within your project's directory hierarchy. The good news is that you don't need to infer the name at all. Instead, simply open your rule assembly in Reflector, and copy the full name of your resource file (remember to omit the .xml extension):

Posted by calinoiu | 2 comment(s)
Filed under: