[Mono.Android] fix a set of the "easiest" trimmer warnings

Context: dotnet#5652 Introduce a new `trim-analyzers.targets` file to be used in this repo: 1. Enables appropriate analyzers. 2. `$(ILLinkTreatWarningsAsErrors)` only does anything in builds that would run `ILLink`, such as an application build. Useful to be here. 3. List all the explicit ILLink and AOT warnings, so that they trigger build errors. Eventually we should be able to import this file for `Mono.Android.csproj`. Fixing the following so far: ~~ TypeConverter ~~ Context: https://source.dot.net/#System.ComponentModel.TypeConverter/System/ComponentModel/TypeConverter.cs,226 src\Mono.Android\System.Drawing\SizeFConverter.cs(121,49): error IL2046: Base member 'System.ComponentModel.TypeConverter.GetProperties(ITypeDescriptorContext, Object, Attribute[])' with 'RequiresUnreferencedCodeAttribute' has a derived member 'System.Drawing.SizeFConverter.GetProperties(ITypeDescriptorContext, Object, Attribute[])' without 'RequiresUnreferencedCodeAttribute'. 'RequiresUnreferencedCodeAttribute' annotations must match across all interface implementations or overrides. Various `TypeConverter` implementations need to specify `[RequiresUnreferencedCode]` to match the base type. ~~ ColorValueMarshaler & IJavaObjectValueMarshaler ~~ Context: dotnet/java-interop@7d1e705 From the trimmer warnings solved in `Java.Interop.dll`, we need to bring these changes forward to any `*Marshaler` types. ~~ AndroidClientHandler ~~ 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.NonPublicFields' in call to 'System.Type.GetField(String, BindingFlags)'. The return value of method 'System.Collections.IEnumerator.Current.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. This class had a loop that was not trimming safe: for (var type = GetType (); type != null; type = type.BaseType) { field = type.GetField (fieldName, BindingFlags.Instance | BindingFlags.NonPublic); if (field != null) break; } If we *look* at the actual base types of `AndroidClientHandler`, we can simplify this loop to something that *is* trimming safe: const BindingFlags flags = BindingFlags.Instance | BindingFlags.NonPublic; FieldInfo? field = typeof (HttpClientHandler).GetField (fieldName, flags) ?? typeof (HttpMessageHandler).GetField (fieldName, flags); There should be no need to iterate *beyond* `HttpMessageHandler`, the code is looking for this field: https://github.com/dotnet/runtime/blob/135fec006e727a31763271984cd712f1659ccbd3/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs#L25
jonathanpeppers · Feb 16, 2024 · 71039b4 · 71039b4
1 parent e987ac4
commit 71039b4
Show file tree

Hide file tree

Showing 9 changed files with 49 additions and 10 deletions.
diff --git a/build-tools/trim-analyzers/trim-analyzers.targets b/build-tools/trim-analyzers/trim-analyzers.targets
@@ -0,0 +1,15 @@
+<!-- Import this to enable trim warnings of all kinds -->
+<Project>
+  <PropertyGroup>
+    <!-- Sets assembly metadata, enable analyzers -->
+    <IsTrimmable>true</IsTrimmable>
+    <EnableSingleFileAnalyzer>true</EnableSingleFileAnalyzer>
+    <EnableAotAnalyzer>true</EnableAotAnalyzer>
+    <!-- In app projects, tells ILLink to emit warnings as errors -->
+    <ILLinkTreatWarningsAsErrors>true</ILLinkTreatWarningsAsErrors>
+    <!-- Trim warnings -->
+    <WarningsAsErrors>$(WarningsAsErrors);IL2000;IL2001;IL2002;IL2003;IL2004;IL2005;IL2006;IL2007;IL2008;IL2009;IL2010;IL2011;IL2012;IL2013;IL2014;IL2015;IL2016;IL2017;IL2018;IL2019;IL2020;IL2021;IL2022;IL2023;IL2024;IL2025;IL2026;IL2027;IL2028;IL2029;IL2030;IL2031;IL2032;IL2033;IL2034;IL2035;IL2036;IL2037;IL2038;IL2039;IL2040;IL2041;IL2042;IL2043;IL2044;IL2045;IL2046;IL2047;IL2048;IL2049;IL2050;IL2051;IL2052;IL2053;IL2054;IL2055;IL2056;IL2057;IL2058;IL2059;IL2060;IL2061;IL2062;IL2063;IL2064;IL2065;IL2066;IL2067;IL2068;IL2069;IL2070;IL2071;IL2072;IL2073;IL2074;IL2075;IL2076;IL2077;IL2078;IL2079;IL2080;IL2081;IL2082;IL2083;IL2084;IL2085;IL2086;IL2087;IL2088;IL2089;IL2090;IL2091;IL2092;IL2093;IL2094;IL2095;IL2096;IL2097;IL2098;IL2099;IL2100;IL2101;IL2102;IL2103;IL2104;IL2105;IL2106;IL2107;IL2108;IL2109;IL2110;IL2111;IL2112;IL2113;IL2114;IL2115;IL2116;IL2117;IL2118;IL2119;IL2120;IL2121;IL2122;IL2123;IL2124;IL2125;IL2126;IL2127;IL2128;IL2129;IL2130;IL2131;IL2132;IL2133;IL2134;IL2135;IL2136;IL2137;IL2138;IL2139;IL2140;IL2141;IL2142;IL2143;IL2144;IL2145;IL2146;IL2147;IL2148;IL2149;IL2150;IL2151;IL2152;IL2153;IL2154;IL2155;IL2156;IL2157</WarningsAsErrors>
+    <!-- NativeAOT warnings -->
+    <WarningsAsErrors>$(WarningsAsErrors);IL3050;IL3051;IL3052;IL3053;IL3054;IL3055;IL3056</WarningsAsErrors>
+  </PropertyGroup>
+</Project>
diff --git a/src/Mono.Android/Android.Graphics/Color.cs b/src/Mono.Android/Android.Graphics/Color.cs
@@ -395,11 +395,18 @@ public static void RGBToHSV (int red, int green, int blue, float[] hsv)
 
 	public class ColorValueMarshaler : JniValueMarshaler<Color>
 	{
+		const DynamicallyAccessedMemberTypes ConstructorsAndInterfaces = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors | DynamicallyAccessedMemberTypes.Interfaces;
+		const string ExpressionRequiresUnreferencedCode = "System.Linq.Expression usage may trim away required code.";
+
 		public override Type MarshalType {
 			get { return typeof (int); }
 		}
 
-		public override Color CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type targetType)
+		public override Color CreateGenericValue (
+				ref JniObjectReference reference,
+				JniObjectReferenceOptions options,
+				[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
+				Type targetType)
 		{
 			throw new NotImplementedException ();
 		}
@@ -414,6 +421,7 @@ public override void DestroyGenericArgumentState (Color value, ref JniValueMarsh
 			throw new NotImplementedException ();
 		}
 
+		[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
 		public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize, Type targetType)
 		{
 			var c = typeof (Color).GetConstructor (new[]{typeof (int)})!;
@@ -424,6 +432,7 @@ public override Expression CreateParameterToManagedExpression (JniValueMarshaler
 			return v;
 		}
 
+		[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
 		public override Expression CreateParameterFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize)
 		{
 			var r = Expression.Variable (MarshalType, sourceValue.Name + "_p");
@@ -433,6 +442,7 @@ public override Expression CreateParameterFromManagedExpression (JniValueMarshal
 			return r;
 		}
 
+		[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
 		public override Expression CreateReturnValueFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue)
 		{
 			return CreateParameterFromManagedExpression (context, sourceValue, 0);

diff --git a/src/Mono.Android/Android.Runtime/IJavaObjectValueMarshaler.cs b/src/Mono.Android/Android.Runtime/IJavaObjectValueMarshaler.cs
@@ -10,9 +10,16 @@ namespace Android.Runtime
 {
 	sealed class IJavaObjectValueMarshaler : JniValueMarshaler<IJavaObject> {
 
+		const DynamicallyAccessedMemberTypes ConstructorsAndInterfaces = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors | DynamicallyAccessedMemberTypes.Interfaces;
+		const string ExpressionRequiresUnreferencedCode = "System.Linq.Expression usage may trim away required code.";
+
 		internal    static  IJavaObjectValueMarshaler              Instance    = new IJavaObjectValueMarshaler ();
 
-		public override IJavaObject CreateGenericValue (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
+		public override IJavaObject CreateGenericValue (
+				ref JniObjectReference reference,
+				JniObjectReferenceOptions options,
+				[DynamicallyAccessedMembers (ConstructorsAndInterfaces)]
+				Type? targetType)
 		{
 			throw new NotImplementedException ();
 		}
@@ -27,6 +34,7 @@ public override void DestroyGenericArgumentState ([AllowNull]IJavaObject value,
 			throw new NotImplementedException ();
 		}
 
+		[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
 		public override Expression CreateReturnValueFromManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue)
 		{
 			return Expression.Call (
@@ -36,6 +44,7 @@ public override Expression CreateReturnValueFromManagedExpression (JniValueMarsh
 				sourceValue);
 		}
 
+		[RequiresUnreferencedCode (ExpressionRequiresUnreferencedCode)]
 		public override Expression CreateParameterToManagedExpression (JniValueMarshalerContext context, ParameterExpression sourceValue, ParameterAttributes synchronize, Type? targetType)
 		{
 			var r   = Expression.Variable (targetType, sourceValue.Name + "_val");

diff --git a/src/Mono.Android/Mono.Android.csproj b/src/Mono.Android/Mono.Android.csproj
@@ -3,6 +3,8 @@
 
   <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />
   <Import Project="..\..\Configuration.props" />
+  <!-- TODO: To be enabled in a future PR -->
+  <!-- <Import Project="$(XamarinAndroidSourcePath)build-tools/trim-analyzers/trim-analyzers.targets" /> -->
 
   <PropertyGroup>
     <TargetFrameworks>$(DotNetTargetFramework)</TargetFrameworks>

diff --git a/src/Mono.Android/System.Drawing/PointConverter.cs b/src/Mono.Android/System.Drawing/PointConverter.cs
@@ -30,6 +30,7 @@
 
 using System.Collections;
 using System.ComponentModel;
+using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.ComponentModel.Design.Serialization;
 using System.Runtime.CompilerServices;
@@ -133,6 +134,7 @@ public override bool GetCreateInstanceSupported (ITypeDescriptorContext context)
 			return true;
 		}
 
+		[RequiresUnreferencedCode("The Type of value cannot be statically discovered.")]
 		public override PropertyDescriptorCollection? GetProperties (
 							ITypeDescriptorContext context,
 							object value, Attribute[] attributes)

diff --git a/src/Mono.Android/System.Drawing/RectangleConverter.cs b/src/Mono.Android/System.Drawing/RectangleConverter.cs
@@ -31,6 +31,7 @@
 
 using System.ComponentModel;
 using System.Collections;
+using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Text;
 using System.ComponentModel.Design.Serialization;
@@ -147,6 +148,7 @@ public override bool GetCreateInstanceSupported (ITypeDescriptorContext context)
 			return true;
 		}
 
+		[RequiresUnreferencedCode("The Type of value cannot be statically discovered.")]
 		public override PropertyDescriptorCollection? GetProperties (
 							ITypeDescriptorContext context,
 							object value, Attribute[] attributes)

diff --git a/src/Mono.Android/System.Drawing/SizeConverter.cs b/src/Mono.Android/System.Drawing/SizeConverter.cs
@@ -31,6 +31,7 @@
 
 using System.Collections;
 using System.ComponentModel;
+using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.ComponentModel.Design.Serialization;
 using System.Reflection;
@@ -135,6 +136,7 @@ public override bool GetCreateInstanceSupported (ITypeDescriptorContext context)
 			return true;
 		}
 
+		[RequiresUnreferencedCode("The Type of value cannot be statically discovered.")]
 		public override PropertyDescriptorCollection? GetProperties (
 							ITypeDescriptorContext context,
 							object value, Attribute[] attributes)

diff --git a/src/Mono.Android/System.Drawing/SizeFConverter.cs b/src/Mono.Android/System.Drawing/SizeFConverter.cs
@@ -32,6 +32,7 @@
 using System;
 using System.Collections;
 using System.ComponentModel;
+using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.ComponentModel.Design.Serialization;
 using System.Reflection;
@@ -118,6 +119,7 @@ public override bool GetCreateInstanceSupported (ITypeDescriptorContext context)
 			return true;
 		}
 
+		[RequiresUnreferencedCode("The Type of value cannot be statically discovered.")]
 		public override PropertyDescriptorCollection? GetProperties (ITypeDescriptorContext context, object value, Attribute[] attributes)
 		{
 			if (value is SizeF)

diff --git a/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs b/src/Mono.Android/Xamarin.Android.Net/AndroidClientHandler.cs
@@ -313,14 +313,9 @@ protected virtual Task SetupRequest (HttpRequestMessage request, HttpURLConnecti
 		object? GetUnderlyingHandler ()
 		{
 			var fieldName = "_nativeHandler";
-			FieldInfo? field = null;
-
-			for (var type = GetType (); type != null; type = type.BaseType) {
-				field = type.GetField (fieldName, BindingFlags.Instance | BindingFlags.NonPublic);
-				if (field != null)
-					break;
-			}
-
+			const BindingFlags flags = BindingFlags.Instance | BindingFlags.NonPublic;
+			FieldInfo? field = typeof (HttpClientHandler).GetField (fieldName, flags) ??
+				typeof (HttpMessageHandler).GetField (fieldName, flags);
 			if (field == null) {
 				throw new InvalidOperationException ($"Field '{fieldName}' is missing from type '{GetType ()}'.");
 			}