Skip to content

Improve nullability check for generic .ctor parameters #92514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 26, 2023
Merged

Improve nullability check for generic .ctor parameters #92514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b62f167
implement absent generic ctor param check
karakasa Sep 23, 2023
8a03bbb
fix code style
karakasa Sep 23, 2023
31afd2a
Improve nullability check for generic parameters in ctor
karakasa Sep 23, 2023
440f99e
add tests on nullability of ctors and methods with generic parameters
karakasa Sep 25, 2023
2ea7f5e
fix test issues with AOT trimming
karakasa Sep 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 35 additions & 23 deletions src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs
View file
Open in desktop
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
Open in desktop
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();
}
}