Skip to content

Commit a1f1624

Browse files
karakasakarakasa
authored
Improve nullability check for generic .ctor parameters (#92514)
* 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
1 parent ac1cf4c commit a1f1624

File tree

2 files changed

+228
-23
lines changed

2 files changed

+228
-23
lines changed

‎src/libraries/System.Private.CoreLib/src/System/Reflection/NullabilityInfoContext.cs

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -86,35 +86,47 @@ public NullabilityInfo Create(ParameterInfo parameterInfo)
8686

8787
private void CheckParameterMetadataType(ParameterInfo parameter, NullabilityInfo nullability)
8888
{
89-
if (parameter.Member is MethodInfo method)
89+
ParameterInfo? metaParameter;
90+
MemberInfo metaMember;
91+
92+
switch (parameter.Member)
9093
{
91-
MethodInfo metaMethod = GetMethodMetadataDefinition(method);
92-
ParameterInfo? metaParameter = null;
93-
if (string.IsNullOrEmpty(parameter.Name))
94-
{
95-
metaParameter = metaMethod.ReturnParameter;
96-
}
97-
else
98-
{
99-
ReadOnlySpan<ParameterInfo> parameters = metaMethod.GetParametersAsSpan();
100-
for (int i = 0; i < parameters.Length; i++)
101-
{
102-
if (parameter.Position == i &&
103-
parameter.Name == parameters[i].Name)
104-
{
105-
metaParameter = parameters[i];
106-
break;
107-
}
108-
}
109-
}
94+
case ConstructorInfo ctor:
95+
var metaCtor = (ConstructorInfo)GetMemberMetadataDefinition(ctor);
96+
metaMember = metaCtor;
97+
metaParameter = GetMetaParameter(metaCtor, parameter);
98+
break;
99+
100+
case MethodInfo method:
101+
MethodInfo metaMethod = GetMethodMetadataDefinition(method);
102+
metaMember = metaMethod;
103+
metaParameter = string.IsNullOrEmpty(parameter.Name) ? metaMethod.ReturnParameter : GetMetaParameter(metaMethod, parameter);
104+
break;
105+
106+
default:
107+
return;
108+
}
109+
110+
if (metaParameter != null)
111+
{
112+
CheckGenericParameters(nullability, metaMember, metaParameter.ParameterType, parameter.Member.ReflectedType);
113+
}
114+
}
110115

111-
if (metaParameter != null)
116+
private static ParameterInfo? GetMetaParameter(MethodBase metaMethod, ParameterInfo parameter)
117+
{
118+
ReadOnlySpan<ParameterInfo> parameters = metaMethod.GetParametersAsSpan();
119+
for (int i = 0; i < parameters.Length; i++)
120+
{
121+
if (parameter.Position == i &&
122+
parameter.Name == parameters[i].Name)
112123
{
113-
CheckGenericParameters(nullability, metaMethod, metaParameter.ParameterType, parameter.Member.ReflectedType);
124+
return parameters[i];
114125
}
115126
}
116-
}
117127

128+
return null;
129+
}
118130
private static MethodInfo GetMethodMetadataDefinition(MethodInfo method)
119131
{
120132
if (method.IsGenericMethod && !method.IsGenericMethodDefinition)

‎src/libraries/System.Runtime/tests/System/Reflection/NullabilityInfoContextTests.cs

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1317,6 +1317,146 @@ public void TestNullabilityInfoCreationOnPropertiesWithNestedGenericTypeArgument
13171317
Assert.Equal(expectedGenericArgumentNullability, info.GenericTypeArguments[2].GenericTypeArguments[0].ReadState);
13181318
Assert.Equal(NullabilityState.NotNull, info.GenericTypeArguments[2].GenericTypeArguments[1].ReadState);
13191319
}
1320+
1321+
private static Type EnsureReflection([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type type) => type;
1322+
1323+
public static IEnumerable<object[]> TestCtorParametersData() => new object[][]
1324+
{
1325+
[EnsureReflection(typeof(GenericTypeWithCtor<>)), NullabilityState.Nullable, NullabilityState.Nullable],
1326+
[EnsureReflection(typeof(GenericTypeWithCtor<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
1327+
[EnsureReflection(typeof(GenericTypeWithCtor<int?>)), NullabilityState.Nullable, NullabilityState.Nullable],
1328+
[EnsureReflection(typeof(GenericTypeWithCtor<object>)), NullabilityState.Nullable, NullabilityState.Nullable],
1329+
1330+
[EnsureReflection(typeof(GenericTypeWithCtor_Disallow<>)), NullabilityState.Nullable, NullabilityState.NotNull],
1331+
[EnsureReflection(typeof(GenericTypeWithCtor_Disallow<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
1332+
[EnsureReflection(typeof(GenericTypeWithCtor_Disallow<int?>)), NullabilityState.Nullable, NullabilityState.NotNull],
1333+
[EnsureReflection(typeof(GenericTypeWithCtor_Disallow<object>)), NullabilityState.Nullable, NullabilityState.NotNull],
1334+
1335+
[EnsureReflection(typeof(GenericTypeWithCtor_Maybe<>)), NullabilityState.Nullable, NullabilityState.Nullable],
1336+
[EnsureReflection(typeof(GenericTypeWithCtor_Maybe<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
1337+
[EnsureReflection(typeof(GenericTypeWithCtor_Maybe<int?>)), NullabilityState.Nullable, NullabilityState.Nullable],
1338+
[EnsureReflection(typeof(GenericTypeWithCtor_Maybe<object>)), NullabilityState.Nullable, NullabilityState.Nullable],
1339+
1340+
[EnsureReflection(typeof(GenericTypeWithCtor_Allow<>)), NullabilityState.Nullable, NullabilityState.Nullable],
1341+
[EnsureReflection(typeof(GenericTypeWithCtor_Allow<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
1342+
[EnsureReflection(typeof(GenericTypeWithCtor_Allow<int?>)), NullabilityState.Nullable, NullabilityState.Nullable],
1343+
[EnsureReflection(typeof(GenericTypeWithCtor_Allow<object>)), NullabilityState.Nullable, NullabilityState.Nullable],
1344+
1345+
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor<>)), NullabilityState.NotNull, NullabilityState.NotNull],
1346+
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
1347+
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor<object>)), NullabilityState.NotNull, NullabilityState.NotNull],
1348+
1349+
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Disallow<>)), NullabilityState.NotNull, NullabilityState.NotNull],
1350+
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Disallow<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
1351+
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Disallow<object>)), NullabilityState.NotNull, NullabilityState.NotNull],
1352+
1353+
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Maybe<>)), NullabilityState.Nullable, NullabilityState.NotNull],
1354+
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Maybe<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
1355+
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Maybe<object>)), NullabilityState.Nullable, NullabilityState.NotNull],
1356+
1357+
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Allow<>)), NullabilityState.NotNull, NullabilityState.Nullable],
1358+
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Allow<int>)), NullabilityState.NotNull, NullabilityState.NotNull],
1359+
[EnsureReflection(typeof(GenericNonNullableTypeWithCtor_Allow<object>)), NullabilityState.NotNull, NullabilityState.Nullable],
1360+
};
1361+
1362+
[Theory]
1363+
[MemberData(nameof(TestCtorParametersData))]
1364+
public void TestCtorParameters([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] Type type,
1365+
NullabilityState expectedRead, NullabilityState expectedWrite)
1366+
{
1367+
var ctx = new NullabilityInfoContext();
1368+
1369+
ParameterInfo param = type.GetConstructors()[0].GetParameters()[0];
1370+
NullabilityInfo info = ctx.Create(param);
1371+
1372+
Assert.Equal(expectedRead, info.ReadState);
1373+
Assert.Equal(expectedWrite, info.WriteState);
1374+
}
1375+
1376+
public static IEnumerable<object[]> TestMethodsWithOpenGenericParametersData()
1377+
{
1378+
const string MethodNullable = "GenericMethod";
1379+
const string MethodNonNullable = "GenericNotNullMethod";
1380+
1381+
return new object[][]
1382+
{
1383+
[EnsureReflection(typeof(ClassWithGenericMethods)), MethodNullable, NullabilityState.Nullable, NullabilityState.Nullable],
1384+
[EnsureReflection(typeof(ClassWithGenericMethods)), MethodNonNullable, NullabilityState.NotNull, NullabilityState.NotNull],
1385+
[EnsureReflection(typeof(ClassWithGenericMethods_Disallow)), MethodNullable, NullabilityState.Nullable, NullabilityState.NotNull],
1386+
[EnsureReflection(typeof(ClassWithGenericMethods_Disallow)), MethodNonNullable, NullabilityState.NotNull, NullabilityState.NotNull],
1387+
[EnsureReflection(typeof(ClassWithGenericMethods_Maybe)), MethodNullable, NullabilityState.Nullable, NullabilityState.Nullable],
1388+
[EnsureReflection(typeof(ClassWithGenericMethods_Maybe)), MethodNonNullable, NullabilityState.Nullable, NullabilityState.NotNull],
1389+
[EnsureReflection(typeof(ClassWithGenericMethods_Allow)), MethodNullable, NullabilityState.Nullable, NullabilityState.Nullable],
1390+
[EnsureReflection(typeof(ClassWithGenericMethods_Allow)), MethodNonNullable, NullabilityState.NotNull, NullabilityState.Nullable],
1391+
};
1392+
}
1393+
1394+
[Theory]
1395+
[MemberData(nameof(TestMethodsWithOpenGenericParametersData))]
1396+
public void TestMethodsWithOpenGenericParameters([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)] Type type,
1397+
string methodName, NullabilityState expectedRead, NullabilityState expectedWrite)
1398+
{
1399+
var ctx = new NullabilityInfoContext();
1400+
1401+
MethodInfo method = type.GetMethod(methodName)!;
1402+
1403+
ParameterInfo paramInfo = method.GetParameters()[0];
1404+
NullabilityInfo info = ctx.Create(paramInfo);
1405+
1406+
Assert.Equal(expectedRead, info.ReadState);
1407+
Assert.Equal(expectedWrite, info.WriteState);
1408+
}
1409+
1410+
private delegate void GenericMethod<T>(T value);
1411+
private delegate void GenericMethodRef<T>([MaybeNull] out T value);
1412+
private delegate void GenericMethodDisallow<T>([DisallowNull] T value);
1413+
public static IEnumerable<object[]> TestMethodsWithGenericParametersData() => new object[][]
1414+
{
1415+
[(GenericMethod<int>)ClassWithGenericMethods.GenericMethod, NullabilityState.NotNull, NullabilityState.NotNull],
1416+
[(GenericMethod<int?>)ClassWithGenericMethods.GenericMethod, NullabilityState.Nullable, NullabilityState.Nullable],
1417+
[(GenericMethod<object>)ClassWithGenericMethods.GenericMethod, NullabilityState.Nullable, NullabilityState.Nullable],
1418+
1419+
[(GenericMethod<int>)ClassWithGenericMethods.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.NotNull],
1420+
[(GenericMethod<object>)ClassWithGenericMethods.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.NotNull],
1421+
1422+
[(GenericMethodRef<int>)ClassWithGenericMethods_Maybe.GenericMethod, NullabilityState.NotNull, NullabilityState.NotNull],
1423+
[(GenericMethodRef<int?>)ClassWithGenericMethods_Maybe.GenericMethod, NullabilityState.Nullable, NullabilityState.Nullable],
1424+
[(GenericMethodRef<object>)ClassWithGenericMethods_Maybe.GenericMethod, NullabilityState.Nullable, NullabilityState.Nullable],
1425+
1426+
[(GenericMethodRef<int>)ClassWithGenericMethods_Maybe.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.NotNull],
1427+
[(GenericMethodRef<object>)ClassWithGenericMethods_Maybe.GenericNotNullMethod, NullabilityState.Nullable, NullabilityState.NotNull],
1428+
1429+
[(GenericMethod<int>)ClassWithGenericMethods_Allow.GenericMethod, NullabilityState.NotNull, NullabilityState.NotNull],
1430+
[(GenericMethod<int?>)ClassWithGenericMethods_Allow.GenericMethod, NullabilityState.Nullable, NullabilityState.Nullable],
1431+
[(GenericMethod<object>)ClassWithGenericMethods_Allow.GenericMethod, NullabilityState.Nullable, NullabilityState.Nullable],
1432+
1433+
[(GenericMethod<int>)ClassWithGenericMethods_Allow.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.NotNull],
1434+
[(GenericMethod<object>)ClassWithGenericMethods_Allow.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.Nullable],
1435+
1436+
// Specialized delegates are required due to CS8622
1437+
1438+
[(GenericMethodDisallow<int>)ClassWithGenericMethods_Disallow.GenericMethod, NullabilityState.NotNull, NullabilityState.NotNull],
1439+
[(GenericMethodDisallow<int?>)ClassWithGenericMethods_Disallow.GenericMethod, NullabilityState.Nullable, NullabilityState.NotNull],
1440+
[(GenericMethodDisallow<object>)ClassWithGenericMethods_Disallow.GenericMethod, NullabilityState.Nullable, NullabilityState.NotNull],
1441+
1442+
[(GenericMethodDisallow<int>)ClassWithGenericMethods_Disallow.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.NotNull],
1443+
[(GenericMethodDisallow<object>)ClassWithGenericMethods_Disallow.GenericNotNullMethod, NullabilityState.NotNull, NullabilityState.NotNull],
1444+
};
1445+
1446+
[Theory]
1447+
[MemberData(nameof(TestMethodsWithGenericParametersData))]
1448+
public void TestMethodsWithGenericParameters(Delegate @delegate, NullabilityState expectedRead, NullabilityState expectedWrite)
1449+
{
1450+
var ctx = new NullabilityInfoContext();
1451+
1452+
MethodInfo method = @delegate.Method;
1453+
1454+
ParameterInfo paramInfo = method.GetParameters()[0];
1455+
NullabilityInfo info = ctx.Create(paramInfo);
1456+
1457+
Assert.Equal(expectedRead, info.ReadState);
1458+
Assert.Equal(expectedWrite, info.WriteState);
1459+
}
13201460
}
13211461

13221462
#pragma warning disable CS0649, CS0067, CS0414
@@ -1599,4 +1739,57 @@ public class TypeWithPropertiesNestingItsGenericTypeArgument<T>
15991739
public Tuple<int, int?, Tuple<T?>>? Deep4 { get; set; }
16001740
public Tuple<int, int, Tuple<T, int>?>? Deep5 { get; set; }
16011741
}
1742+
1743+
public class GenericTypeWithCtor<T>
1744+
{
1745+
public GenericTypeWithCtor(T value) { }
1746+
}
1747+
public class GenericTypeWithCtor_Disallow<T>
1748+
{
1749+
public GenericTypeWithCtor_Disallow([DisallowNull] T value) { }
1750+
}
1751+
public class GenericTypeWithCtor_Maybe<T>
1752+
{
1753+
public GenericTypeWithCtor_Maybe([MaybeNull] out T value) { value = default; }
1754+
}
1755+
public class GenericTypeWithCtor_Allow<T>
1756+
{
1757+
public GenericTypeWithCtor_Allow([AllowNull] T value) { }
1758+
}
1759+
public class GenericNonNullableTypeWithCtor<T> where T : notnull
1760+
{
1761+
public GenericNonNullableTypeWithCtor(T value) { }
1762+
}
1763+
public class GenericNonNullableTypeWithCtor_Disallow<T> where T : notnull
1764+
{
1765+
public GenericNonNullableTypeWithCtor_Disallow([DisallowNull] T value) { }
1766+
}
1767+
public class GenericNonNullableTypeWithCtor_Maybe<T> where T : notnull
1768+
{
1769+
public GenericNonNullableTypeWithCtor_Maybe([MaybeNull] out T value) { value = default; }
1770+
}
1771+
public class GenericNonNullableTypeWithCtor_Allow<T> where T : notnull
1772+
{
1773+
public GenericNonNullableTypeWithCtor_Allow([AllowNull] T value) { }
1774+
}
1775+
public class ClassWithGenericMethods
1776+
{
1777+
public static void GenericMethod<T>(T value) => throw new Exception();
1778+
public static void GenericNotNullMethod<T>(T value) where T : notnull => throw new Exception();
1779+
}
1780+
public class ClassWithGenericMethods_Disallow
1781+
{
1782+
public static void GenericMethod<T>([DisallowNull] T value) => throw new Exception();
1783+
public static void GenericNotNullMethod<T>([DisallowNull] T value) where T : notnull => throw new Exception();
1784+
}
1785+
public class ClassWithGenericMethods_Maybe
1786+
{
1787+
public static void GenericMethod<T>([MaybeNull] out T value) => throw new Exception();
1788+
public static void GenericNotNullMethod<T>([MaybeNull] out T value) where T : notnull => throw new Exception();
1789+
}
1790+
public class ClassWithGenericMethods_Allow
1791+
{
1792+
public static void GenericMethod<T>([AllowNull] T value) => throw new Exception();
1793+
public static void GenericNotNullMethod<T>([AllowNull] T value) where T : notnull => throw new Exception();
1794+
}
16021795
}

0 commit comments

Comments
 (0)