No Visible Mutable Static Field

ID

csharp.no_visible_mutable_static_field

Severity

high

Remediation Complexity

medium

Remediation Risk

low

Remediation Effort

medium

Resource

Code Smell

Language

CSharp

Tags

api-design, code_smell, concurrency

Description

Reports static fields that are externally visible — public, protected or protected internal — and can still be reassigned because they are neither const nor readonly. private protected fields are not reported: only derived types inside the declaring assembly can reach them.

The visibility of the containing type is deliberately not considered: shared mutable static state is a defect on its own terms, and an internal type merely narrows the set of writers to one assembly.

Rationale

A visible mutable static field is global state with the access control removed. Any code in any referencing assembly can overwrite it, and there is no way for the declaring type to validate the new value, to log the change, or to keep a related field consistent with it. On top of that the field is unsynchronised: concurrent writes from two threads race, and a reader may observe a value that no writer ever intended. Wrapping the state behind a property restores the seam where validation and locking belong, and static readonly states the more common intent — a value initialised once and read from then on.

public class Registry
{
    public static int Counter;                              // FLAW - anyone can overwrite it
    protected static string Prefix = "cache:";              // FLAW - visible to derived types elsewhere
    public static volatile bool Ready;                      // FLAW - volatile does not make it safe to share

    public const int MaxRetries = 3;                        // OK - immutable
    public static readonly int Limit = 5;                   // OK - assigned once
    private static int hidden;                              // OK - not visible outside the type
    internal static int shared;                             // OK - assembly-scoped
    public int instanceCounter;                             // OK - not static
}

Remediation

Make the field static readonly when it is initialised once, or const when the value is a compile-time literal that will never change. When the value genuinely has to change at run time, make the field private and expose a property or method that owns the invariants and the synchronisation; better still, hold the state in an instance whose lifetime the caller controls, so tests can substitute it.

References