Wednesday, June 29, 2011

Build Server Code Analysis Settings

I've been looking at ways to improve the reporting of Code Analysis as part of our Team Build. During my research I found that the RunCodeAnalysis setting as defined in the TFSBuild.proj differs significantly from the local MSBuild project schema options (see this post for a detailed break down of project level analysis settings).

Specifically, TFSBuild defines RunCodeAnalysis as "Always", "Default" and "Never" while MSBuild defines this as a simple true/false Boolean.  To make sense of this I hunted this down to this section of the Microsoft.TeamFoundation.Build.targets file:

<Target Name="CoreCompileSolution">

  <PropertyGroup>
    <CodeAnalysisOption Condition=" '$(RunCodeAnalysis)'=='Always'">RunCodeAnalysis=true</CodeAnalysisOption>
    <CodeAnalysisOption Condition=" '$(RunCodeAnalysis)'=='Never'">RunCodeAnalysis=false</CodeAnalysisOption>
    <!-- ... -->
  </PropertyGroup>
  <!-- ... -->
</Target>

From this we can infer that "Default" setting does not provide a value to the runtime, while "Always" and "Never" map to True/False respectively.  But this raises the question, what's the difference between Default and Always, and what should I specify to effectively run Code Analysis?

To answer this question, let's take an example of a small project with two projects.  Both projects are configured in the csproj with default settings.  For the purposes of the demo, I've altered the output folder to be at a folder called Output at the root of the solution:

<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
  <DebugSymbols>true</DebugSymbols>
  <DebugType>full</DebugType>
  <Optimize>false</Optimize>
  <OutputPath>..\Output</OutputPath>
  <DefineConstants>DEBUG;TRACE</DefineConstants>
  <ErrorReport>prompt</ErrorReport>
  <WarningLevel>4</WarningLevel>
</PropertyGroup>

When we specify code analysis to run with the "Always" setting:

Msbuild.exe Example.sln /p:RunCodeAnalysis=True

The Output folder contains the following files:

CodeAnalysisExperiment1.dll 
CodeAnalysisExperiment1.dll.CodeAnalysisLog.xml 
CodeAnalysisExperiment1.dll.lastcodeanalysissucceeded 
CodeAnalysisExperiment1.pdb 
CodeAnalysisExperiment2.dll 
CodeAnalysisExperiment2.dll.CodeAnalysisLog.xml 
CodeAnalysisExperiment2.dll.lastcodeanalysissucceeded 
CodeAnalysisExperiment2.pdb

Close inspection of the console output shows that the Minimal ruleset was applied to my empty projects, despite having not configured either project for code analysis.

DefaultRuleset

If I tweak the project settings slightly such that one project explicitly declares RunCodeAnalysis as True and the other as False

<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "> 
  <DebugSymbols>true</DebugSymbols> 
  <DebugType>full</DebugType> 
  <Optimize>false</Optimize> 
  <OutputPath>..\Output</OutputPath> 
  <DefineConstants>DEBUG;TRACE</DefineConstants> 
  <ErrorReport>prompt</ErrorReport> 
  <WarningLevel>4</WarningLevel> 
  <RunCodeAnalysis>false</RunCodeAnalysis> 
</PropertyGroup>

and…

<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "> 
  <DebugSymbols>true</DebugSymbols> 
  <DebugType>full</DebugType> 
  <Optimize>false</Optimize> 
  <OutputPath>..\Output</OutputPath> 
  <DefineConstants>DEBUG;TRACE</DefineConstants> 
  <ErrorReport>prompt</ErrorReport> 
  <WarningLevel>4</WarningLevel> 
  <RunCodeAnalysis>true</RunCodeAnalysis> 
</PropertyGroup>

...and then specify the "Default" setting:

msbuild Example.sln

The Output folder now contains the following files:

CodeAnalysisExperiment1.dll 
CodeAnalysisExperiment1.dll.CodeAnalysisLog.xml 
CodeAnalysisExperiment1.dll.lastcodeanalysissucceeded 
CodeAnalysisExperiment1.pdb 
CodeAnalysisExperiment2.dll 
CodeAnalysisExperiment2.pdb

From this we can infer that the "Default" setting looks to the project settings to determine if analysis should be done, whereas the "Always" setting will override the analysis and "Never" will disable analysis altogether.

For my purposes, I have some projects (unit tests, etc) that I don't necessarily want to run code analysis on -- for me, "Always" is a bit of a deal breaker.  However, there's an interesting lesson to take away here: command line arguments passed to msbuild are carried to all targets.  Which means we can override most project settings this way:

// use 'all rules' for projects that have analysis turned on 
msbuild.exe Example.sln /p:CodeAnalysisRuleSet="Allrules.set"

// use a custom dictionary during code analysis 
msbulid.exe Example.sln /p:CodeAnalysisDictionary="..\AnalysisDictionary.xml"

// redirect output to a different folder 
msbulid.exe Example.sln /p:OutputPath="..\Output2"

Cheers!

Happy Coding.

submit to reddit

Tuesday, June 28, 2011

The Mystery of Cascading FxCop Errors

As I mentioned in my last post, I've been working through FxCop concerns on my current project. To provide some context, this is a large project with more than 500K lines of code, and FxCop was added late to the development process. Naturally, out of the gate there were thousands of errors, so we had to ease it into the development process by slowly turning on rules until we had the warnings under control.

To facilitate this, we had a dedicated team go through the entire code base and address as many of the errors as they could: implementing IDispose correctly, validating arguments, etc. In the end, we had a hand picked set of rules that applied to our scenarios and a code base that adhered to them. Once the existing violations were under control, we flipped the switch to treat new analysis warnings as errors to ensure that the violations were not introduced into source control.

There were some bumps along the way, but in the end it was manageable (royal pain in the butt, but manageable).

Recently however, my team started to see serious problems with code analysis throughout parts of the application. They found that if they made a minor change, a series of FxCop violations would appear -- even in areas that they didn't touch or had previously been reviewed and considered "warning free". The team believed that FxCop was somehow incrementally analyzing files, thereby finding new problems which had previously been skipped. While this theory fit into the symptom of new violations for existing code, it just didn't seem right: FxCop analyzes compiled assemblies, not individual files.

The problem with code analysis errors is that they're not always obvious what is causing the problem. It's easy to fall into a trap to believe that Code Analysis is only looking at the internal structure or logic of a class for correctness. There are many FxCop rules that look for correctness, such as Methods that can be private or static, arguments that can be null, etc. The really obscure errors are the ones that look at how a class is used by other classes. What may seem like a trivial change can introduce a different execution path that suddenly exposes the class and its dependencies for consideration of new FxCop violations.

Let's take a look at a contrived example. Let's say I'm using a Service Locator to wire up a ViewModel and like any good Inversion of Control junkie, I'm using an interface for that ViewModel. According to FxCop, the following example is good and life is fine.

public class CustomApp : Application 
{ 
    public override void OnStartup() 
    { 
        var view = new MainView(); 
        view.DataContext = new MainViewModel(); 
        view.Show(); 
    } 
}

internal class MainViewModel 
{ 
    public ISearchViewModel SearchViewModel { get; set;}

    public MainViewModel() 
    { 
        SearchViewModel = ServiceLocator.Get<ISearchViewModel>(); 
    } 
}

As smart developers, we decide that we don't really need the ServiceLocator to get our SearchViewModel. The concrete SearchViewModel doesn't have any complex construction logic and we probably will never swap it out with a different instance at runtime -- this is a little hit against dependency inversion but from a testing perspective, I can always introduce a Mock through property injection. Upon introducing this change we discover several new FxCop violations that weren't present before -- nearly a dozen warnings in unrelated classes about objects not being properly disposed. Out of all the warnings, only one warning is specific to the change we've made in this class but it doesn't seem obvious: CA2000: Dispose objects before losing scope.

public class MainViewModel 
{ 
    public ISearchViewModel SearchViewModel { get; set;}

    public MainViewModel() 
    { 
        // FxCop: CA2000 - Dispose object before losing scope
        SearchViewModel = new PrimarySearchViewModel();
    } 
}

This violation takes a few minutes to sort out because we're assigning the object to a property, and it isn't going out of scope. Upon further inpsection, it seems that the concrete class implements IDisposable, but the interface does not. FxCop is warning us that once the constructor goes out of scope, our concrete class will become polymorphically assigned to an interface that doesn't support IDispose and as such, we won't have an opportunity to call the Dispose method on the concrete class. Clever girl.

The easy fix is to add IDispose onto the interface and then implement the dispose pattern for the MainViewModel. All of this makes sense, and we've just fixed a memory leak that was there all along. But why did we have violations in all these other classes?

It turns out that PrimarySearchViewModel implemented IDispose improperly and it also constructed several other IDisposable types with the same problem in its constructor. These violations, like the PrimarySearchViewModel one, are legitimate. Now, if you're following along, you'll realize I haven't answered the question. If all these classes implemented IDispose improperly, why weren't these found earlier?

The answer is because we changed the usage. No one was calling the constructor previously -- the ServiceLocator used reflection to hunt down and construct our object for us, but by switching to use the constructor directly the analysis engine did it's part to highlight problems. It's hard to say whether the analysis engine should have caught this potential problem or if it's a feature that only inspects currently active code. At any rate, I hope this help you or your team -- before dismissing FxCop violations as fluke, look at how the usage of your objects has changed.

submit to reddit

Monday, June 27, 2011

Visual Studio Code Analysis Settings

I've just spent the last week or so working with Code Analysis for my Visual Studio 2010 projects.  I still haven't figured out if I'm using Visual Studio's built in Code Analysis tool or FxCop 10.0, because the two tools share a lot of the same engine and rulesets.  At any rate, one of the pain points I had to work through was ensuring that the configuration settings between my Visual Studio configurations were the same.  Strangely enough, I had more confidence in my changes when manually editing the csproj files in Notepad++ than from Visual Studio -- it seemed like each configuration had slightly different settings, and being able to see the msbuild settings seemed to make more sense.

During this process, I found it quite frustrating that none of the CodeAnalysis settings that are configurable from the csproj schema were documented online.  However, the intellisense for the schema is quite good.  The schema is located at: <Program Files>\Microsoft Visual Studio 10.0\Xml\Schemas\1033\MSBuild\Microsoft.Build.CommonTypes.xsd

Most of the settings generated by Visual Studio are often the default values and can be discarded.

For my reference, here's the documentation, lifted from the schema:

Project Element Description
CodeAnalysisAdditionalOptions Additional options to pass to the Code Analysis command line tool.
CodeAnalysisApplyLogFileXsl Indicates whether to apply the XSL style sheet specified in $(CodeAnalysisLogFileXsl) to the Code Analysis report. This report is specified in $(CodeAnalysisLogFile). The default is false.
CodeAnalysisConsoleXsl Path to the XSL style sheet that will be applied to the Code Analysis console output. The default is an empty string (''), which causes Code Analysis to use its default console output.
CodeAnalysisCulture Culture to use for Code Analysis spelling rules, for example, 'en-US' or 'en-AU'. The default is the current user interface language for Windows.
CodeAnalysisDependentAssemblyPaths Additional reference assembly paths to pass to the Code Analysis command line tool. A fully qualified path to a directory containing reference assemblies to be used by Code Analysis.
CodeAnalysisDictionary Code Analysis custom dictionaries.  Semicolon-separated list of Code Analysis custom dictionaries. Wildcards are allowed.
CodeAnalysisFailOnMissingRules Indicates whether Code Analysis should fail if a rule or rule set is missing. The default is false.
CodeAnalysisForceOutput Indicates whether Code Analysis generates a report file, even when there are no active warnings or errors. The default is true.
CodeAnalysisGenerateSuccessFile Indicates whether Code Analysis generates a '$(CodeAnalysisInputAssembly).lastcodeanalysissucceeded' file in the output folder when no build-breaking errors occur. The default is true.
CodeAnalysisIgnoreBuiltInRules Indicates whether Code Analysis will ignore the default rule directories when searching for rules. The default is false.
CodeAnalysisIgnoreBuiltInRuleSets Indicates whether Code Analysis will ignore the default rule set directories when searching for rule sets. The default is false.
CodeAnalysisIgnoreInvalidTargets Indicates whether Code Analysis should silently fail when analyzing invalid assemblies, such as those without managed code. The default is true.
CodeAnalysisIgnoreGeneratedCode Indicates whether Code Analysis should fail silently when it analyzes invalid assemblies, such as those without managed code. The default is true.
CodeAnalysisImport Code Analysis projects (*.fxcop) or reports to import. Semicolon-separated list of Code Analysis projects (*.fxcop) or reports to import. Wildcards are allowed.
CodeAnalysisInputAssembly Path to the assembly to be analyzed by Code Analysis. The default is '$(OutDir)$(TargetName)$(TargetExt)'.
CodeAnalysisLogFile Path to the output file for the Code Analysis report. The default is '$(CodeAnalysisInputAssembly).CodeAnalysisLog.xml'.
CodeAnalysisLogFileXsl Path to the XSL style sheet to reference in the Code Analysis output report. This report is specified in $(CodeAnalysisLogFile). The default is an empty string ('').
CodeAnalysisModuleSuppressionsFile Name of the file, without the path, where Code Analysis project-level suppressions are stored. The default is 'GlobalSuppressions$(DefaultLanguageSourceExtension)'.
CodeAnalysisOverrideRuleVisibilities Indicates whether to run all overridable Code Analysis rules against all targets. This will cause specific rules, such as those within the Design and Naming categories, to run against both public and internal APIs, instead of only public APIs. The default is false.
CodeAnalysisOutputToConsole Indicates whether to output Code Analysis warnings and errors to the console. The default is false.
CodeAnalysisVerbose Indicates whether to output verbose Code Analysis diagnostic info to the console. The default is false.
CodeAnalysisPath Path to the Code Analysis installation folder. The default is '$(VSINSTALLDIR)\Team Tools\Static Analysis Tools\FxCop'.
CodeAnalysisPlatformPath Path to the .NET Framework folder that contains platform assemblies, such as mscorlib.dll and System.dll. The default is an empty string ('').
CodeAnalysisProject Path to the Code Analysis project (*.fxcop) to load. The default is an empty string ('').
CodeAnalysisQuiet Indicates whether to suppress all Code Analysis console output other than errors and warnings. This applies when $(CodeAnalysisOutputToConsole) is true. The default is false.
CodeAnalysisRuleAssemblies Semicolon-separated list of paths either to Code Analysis rule assemblies or to folders that contain Code Analysis rule assemblies. The paths are in the form '[+|-][!][file|folder]', where '+' enables all rules in rule assembly, '-' disables all rules in rule assembly, and '!' causes all rules in rule assembly to be treated as errors. For example '+D:\Projects\Rules\NamingRules.dll;+!D:\Projects\Rules\SecurityRules.dll'. The default is '$(CodeAnalysisPath)\Rules'.
CodeAnalysisRuleDirectories Semicolon-separated list of directories in which to search for rules when resolving a rule set. The default is '$(CodeAnalysisPath)\Rules' unless the CodeAnalysisIgnoreBuiltInRules property is set to true.
CodeAnalysisRules Semicolon-separated list of Code Analysis rules. The rules are in the form '[+|-][!]Category#CheckId', where '+' enables the rule, '-' disables the rule, and '!' causes the rule to be treated as an error. For example, '-Microsoft.Naming#CA1700;+!Microsoft.Naming#CA1701'. The default is an empty string ('') which enables all rules.
CodeAnalysisRuleSet A .ruleset file which contains a list of rules to run during analysis. The string can be a full path, a path relative to the project file, or a file name. If a file name is specified, the CodeAnalysisRuleSetDirectories property will be searched to find the file. The default is an empty string ('').
CodeAnalysisRuleSetDirectories Semicolon-separated list of directories in which to search for rule sets. The default is '$(VSINSTALLDIR)\Team Tools\Static Analysis Tools\Rule Sets' unless the CodeAnalysisIgnoreBuiltInRuleSets property is set to true.
CodeAnalysisSaveMessagesToReport Comma-separated list of the type ('Active', 'Excluded', or 'Absent') of warnings and errors to save to the output report file. The default is 'Active'.
CodeAnalysisSearchGlobalAssemblyCache Indicates whether Code Analysis should search the Global Assembly Cache (GAC) for missing references that are encountered during analysis. The default is true.
CodeAnalysisSummary Indicates whether to output a Code Analysis summary to the console after analysis. The default is false.
CodeAnalysisTimeout The time, in seconds, that Code Analysis should wait for analysis of a single item to complete before it aborts analysis. Specify 0 to cause Code Analysis to wait indefinitely. The default is 120.
CodeAnalysisTreatWarningsAsErrors Indicates whether to treat all Code Analysis warnings as errors. The default is false.
CodeAnalysisUpdateProject Indicates whether to update the Code Analysis project (*.fxcop) specified in $(CodeAnalysisProject). This applies when there are changes during analysis. The default is false.
CodeAnalysisUseTypeNameInSuppression Indicates whether to include the name of the rule when Code Analysis emits a suppression. The default is true.
RunCodeAnalysis Indicates whether to run Code Analysis during the build.

As this has been a fun week dealing with FxCop, expect to see a few more posts related to the above settings.  Stay right here and click refresh endlessly, or subscribe to the rss feed.

Happy coding.

submit to reddit