Skip to content

Commit

Permalink
[Mono.Android] fix trimming warnings, part 2
Browse files Browse the repository at this point in the history
This fixes the next set of trimmer warnings found via:

    <IsTrimmable>true</IsTrimmable>
    <EnableAotAnalyzer>true</EnableAotAnalyzer>

~~ JavaObjectExtensions ~~

`JavaCast<T>()` now requires `PublicConstructors` and
`NonPublicConstructors` because of `Activator.CreateInstance<T>()`.
This change bubbles up to various other types that have a
`Find*ById<T>()` method:

* `Activity`
* `Dialog`
* `FragmentManager`
* `View`
* `Window`

`GetInvokerType()` also has suppressions around `Assembly.GetType()`
and `Type.MakeGenericType()`. We track this for the future at:

dotnet#8724

~~ AndroidRuntime ~~

Update `[DynamicallyAccessedMembers]` based on changes to
`RegisterNativeMembers` in:

dotnet/java-interop@b8f6f88

~~ JNINativeWrapper ~~

`$(EnableAotAnalyzer)` found usage of `DynamicMethod`. Suppress for now,
as we track this for the future at:

dotnet#8724

~~ ResourceIdManager ~~

Usage of `Type.GetMethod ("UpdateIdValues")` leads to decoration of
`[ResourceDesignerAttribute]` with:

    [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
    public string FullName { get; set; }

I also had to suppress warnings around `Assembly.GetType()`. This
*should* be OK, as `Resource.designer.cs` is always in the "root
assembly" of Android application projects.

~~ JavaProxyThrowable ~~

Suppress warning around `StackFrame.GetMethod()`, we already handle
null return values and exceptions. The existing code appears to be
"best effort" to provide additional stack trace information.

~~ TypeManager ~~

Suppress warning around a call to `Type.GetType()` with a string
passed in from Java. Not really much we can do yet, except rely on
`MarkJavaObjects` trimmer step.

Likely also a problem for the future:

dotnet#8724
  • Loading branch information
jonathanpeppers committed Feb 29, 2024
1 parent e873731 commit 6a8022a
Show file tree
Hide file tree
Showing 14 changed files with 211 additions and 102 deletions.
14 changes: 11 additions & 3 deletions src/Mono.Android/Android.App/Activity.cs
Original file line number Diff line number Diff line change
@@ -1,19 +1,27 @@
using System;

using System.Diagnostics.CodeAnalysis;
using Android.Runtime;

namespace Android.App {

partial class Activity {

public T? FindViewById<T> (int id)
internal const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors;

public T? FindViewById<
[DynamicallyAccessedMembers (Constructors)]
T
> (int id)
where T : Android.Views.View
{
return this.FindViewById (id)!.JavaCast<T> ();
}

// See: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/app/Activity.java;l=3430
public T RequireViewById<T> (int id)
public T RequireViewById<
[DynamicallyAccessedMembers (Constructors)]
T
> (int id)
where T : Android.Views.View
{
var view = FindViewById<T> (id);
Expand Down
6 changes: 5 additions & 1 deletion src/Mono.Android/Android.App/Dialog.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Diagnostics.CodeAnalysis;
using Android.Runtime;

namespace Android.App {
Expand All @@ -8,7 +9,10 @@ public partial class Dialog {
protected Dialog (Android.Content.Context context, bool cancelable, EventHandler cancelHandler)
: this (context, cancelable, new Android.Content.IDialogInterfaceOnCancelListenerImplementor () { Handler = cancelHandler }) {}

public T? FindViewById<T> (int id)
public T? FindViewById<
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)]
T
> (int id)
where T : Android.Views.View
{
return this.FindViewById (id).JavaCast<T> ();
Expand Down
20 changes: 17 additions & 3 deletions src/Mono.Android/Android.App/FragmentManager.cs
Original file line number Diff line number Diff line change
@@ -1,18 +1,32 @@
using Android.OS;
using Android.Runtime;
using System.Diagnostics.CodeAnalysis;

#if ANDROID_11
namespace Android.App {
public partial class FragmentManager {
public T? FindFragmentById<T> (int id) where T : Fragment
const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors;

public T? FindFragmentById<
[DynamicallyAccessedMembers (Constructors)]
T
> (int id) where T : Fragment
{
return FindFragmentById (id).JavaCast<T> ();
}
public T? FindFragmentByTag<T> (string tag) where T : Fragment

public T? FindFragmentByTag<
[DynamicallyAccessedMembers (Constructors)]
T
> (string tag) where T : Fragment
{
return FindFragmentByTag (tag).JavaCast<T> ();
}
public T? GetFragment<T> (Bundle bundle, string key) where T : Fragment

public T? GetFragment<
[DynamicallyAccessedMembers (Constructors)]
T
> (Bundle bundle, string key) where T : Fragment
{
return GetFragment (bundle, key).JavaCast<T> ();
}
Expand Down
13 changes: 10 additions & 3 deletions src/Mono.Android/Android.Runtime/AndroidRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,9 @@ struct JniRemappingReplacementMethod

bool jniAddNativeMethodRegistrationAttributePresent;

const DynamicallyAccessedMemberTypes Methods = DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods;
const DynamicallyAccessedMemberTypes MethodsAndPrivateNested = Methods | DynamicallyAccessedMemberTypes.NonPublicNestedTypes;

public AndroidTypeManager (bool jniAddNativeMethodRegistrationAttributePresent)
{
this.jniAddNativeMethodRegistrationAttributePresent = jniAddNativeMethodRegistrationAttributePresent;
Expand Down Expand Up @@ -473,7 +476,7 @@ static bool CallRegisterMethodByIndex (JniNativeMethodRegistrationArguments argu

public override void RegisterNativeMembers (
JniType nativeClass,
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)]
[DynamicallyAccessedMembers (MethodsAndPrivateNested)]
Type type,
string? methods) =>
RegisterNativeMembers (nativeClass, type, methods.AsSpan ());
Expand All @@ -483,7 +486,7 @@ public override void RegisterNativeMembers (
[UnconditionalSuppressMessage ("Trimming", "IL2072", Justification = "Delegate.CreateDelegate() can never statically know the string value parsed from parameter 'methods'.")]
public void RegisterNativeMembers (
JniType nativeClass,
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)] Type type,
[DynamicallyAccessedMembers (MethodsAndPrivateNested)] Type type,
ReadOnlySpan<char> methods)
{
try {
Expand Down Expand Up @@ -619,7 +622,11 @@ public override void WaitForGCBridgeProcessing ()
AndroidRuntimeInternal.WaitForBridgeProcessing ();
}

public override IJavaPeerable? CreatePeer (ref JniObjectReference reference, JniObjectReferenceOptions options, Type? targetType)
public override IJavaPeerable? CreatePeer (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)]
Type? targetType)
{
if (!reference.IsValid)
return null;
Expand Down
5 changes: 4 additions & 1 deletion src/Mono.Android/Android.Runtime/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ namespace Android.Runtime {
public static class Extensions {

[return: NotNullIfNotNull ("instance")]
public static TResult? JavaCast<TResult> (this IJavaObject? instance)
public static TResult? JavaCast<
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)]
TResult
> (this IJavaObject? instance)
where TResult : class, IJavaObject
{
return Java.Interop.JavaObjectExtensions.JavaCast<TResult>(instance);
Expand Down
5 changes: 5 additions & 0 deletions src/Mono.Android/Android.Runtime/JNINativeWrapper.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Reflection.Emit;
using System.Threading;
Expand Down Expand Up @@ -52,7 +53,11 @@ public static Delegate CreateDelegate (Delegate dlg)
param_types [i] = parameters [i].ParameterType;
}

// FIXME: https://github.com/xamarin/xamarin-android/issues/8724
// IL3050 disabled in source: if someone uses NativeAOT, they will get the warning.
#pragma warning disable IL3050
var dynamic = new DynamicMethod (DynamicMethodNameCounter.GetUniqueName (), ret_type, param_types, typeof (DynamicMethodNameCounter), true);
#pragma warning restore IL3050
var ig = dynamic.GetILGenerator ();

LocalBuilder? retval = null;
Expand Down
11 changes: 10 additions & 1 deletion src/Mono.Android/Android.Runtime/JavaProxyThrowable.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;

using StackTraceElement = Java.Lang.StackTraceElement;
Expand Down Expand Up @@ -38,6 +39,14 @@ public static JavaProxyThrowable Create (Exception innerException)

void TranslateStackTrace ()
{
// FIXME: https://github.com/xamarin/xamarin-android/issues/8724
// StackFrame.GetMethod() will return null under NativeAOT;
// However, you can still get useful information from StackFrame.ToString():
// MainActivity.OnCreate() + 0x37 at offset 55 in file:line:column <filename unknown>:0:0
[UnconditionalSuppressMessage ("Trimming", "IL2026", Justification = "StackFrame.GetMethod() is \"best attempt\", we handle null & exceptions")]
static MethodBase? StackFrameGetMethod (StackFrame frame) =>
frame.GetMethod ();

var trace = new StackTrace (InnerException, fNeedFileInfo: true);
if (trace.FrameCount <= 0) {
return;
Expand All @@ -59,7 +68,7 @@ void TranslateStackTrace ()

for (int i = 0; i < frames.Length; i++) {
StackFrame managedFrame = frames[i];
MethodBase? managedMethod = managedFrame.GetMethod ();
MethodBase? managedMethod = StackFrameGetMethod (managedFrame);

var throwableFrame = new StackTraceElement (
declaringClass: managedMethod?.DeclaringType?.FullName,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
using System;
using System.Diagnostics.CodeAnalysis;

namespace Android.Runtime
{
[AttributeUsage (AttributeTargets.Assembly)]
public class ResourceDesignerAttribute : Attribute
{
public ResourceDesignerAttribute (string fullName)
public ResourceDesignerAttribute (
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
string fullName)
{
FullName = fullName;
}

[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
public string FullName { get; set; }

public bool IsApplication { get; set; }
Expand Down
11 changes: 9 additions & 2 deletions src/Mono.Android/Android.Runtime/ResourceIdManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,19 @@ public static void UpdateIdValues ()
}
}

[UnconditionalSuppressMessage ("Trimming", "IL2026", Justification = "Types in Resource.designer.cs are preserved, because it is the root assembly passed to the linker.")]
[return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
static Type? GetResourceTypeFromAssembly (Assembly assembly)
{
const string rootAssembly = "Types in Resource.designer.cs are preserved, because it is the @(TrimmerRootAssembly).";

[UnconditionalSuppressMessage ("Trimming", "IL2026", Justification = rootAssembly)]
[UnconditionalSuppressMessage ("Trimming", "IL2073", Justification = rootAssembly)]
[return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
static Type AssemblyGetType (Assembly a, string name) => a.GetType (name);

foreach (var customAttribute in assembly.GetCustomAttributes (typeof (ResourceDesignerAttribute), true)) {
if (customAttribute is ResourceDesignerAttribute resourceDesignerAttribute && resourceDesignerAttribute.IsApplication) {
var type = assembly.GetType (resourceDesignerAttribute.FullName);
var type = AssemblyGetType (assembly, resourceDesignerAttribute.FullName);
if (type != null)
return type;
}
Expand Down
12 changes: 10 additions & 2 deletions src/Mono.Android/Android.Views/View.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ public enum SystemUiFlags {

public partial class View {

internal const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors;

#if ANDROID_16
[Obsolete ("This method uses wrong enum type. Please use PerformAccessibilityAction(Action) instead.")]
public bool PerformAccessibilityAction (GlobalAction action, Bundle arguments)
Expand All @@ -22,14 +24,20 @@ public bool PerformAccessibilityAction (GlobalAction action, Bundle arguments)
}
#endif

public T? FindViewById<T> (int id)
public T? FindViewById<
[DynamicallyAccessedMembers (Constructors)]
T
> (int id)
where T : Android.Views.View
{
return this.FindViewById (id).JavaCast<T> ();
}

// See: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/view/View.java;l=25322
public T RequireViewById<T> (int id)
public T RequireViewById<
[DynamicallyAccessedMembers (Constructors)]
T
> (int id)
where T : Android.Views.View
{
var view = FindViewById<T> (id);
Expand Down
5 changes: 4 additions & 1 deletion src/Mono.Android/Android.Views/Window.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ namespace Android.Views {

partial class Window {

public T? FindViewById<T> (int id)
public T? FindViewById<
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)]
T
> (int id)
where T : Android.Views.View
{
return this.FindViewById (id).JavaCast<T> ();
Expand Down
51 changes: 42 additions & 9 deletions src/Mono.Android/Java.Interop/JavaObjectExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
namespace Java.Interop {

public static class JavaObjectExtensions {
const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors;

[Obsolete ("Use Android.Runtime.JavaCollection.ToLocalJniHandle()")]
public static JavaCollection ToInteroperableCollection (this ICollection instance)
Expand Down Expand Up @@ -46,13 +47,19 @@ public static JavaDictionary<K,V> ToInteroperableCollection<K,V> (this IDictiona
}

[return: NotNullIfNotNull ("instance")]
public static TResult? JavaCast<TResult> (this IJavaObject? instance)
public static TResult? JavaCast<
[DynamicallyAccessedMembers (Constructors)]
TResult
> (this IJavaObject? instance)
where TResult : class, IJavaObject
{
return _JavaCast<TResult> (instance);
}

internal static TResult? _JavaCast<TResult> (this IJavaObject? instance)
internal static TResult? _JavaCast<
[DynamicallyAccessedMembers (Constructors)]
TResult
> (this IJavaObject? instance)
{
if (instance == null)
return default (TResult);
Expand All @@ -74,7 +81,10 @@ public static JavaDictionary<K,V> ToInteroperableCollection<K,V> (this IDictiona
throw new NotSupportedException (FormattableString.Invariant ($"Unable to convert type '{instance.GetType ().FullName}' to '{resultType.FullName}'."));
}

static IJavaObject CastClass (IJavaObject instance, Type resultType)
static IJavaObject CastClass (
IJavaObject instance,
[DynamicallyAccessedMembers (Constructors)]
Type resultType)
{
var klass = JNIEnv.FindClass (resultType);
try {
Expand All @@ -97,7 +107,10 @@ static IJavaObject CastClass (IJavaObject instance, Type resultType)
return (IJavaObject) TypeManager.CreateProxy (resultType, instance.Handle, JniHandleOwnership.DoNotTransfer);
}

internal static IJavaObject? JavaCast (IJavaObject? instance, Type resultType)
internal static IJavaObject? JavaCast (
IJavaObject? instance,
[DynamicallyAccessedMembers (Constructors)]
Type resultType)
{
if (resultType == null)
throw new ArgumentNullException ("resultType");
Expand All @@ -120,23 +133,43 @@ static IJavaObject CastClass (IJavaObject instance, Type resultType)

// typeof(Foo) -> FooInvoker
// typeof(Foo<>) -> FooInvoker`1
[UnconditionalSuppressMessage ("Trimming", "IL2026", Justification = "*Invoker types are preserved by the MarkJavaObjects linker step.")]
[UnconditionalSuppressMessage ("Trimming", "IL2055", Justification = "*Invoker types are preserved by the MarkJavaObjects linker step.")]
[return: DynamicallyAccessedMembers (Constructors)]
internal static Type? GetInvokerType (Type type)
{
const string InvokerTypes = "*Invoker types are preserved by the MarkJavaObjects linker step.";

[UnconditionalSuppressMessage ("Trimming", "IL2026", Justification = InvokerTypes)]
[UnconditionalSuppressMessage ("Trimming", "IL2055", Justification = InvokerTypes)]
[UnconditionalSuppressMessage ("Trimming", "IL2073", Justification = InvokerTypes)]
[return: DynamicallyAccessedMembers (Constructors)]
static Type? AssemblyGetType (Assembly assembly, string typeName) =>
assembly.GetType (typeName);

// FIXME: https://github.com/xamarin/xamarin-android/issues/8724
// IL3050 disabled in source: if someone uses NativeAOT, they will get the warning.
[UnconditionalSuppressMessage ("Trimming", "IL2055", Justification = InvokerTypes)]
[UnconditionalSuppressMessage ("Trimming", "IL2068", Justification = InvokerTypes)]
[return: DynamicallyAccessedMembers (Constructors)]
static Type MakeGenericType (Type type, params Type [] typeArguments) =>
#pragma warning disable IL3050
type.MakeGenericType (typeArguments);
#pragma warning restore IL3050

const string suffix = "Invoker";

Type[] arguments = type.GetGenericArguments ();
if (arguments.Length == 0)
return type.Assembly.GetType (type + suffix);
return AssemblyGetType (type.Assembly, type + suffix);
Type definition = type.GetGenericTypeDefinition ();
int bt = definition.FullName!.IndexOf ("`", StringComparison.Ordinal);
if (bt == -1)
throw new NotSupportedException ("Generic type doesn't follow generic type naming convention! " + type.FullName);
Type? suffixDefinition = definition.Assembly.GetType (
Type? suffixDefinition = AssemblyGetType (
definition.Assembly,
definition.FullName.Substring (0, bt) + suffix + definition.FullName.Substring (bt));
if (suffixDefinition == null)
return null;
return suffixDefinition.MakeGenericType (arguments);
return MakeGenericType (suffixDefinition, arguments);
}
}
}
6 changes: 5 additions & 1 deletion src/Mono.Android/Java.Interop/TypeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,10 @@ static Exception CreateJavaLocationException ()

internal static Type? TypeRegistrationFallback (string class_name)
{
[UnconditionalSuppressMessage ("Trimming", "IL2057", Justification = "Type should be preserved by the MarkJavaObjects trimmer step.")]
static Type? TypeGetType (string name) =>
Type.GetType (name, throwOnError: false);

__TypeRegistrations.RegisterPackages ();

Type? type = null;
Expand All @@ -250,7 +254,7 @@ static Exception CreateJavaLocationException ()
return type;
}
}
if ((type = Type.GetType (JavaNativeTypeManager.ToCliType (class_name))) != null) {
if ((type = TypeGetType (JavaNativeTypeManager.ToCliType (class_name))) != null) {
return type;
}
return null;
Expand Down
Loading

0 comments on commit 6a8022a

Please sign in to comment.