Nullable GetType Comparison

ID

csharp.nullable_gettype_comparison

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

CSharp

Tags

nullable, reflection, reliability, suspicious-comparison

Description

Reports the runtime type returned by GetType() being compared with typeof of a nullable value type, with == or != and in either operand order.

Three spellings of the nullable type are recognised: the language shorthand typeof(int?), the constructed form typeof(Nullable<int>), and the open form typeof(Nullable<>).

A typeof(Nullable<…​>) that is not compared with a GetType() call is ordinary reflection code and is not reported.

Rationale

Nullable<T> never appears as a runtime type. GetType() is declared on object, so a nullable value is boxed before the call, and boxing a nullable does one of two things: if it has a value, the box holds the underlying value, so ((int?)1).GetType() is typeof(int); if it has none, boxing yields a null reference and the call throws NullReferenceException.

The comparison therefore never holds. An == branch is dead code, an != branch is always taken, and either way the expression may throw before it is even evaluated. Nothing in the source hints at the cause — the test reads exactly like the correct one written against the underlying type.

using System;

public class TypeProbe
{
    public bool IsNullableInt(object value)
    {
        return value.GetType() == typeof(int?);              // FLAW
    }

    public bool IsNotNullableLong(object value)
    {
        return value.GetType() != typeof(long?);             // FLAW
    }

    public bool Constructed(object value)
    {
        return value.GetType() == typeof(Nullable<decimal>); // FLAW
    }

    public bool IsInt(object value)
    {
        return value.GetType() == typeof(int);               // OK, the underlying type
    }

    public bool IsAnyNullable(object value)
    {
        return Nullable.GetUnderlyingType(value.GetType()) != null;  // OK
    }

    public Type OpenNullable()
    {
        return typeof(Nullable<>);                           // OK, not compared with GetType
    }
}

Remediation

Decide which question the code is really asking.

To test whether a boxed value is an int, compare against the underlying type: value.GetType() == typeof(int). A boxed int? that has a value satisfies this, which is what the original test was reaching for.

To test whether a declared type is nullable, ask the reflection API rather than a runtime value: Nullable.GetUnderlyingType(type) != null, where type comes from a field, property or parameter declaration. That is the only place Nullable<T> survives as a type.

Guard the call as well when the receiver may be an empty nullable, or use a pattern match such as value is int, which is null-safe and needs no boxing rules to be understood.

References