Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

CA2213 code analysis rule and auto-implemented properties

I use static code analysis in our projects to check for code violations. One of extensively used rules is CA2213, which checks for correct disposing of disposable fields.

I noticed CA2213 does not check disposing of auto implemented properties.

Furthermore CA2213 does not check disposing of neither fields nor auto implemented properties if the class inherits from class which implements IDisposable and does not override the Dispose method.

Practical example:

public sealed class Good : IDisposable {
    private Font font;
    public Font Font {
        get { return font; }
        set { font = value; }
    }
    public Good() { font = new Font("Arial", 9); }
    public void Dispose() { /* Do nothing */ }       // CA2213
}

public sealed class Bad : IDisposable {
    public Font Font { get; set; }
    public Bad() { Font = new Font("Arial", 9); }
    public void Dispose() { /* Do nothing */ }       // No warning
}

Has anyone else encountered this behavior? Is this by design or bug in CA2213 rule?

like image 896
Zvonko Avatar asked Oct 29 '14 07:10

Zvonko


1 Answers

Code analyzers do have limitations, the one present in this code is that it needs to generate an actionable warning. One that the programmer can actually follow up on to fix his code. A key problem with the current code analyzer is that it does not analyze source code, it works from the assembly that's generated by the compiler. Which is nice, it works for any .NET language. But nobody will like this warning:

CA2213 Disposable fields should be disposed
'Bad' contains field <Font>k__BackingField that is of IDisposable type: 'Font'. Change the Dispose method on 'Bad' to call Dispose or Close on this field.

Yikes, of course there is no such field in the program. To generate a better message, the analyzer would have to figure out that the <Font>k__BackingField field in the metadata is in fact associated with the auto-implemented Font property. There is nothing available in the metadata that makes that connection unambiguously. The field only carries the [CompilerGenerated] attribute and the auto-generated field name is a compiler implementation detail. Different language compilers generate different names.

It is the kind of problem that requires source-code analysis, not IL analysis as currently implemented. Opportunity is knocking on the door, source-code analysis is going to be a lot easier to implement with Roslyn available today. VS2015 is the first VS version that supports "Live Code" analysis. I don't know yet whether it looks for CA2213 style errors, soon.

like image 60
Hans Passant Avatar answered Nov 10 '22 15:11

Hans Passant