26 January, 2011

Static code analysis and CA2000 warning

After upgrading to visual studio 2010, i started to get a lot of CA2000 warnings when running static code analysis.

Let's look at this example function:
public static void PopulateMyTable1(HtmlTable table)
{
    table.Rows.Clear();
    HtmlTableRow row = new HtmlTableRow();
    row.ID = "MyRow";
    row.Visible = false;
    HtmlTableCell cell = new HtmlTableCell();
    cell.Attributes.Add("class", "MyClass");
    cell.Height = "20px";
    row.Cells.Add(cell);
    table.Rows.Add(row);
}

Generates these warnings:
CA2000 : Microsoft.Reliability : In method 'TestClass.PopulateMyTable1(HtmlTable)', object 'cell' is not disposed along all exception paths.
Call System.IDisposable.Dispose on object 'cell' before all references to it are out of scope.
CA2000 : Microsoft.Reliability : In method 'TestClass.PopulateMyTable1(HtmlTable)', object 'row' is not disposed along all exception paths.
Call System.IDisposable.Dispose on object 'row' before all references to it are out of scope.


The warnings are generated because we are calling properties that can cause an exception, which will cause the object not beeing disposed properly.
So how can we fix this ?

public static void PopulateMyTable2(HtmlTable table)
{
    table.Rows.Clear();
    HtmlTableRow row = null;
    try
    {
        row = new HtmlTableRow();
        row.ID = "MyRow";
        row.Visible = false;
    }
    catch (Exception)
    {
        row.Dispose();
        throw;
    }
 
    table.Rows.Add(row);
 
    HtmlTableCell cell = null;
    try
    {
        cell = new HtmlTableCell();
        cell.Attributes.Add("class", "MyClass");
        cell.Height = "20px";
    }
    catch (Exception)
    {
        cell.Dispose();
        throw;
    }
 
    row.Cells.Add(cell);            
}

This generates no CA2000 warnings, but talk about code cluttering.
Let's try something else:

public static void PopulateMyTable3(HtmlTable table)
{
    table.Rows.Clear();
    HtmlTableRow row = new HtmlTableRow();
    table.Rows.Add(row);
    row.ID = "MyRow";
    row.Visible = false;
    HtmlTableCell cell = new HtmlTableCell();
    row.Cells.Add(cell);
    cell.Attributes.Add("class", "MyClass");
    cell.Height = "20px";
}

Since we always add the new objects to the collection before calling any properties/methods then there will be no CA2000 warnings.

A much safer approach, because if an exception is thrown by calling a property then it will be taken care of when disposing the collection.

24 January, 2011

Annoying static code analysis errors in visual studio 2010

I have just upgraded to VisualStudio 2010, and ran into these annoying warnings.

CA0503 : The CodeAnalysisRules property is deprecated. Use the CodeAnalysisRuleSet property instead.
CA0505 : The CodeAnalysisRules property will be ignored because the CodeAnalysisRuleSet property is defined.

In visual studio 2008 the only way of using a centralized rules file was to add the following line to the end of
our project file:

<Import Project="C:\Projects\Applications\Common\Rules\MyCompany.CodeAnalysis.Rules.targets"/>

In 2010 you can browse for an existing rules file from within the project options.
And it will store the rules file like this:

<CodeAnalysisRuleSet>..\..\Common\Rules\MyCompany.CodeAnalysis.ruleset</CodeAnalysisRuleSet>

If you have defined both, then you will get the CA503 and CA505 errors.
So in visual studio 2010, just remove the old style import tag.