Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C# CA2104 - Automated Code Analysis dislikes static readonly mutable types

I've got a code like this:

public abstract class Base
{
    // is going to be used in deriving classes
    // let's assume foo is threadsafe
    protected static readonly Foo StaticFoo = new Foo();
}

Visual Studio 2008's Code Analysis pops up this message:

CA2104 : Microsoft.Security : Remove the read-only designation from 'Base.StaticFoo' or change the field to one that is an immutable reference type. If the reference type 'Foo' is, in fact, immutable, exclude this message.

Is my design instrinsically flawed, or can i add a [SuppressMessage] in the source?

like image 439
mafu Avatar asked Dec 17 '22 08:12

mafu


1 Answers

The problem is that it gives the wrong impression - it makes it look like the value can't change, when actually it's only the field value that can't change, rather than the object. I think the code analysis is being overly cautious here - there are plenty of places where you might want a mutable type stored in a readonly field, particularly if it's private, initialized in a static initializer and then never mutated within your code.

While the field is protected, however, there's more of a risk of derived classes abusing it. Personally I like all my fields to be private. On the other hand, I don't know your code - it could be that you've weighed that up against the possibilities and decided it's the right thing to do.

If Foo really is thread-safe, there's less to worry about, other than the general impression of immutability given by the fact that the field is readonly. I'd disable the warning for that line, and add a comment to emphasize that the object is mutable even though the field is readonly.

like image 174
Jon Skeet Avatar answered Jan 27 '23 03:01

Jon Skeet