-
Notifications
You must be signed in to change notification settings - Fork 57
Public types nested in internal Kotlin types not marked as internal #682
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
Comments
Fixes: #682 Context: dotnet/android#4955 We added support for reading Kotlin-provided metadata in commit 439bd83. Part of this was recognizing when a type was "Kotlin-internal" and marking it as `private` or "package-private" to prevent `generator` from attempting to bind it. However, we didn't consider *nested* types of `private` or "package-private" types in 439bd83, leaving the nested type visibility unchanged. This could result in a warning when binding such nested types: warning BG8604: top ancestor ParentType not found for nested type ParentType.ChildEnum Note: this doesn't prevent building -- unless `csc /warnaserror` is used -- and the type is not bound, so we are doing the "correct" thing. This warning *does* create a "less-than-desirable" user experience, possibly confusing users. Update `generator` to change the visibility of nested types within non-`public` types so that `generator` doesn't attempt to bind the nested types. This avoids the BG8604 warning.
Fixes: #682 Context: dotnet/android#4955 We added support for reading Kotlin-provided metadata in commit 439bd83. Part of this was recognizing when a type was "Kotlin-internal" and marking it as `private` or "package-private" to prevent `generator` from attempting to bind it. However, we didn't consider *nested* types of `private` or "package-private" types in 439bd83, leaving the nested type visibility unchanged. This could result in a warning when binding such nested types: warning BG8604: top ancestor ParentType not found for nested type ParentType.ChildEnum Note: this doesn't prevent building -- unless `csc /warnaserror` is used -- and the type is not bound, so we are doing the "correct" thing. This warning *does* create a "less-than-desirable" user experience, possibly confusing users. Update `generator` to change the visibility of nested types within non-`public` types so that `generator` doesn't attempt to bind the nested types. This avoids the BG8604 warning.
Fixes: dotnet/java-interop#682 Fixes: dotnet/java-interop#717 Context: dotnet/java-interop#719 Changes: dotnet/java-interop@a807961...79d9533 * dotnet/java-interop@79d95334: [generator] Use GC.KeepAlive for reference type method parameters. (dotnet#722) * dotnet/java-interop@1a19ec04: [Xamarin.Android.Tools.Bytecode] Hide Kotlin nested types inside (dotnet#723) * dotnet/java-interop@24a9abdb: [Xamarin.Android.Tools.ApiXmlAdjuster] Find app.android.IntentService (dotnet#718)
Fixes: dotnet/java-interop#682 Fixes: dotnet/java-interop#717 Context: dotnet/java-interop#719 Changes: dotnet/java-interop@a807961...79d9533 * dotnet/java-interop@79d95334: [generator] Use GC.KeepAlive for reference type method parameters. (#722) * dotnet/java-interop@1a19ec04: [Xamarin.Android.Tools.Bytecode] Hide Kotlin nested types inside (#723) * dotnet/java-interop@24a9abdb: [Xamarin.Android.Tools.ApiXmlAdjuster] Find app.android.IntentService (#718)
Release status update A new Preview version of Xamarin.Android has now been published that includes the fix for this item. The fix is not yet included in a Release version. I will update this item again when a Release version is available that includes the fix. Fix included in Xamarin.Android SDK version 11.1.0.15. Fix included on Windows in Visual Studio 2019 version 16.8 Preview 4. To try the Preview version that includes the fix, check for the latest updates in Visual Studio Preview. Fix included on macOS in Visual Studio 2019 for Mac version 8.8 Preview 4. To try the Preview version that includes the fix, check for the latest updates on the Preview updater channel. |
Fixes: dotnet/java-interop#461 Fixes: dotnet/java-interop#682 Fixes: dotnet/java-interop#717 Fixes: dotnet/java-interop#719 Fixes: dotnet/java-interop#728 Changes: dotnet/java-interop@ac914ce...b991bb8 * dotnet/java-interop@b991bb86: [generator] Revert change to use auto-properties in EventArgs classes (#736) * dotnet/java-interop@ee50d89b: Bump to xamarin/xamarin-android-tools/master@f2af06f2 (#733) * dotnet/java-interop@a0b895c1: [build] Suppress NuGet warnings (#730) * dotnet/java-interop@8b1b0507: [generator] Fix parsing of complex generic types (#729) * dotnet/java-interop@ee7afeed: [generator] Prevent generating duplicate EventArgs classes (#726) * dotnet/java-interop@1f21f38c: [generator] Use GC.KeepAlive for reference type method parameters. (#725) * dotnet/java-interop@5136ef98: [Xamarin.Android.Tools.Bytecode] Hide Kotlin nested types inside (#723) * dotnet/java-interop@53d60513: [jnimarshalmethod-gen] Fix registration on Windows (#721) * dotnet/java-interop@5a834d42: [jnimarshalmethod-gen] Avoid creating AppDomains (#720) * dotnet/java-interop@a76edb8c: [Xamarin.Android.Tools.ApiXmlAdjuster] Find app.android.IntentService (#718) * dotnet/java-interop@6cde0877: [Java.Interop] Emit a reference assembly for Java.Interop.dll (#716) * dotnet/java-interop@b858dc59: [generator] Provide line/col numbers for api.xml warnings (#715) * dotnet/java-interop@9be92a04: [ci] Don't kick off CI for documentation only changes. (#712) * dotnet/java-interop@03c22722: [jnimarshalmethod-gen] Fix type resolution crash (#706)
Release status update A new Release version of Xamarin.Android has now been published that includes the fix for this item. Fix included in Xamarin.Android SDK version 11.1.0.17. Fix included on Windows in Visual Studio 2019 version 16.8. To get the new version that includes the fix, check for the latest updates or install the most recent release from https://visualstudio.microsoft.com/downloads/. Fix included on macOS in Visual Studio 2019 for Mac version 8.8. To get the new version that includes the fix, check for the latest updates on the Stable updater channel. |
) Fixes: #826 Context: #682 Given the following Kotlin code: internal class MyClass { public interface MyInterface { } } The default Java representation of this would be: public MyClass { public MyInterface { } } In order to prevent this from being bound, our Kotlin fixups attempted to change the Java representation to: private class MyClass { private interface MyInterface { } } However there was a bug preventing the internal interface from being marked as `private`, because we are setting the visibility on the parent type's `InnerClassAccessFlags`, *not* the nested type itself. When we output the XML we use use the declaring class' `InnerClassAccessFlags`, we instead look at the flags on the nested type's `.class` file, which was still `public`: <class name="MyClass" visibility="private" … /> <class name="MyClass.MyInterface" visibility="public" … /> This would result in a `generator` warning when trying to bind `MyClass.MyInterface`, as the parent `MyClass` type is skipped: warning BG8604: top ancestor MyClass not found for nested type MyClass.MyInterface Fix this by finding the appropriate inner class `.class` file and updating the access flags there, recursively: <class name="MyClass" visibility="private" … /> <class name="MyClass.MyInterface" visibility="private" … /> Note: the [`InnerClasses` Attribute][0] for an inner class may contain the declaring class. For example, the `InnerClasses` for `InternalClassWithNestedInterface$NestedInterface` contains `InternalClassWithNestedInterface$NestedInterface`. We must thus protect recursive `HideInternalInnerClass()` invocations from processing the current type, to avoid infinite recursion. [0]: https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-4.html#jvms-4.7.6
Context: dotnet/android#4955
Given a public nested type inside a private/internal type in Kotlin:
We correctly hide
ParentType
but we do not hideChildEnum
because we do not check parent visibility. This results in a warning such as this whengenerator
is trying nest theChildEnum
object inside toParentType
object:The good news is
ChildEnum
will not be bound due to the warning, so the final binding is correct, but we should handle this correctly.The text was updated successfully, but these errors were encountered: