NullabilityInfoContext reports null/notnull for unspecified generic parameters

[madelson]

Description

For a generic parameter T on a class with no type constraints indicating nullability, Unknown would seem most appropriate nullability state to return from NullabilityInfoContext because there are no constraints on that type with respect to nullability.

However, the value returned seems to depend on whether the type is declared in a nullable or non-nullable context

Reproduction Steps

public static void Main()
    {
        var context = new NullabilityInfoContext();

        var fooInfo = context.Create(typeof(Foo<string?>).GetConstructors().Single().GetParameters().Single());
        Console.WriteLine(fooInfo.WriteState); // NotNull (I would expect Unknown; NotNull is clearly wrong here given that I can make Foo<string?>)

        var barInfo = context.Create(typeof(Bar<string?>).GetConstructors().Single().GetParameters().Single());
        Console.WriteLine(barInfo.WriteState); // Unknown
    }

#nullable disable
    private class Foo<T> { public Foo(T t) {  } }
#nullable enable

#nullable disable
    private class Bar<T> { public Bar(T t) { } }
#nullable enable

Expected behavior

Consistently returns Unknown.

Actual behavior

Sometimes returns NotNull.

Regression?

No response

Known Workarounds

No response

Configuration

VS 17.0.2, .NET 6, Windows 10 x64.

I don't have any reason to believe this is configuration specific.

Other information

C#9 allows for both T and T? to be used in generic classes even without any constraint that indicates whether or not T can be null:

class Generic<T> { public void Foo(T a, T? b) { } }

It's unclear what NullabilityInfoContext should return when reflecting over such types. On one hand, both could be either non-nullable or nullable depending on the particular generic argument that ends up getting used, which makes Unknown seem reasonable for both from that perspective.

On the other hand, it seems nice to be able to differentiate between the two in some way. Perhaps it makes sense to add another null state that explicitly represents the case of NotNullIfGenericArgumentIsNonNullableReferenceType?

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

For a generic parameter T on a class with no type constraints indicating nullability, Unknown would seem most appropriate nullability state to return from NullabilityInfoContext because there are no constraints on that type with respect to nullability.

However, the value returned seems to depend on whether the type is declared in a nullable or non-nullable context

Reproduction Steps

public static void Main()
    {
        var context = new NullabilityInfoContext();

        var fooInfo = context.Create(typeof(Foo<string?>).GetConstructors().Single().GetParameters().Single());
        Console.WriteLine(fooInfo.WriteState); // NotNull (I would expect Unknown; NotNull is clearly wrong here given that I can make Foo<string?>)

        var barInfo = context.Create(typeof(Bar<string?>).GetConstructors().Single().GetParameters().Single());
        Console.WriteLine(barInfo.WriteState); // Unknown
    }

#nullable disable
    private class Foo<T> { public Foo(T t) {  } }
#nullable enable

#nullable disable
    private class Bar<T> { public Bar(T t) { } }
#nullable enable

Expected behavior

Consistently returns Unknown.

Actual behavior

Sometimes returns NotNull.

Regression?

No response

Known Workarounds

No response

Configuration

VS 17.0.2, .NET 6, Windows 10 x64.

I don't have any reason to believe this is configuration specific.

Other information

C#9 allows for both T and T? to be used in generic classes even without any constraint that indicates whether or not T can be null:

class Generic<T> { public void Foo(T a, T? b) { } }

It's unclear what NullabilityInfoContext should return when reflecting over such types. On one hand, both could be either non-nullable or nullable depending on the particular generic argument that ends up getting used, which makes Unknown seem reasonable for both from that perspective.

On the other hand, it seems nice to be able to differentiate between the two in some way. Perhaps it makes sense to add another null state that explicitly represents the case of NotNullIfGenericArgumentIsNonNullableReferenceType?

Author:	madelson
Assignees:	-
Labels:	area-System.Reflection, untriaged
Milestone:	-

[Shane32]

Somewhat related issue here - see below PR just to fix Tuple<Tuple<string?, string?>, string> to work right:

• Fix NullabilityInfoContext.Create to properly analyze types with nested generic types #63556

[Shane32]

As it relates to this issue, it appears that code was added to inspect the generic type (e.g. perhaps it was a closed type), and return the correct nullability status. But the code is so broken that the value is completely useless. It would be more useful if it did not do any inspection and simply returned the nullability flag of T given a method Foo<T>(). (meaning, if it was declared T or T?) That information is readily available with no deep inspection required.

I have not spent the time to write a fix and put a PR here (besides the small fix above), but I have written a great many tests that demonstrate the wide variety of ways that T could be declared and what the code must account for. (And hence the great many situations in which it currently fails.) Either that, or it should completely ignore what T is and return the nullability flag declared on the member.

See:

• https://github.com/Shane32/NullabilityInfo/tree/with_net6_code

[Shane32]

On one hand, both could be either non-nullable or nullable depending on the particular generic argument that ends up getting used, which makes Unknown seem reasonable for both from that perspective.

I agree. I would say that "Unknown" should be returned for T when T is a unrestricted generic type, as it could be a nullable reference type. But "Not-Null" should be returned for T when T is restricted to notnull, a non-null reference type, or a struct type. And "Nullable" should be returned for T? regardless. Oh, and if it's a closed generic, then the proper nullability status should be returned. All of the above is possible with reflection for public members/types, and for private members/types when the necessary attributes are not suppressed by the compiler. I believe this would include closed generic types generated at compile time (like List<int>), but not closed generic types generated at runtime. But I have not tested that yet. At least the code should provide what information it can.

[buyaa-n]

C#9 allows for both T and T? to be used in generic classes even without any constraint that indicates whether or not T can be null:

class Generic<T> { public void Foo(T a, T? b) { } }

It's unclear what NullabilityInfoContext should return when reflecting over such types. On one hand, both could be either non-nullable or nullable depending on the particular generic argument that ends up getting used, which makes Unknown seem reasonable for both from that perspective.

T can be nullable in case refr type used for the generic argument, will be non null if value type used, but T? will always be nullable even for value type.

Therefore the nullability result of T? always will be Nullable for any concrete type used as type parameter, but for T it depened from:

1. if nullability context disabled Unknown for ref types
2. if nullability context enabled and if a ref type used as type parameter (or open generic) it is Nullable and NotNull for value type

With that being said i see Foo(T t) is returning NotNull when nullability enabled and string type is used as type parameter, that is a bug, it should be Nullable by current design

On the other hand, it seems nice to be able to differentiate between the two in some way. Perhaps it makes sense to add another null state that explicitly represents the case of NotNullIfGenericArgumentIsNonNullableReferenceType?

That is interesting idea

[buyaa-n]

As it relates to this issue, it appears that code was added to inspect the generic type (e.g. perhaps it was a closed type), and return the correct nullability status. But the code is so broken that the value is completely useless. It would be more useful if it did not do any inspection and simply returned the nullability flag of T given a method Foo(). (meaning, if it was declared T or T?) That information is readily available with no deep inspection required.

We are doing that inspection exactly for returning the nullability flag of T given a method Foo(). (meaning, if it was declared T or T?) , but that information is not readily available, only available if it is open generic. When Foo<string?> is used we would see only Foo<string>, the compiler would not retain the ?, therefore we inspect to the original generic type declaration to get the nullability info. Though I agree that that inspection has bugs need to be fixed

[Shane32]

Thanks for the input. I will update my test project to match your description of C# semantics and internals.

[madelson]

Based on https://docs.microsoft.com/en-us/dotnet/csharp/nullable-references, there is zero difference between T and T? in unrestricted generic types except in the case that T is bound to a non-nullable reference type:

Type	T	T?
int	non-nullable	non-nullable
int?	nullable	nullable
object	non-nullable	nullable
object?	nullable	nullable

That's why I feel it is misleading to return either NotNull or Nullable when examining a generic parameter of type T without further information.

Curious whether we agree on what NullabilityInfoContext should return in these different cases:

// classes
#nullable disable
class ObliviousGeneric<T> { T A; }
#nullable enable
class Generic<T> { T B; T? C; }
class HasGeneric { ObliviousGeneric<object> D; ObliviousGeneric<object?> E; Generic<object> F; Generic<object?> G; }
class DerivesFromObliviousNonNullGeneric : ObliviousGeneric<object> { }
class DerivesFromObliviousNullGeneric : ObliviousGeneric<object?> { }
class DerivesFromNonNullGeneric : Generic<object> { }
class DerivesFromNullGeneric : Generic<object?> { }
class NotNullGeneric<T> where T : notnull { T I; T? J; }
class ClassGeneric<T> where T : class { T K; T? L; }
class NullClassGeneric<T> where T : class? { T M; T? N; }
class StructGeneric<T> where T : struct { T O; T? P; }
class NotNullGenericDerivesFromObliviousGeneric<T> : ObliviousGeneric<T> where T : notnull { }
class DerivesFromNotNullGenericDerivesFromObliviousGeneric : NotNullGenericDerivesFromObliviousGeneric<object> { }
class ClassRestrictedGeneric<T> where T : Class1 { T K; T? L; }
class NullClassRestrictedGeneric<T> where T : Class1? { T M; T? N; }
class Class1 { }

Case	Desired result	Notes
typeof(ObliviousGeneric<>).A	Unknown
typeof(ObliviousGeneric<object>).A	Unknown	It would be nice if this could return NotNull, but we won't have any metadata to determine that
typeof(Generic<>).B	Unknown	I could also see an argument for the more nuanced NotNullIfGenericArgumentIsNonNullableReferenceType here
typeof(Generic<>).C	Unknown	I could also see an argument for the more nuanced NullableIfGenericArgumentIsNonNullableReferenceType here
typeof(HasGeneric).D (generic parameter)	NotNull
typeof(HashGeneric).E (generic parameter)	Nullable
typeof(HasGeneric).F (generic parameter)	NotNull
typeof(HashGeneric).G (generic parameter)	Nullable
typeof(DerivesFromObliviousNonNullGeneric).A	NotNull	Metadata available on DerivesFromObliviousNonNullGeneric
typeof(DerivesFromObliviousNullGeneric ).A	Nullable	Metadata available on DerivesFromObliviousNullGeneric
typeof(DerivesFromNonNullGeneric).B	NotNull	Metadata available on DerivesFromNonNullGeneric
typeof(DerivesFromNonNullGeneric).C	Nullable	Metadata available on DerivesFromNonNullGeneric
typeof(DerivesFromNullGeneric).B	NotNull	Metadata available on DerivesFromNullGeneric
typeof(DerivesFromNullGeneric).C	Nullable	Metadata available on DerivesFromNullGeneric
typeof(NotNullGeneric<>).I	NotNull
typeof(NotNullGeneric<>).J	Unknown	Could be NullableIfGenericArgumentIsNonNullableReferenceType. If T ends up being int then J's type will be int not int? so Nullable feels inaccurrate
typeof(NotNullGeneric<object>).I	NotNull
typeof(NotNullGeneric<object>).J	Nullable
typeof(NotNullGeneric<int>).I	NotNull
typeof(NotNullGeneric<int>).J	NotNull
typeof(ClassGeneric<>).K	NotNull
typeof(ClassGeneric<>).L	Nullable
typeof(NullClassGeneric<>).M	Unknown	Could be NotNullIfGenericArgumentIsNonNullableReferenceType
typeof(NullClassGeneric<>).N	Nullable
typeof(StructGeneric<>).O	NotNull
typeof(StructGeneric<>).P	Nullable
typeof(NotNullGenericDerivesFromObliviousGeneric<>).A	NotNull
typeof(DerivesFromNotNullGenericDerivesFromObliviousGeneric).A	NotNull
typeof(ClassRestrictedGeneric<>).K	NotNull	Unclear where the metadata is
typeof(ClassRestrictedGeneric<>).L	Nullable	Unclear where the metadata is
typeof(NullClassRestrictedGeneric<>).M	Unknown	Could be NotNullIfGenericArgumentIsNonNullableReferenceType
typeof(NullClassRestrictedGeneric<>).N	Nullable	Unclear where the metadata is

[Shane32]

Makes sense to me.

Notes:

Type	Notes
class StructGeneric<T> where T : struct { T O; T P; }	should be T? P;
typeof(DerivesFromObliviousNonNullGeneric ).B	method does not exist
typeof(DerivesFromObliviousNullGeneric ).B	method does not exist
typeof(DerivesFromNonNullGeneric).A	should be .B
typeof(DerivesFromNonNullGeneric).B	should be .C
typeof(DerivesFromNullGeneric).A	should be .B
typeof(DerivesFromNullGeneric).B	should be .C

[Shane32]

@buyaa-n If you can review and confirm the list provided above by @madelson , it will give us a chance to test the existing functionality, and if necessary, write a pull request that fixes it.

[madelson]

@Shane32 good catch! I've updated my list and added a few more cases too.

From poking around the NullabilityInfoContext code, I think the root cause(s) are that:

• We never inspect the ReflectedType of the passed in parameter/member which would allow us to leverage information layered on by derived types
• We seem to override nullability info based on nullability info from generic parameters, whereas I think that generic parameter nullability should act as more of a constraining factor layered on top (even if concepts like NotNullIfGenericArgumentIsNonNullableReferenceType aren't exposed via the API, I think we may need them internally while building up to the answer).