Skip to content

Contradiction between IReadOnlyTypeBase.ClrType XML documentation and NRT annotation #39044

Description

@bkoelman

Summary

There is an inconsistency in Microsoft.EntityFrameworkCore.Metadata.IReadOnlyTypeBase between the XML documentation of ClrType (which indicates it can return null for shadow types) and its C# Nullable Reference Type (NRT) annotation (which is non-nullable Type).

Because ClrType is annotated as non-nullable Type, static analysis tools (Roslyn analyzers, ReSharper, Rider) flag null checks such as if (entityType.ClrType != null) as redundant ("Expression is always true"). However, callers who follow the XML documentation and assume it can be null face contradictory compiler/analyzer behavior.


Detailed Description

In src/EFCore/Metadata/IReadOnlyTypeBase.cs:

/// <summary>
///     Gets the CLR class that is used to represent instances of this type.
///     Returns <see langword="null" /> if the type does not have a corresponding CLR class (known as a shadow type).
/// </summary>
/// <remarks>
///     Shadow types are not currently supported in a model that is used at runtime with a <see cref="DbContext" />.
///     Therefore, shadow types will only exist in migration model snapshots, etc.
/// </remarks>
[DynamicallyAccessedMembers(IEntityType.DynamicallyAccessedMemberTypes)]
Type ClrType { get; }

1. XML Documentation vs. NRT Annotation

  • The XML <summary> states:

    "Returns null if the type does not have a corresponding CLR class (known as a shadow type)."

  • The return type is annotated as non-nullable Type, not Type?.
    • Other properties in the same interface that can return null are explicitly annotated as nullable (for example, IReadOnlyTypeBase? BaseType { get; }).

2. Default Interface Methods Dereference ClrType Unconditionally

In the same file (IReadOnlyTypeBase.cs), default interface implementations dereference ClrType directly without null checks:

[DebuggerStepThrough]
bool IsAbstract()
    => ClrType.IsAbstract;

[DebuggerStepThrough]
string ShortName()
{
    if (!HasSharedClrType)
    {
        var name = ClrType.ShortDisplayName(); // Will throw NullReferenceException if ClrType is null
        ...
    }
    ...
}

If ClrType were ever null, invoking IsAbstract() or ShortName() would throw a NullReferenceException.

3. Impact on Consuming Code and Static Analysis

When consumers write defensive code based on the XML documentation:

if (foreignKey.DeclaringEntityType.ClrType != null)
{
    ...
}

The C# compiler and static analyzers (Roslyn, ReSharper, Rider) emit warnings:

  • ReSharper / Rider: "Expression is always true"
  • Roslyn: CS8073: The result of the expression is always 'true' since a value of type 'Type' is never equal to 'null' (when treated in strict nullable contexts).

In projects configured with <TreatWarningsAsErrors>true</TreatWarningsAsErrors>, this breaks builds unless suppressed via #pragma or discarded.

4. Implementation Analysis

Looking at the implementations across EF Core:

  • TypeBase (design-time / model builder): Both constructors (TypeBase(Type, Model, ...) and TypeBase(string, Type, Model, ...)) require a non-null Type type.
  • EntityType configured only by string name (modelBuilder.Entity("SomeName")): EF Core passes Model.DefaultPropertyBagType (typeof(Dictionary<string, object>)) to the base constructor rather than null.
  • RuntimeTypeBase (compiled runtime model): Constructor takes Type type and assigns ClrType = type;.

It appears that ClrType is practically never null across all supported runtime and design-time types today. The XML comment seems to be a legacy doc comment from early EF Core design phases (when true CLR-less shadow entities were envisioned).


Suggested Resolutions

  • Option A (If ClrType is never null):
    Update the XML documentation of IReadOnlyTypeBase.ClrType to remove the statement:
    Returns null if the type does not have a corresponding CLR class (known as a shadow type).
    Clarify that even shadow/shared-type entities have a CLR representation (e.g. Dictionary<string, object>).

  • Option B (If ClrType can legitimately be null in some design-time / migration contexts):

    1. Change the signature to Type? ClrType { get; }.
    2. Update default interface implementations (e.g. IsAbstract(), ShortName()) to safely handle null.

Note: This issue description was drafted with AI assistance.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions