rspec/rules/S2223/csharp/rule.adoc

== Why is this an issue?

Unlike instance fields, which can only be accessed by code having a hold on the instance, `static` fields can be accessed by any code having visibility of the field and its type.

[source,csharp,diff-id=1,diff-type=noncompliant]
----
public class Math
{
    public static double Pi = 3.14;  // Noncompliant
}

// Somewhere else, where Math and Math.Pi are visible
var pi = Math.Pi; // Reading the value 
Math.Pi = 3.1416; // Mutating the value
----

Another typical scenario of the use of a non-private mutable `static` field is the following:

[source,csharp,diff-id=2,diff-type=noncompliant]
----
public class Shape
{
    public static Shape Empty = new EmptyShape();  // Noncompliant

    private class EmptyShape : Shape
    {
    }
}
----

Non-private `static` fields that are neither `const` nor `readonly`, like the ones in the examples above, can lead to errors and unpredictable behavior.

This can happen because:

* Any object can modify these fields and alter the global state. This makes the code more difficult to read, debug and test.
+
[source,csharp]
----
class Counters
{
    public static int ErrorCounter = 0;
}

class Program
{
    public static void Thread1()
    {
        // ...
        Counters.ErrorCounter = 0; // Error counter reset
        // ...
    }

    public static void Thread2()
    {
        // ...
        if (Counters.ErrorCounter > 0)
        {
            Trace.TraceError($"There are {Counters.ErrorCounter} errors"); // It may print "There are 0 errors"
        }
        // ...
    }
}
----

* Correctly accessing these fields from different threads needs synchronization with `lock` or equivalent mechanisms. Improper synchronization may lead to unexpected results.
+
[source,csharp]
----
class Counters
{
    public static volatile int ErrorCounter;
}

class Program
{
    public static void ImproperSynchronization()
    {
        Counters.ErrorCounter = 0;
        Parallel.ForEach(Enumerable.Range(0, 1000), _ => Counters.ErrorCounter++); // Volatile is not enough
        Console.WriteLine(Counters.ErrorCounter); // May print less than 1000
    }

    public static void ProperSynchronization()
    {
        Counters.ErrorCounter = 0;
        Parallel.ForEach(Enumerable.Range(0, 1000), _ => Interlocked.Increment(ref Counters.ErrorCounter));
        Console.WriteLine(Counters.ErrorCounter); // Always prints 1000
    }
}
----

Publicly visible `static` fields should only be used to store shared data that does not change.
To enforce this intent, these fields should be marked `readonly` or converted to `const`.

[source,csharp,diff-id=1,diff-type=compliant]
----
public class Math
{
    public const double Pi = 3.14;
}
----

[source,csharp,diff-id=2,diff-type=compliant]
----
public class Shape
{
    public static readonly Shape Empty = new EmptyShape();
  
    private class EmptyShape : Shape
    {
    }
}
----

include::../resources-dotnet.adoc[]

ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

=== Message

Change the visibility of "xxx" or make it "const" or "readonly".

'''
== Comments And Links
(visible only on this page)

include::../comments-and-links.adoc[]

endif::env-github,rspecator-view[]