[Mono.Android] is now "trimming safe"

[jonathanpeppers]

Fixes: #5652

Each trimming problem listed below.

AndroidEnvironment

src\Mono.Android\Android.Runtime\AndroidEnvironment.cs(373,19): error IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
src\Mono.Android\Android.Runtime\AndroidEnvironment.cs(379,22): error IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
src\Mono.Android\Android.Runtime\AndroidEnvironment.cs(342,20): error IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
src\Mono.Android\Android.Runtime\AndroidEnvironment.cs(352,11): error IL2077: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'. The field 'Android.Runtime.AndroidEnvironment.httpMessageHandlerType' 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.

To fix these:

• Use constant strings for calls like:

Type.GetType ("System.Net.Http.HttpMessageHandler, System.Net.Http", throwOnError: false)

Use Type.IsAssignableFrom() inline, and the trimmer now understands
the code paths.

There is a problem here if either of these are used with a custom type:

• $(AndroidHttpClientHandlerType)
• $XA_HTTP_CLIENT_HANDLER_TYPE

This is a rarely used feature, I found one usage in GitHub:

https://github.com/search?q=%3CAndroidHttpClientHandlerType%3E+NOT+Xamarin.Android.Net+NOT+System.Net.Http+NOT+Default&type=code

Filed an issue to fix this in the future:

• When using TrimMode=link custom AndroidHttpClientHandlerType is not preserved #8797

JavaConvert

src\Mono.Android\Java.Interop\JavaConvert.cs(223,12): error IL2091: 'TResult' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Java.Interop.JavaObjectExtensions._JavaCast<TResult>(IJavaObject)'. The generic parameter 'T' of 'Java.Interop.JavaConvert.FromJavaObject<T>(IJavaObject, out Boolean)' 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.
src\Mono.Android\Java.Interop\JavaConvert.cs(254,12): error IL2067: 'resultType' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' in call to 'Java.Interop.JavaObjectExtensions.JavaCast(IJavaObject, Type)'. The parameter 'targetType' of method 'Java.Interop.JavaConvert.FromJavaObject(IJavaObject, Type)' 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.
src\Mono.Android\Java.Interop\JavaConvert.cs(67,14): error IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
src\Mono.Android\Java.Interop\JavaConvert.cs(73,14): error IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
src\Mono.Android\Java.Interop\JavaConvert.cs(79,14): error IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.

Adding attributes fixed some of these.

I suppressed warnings for:

• Type.MakeGenericType(): JavaDictionary<,>, JavaList<>, and
  JavaCollection<> use DynamicDependency to preserve FromJniHandle
  method.

• Type.GetElementType(): for calling MyJavaObject[] constructors,
  the MarkJavaObjects trimmer step should preserve these
  constructors.

This trickled over to require more attributes for:

• AdapterView
• ArrayAdapter
• AsyncTask
• JavaCollection
• JavaDictionary
• JavaList
• JavaList
• JavaObjectExtensions
• JavaSet
• SparseArray
• System.Linq\Extensions

JNIEnv

src\Mono.Android\Android.Runtime\JNIEnv.cs(810,38): error IL3050: Using member 'System.Type.MakeArrayType()' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
src\Mono.Android\Android.Runtime\JNIEnv.cs(953,33): error IL3050: Using member 'System.Type.MakeArrayType()' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
src\Mono.Android\Android.Runtime\JNIEnv.cs(1078,44): error IL3050: Using member 'System.Type.MakeArrayType()' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
src\Mono.Android\Android.Runtime\JNIEnv.cs(1139,15): error IL2091: 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Java.Interop.JavaConvert.FromJavaObject<T>(IJavaObject)'. The generic parameter 'T' of 'Android.Runtime.JNIEnv.GetArray<T>(Object[])' 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.
src\Mono.Android\Android.Runtime\JNIEnv.cs(1060,14): error IL3050: Using member 'System.Array.CreateInstance(Type, Int32)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
src\Mono.Android\Android.Runtime\JNIEnv.cs(1065,14): error IL3050: Using member 'System.Array.CreateInstance(Type, Int32)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
src\Mono.Android\Android.Runtime\JNIEnv.cs(1257,23): error IL2091: 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Java.Interop.JavaConvert.FromJniHandle<T>(nint, JniHandleOwnership)'. The generic parameter 'T' of 'Android.Runtime.JNIEnv.CopyObjectArray<T>(nint, T[])' 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.

Adding some attributes fixed two of these.

Suppress NativeAOT warnings, in source, for:

• Array.CreateInstance()

• Type.MakeArrayType()

Link to an issue to fix in the future.

[jonathanpeppers]

/cc @simonrozsival @vitek-karas

[vitek-karas]

Is this to get the analyzer warnings to be treated as errors?
For trimmer the ILLinkTreatWarningsAsErrors should be enough to do that.

[jonathanpeppers]

I found it didn't work in a class library, unless I added these.

Let me look at an MSBuild log to see why.

[jonathanpeppers]

I can't find anything in the ILLink analyzer that would look for $(ILLinkTreatWarningsAsErrors) and promote the warnings to errors.

I was looking here:

• https://github.com/search?q=repo%3Adotnet/runtime%20IsMSBuildPropertyValueTrue&type=code

[vitek-karas]

Analyzer doesn't do this, if you need this for the analyzer you do need to use the Roslyn's properties - so WarnAsError or TreatWarningsAsErrors (if you can enable it for all warnings). The ILLinkTreatWarningsAsErrors is only for ILLink.

[jonathanpeppers]

I think what we have here is OK for now. We could consider $(TreatWarningsAsErrors) as some point, but we have ~31 long-standing warnings from binding the Android API surface. There are a lot fewer now than there used to be, so it might be reasonable to fix these in .NET 9.

[vitek-karas]

One of these is about the fact that the type is unknown and it might have annotations on generic parameters. And in fact that happens here. For example JavaList<[DAM(Ctors)] T>. Suppressing the warning here means that we can create JavaList<SomeType> but not guarantee that SomeType.ctor is preserved -> break.

For the warning about the return type, it should be enough to annotate the Type type with the same DAM - the MakeGenericType "transfers" the annotation from the open generic to the instantiated generic. And since this helper is used on statically known types, that should be 100% fixable (no need for suppression).

[jonathanpeppers]

It looks like the warnings were about public methods:

src\Mono.Android\Java.Interop\JavaConvert.cs(68,5): error IL2068: 'MakeGenericType(Type, params Type[])' method return value does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' requirements. The parameter 'type' of method 'MakeGenericType(Type, params Type[])' 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.
src\Mono.Android\Java.Interop\JavaConvert.cs(68,5): error IL2055: Call to 'System.Type.MakeGenericType(params Type[])' can not be statically analyzed. It's not possible to guarantee the availability of requirements of the generic type.

There is some code below that would call:

typeof(JavaList<T>).GetMethod ("FromJniHandle", BindingFlags.Static | BindingFlags.Public)

I added [DAM(PublicMethods)] on JavaList<T>, does that seem like it would work for now?

I think we are good on the generic parameters on the three types, I have [DAM(Ctors)] for generics on JavaList<T>, JavaCollection<T>, and JavaDictionary<K,V>:

• https://github.com/xamarin/xamarin-android/pull/8778/files#diff-60dc6156fcc89d17dc9ae7ae9fd821c59bfc4d75a6830b36054c130b2d69d964R688
• https://github.com/xamarin/xamarin-android/pull/8778/files#diff-3c160b3d5560b1c37d5d0bb5e97f21ab78e6238d71558552142b56e513797d02R404
• https://github.com/xamarin/xamarin-android/pull/8778/files#diff-60dc6156fcc89d17dc9ae7ae9fd821c59bfc4d75a6830b36054c130b2d69d964R29

Warnings for these appeared from other places.

[vitek-karas]

Sorry - that's not how it works.
Putting DAM on a type only works if you have a variable statically declared as that type and then call .GetType() on the variable. Otherwise it will NOT apply the DAM to the type. The goal of DAM is to make things verifiable... so if you still need to use suppressions, it means you probably got it wrong.

The first warning should go away if you change the local method to:

			static Type MakeGenericType ([DynamicallyAccessedMembers(PublicMethods)] Type type, params Type [] typeArguments) 

You should also remove the DAM annotations on the specific types (JavaList and so on).

The second one doesn't seem to be fixable though:

class TestType
{
    public TestType() {}
}

// Internally this will create:
//     JavaList<TestType> instance, but that requires TestType to keep the public constructor
//  But the trimmer will remove the constructor, since it doesn't see TestType used anywhere
//  where it would need a constructor.
GetJniHandleConverter(typeof(IList<TestType>));

There's no good solution as-is... DI had the same problem and didn't really solve it either. It workaround this by checking the DAM annotations on the generic parameters of the interface at runtime, but that would not help here either, since we can't put the annotation on the interface. I don't know what to do with this second one. But I don't understand the higher level of this, so maybe there's something we can do there.

[jonathanpeppers]

Ok, I was able to remove a [UnconditionalSuppressMessage], but the IL2055 is still suppressed for now.

After this is merged, next step I'm going to enable more of our tests to verify TrimMode=full works on-device at runtime. That might find some cases that don't work in practice.

[vitek-karas]

Nice :-)

Nit: The justification doesn't apply to the warning - see my other comment about the reason for the warning.

[jonpryor]

Fixes: https://github.com/xamarin/xamarin-android/issues/5652

Add `build-tools/trim-analyzers/trim-analyzers.props`, which when
`<Import/>`ed into a project will enable trimmer analyzers and enable
`$(WarningsAsErrors)` for the warnings listed below so that that these
trimmer warnings need to be fixed as part of the build:

  * IL2000 through IL2129
  * IL3050 through IL3056

Update `Mono.Android.csproj` to import `trim-analyzers.props`.

Address the trimmer warnings that are now errors


~~ `$(AndroidHttpClientHandlerType)` and AndroidEnvironment ~~

The default `HttpClientHandler` type used with `new HttpClient()` can
be controlled via the [`$(AndroidHttpClientHandlerType)`][0] MSBuild
property, which sets the `$XA_HTTP_CLIENT_HANDLER_TYPE` environment
variable, which is used with `Type.GetType()`.  (Setting
`$(AndroidHttpClientHandlerType)` [is rare][1], has not been
deprecated, and thus is supported.)

The trimmer only supports `Type.GetType()` with string constants, not
with runtime values such as environment variables.

This results in numerous trimmer warnings:

	src\Mono.Android\Android.Runtime\AndroidEnvironment.cs(373,19): error IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
	src\Mono.Android\Android.Runtime\AndroidEnvironment.cs(379,22): error IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
	src\Mono.Android\Android.Runtime\AndroidEnvironment.cs(342,20): error IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
	src\Mono.Android\Android.Runtime\AndroidEnvironment.cs(352,11): error IL2077: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'.
	  The field 'Android.Runtime.AndroidEnvironment.httpMessageHandlerType' 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.

Partially address these issues by:

  * Using string constants instead of default parameter values.

        // This works
        static Type GetFallbackHttpMessageHandlerType() {
            const string typeName = "Xamarin.Android.Net.AndroidMessageHandler, Mono.Android";
            Type.GetType(typeName, …);
        }

        // This does not, as the *caller* gets the value
        static Type GetFallbackHttpMessageHandlerType(string typeName = "Xamarin.Android.Net.AndroidMessageHandler, Mono.Android") {
            Type.GetType(typeName, …);
        }

  * Spread `[DynamicallyAccessedMembers]` around to fix IL2077.

  * Use `[UnconditionalSuppressMessage]` to explicitly suppress IL2057

TODO: we need to *actually preserve* the type referenced by
`$(AndroidHttpClientHandlerType)`.

  * https://github.com/xamarin/xamarin-android/issues/8797


~~ JavaConvert ~~

`JavaConvert` is used internally to marshal between Java and managed
types, and elicits numerous trimmer warnings:

	src\Mono.Android\Java.Interop\JavaConvert.cs(223,12): error IL2091: 'TResult' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors',
	  'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Java.Interop.JavaObjectExtensions._JavaCast<TResult>(IJavaObject)'.
	  The generic parameter 'T' of 'Java.Interop.JavaConvert.FromJavaObject<T>(IJavaObject, out Boolean)' 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.
	src\Mono.Android\Java.Interop\JavaConvert.cs(254,12): error IL2067: 'resultType' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors',
	  'DynamicallyAccessedMemberTypes.NonPublicConstructors' in call to 'Java.Interop.JavaObjectExtensions.JavaCast(IJavaObject, Type)'.
	  The parameter 'targetType' of method 'Java.Interop.JavaConvert.FromJavaObject(IJavaObject, Type)' 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.
	src\Mono.Android\Java.Interop\JavaConvert.cs(67,14): error IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
	src\Mono.Android\Java.Interop\JavaConvert.cs(73,14): error IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
	src\Mono.Android\Java.Interop\JavaConvert.cs(79,14): error IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.

Address these by adding `[DynamicallyAccessedMembers]` to assist the
trimmer, `#pragma warning disable` to suppress the IL3050 warning,
and `[UnconditionalSuppressMessage]` to ignore the IL2055 warnings.

This trickled over to require more attributes for:

  * `AdapterView`
  * `ArrayAdapter`
  * `AsyncTask`
  * `JavaCollection`
  * `JavaDictionary`
  * `JavaList`
  * `JavaList`
  * `JavaObjectExtensions`
  * `JavaSet`
  * `SparseArray`
  * `System.Linq\Extensions`


~~ JNIEnv ~~

`JNIENv` is also used internally to marshal between Java and managed
types, and elicits numerous trimmer warnings:

	src\Mono.Android\Android.Runtime\JNIEnv.cs(810,38): error IL3050: Using member 'System.Type.MakeArrayType()' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
	src\Mono.Android\Android.Runtime\JNIEnv.cs(953,33): error IL3050: Using member 'System.Type.MakeArrayType()' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
	src\Mono.Android\Android.Runtime\JNIEnv.cs(1078,44): error IL3050: Using member 'System.Type.MakeArrayType()' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
	src\Mono.Android\Android.Runtime\JNIEnv.cs(1139,15): error IL2091: 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors',
	  'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Java.Interop.JavaConvert.FromJavaObject<T>(IJavaObject)'.
	  The generic parameter 'T' of 'Android.Runtime.JNIEnv.GetArray<T>(Object[])' 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.
	src\Mono.Android\Android.Runtime\JNIEnv.cs(1060,14): error IL3050: Using member 'System.Array.CreateInstance(Type, Int32)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
	src\Mono.Android\Android.Runtime\JNIEnv.cs(1065,14): error IL3050: Using member 'System.Array.CreateInstance(Type, Int32)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
	src\Mono.Android\Android.Runtime\JNIEnv.cs(1257,23): error IL2091: 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors',
	  'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Java.Interop.JavaConvert.FromJniHandle<T>(nint, JniHandleOwnership)'.
	  The generic parameter 'T' of 'Android.Runtime.JNIEnv.CopyObjectArray<T>(nint, T[])' 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.

Adding `[DynamicallyAccessedMembers]` fixes the IL2091 warnings.

Use `#pragma warning disable` to ignore the IL3050 warnings around
`Array.CreateInstance()` and `Type.MakeArrayType()`.

TODO: try to fix these issues instead of suppressing them:

  * https://github.com/xamarin/xamarin-android/issues/8724

[0]: https://github.com/xamarin/xamarin-android/blob/c20d51fcf8e910b8fb46c5351c26e55ed1fab90c/Documentation/guides/building-apps/build-properties.md#androidhttpclienthandlertype
[1]: https://github.com/search?q=%3CAndroidHttpClientHandlerType%3E+NOT+Xamarin.Android.Net+NOT+System.Net.Http+NOT+Default&type=code