Unwrap NullableValue for diagnostics creation and add try/catch for formatting diagnostics (#110229)

Fixes #109536

See #93800 for more context.

When a NullableValue is the source of an assignment, we consider use the TypeArgumentTargetsX diagnostic since the underlying value is a generic argument, but use the source of the generic argument for diagnostics. This leads to some confusing warning messages, and sometimes crashes in formatting the diagnostics. Ideally, we would want to unwrap the NullableValues and use the UnderlyingTypeValue as the source, but this would be a breaking change for the situations that don't crash.

FieldValue and MethodReturnValue only provide 1 diagnostic arguments, but the diagnostics for TypeArgumentTargetsX expects 2 arguments from the source. This caused crashes when the situation was encountered. Since they didn't work before at all, this won't be a breaking change to provide the correct warning here.

Author: jtschuster

src/tools/illink/src/ILLink.Shared/Annotations.cs

@@ -85,6 +85,15 @@ private static DynamicallyAccessedMemberTypes[] GetAllDynamicallyAccessedMemberT
 
 		public static (DiagnosticId Id, string[] Arguments) GetDiagnosticForAnnotationMismatch (ValueWithDynamicallyAccessedMembers source, ValueWithDynamicallyAccessedMembers target, string missingAnnotations)
 		{
+			source = source switch {
+				// FieldValue and MethodReturnValue have only one diagnostic argument, so formatting throws when the source
+				// is a NullableValueWithDynamicallyAccessedMembers.
+				// The correct behavior here is to unwrap always, as the underlying type is the one that has the annotations,
+				// but it is a breaking change for other UnderlyingTypeValues.
+				// https://github.com/dotnet/runtime/issues/93800
+				NullableValueWithDynamicallyAccessedMembers { UnderlyingTypeValue: FieldValue or MethodReturnValue } nullable => nullable.UnderlyingTypeValue,
+				_ => source
+			};
 			DiagnosticId diagnosticId = (source, target) switch {
 				(MethodParameterValue maybeThisSource, MethodParameterValue maybeThisTarget) when maybeThisSource.IsThisParameter () && maybeThisTarget.IsThisParameter () => DiagnosticId.DynamicallyAccessedMembersMismatchThisParameterTargetsThisParameter,
 				(MethodParameterValue maybeThis, MethodParameterValue) when maybeThis.IsThisParameter () => DiagnosticId.DynamicallyAccessedMembersMismatchThisParameterTargetsParameter,

src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/NullableAnnotations.cs

@@ -65,6 +65,7 @@ public static void Main ()
 			GetUnderlyingTypeOnNonNullableKnownType.Test ();
 			MakeGenericTypeWithUnknownValue (new object[2] { 1, 2 });
 			MakeGenericTypeWithKnowAndUnknownArray ();
+			RequiresOnNullableMakeGenericType.Test();
 
 			// Prevents optimizing away 'as Type' conversion.
 			PreserveSystemType ();
@@ -100,6 +101,68 @@ static void RequireAllFromMadeGenericNullableOfTypeWithMethodWithRuc ()
 			typeof (Nullable<>).MakeGenericType (typeof (TestStructWithRucMethod)).RequiresAll ();
 		}
 
+		public class RequiresOnNullableMakeGenericType
+		{
+			[Kept]
+			static Type UnannotatedField;
+			[Kept]
+			[KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))]
+			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+			static Type FieldWithMethods;
+			[Kept]
+			[UnexpectedWarning("IL2080", nameof(UnannotatedField), Tool.TrimmerAnalyzerAndNativeAot, "https://github.com/dotnet/runtime/issues/93800")]
+			static void Field()
+			{
+				typeof (Nullable<>).MakeGenericType (UnannotatedField).GetMethods ();
+				typeof (Nullable<>).MakeGenericType (FieldWithMethods).GetMethods ();
+			}
+
+			[Kept]
+			[UnexpectedWarning("IL2090", nameof(unannotated), Tool.TrimmerAnalyzerAndNativeAot, "https://github.com/dotnet/runtime/issues/93800")]
+			static void Parameter(
+				Type unannotated,
+				[KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))]
+				[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type annotated)
+			{
+				typeof (Nullable<>).MakeGenericType (unannotated).GetMethods ();
+				typeof (Nullable<>).MakeGenericType (annotated).GetMethods ();
+			}
+
+			[Kept]
+			[ExpectedWarning("IL2090", "TUnannotated")]
+			static void TypeParameter<
+				TUnannotated,
+				[KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))]
+				[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] TAnnotated>()
+			{
+				typeof (Nullable<>).MakeGenericType (typeof(TUnannotated)).GetMethods ();
+				typeof (Nullable<>).MakeGenericType (typeof(TAnnotated)).GetMethods ();
+			}
+
+			[Kept]
+			[UnexpectedWarning("IL2075", nameof(GetUnannotated), Tool.TrimmerAnalyzerAndNativeAot, "https://github.com/dotnet/runtime/issues/93800")]
+			static void ReturnValue()
+			{
+				typeof (Nullable<>).MakeGenericType (GetUnannotated()).GetMethods ();
+				typeof (Nullable<>).MakeGenericType (GetAnnotated()).GetMethods ();
+			}
+			[Kept]
+			static Type GetUnannotated() => null;
+			[Kept]
+			[return: KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))]
+			[return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]
+			static Type GetAnnotated() => null;
+
+			[Kept]
+			public static void Test()
+			{
+				Field();
+				Parameter(null, null);
+				TypeParameter<object, object>();
+				ReturnValue();
+			}
+		}
+
 		[Kept]
 		static void TestRequireRucMethodThroughNullable ()
 		{