-
Notifications
You must be signed in to change notification settings - Fork 52
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
[generator] Support [Obsolete]/[SupportedOSPlatform] attributes for enum members. #1066
Conversation
484006e
to
c2f4169
Compare
All the examples provided in #1037 appear correct with this PR: [Register("POST_NOTIFICATIONS", ApiSince=33)]
[SupportedOSPlatform("android33.0")]
public const string PostNotifications = "android.permission.POST_NOTIFICATIONS"; public enum VibrationEffectSupport
{
[IntDefinition("Android.OS.Vibrator.VibrationEffectSupportUnknown", JniField="android/os/Vibrator.VIBRATION_EFFECT_SUPPORT_UNKNOWN")]
[SupportedOSPlatform("android30.0")]
Unknown = 0,
[IntDefinition("Android.OS.Vibrator.VibrationEffectSupportYes", JniField="android/os/Vibrator.VIBRATION_EFFECT_SUPPORT_YES")]
[SupportedOSPlatform("android30.0")]
Yes = 1,
[IntDefinition("Android.OS.Vibrator.VibrationEffectSupportNo", JniField="android/os/Vibrator.VIBRATION_EFFECT_SUPPORT_NO")]
[SupportedOSPlatform("android30.0")]
No = 2
} [Register("ACTION_POST_CALL", ApiSince=30)]
[SupportedOSPlatform("android30.0")]
public const string ActionPostCall = "android.telecom.action.POST_CALL"; |
c2f4169
to
e3abb97
Compare
One oddity reviewing this PR is that the description doesn't match my expected usage. For example, the description mentions a I've attempted to update the commit message to "make sense in context". [generator] Obsolete&SupportedOSPlatform attributes on enum members (#1066)
Fixes: https://github.com/xamarin/java.interop/issues/1037
Adds support for emitting `[Obsolete]`, `[ObsoletedOSPlatform]`, and
`[SupportedOSPlatform]` custom attributes on enum members.
~~ [SupportedOSPlatform] ~~
The data for `[SupportedOSPlatform]` comes from the values provided
in the enum-mapping file (b00e644e), e.g.
[`xamarin-android/src/Mono.Android/map.csv`][0]:
// src/Mono.Android/map.csv
E,29,android/media/MediaRecorder$AudioEncoder.OPUS,7,Android.Media.AudioEncoder,Opus,keep,
The `29` is the API in which we added the enum, which becomes
`[SupportedOSPlatform("android-29.0")]`.
~~ [Obsolete] and [ObsoletedOSPlatform] ~~
The data for the "obsolete" attributes comes from the `deprecated`
and `deprecated-since` attributes on the original field in the
`api.xml`, if it can be found. For example, given:
<class name="AccessibilityServiceInfo" jni-signature="Landroid/accessibilityservice/AccessibilityServiceInfo;" …>
<field
deprecated="deprecated"
final="true"
name="CAPABILITY_CAN_REQUEST_ENHANCED_WEB_ACCESSIBILITY"
jni-signature="I"
static="true"
transient="false"
type="int"
type-generic-aware="int"
value="4"
visibility="public"
volatile="false"
deprecated-since="26"
/>
</class>
in combination with these `map.csv` (b00e644e) entries:
A,0,,0,Android.AccessibilityServices.AccessibilityServiceCapabilities,None,remove,
E,18,android/accessibilityservice/AccessibilityServiceInfo.CAPABILITY_CAN_REQUEST_ENHANCED_WEB_ACCESSIBILITY,4,Android.AccessibilityServices.AccessibilityServiceCapabilities,CanRequestEnhancedWebAccessibility,remove,
results in:
/* partial */ enum AccessibilityServiceCapabilities {
// When using classic obsolete attributes:
[global::System.Obsolete("deprecated")]
// When using new obsolete attributes:
[global::System.Runtime.Versioning.ObsoletedOSPlatform("android31.0")]
CanRequestEnhancedWebAccessibility = 4,
}
One wrinkle is we may have obsoleted the field because we want the
user to use the enum instead:
<field
deprecated='This constant will be removed in the future version. Use Android.App.RecentTaskFlags enum directly instead of this field.'
name='RECENT_IGNORE_UNAVAILABLE'
…
/>
We need to detect this message and not obsolete the enum in this case.
[0]: https://github.com/xamarin/xamarin-android/blob/17213ea184e23a9020451b265fec459558278489/src/Mono.Android/map.csv |
That's fine, I likely pulled some examples from the reported issue, which covered both |
Context: 4da2792 Context: dotnet/java-interop@d3ea180 Context: dotnet/java-interop@15c8879 Changes: dotnet/java-interop@149d70f...f8d77fa * dotnet/java-interop@f8d77faf: [generator] Better support deprecated property getter/setters. (dotnet/java-interop#1062) * dotnet/java-interop@5e6209ea: [generator] Obsolete&SupportedOSPlatform attributes on enum members (dotnet/java-interop#1066) * dotnet/java-interop@15c88797: [generator] Use decl type's @deprecated-since if < member's (dotnet/java-interop#1068) * dotnet/java-interop@525a45d5: [Java.Interop.Dynamic-Tests] Use Microsoft.CSharp NuGet package (dotnet/java-interop#1067) Background: member deprecations can be "historically weird" in Android. For example, in API-21 [`android/app/ActionBar.TabListener`][0] was deprecated: @deprecated /* partial */ interface TabListener { void onTabReselected(ActionBar.Tab tab, FragmentTransaction ft); // … } The type being deprecated means that its members are *implicitly* deprecated. In API-29 the members were *explicitly* deprecated: @deprecated /* partial */ interface TabListener { @deprecated void onTabReselected(ActionBar.Tab tab, FragmentTransaction ft); // … } Before dotnet/java-interop@d3ea180c, this resulted in the binding: [Obsolete] partial interface ITabListener { [Obsolete] void OnTabReselected(ActionBar.Tab? tab, FragmentTransaction? ft); // … } Commit dotnet/java-interop@d3ea180c added support for `[ObsoletedOSPlatform]` to `generator`, which *changed* the binding to: [Obsolete] // because it was deprecated in our min-supported API-21 partial interface ITabListener { [ObsoletedOSPlatform ("android29.0")] void OnTabReselected(ActionBar.Tab? tab, FragmentTransaction? ft); // … } This resulted in *lots* of changes in commit 4da2792 to `tests/api-compatibility/acceptable-breakages-vReference-net7.0.txt` because `[Obsolete]` was were replaced by `[ObsoletedOSPlatform]`: CannotRemoveAttribute : Attribute 'System.ObsoleteAttribute' exists on 'Android.App.ActionBar.ITabListener.OnTabReselected(Android.App.ActionBar.Tab, Android.App.FragmentTransaction)' in the contract but not the implementation Commit dotnet/java-interop@15c88797 updates type members to have the same deprecation status as their declaring type, when the member was deprecated *after* the type was deprecated. This means we *now* bind `ITabListener` as: [Obsolete] // because it was deprecated in our min-supported API-21 partial interface ITabListener { [Obsolete] void OnTabReselected(ActionBar.Tab? tab, FragmentTransaction? ft); // … } which matches the state of things pre- dotnet/java-interop@d3ea180c, which means we no longer need to ignore all those `CannotRemoveAttribute` messages. dotnet/java-interop@15c88797 thus "unintentionally partially reverts" 4da2792 (yay?), simply because the prior state of affairs in which a member was deprecated *after* the declaring type, while valid, didn't make any sense. Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jonathan Pobst <jonathan.pobst@microsoft.com> [0]: https://developer.android.com/reference/android/app/ActionBar.TabListener
Fixes: #1037
Adds support for
[Obsolete]
/[ObsoletedOSPlatform]
and[SupportedOSPlatform]
attributes on enum members.[SupportedOSPlatform]
The data for
[SupportedOSPlatform]
comes from the values provided in the enum-mapping file:The
29
is the API in which we added the enum, which becomes[SupportedOSPlatform ("android-29.0")]
.[Obsolete]/[ObsoletedOSPlatform]
The data for the "obsolete" attributes comes from the
deprecated
anddeprecated-since
attributes on the original field in theapi.xml
(if it can be found):This example results in:
One wrinkle is we may have obsoleted the field because we want the user to use the enum instead:
We need to detect this message and not obsolete the enum in this case.