Warn user if using DIM with minSdkVersion < 24

Saratsin commented 3 years ago

Steps to Reproduce

In Xamarin.Android create some C# class implement with it some java interface which has default interface method, not override this interface
Set your minSdkVersion to 23 or lower (targetSdk is 29)
Somewhere in the code cast your C# class to the java interface and call the method which has default implementation

To simplify these steps, here's the repro project: https://github.com/Saratsin/DefaultInterfaceMemberIssue

Expected Behavior

Everything works fine, default implementation of method is called

Actual Behavior

AbstractMethodError throws

Version Information

Click to expand details

``` === Visual Studio Community 2019 for Mac === Version 8.9.3 (build 13) Installation UUID: ef163811-a02f-40eb-86ad-f68e51ff3f84 GTK+ 2.24.23 (Raleigh theme) Xamarin.Mac 6.18.0.23 (d16-6 / 088c73638) Package version: 612000125 === Mono Framework MDK === Runtime: Mono 6.12.0.125 (2020-02/8c552e98bd6) (64-bit) Package version: 612000125 === Roslyn (Language Service) === 3.9.0-6.21152.10+c10f884b30737542ddd84ca889a4aad9281ce210 === NuGet === Version: 5.8.0.6860 === .NET Core SDK === SDK: /usr/local/share/dotnet/sdk/5.0.201/Sdks SDK Versions: 5.0.201 5.0.103 5.0.102 5.0.100 3.1.407 3.1.406 3.1.405 3.1.404 3.1.403 3.1.402 3.1.401 3.1.302 3.1.301 MSBuild SDKs: /Applications/Visual Studio.app/Contents/Resources/lib/monodevelop/bin/MSBuild/Current/bin/Sdks === .NET Core Runtime === Runtime: /usr/local/share/dotnet/dotnet Runtime Versions: 5.0.4 5.0.3 5.0.2 5.0.0 3.1.13 3.1.12 3.1.11 3.1.10 3.1.9 3.1.8 3.1.7 3.1.6 3.1.5 2.1.23 2.1.22 2.1.21 2.1.20 2.1.19 === .NET Core 3.1 SDK === SDK: 3.1.407 === Xamarin.Profiler === Version: 1.6.15.68 Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler === Updater === Version: 11 === Apple Developer Tools === Xcode 12.4 (17801) Build 12D4e === Xamarin.Mac === Version: 7.8.2.5 (Visual Studio Community) Hash: 3836759d4 Branch: d16-9 Build date: 2021-02-10 17:56:43-0500 === Xamarin.iOS === Version: 14.10.0.4 (Visual Studio Community) Hash: 5a05865f6 Branch: xcode12.4 Build date: 2021-01-28 02:30:23-0500 === Xamarin Designer === Version: 16.9.0.316 Hash: 2241b204a Branch: tags/vsm-rel/d16.9-4540908 Build date: 2021-03-10 21:18:10 UTC === Xamarin.Android === Version: 11.2.2.1 (Visual Studio Community) Commit: xamarin-android/d16-9/877f572 Android SDK: /Users/taras/Library/Android/sdk Supported Android versions: 8.1 (API level 27) SDK Tools Version: 26.1.1 SDK Platform Tools Version: 30.0.4 SDK Build Tools Version: 31.0.0 rc1 Build Information: Mono: 5e9cb6d Java.Interop: xamarin/java.interop/d16-9@54f8c24 ProGuard: Guardsquare/proguard/v7.0.1@912d149 SQLite: xamarin/sqlite/3.34.1@daff8f4 Xamarin.Android Tools: xamarin/xamarin-android-tools/d16-9@d210f11 === Microsoft OpenJDK for Mobile === Java SDK: /Users/taras/Library/Developer/Xamarin/jdk/microsoft_dist_openjdk_1.8.0.25 1.8.0-25 Android Designer EPL code available here: https://github.com/xamarin/AndroidDesigner.EPL === Android SDK Manager === Version: 16.9.0.22 Hash: a391de2 Branch: remotes/origin/d16-9 Build date: 2021-03-05 18:52:30 UTC === Android Device Manager === Version: 16.9.0.17 Hash: fc2b3db Branch: remotes/origin/d16-9 Build date: 2021-03-05 18:52:54 UTC === Build Information === Release ID: 809030013 Git revision: 9f03968ddc9d55da84019ce7a972b9fe225009fe Build date: 2021-03-18 10:15:43-04 Build branch: release-8.9 Xamarin extensions: 9f03968ddc9d55da84019ce7a972b9fe225009fe === Operating System === Mac OS X 10.16.0 Darwin 20.3.0 Darwin Kernel Version 20.3.0 Thu Jan 21 00:07:06 PST 2021 root:xnu-7195.81.3~1/RELEASE_X86_64 x86_64 === Enabled user installed extensions === DeepClean 1.2.5 Snippet Manager Addin 1.1 Xamarin.Forms HotReload extension 1.4.0 Xamarin.AndroidStudio 0.7.0 AddinMaker 1.6.0 Code Coverage 1.1 FileNesting 0.1.2 XAML Styler 2.0.1 MvvmCross Template pack 2.1.0 ```

Log File

Log.txt

jpobst commented 3 years ago

Looks like default interface methods are not natively supported on Android on API levels lower than 24. Here's the error Android Studio reports:

Default interface methods are only supported starting with Android N (--min-api 24)

(https://stackoverflow.com/questions/49512629/default-interface-methods-are-only-supported-starting-with-android-n)

It looks like this can be worked around with desugar. I'm not sure if we already do that or not. Even if we do, it is plausible that the desugaring moves the method so that our binding to it through JNI no longer works.

We might could add a build warning if a project targets C# 8 and a min-sdk < 24.

Saratsin commented 3 years ago

I thought initially that maybe it is only Java problem, so I tried the same code with native android application and java code. However, in java world everything works fine, all you need is to add this to build.gradle

compileOptions {
        sourceCompatibility JavaVersion.VERSION_1_8
        targetCompatibility JavaVersion.VERSION_1_8
}

In sample project I've made 2 examples:

Java Class which inherits interface with default method (but not implements it) in Java world and we use this class via C# wrapper generated by bindings generator.
C# Class which inherits interface with default method (but not implements it) in C# world.

First example works without any problems. Second example throws this error, but visually there's no difference, so I guess there might be some issue with exposing this C# class to the Java world.

jpobst commented 3 years ago

Yeah, that's what I would expect.

My guess is our JNI code says "call the default method Foo on interface MyInterface". However, since it isn't valid to have a default method in an interface pre-24, the desugar process has moved the method to another location and updated any Java callers to know the new location.

But this all happens after the C# code has been generated and compiled, and there's no way for it to know that the method has been moved. So it attempts to invoke the method at the original location, which fails.

Saratsin commented 3 years ago

Is it possible then somehow to warn user that

IF minSdkVersion < 24 AND 
   Java Interface which was implemented by some C# class has default methods AND
   There is at least one default method that was not implemented by this C# class
THEN
   Throw a compilation error. Or at least a warning

I guess that it might be even not possible to fix this issue with desugar and this problem appears only if minSdkVersion is lower than 24.

jpobst commented 3 years ago

I think it might be possible to do it for Release builds. It will require reflecting over every assembly in the application looking for this case, which takes a non-trivial amount of time. This is something the Linker already does on Release builds. It could probably be extended to look for this case as well.

Saratsin commented 3 years ago

Okay, then I'll be watching for this issue, as I understand such updates will be posted here as well. I guess it might be even an Error for Release build, since such issue can easily cause crashes on production of any application which uses such library and such crashes might look completely random

jpobst commented 1 year ago

This enhancement should no longer be necessary, as DIM should now work on pre-API-24 runtimes in .NET 8: https://github.com/xamarin/xamarin-android/issues/6784

dotnet / android