Skip to content

[Java.Interop.Tools.TypeNameMappings] fix trimmer warnings #1194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Feb 16, 2024

Context: https://github.com/xamarin/xamarin-android/blob/e987ac458536e59a8329a06d5c5d5f4d4ea2c6b6/src/Mono.Android/Mono.Android.csproj#L69-L71

In xamarin/xamarin-android, we import the source for Java.Interop.Tools.TypeNameMappings\JavaNativeTypeManager.cs, and unfortunately there are some trimmer warnings:

external\Java.Interop\src\Java.Interop.Tools.TypeNameMappings\Java.Interop.Tools.TypeNameMappings\JavaNativeTypeManager.cs(182,9):
error IL2070: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.Interfaces' in call to 'System.Type.GetInterfaces()'. The parameter 'type' of method 'Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToJniName(Type, ExportParameterKind)' 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.

From the code:

if (!type.GetInterfaces ().Any (t => t.FullName == "Android.Runtime.IJavaObject"))

It appears we can instead look for IJavaPeerable and use trim-safe behavior instead:

if (Type.GetType ("Java.Interop.IJavaPeerable, Java.Interop", throwOnError: true)
    .IsAssignableFrom (type))

I also cached the Type.GetType() call with Lazy<T>.

However, in some cases we run this code under an MSBuild context. In
this case, we don't have Java.Interop.dll to load, so we should
instead use:

type.GetInterfaces ().Any (t => t.FullName == "Java.Interop.IJavaPeerable");

To catch warnings in this project going forward:

  • Multi-target to netstandard2.0 and net8.0 using $(DotNetTargetFramework). netstandard2.0 is used for MSBuild-task assemblies inside VS.

  • Enable trimmer warnings for net8.0

As I targeted net8.0, various null-reference-type warnings appeared, which I also fixed.

@jonathanpeppers jonathanpeppers force-pushed the Java.Interop.Tools.TypeNameMappings branch from 81405c2 to 9ac0133 Compare February 16, 2024 16:31
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
<LangVersion>8.0</LangVersion>
<Nullable>enable</Nullable>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<SignAssembly>true</SignAssembly>
<ProduceReferenceAssembly>true</ProduceReferenceAssembly>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The net8.0 build failed without this:

https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=9093665&view=logs&j=4ab9874f-3b82-50ef-2a6b-4340d0043621&t=1d9063ca-c961-5752-dc23-cb1130ee3d46&l=1141

Which is kind of odd, I thought that was the default in net5.0+? We should use it anyway to make incremental builds quicker.

@jonathanpeppers jonathanpeppers marked this pull request as ready for review February 16, 2024 16:39
@jonathanpeppers
Copy link
Member Author

/cc @simonrozsival @vitek-karas

Context: https://github.com/xamarin/xamarin-android/blob/e987ac458536e59a8329a06d5c5d5f4d4ea2c6b6/src/Mono.Android/Mono.Android.csproj#L69-L71

In xamarin/xamarin-android, we import the source for
`Java.Interop.Tools.TypeNameMappings\JavaNativeTypeManager.cs`, and
unfortunately there are some trimmer warnings:

    external\Java.Interop\src\Java.Interop.Tools.TypeNameMappings\Java.Interop.Tools.TypeNameMappings\JavaNativeTypeManager.cs(182,9):
    error IL2070: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.Interfaces' in call to 'System.Type.GetInterfaces()'. The parameter 'type' of method 'Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToJniName(Type, ExportParameterKind)' 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.

From the code:

    if (!type.GetInterfaces ().Any (t => t.FullName == "Android.Runtime.IJavaObject"))

It appears we can instead look for `IJavaPeerable` and use trim-safe
behavior instead:

    if (Type.GetType ("Java.Interop.IJavaPeerable, Java.Interop", throwOnError: true)
        .IsAssignableFrom (type))

I also cached the `Type.GetType()` call with `Lazy<T>`.

However, in some cases we run this code under an MSBuild context. In
this case, we don't have `Java.Interop.dll` to load, so we should
instead use:

    type.GetInterfaces ().Any (t => t.FullName == "Java.Interop.IJavaPeerable");

To catch warnings in this project going forward:

* Multi-target to `netstandard2.0` and `net8.0` using
`$(DotNetTargetFramework)`. `netstandard2.0` is used for
MSBuild-task assemblies inside VS.

* Enable trimmer warnings for `net8.0`

As I targeted `net8.0`, various null-reference-type warnings appeared,
which I also fixed.
@jonathanpeppers jonathanpeppers force-pushed the Java.Interop.Tools.TypeNameMappings branch from 6845af4 to 889057c Compare February 21, 2024 17:12
@jonpryor jonpryor merged commit 56b7eeb into dotnet:main Feb 21, 2024
@jonathanpeppers jonathanpeppers deleted the Java.Interop.Tools.TypeNameMappings branch February 21, 2024 21:06
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants