A writable shared field is exposed outside its type

ID

vbnet.maintainability.no_visible_mutable_static_field

Severity

high

Remediation Complexity

medium

Remediation Risk

medium

Remediation Effort

medium

Resource

Api Design

Language

VB.NET

Description

Reports a Shared field declared Public, Protected or Protected Friend that is neither ReadOnly nor Const, with or without an initialiser. Private and Friend shared fields are not reported, because they are not reachable from outside the assembly’s own code, and neither are instance fields.

Rationale

A writable shared field is a single value with no owner. Every assembly that can see the type can assign to it, so the set of writers is the set of callers rather than a list you can enumerate, and when the value turns out to be wrong there is no method to breakpoint and no property setter to log - the write that caused it may be in code you do not ship. That alone makes the field expensive to reason about, and it gets worse in two directions. Concurrency: shared state written without coordination is a data race by construction, and since the field is public the type cannot impose any locking discipline on the writers, so the usual symptoms - a setting that reverts, a counter that drifts, a cache that is briefly inconsistent - appear under load and not in tests. Evolution: the field is part of the public contract, so adding validation, lazy initialisation, change notification or a per-tenant lookup later means changing a field into a property, which is a source and binary breaking change for every consumer. Initialisation order compounds it, because a caller can read the field before the code that was supposed to populate it has run and will see the type default with nothing to indicate the value was never set.

The following code illustrates the pattern detected by this rule:

Public Class RuntimeSettings

    ' FLAGGED: A writable shared field is exposed outside its type
    Public Shared ConnectionString As String = "Server=.;Database=acme"

    ' FLAGGED: A writable shared field is exposed outside its type
    Public Shared RetryCount As Integer

    ' FLAGGED: A writable shared field is exposed outside its type
    Public Shared Handlers As New List(Of String)()

    ' FLAGGED: A writable shared field is exposed outside its type
    Protected Shared Culture As String = "en-US"

    ' FLAGGED: A writable shared field is exposed outside its type
    Protected Friend Shared LastReloadUtc As Date

Remediation

If the value never changes after start-up, mark it ReadOnly (or Const for a compile-time literal) - that is usually the whole fix, and it is source-compatible. If it does change, take the field out of the public surface: make it Private Shared and expose a Shared property, so validation, locking and logging have somewhere to live and the accessor can evolve without breaking callers. Better still, drop the shared state: pass the value in through a constructor or an options object so each consumer gets its own instance and the lifetime is explicit. Where a writable shared collection is used as a registry, expose it as an immutable snapshot and mutate it through methods that take a lock.

' Before: any assembly can assign this, from any thread
Public Class RuntimeSettings
    Public Shared ConnectionString As String = "Server=.;Database=acme"
End Class

' After: the field is owned by its type, and the accessor can evolve
Public Class RuntimeSettings
    Private Shared _connectionString As String = "Server=.;Database=acme"

    Public Shared Property ConnectionString As String
        Get
            Return _connectionString
        End Get
        Set(value As String)
            If String.IsNullOrWhiteSpace(value) Then Throw New ArgumentException(NameOf(value))
            _connectionString = value
        End Set
    End Property
End Class

Configuration

This detector does not need any configuration.

References