Code analysis - FxCop vs. Visual Studio Team Edition

Overview

The external FxCop utility is used to perform code analysis. For .NET 2.0, version 1.35 is the release candidate to use for this purpose. Visual Studio 2003 can be configured to run this tool from within its IDE. However, Visual Studio 2005 Team Edition for Software Developers embeds a version of FxCop directly into the IDE.

This article invites discussion of how we should actually utilize managed code analysis in our development process via some initial proposals, and provides some background information, based on some investigation I've done.

Recommended Direction

Despite the fact that some reporting and history function has been lost in the integration of FxCop into Visual Studio Team Edition, I recommend that developers working with code held in TFS utilize the managed code analysis in Team Edition and change the check-in policy to require that messages be addressed (i.e., code fixed or messages suppressed) before a check-in is allowed to proceed. The reasons for this include:

  • This automates requiring new/changed lines of code to be analyzed and addressed before checking in, as opposed to relying on the procedural 'honor system' method that is easily circumvented (typically due to schedule pressure).
  • This allows for code analysis of the Asi.WebRoot 'web site' without requiring conversion of that code to a web application project via 'aftermarket' tools.

Given the significant number of excluded FxCop messages that currently exist, I imagine we'll initially suppress all messages created by managed code analysis, then fix any subsequent messages as they are encountered. Suppressed messages can then be addressed as development schedules permit.

FxCop 1.35 vs. Visual Studio Team Edition Code Analysis

FxCop 1.35 Release Candidate 1

The FxCop discussion forum is here.

Pros

  • Easily installed over previous versions
  • Displays active, excluded message groups.
  • Allows excluded messages to have text explaining they're excluded.
  • Can analyze separately from assembly compilation, allowing developers to quickly compile/test code until the desired function is achieved, then perform any analysis/cleanup.

Cons

  • Developers can check in code with active messages.
  • No ability to analyze Web Projects without converting them to 'Web Application Projects', using an an 'aftermarket' tool.

Visual Studio Team Edition Code Analysis

A quick blurb about this capability is here. Team System forum discussions are here.

Pros

  • Code analysis is nicely integrated into the IDE, and runs when a project is built, or the user explicitly requires it.
  • Check-ins can be dependent upon a clean run of the analysis (i.e., messages are fixed or suppressed).
  • Rules can be defined and migrated to client workstations (except for Web Pages, see Cons)
  • Our AsiBuildUtility can be modified slightly to ensure that code analysis is performed when msbuild is run (see "Under The Covers..." below).

Cons

  • Requires Visual Studio Team Edition to be installed.
  • No ability to provide notes on why a message is being excluded.
  • Maintenance of suppressed messages is a bit tedious (see "Under The Covers..." below).
  • No ability to migrate Web site project rules to a developer workstation without converting them to 'Web Application Projects', using an an 'aftermarket' tool.

"Under the covers" with Visual Studio Team Edition Code Analysis

When Visual Studio Team Edition is installed using the default installation directory, the FxCop engine is installed to

C:\Program Files\Microsoft Visual Studio 8\Team Tools\Static Analysis Tools\FxCop

Code analysis is turned on in the IDE as follows:

  • Standard project: (Project Properties) > Code Analysis tab > Enable Code Analysis
  • Web site: (Web site property pages) > Build section > Enable Code Analysis

For normal (non-web site) assemblies, building the project or explicitly running the code analysis creates the file

...\bin\(configuration)\(assembly name).dll.CodeAnalysisLog.xml

which can be viewed via IE. Note that this generated file is missing an XML statement which ensures that the "Expand All" button is displayed in the browser. To fix this, do either of the following:

  1. (recommended) In the file C:\Program Files\Microsoft Visual Studio 8\Team Tools\Static Analysis Tools\FxCop\Xml\CodeAnalysisReport.xsl, search for @key='ExpandAll', and replace the enclosing xsl:value-of... tag with the string Expand All.
  2. In the generated file, add the tag Expand All to the section at the bottom of the file.

When performing code analysis on a Web site, a file named

...\(web project name)\(web project name\{(some guid string}\CodeAnalysisLog.xml

is created.

Message Suppression

When suppressing a global message, i.e., one relating to the entire assembly vs. a particular line of code, a file named GlobalSuppressions.cs is created, containing the appropriate SuppressMessage attributes. Suppressing a message for a particular file/line results in a SuppressMessage attribute being added prior to the related class in the given file.

Correction of code involved in previously suppressed messages is slightly tedious. If you decide to fix a given line of code for which there is a related SuppressMessage attribute, rerunning code analysis will not subsequently remove the attribute from the source file or Global Suppressions.cs. Additionally, the relationship between the offending line of code and the corresponding attribute can be quite vague. For example, given the line

string fmt = string.Format("{0} - {1}", something, other);

suppressing the related code analysis message creates

[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Globalization", "CA1305:SpecifyIFormatProvider", MessageId = "System.String.Format(System.String,System.Object,System.Object)")]

Furthermore, new lines of code that would cause the same analysis message would also be suppressed. Yell

The best thing to do in this case is to delete the particular attribute, and rebuild the project in order to have code analysis point to the offending lines of code.

A nominal number (201?) of warning messages are displayed in the Error List window of the IDE; if there are more messages than that, an error message is displayed indicating additional messages cannot be displayed.

MSBuild

When running msbuild.exe external to the IDE, one must do either of the following if the projects are enabled for code analysis:

  1. Execute the command from within a Microsoft Visual Studio 2005 command prompt
  2. Set the FXCOPDIR environment variable to point to the directory

"%vs80comntools%..\..\Team Tools\Static Analysis Tools\FxCop"

The AsiBuildUtility can be modified to utilize option #2 above to support performing code analysis during a partial or full build of the code.

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.

Dev team consensus today: pursue the IDE version

Things to do:

* Use Asi.Web as the initial project for this
* Phase in projects
* Account for 3p developers who don't use TFS/Team Edition and figure out how to resolve discrepencies between the 2 FxCop engines.

First project to use this is Asi, not Asi.Web

A fairly arbitrary decision, but that's what we've got right now.

paul

First warning to exclude

We use guid indexers everywhere, and do so legitimately. The code analysis insists that indexers should only be ints or strings. I’d say this “rule” is in error to not consider guid indexers valid. This warning/rule should be turned off universally:

Use integral or string argument for indexers

TypeName

    UseIntegralOrStringArgumentForIndexers

CheckId

    CA1043

Category

    Microsoft.Design

Breaking Change

    Breaking

Cause

    A public or protected type contains a public or protected indexer that uses an index type other than System.Int32, System.Int64, System.Object, or System.String.

Rule Description

    Indexers, that is, indexed properties, should use integer or string types for the index. These types are typically used for indexing data structures and increase the usability of the library. Use of the Object type should be restricted to those cases where the specific integer or string type cannot be specified at design time. If the design requires other types for the index, reconsider whether the type represents a logical data store. If it does not represent a logical data store, use a method.

How to Fix Violations

    To fix a violation of this rule, change the index to an integer or string type, or use a method instead of the indexer.

When to Exclude Warnings

    Exclude a warning from this rule only after carefully considering the need for the nonstandard indexer.

A few points on managing code analysis rule sets

  1. I've got no problem with working to exclude this (or any other) message, but I'm wondering if there shouldn't be some level of blessing by the design team for any message that is to be excluded?
  2. I've been mucking around with the check-in policies with respect to changing what rules are employed in code analysis, and ran into unexpected behavior. Specifically, this post clarifies that when you turn off a rule in the check-in policy, it won't migrate to a project if that project has a more restrictive ruleset (i.e., that same rule is still turned on). This just means that policy maintenance requires all rules for a given project should be turned off, then migrate the check-in policy to it.

paul

Change to VS Team Edition code analysis is nigh

I'm inching closer to checking in file updates to stop using the external FxCop utility and move to Visual Studio Team Edition team analysis.

Initially, I will turn on code analysis for the following solutions only:

* Asi.Web
* Asi.WebRoot

No rules or messages are being excluded. "Pre-2005 Excluded " messages will be a thing of the past. The build web page will simply show all messages related to any solution for which code analysis has been turned on.

I'm assuming all folks using TFS have installed the Visual Studio Team Edition (EricM had excellent notes on installation, etc., here).

PLEASE let me know if you are NOT yet set up on VS Team Edition.

Additionally, I'm thinking/hoping there are minimal impacts to MedinaInterval1 as I make updates to the Main branch, even if you somehow synch the affected utility/solution changes. Please let me know if this is an incorrect assumption from your perspective.

An additional note: if, for some reason, you wish to run the ProcessFxCop.exe utility to generate a web page similar to what is linked in the FxCop column of bldindex.htm on devnetsrv1, you will need to add the following directory to your PATH environment variable in order for that tool to find the TF.exe command:

C:\Program Files\Microsoft Visual Studio 8\Common7\IDE

Thx...paul

Baseline tool

Have you seen this?

It's a tool for converting standalone FxCop exclusions to Code Analysis exclusions; I don't know if we really want to go down that path, but if we have a particular project with umpteen million exclusions it might be better than not running CA at all.

Interesting

Cool find, Eric, thanks.

Do you know offhand how to define a compilation flag in a web site/project/whatever? I can use that baseline tool to create a suppressed messages file, but dunno how to set the required CODE_ANALYSIS_BASELINE definition.

Also, Richard mentioned that MarkD may have said that we don't really want to suppress any messages, but show them all and address what we can, when we can.

I agree that we don't want

I agree that we don't want to suppress any messages -- this was more an "in an emergency" thing than a "usual" thing. :) As far as defining flags, you can do that on the Project Properties > Build tab, in the Conditional Compilation Symbols textbox.

Defining compilation flags

Yup, I saw the way to do it for a regular assembly, but was stumped about how to do it for a web site like asi.webroot. Fortunately, the FxCop folks are pretty responsive, and got me an answer, namely, update the compilerOptions attribute in system.codedom/compilers/compiler in web.config (which I'm still not thrilled about, but which would seem better than the other recommendation: defining the flag in every source file!).

paul