rspec/rules/S3925/csharp/rule.adoc

== Why is this an issue?

The https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.iserializable[`ISerializable`] interface is the mechanism to control the type serialization process. If not implemented correctly this could result in an invalid serialization and hard-to-detect bugs.

This rule raises an issue on types that implement `ISerializable` without following the https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/serialization[serialization pattern recommended by Microsoft].

Specifically, this rule checks for these problems:

* The https://learn.microsoft.com/en-us/dotnet/api/system.serializableattribute[`SerializableAttribute`] attribute is missing.
* Non-serializable fields are not marked with the https://learn.microsoft.com/en-us/dotnet/api/system.nonserializedattribute[`NonSerializedAttribute`] attribute.
* There is no serialization constructor.
* An unsealed type has a serialization constructor that is not `protected`.
* A sealed type has a serialization constructor that is not `private`.
* An unsealed type has an https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.iserializable.getobjectdata[`ISerializable.GetObjectData`] that is not both `public` and `virtual`.
* A derived type has a serialization constructor that does not call the `base` constructor.
* A derived type has an `ISerializable.GetObjectData` method that does not call the `base` method.
* A derived type has serializable fields but the `ISerializable.GetObjectData` method is not overridden.

Classes that inherit from https://learn.microsoft.com/en-us/dotnet/api/system.exception[`Exception`] are implementing `ISerializable`. Make sure the `[Serializable]` attribute is used and that `ISerializable` is correctly implemented. Even if you don't plan to explicitly serialize the object yourself, it might still require serialization, for instance when crossing the boundary of an https://learn.microsoft.com/en-us/dotnet/api/system.appdomain[`AppDomain`].

This rule only raises an issue on classes that indicate that they are interested in serialization (see the _Exceptions_ section). That is to reduce noise because a lot of classes in the base class library are implementing `ISerializable`, including the following classes: https://learn.microsoft.com/en-us/dotnet/api/system.exception[`Exception`], https://learn.microsoft.com/en-us/dotnet/api/system.uri[`Uri`], https://learn.microsoft.com/en-us/dotnet/api/system.collections.hashtable[`Hashtable`], https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.dictionary-2[`Dictionary<TKey,TValue>`], https://learn.microsoft.com/en-us/dotnet/api/system.data.dataset[`DataSet`], https://learn.microsoft.com/en-us/dotnet/api/system.net.httpwebrequest[`HttpWebRequest`], https://learn.microsoft.com/en-us/dotnet/api/system.text.regularexpressions.regex[`Regex`] https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.treenode[`TreeNode`], and others. There is often no need to add serialization support in classes derived from these types.

=== Exceptions

* Classes in test projects are not checked.
* Classes need to indicate that they are interested in serialization support by either
  . Applying the `[Serializable]` attribute
  . Having `ISerializable` in their base type list
  . Declaring a https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/serialization#supporting-runtime-serialization[serialization constructor]

[source,csharp]
----
[Serializable]                                                                                 // 1.
public class SerializationOptIn_Attribute
{
}

public class SerializationOptIn_Interface : ISerializable                                      // 2.
{
}

public class SerializationOptIn_Constructor
{
    protected SerializationOptIn_Constructor(SerializationInfo info, StreamingContext context) // 3.
    { 
    }
}
----

== How to fix it

Make sure to follow the https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/serialization[recommended guidelines] when implementing `ISerializable`.

=== Code examples

==== Noncompliant code example

[source,csharp,diff-id=1,diff-type=noncompliant]
----
public class Bar
{
}

public class Foo : ISerializable // Noncompliant: serialization constructor is missing
                                 // Noncompliant: the [Serializable] attribute is missing
{
    private readonly Bar bar; // Noncompliant: the field is not marked with [NonSerialized]
}

public sealed class SealedFoo : Foo
{
    private int val; // Noncompliant: 'val' is serializable and GetObjectData is not overridden

    public SealedFoo()
    {
        // ...
    }

    public SealedFoo(SerializationInfo info, StreamingContext context) // Noncompliant: serialization constructor is not `private`
                                                                       // Noncompliant: serialization constructor does not call base constructor
    {
        // ...
    }
}

public class UnsealedFoo : Foo
{
    public UnsealedFoo()
    {
        // ...
    }

    public UnsealedFoo(SerializationInfo info, StreamingContext context) // Noncompliant: serialization constructor is not `protected`
        : base(info, context)
    {
        // ...
    }

    protected void GetObjectData(SerializationInfo info, StreamingContext context) // Noncompliant: GetObjectData is not public virtual
    {
        // Noncompliant: does not call base.GetObjectData(info, context)
    }
}
----


==== Compliant solution

[source,csharp,diff-id=1,diff-type=compliant]
----
public class Bar
{
}

[Serializable]
public class Foo : ISerializable // Compliant: the class is marked with [Serializable]
{
    [NonSerialized]
    private readonly Bar bar; // Compliant: the field is marked with [NonSerialized]

    public Foo()
    {
        // ...
    }

    protected Foo(SerializationInfo info, StreamingContext context) // Compliant: serialization constructor is present
    {
        // ...
    }

    public virtual void GetObjectData(SerializationInfo info, StreamingContext context)
    {
        // ...
    }
}

[Serializable]
public sealed class SealedFoo : Foo
{
    private int val; // Compliant: 'val' is serializable and GetObjectData is overridden

    public SealedFoo()
    {
        // ...
    }

    private SealedFoo(SerializationInfo info, StreamingContext context) // Compliant: serialization constructor is `private`
        : base(info, context) // Compliant: serialization constructor calls base constructor
    {
        // ...
    }

    public override void GetObjectData(SerializationInfo info, StreamingContext context)
    {
        base.GetObjectData(info, context);
        // ...
    }
}

[Serializable]
public class UnsealedFoo : Foo
{
    public UnsealedFoo()
    {
        // ...
    }

    protected UnsealedFoo(SerializationInfo info, StreamingContext context) // Compliant: serialization constructor is `protected`
        : base(info, context)
    {
        // ...
    }

    public virtual void GetObjectData(SerializationInfo info, StreamingContext context) // Compliant: GetObjectData is public virtual
    {
        base.GetObjectData(info, context); // Compliant: calls base.GetObjectData(info, context)
        // ...

    }
}
----

== Resources

=== Documentation

* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/serialization[Serialization]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.iserializable[`ISerializable` Interface]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.serializableattribute[`SerializableAttribute` Class]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.nonserializedattribute[`NonSerializedAttribute` Class]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.iserializable.getobjectdata[`ISerializable.GetObjectData` Method]
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/api/system.exception[`Exception` Class]

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

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

=== Message

Update this implementation of `ISerializable` to conform to the recommended serialization pattern.
Override 'GetObjectData(SerializationInfo, StreamingContext)' and serialize 'X'.
Invoke 'base.GetObjectData(SerializationInfo, StreamingContext)' in this method.
Make 'GetObjectData' 'public' and 'virtual', or seal 'X'.
Make this constructor 'X'.
Call constructor 'base(SerializationInfo, StreamingContext)'.
Add a 'X' constructor '{typeSymbol.Name}(SerializationInfo, StreamingContext)'.

=== Highlighting

The type identifier.

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

=== on 16 Mar 2017, 12:11:37 Ann Campbell wrote:
\[~amaury.leve] I've edited. Please double-check me.


Also, examples for each noncompliant case aren't necessary IMO. I'd show only one or two particularly damaging or particularly subtle examples.

=== on 24 Mar 2017, 16:05:50 Valeri Hristov wrote:
The following check is not implemented because it is difficult to know exactly which fields should be marked with `NonSerialized` and I am afraid it will generate too many FPs:

* Non-serializable fields are not marked with the `System.NonSerializedAttribute` attribute.

We find mostly classes that derive from Exception in the projects we test and that's why they might not be a good source for checking issues and false positives (statistically).

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