Skip to content

Commit

Permalink
Improve nullability check for generic .ctor parameters (#92514)
Browse files Browse the repository at this point in the history 
* implement absent generic ctor param check

* fix code style

* Improve nullability check for generic parameters in ctor

`NullabilityInfoContext.CheckParameterMetadataType` didn't have
code paths for parameters in constructors, leading to wrong
nullability results.

The PR adds a code path for constructor parameters.

Fix #92487

* add tests on nullability of ctors and methods with generic parameters

* fix test issues with AOT trimming
  • Loading branch information
@karakasa
karakasa authored Sep 26, 2023
1 parent ac1cf4c commit a1f1624
Show file tree
Hide file tree
Showing 2 changed files with 228 additions and 23 deletions.
58 changes: 35 additions & 23 deletions src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,35 +86,47 @@ public NullabilityInfo Create(ParameterInfo parameterInfo)

private void CheckParameterMetadataType(ParameterInfo parameter, NullabilityInfo nullability)
{
if (parameter.Member is MethodInfo method)
ParameterInfo? metaParameter;
MemberInfo metaMember;

switch (parameter.Member)
{
MethodInfo metaMethod = GetMethodMetadataDefinition(method);
ParameterInfo? metaParameter = null;
if (string.IsNullOrEmpty(parameter.Name))
{
metaParameter = metaMethod.ReturnParameter;
}
else
{
ReadOnlySpan<ParameterInfo> parameters = metaMethod.GetParametersAsSpan();
for (int i = 0; i < parameters.Length; i++)
{
if (parameter.Position == i &&
parameter.Name == parameters[i].Name)
{
metaParameter = parameters[i];
break;
}
}
}
case ConstructorInfo ctor:
var metaCtor = (ConstructorInfo)GetMemberMetadataDefinition(ctor);
metaMember = metaCtor;
metaParameter = GetMetaParameter(metaCtor, parameter);
break;

case MethodInfo method:
MethodInfo metaMethod = GetMethodMetadataDefinition(method);
metaMember = metaMethod;
metaParameter = string.IsNullOrEmpty(parameter.Name) ? metaMethod.ReturnParameter : GetMetaParameter(metaMethod, parameter);
break;

default:
return;
}

if (metaParameter != null)
{
CheckGenericParameters(nullability, metaMember, metaParameter.ParameterType, parameter.Member.ReflectedType);
}
}

if (metaParameter != null)
private static ParameterInfo? GetMetaParameter(MethodBase metaMethod, ParameterInfo parameter)
{
ReadOnlySpan<ParameterInfo> parameters = metaMethod.GetParametersAsSpan();
for (int i = 0; i < parameters.Length; i++)
{
if (parameter.Position == i &&
parameter.Name == parameters[i].Name)
{
CheckGenericParameters(nullability, metaMethod, metaParameter.ParameterType, parameter.Member.ReflectedType);
return parameters[i];
}
}
}

return null;
}
private static MethodInfo GetMethodMetadataDefinition(MethodInfo method)
{
if (method.IsGenericMethod && !method.IsGenericMethodDefinition)
Expand Down
193 changes: 193 additions & 0 deletions src/libraries/System.Runtime/tests/System/Reflection/NullabilityInfoContextTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1317,6 +1317,146 @@ public void TestNullabilityInfoCreationOnPropertiesWithNestedGenericTypeArgument
Assert.Equal(expectedGenericArgumentNullability, info.GenericTypeArguments[2].GenericTypeArguments[0].ReadState);
Assert.Equal(NullabilityState.NotNull, info.GenericTypeArguments[2].GenericTypeArguments[1].ReadState);
}

private static Type EnsureReflection([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type type) => type;

public static IEnumerable<object[]> TestCtorParametersData() => new object[][]
{
[EnsureReflection(typeof(GenericTypeWithCtor<>)), NullabilityState.Nullable, NullabilityState.Nullable],
[EnsureReflection(typeof(GenericTypeWithCtor<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericTypeWithCtor<int?>)), NullabilityState.Nullable, NullabilityState.Nullable],
[EnsureReflection(typeof(GenericTypeWithCtor<object>)), NullabilityState.Nullable, NullabilityState.Nullable],

[EnsureReflection(typeof(GenericTypeWithCtor_Disallow<>)), NullabilityState.Nullable, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericTypeWithCtor_Disallow<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericTypeWithCtor_Disallow<int?>)), NullabilityState.Nullable, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericTypeWithCtor_Disallow<object>)), NullabilityState.Nullable, NullabilityState.NotNull],

[EnsureReflection(typeof(GenericTypeWithCtor_Maybe<>)), NullabilityState.Nullable, NullabilityState.Nullable],
[EnsureReflection(typeof(GenericTypeWithCtor_Maybe<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericTypeWithCtor_Maybe<int?>)), NullabilityState.Nullable, NullabilityState.Nullable],
[EnsureReflection(typeof(GenericTypeWithCtor_Maybe<object>)), NullabilityState.Nullable, NullabilityState.Nullable],

[EnsureReflection(typeof(GenericTypeWithCtor_Allow<>)), NullabilityState.Nullable, NullabilityState.Nullable],
[EnsureReflection(typeof(GenericTypeWithCtor_Allow<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericTypeWithCtor_Allow<int?>)), NullabilityState.Nullable, NullabilityState.Nullable],
[EnsureReflection(typeof(GenericTypeWithCtor_Allow<object>)), NullabilityState.Nullable, NullabilityState.Nullable],

[EnsureReflection(typeof(GenericNonNullableTypeWithCtor<>)), NullabilityState.NotNull, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor<object>)), NullabilityState.NotNull, NullabilityState.NotNull],

[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Disallow<>)), NullabilityState.NotNull, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Disallow<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Disallow<object>)), NullabilityState.NotNull, NullabilityState.NotNull],

[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Maybe<>)), NullabilityState.Nullable, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Maybe<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Maybe<object>)), NullabilityState.Nullable, NullabilityState.NotNull],

[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Allow<>)), NullabilityState.NotNull, NullabilityState.Nullable],
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Allow<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Allow<object>)), NullabilityState.NotNull, NullabilityState.Nullable],
};

[Theory]
[MemberData(nameof(TestCtorParametersData))]
public void TestCtorParameters([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type,
NullabilityState expectedRead, NullabilityState expectedWrite)
{
var ctx = new NullabilityInfoContext();

ParameterInfo param = type.GetConstructors()[0].GetParameters()[0];
NullabilityInfo info = ctx.Create(param);

Assert.Equal(expectedRead, info.ReadState);
Assert.Equal(expectedWrite, info.WriteState);
}

public static IEnumerable<object[]> TestMethodsWithOpenGenericParametersData()
{
const string MethodNullable = "GenericMethod";
const string MethodNonNullable = "GenericNotNullMethod";

return new object[][]
{
[EnsureReflection(typeof(ClassWithGenericMethods)), MethodNullable, NullabilityState.Nullable, NullabilityState.Nullable],
[EnsureReflection(typeof(ClassWithGenericMethods)), MethodNonNullable, NullabilityState.NotNull, NullabilityState.NotNull],
[EnsureReflection(typeof(ClassWithGenericMethods_Disallow)), MethodNullable, NullabilityState.Nullable, NullabilityState.NotNull],
[EnsureReflection(typeof(ClassWithGenericMethods_Disallow)), MethodNonNullable, NullabilityState.NotNull, NullabilityState.NotNull],
[EnsureReflection(typeof(ClassWithGenericMethods_Maybe)), MethodNullable, NullabilityState.Nullable, NullabilityState.Nullable],
[EnsureReflection(typeof(ClassWithGenericMethods_Maybe)), MethodNonNullable, NullabilityState.Nullable, NullabilityState.NotNull],
[EnsureReflection(typeof(ClassWithGenericMethods_Allow)), MethodNullable, NullabilityState.Nullable, NullabilityState.Nullable],
[EnsureReflection(typeof(ClassWithGenericMethods_Allow)), MethodNonNullable, NullabilityState.NotNull, NullabilityState.Nullable],
};
}

[Theory]
[MemberData(nameof(TestMethodsWithOpenGenericParametersData))]
public void TestMethodsWithOpenGenericParameters([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] Type type,
string methodName, NullabilityState expectedRead, NullabilityState expectedWrite)
{
var ctx = new NullabilityInfoContext();

MethodInfo method = type.GetMethod(methodName)!;

ParameterInfo paramInfo = method.GetParameters()[0];
NullabilityInfo info = ctx.Create(paramInfo);

Assert.Equal(expectedRead, info.ReadState);
Assert.Equal(expectedWrite, info.WriteState);
}

private delegate void GenericMethod<T>(T value);
private delegate void GenericMethodRef<T>([MaybeNull] out T value);
private delegate void GenericMethodDisallow<T>([DisallowNull] T value);
public static IEnumerable<object[]> TestMethodsWithGenericParametersData() => new object[][]
{
[(GenericMethod<int>)ClassWithGenericMethods.GenericMethod, NullabilityState.NotNull, NullabilityState.NotNull],
[(GenericMethod<int?>)ClassWithGenericMethods.GenericMethod, NullabilityState.Nullable, NullabilityState.Nullable],
[(GenericMethod<object>)ClassWithGenericMethods.GenericMethod, NullabilityState.Nullable, NullabilityState.Nullable],

[(GenericMethod<int>)ClassWithGenericMethods.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.NotNull],
[(GenericMethod<object>)ClassWithGenericMethods.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.NotNull],

[(GenericMethodRef<int>)ClassWithGenericMethods_Maybe.GenericMethod, NullabilityState.NotNull, NullabilityState.NotNull],
[(GenericMethodRef<int?>)ClassWithGenericMethods_Maybe.GenericMethod, NullabilityState.Nullable, NullabilityState.Nullable],
[(GenericMethodRef<object>)ClassWithGenericMethods_Maybe.GenericMethod, NullabilityState.Nullable, NullabilityState.Nullable],

[(GenericMethodRef<int>)ClassWithGenericMethods_Maybe.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.NotNull],
[(GenericMethodRef<object>)ClassWithGenericMethods_Maybe.GenericNotNullMethod, NullabilityState.Nullable, NullabilityState.NotNull],

[(GenericMethod<int>)ClassWithGenericMethods_Allow.GenericMethod, NullabilityState.NotNull, NullabilityState.NotNull],
[(GenericMethod<int?>)ClassWithGenericMethods_Allow.GenericMethod, NullabilityState.Nullable, NullabilityState.Nullable],
[(GenericMethod<object>)ClassWithGenericMethods_Allow.GenericMethod, NullabilityState.Nullable, NullabilityState.Nullable],

[(GenericMethod<int>)ClassWithGenericMethods_Allow.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.NotNull],
[(GenericMethod<object>)ClassWithGenericMethods_Allow.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.Nullable],

// Specialized delegates are required due to CS8622

[(GenericMethodDisallow<int>)ClassWithGenericMethods_Disallow.GenericMethod, NullabilityState.NotNull, NullabilityState.NotNull],
[(GenericMethodDisallow<int?>)ClassWithGenericMethods_Disallow.GenericMethod, NullabilityState.Nullable, NullabilityState.NotNull],
[(GenericMethodDisallow<object>)ClassWithGenericMethods_Disallow.GenericMethod, NullabilityState.Nullable, NullabilityState.NotNull],

[(GenericMethodDisallow<int>)ClassWithGenericMethods_Disallow.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.NotNull],
[(GenericMethodDisallow<object>)ClassWithGenericMethods_Disallow.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.NotNull],
};

[Theory]
[MemberData(nameof(TestMethodsWithGenericParametersData))]
public void TestMethodsWithGenericParameters(Delegate @delegate, NullabilityState expectedRead, NullabilityState expectedWrite)
{
var ctx = new NullabilityInfoContext();

MethodInfo method = @delegate.Method;

ParameterInfo paramInfo = method.GetParameters()[0];
NullabilityInfo info = ctx.Create(paramInfo);

Assert.Equal(expectedRead, info.ReadState);
Assert.Equal(expectedWrite, info.WriteState);
}
}

#pragma warning disable CS0649, CS0067, CS0414
Expand Down Expand Up @@ -1599,4 +1739,57 @@ public class TypeWithPropertiesNestingItsGenericTypeArgument<T>
public Tuple<int, int?, Tuple<T?>>? Deep4 { get; set; }
public Tuple<int, int, Tuple<T, int>?>? Deep5 { get; set; }
}

public class GenericTypeWithCtor<T>
{
public GenericTypeWithCtor(T value) { }
}
public class GenericTypeWithCtor_Disallow<T>
{
public GenericTypeWithCtor_Disallow([DisallowNull] T value) { }
}
public class GenericTypeWithCtor_Maybe<T>
{
public GenericTypeWithCtor_Maybe([MaybeNull] out T value) { value = default; }
}
public class GenericTypeWithCtor_Allow<T>
{
public GenericTypeWithCtor_Allow([AllowNull] T value) { }
}
public class GenericNonNullableTypeWithCtor<T> where T : notnull
{
public GenericNonNullableTypeWithCtor(T value) { }
}
public class GenericNonNullableTypeWithCtor_Disallow<T> where T : notnull
{
public GenericNonNullableTypeWithCtor_Disallow([DisallowNull] T value) { }
}
public class GenericNonNullableTypeWithCtor_Maybe<T> where T : notnull
{
public GenericNonNullableTypeWithCtor_Maybe([MaybeNull] out T value) { value = default; }
}
public class GenericNonNullableTypeWithCtor_Allow<T> where T : notnull
{
public GenericNonNullableTypeWithCtor_Allow([AllowNull] T value) { }
}
public class ClassWithGenericMethods
{
public static void GenericMethod<T>(T value) => throw new Exception();
public static void GenericNotNullMethod<T>(T value) where T : notnull => throw new Exception();
}
public class ClassWithGenericMethods_Disallow
{
public static void GenericMethod<T>([DisallowNull] T value) => throw new Exception();
public static void GenericNotNullMethod<T>([DisallowNull] T value) where T : notnull => throw new Exception();
}
public class ClassWithGenericMethods_Maybe
{
public static void GenericMethod<T>([MaybeNull] out T value) => throw new Exception();
public static void GenericNotNullMethod<T>([MaybeNull] out T value) where T : notnull => throw new Exception();
}
public class ClassWithGenericMethods_Allow
{
public static void GenericMethod<T>([AllowNull] T value) => throw new Exception();
public static void GenericNotNullMethod<T>([AllowNull] T value) where T : notnull => throw new Exception();
}
}

0 comments on commit a1f1624

Please sign in to comment.