Skip to content
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] Inherit declaring type's 'deprecated-since' if lower than member's. #1068

Merged
merged 1 commit into from
Dec 14, 2022

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Dec 13, 2022

Imagine a type is deprecated in API-25 but its members are not explicitly marked as deprecated:

@Deprecated
public class TabActivity extends ActivityGroup {
  public void setDefaultTab(String tag) { ... }
}

Then in API-29 the member is explicitly marked as deprecated:

@Deprecated
public class TabActivity extends ActivityGroup {
  @Deprecated
  public void setDefaultTab(String tag) { ... }
}

Due to the way api-merge calculates deprecated-since, these would end up with different values, and the resulting C# API would look like:

[ObsoletedOSPlatform("android25.0")]
public class TabActivity : ActivityGroup
{
  [ObsoletedOSPlatform("android29.0")]
  public void SetDefaultTab (string tag) { ... }
}

While technically "fine", it is confusing that the method says it was obsoleted in API-29 instead of API-25.

To fix this, if a member has a "higher" deprecated-since than its declaring type, we set the member to the declaring type's value.

@jpobst jpobst marked this pull request as ready for review December 13, 2022 22:05
@jonpryor jonpryor merged commit 15c8879 into main Dec 14, 2022
@jonpryor jonpryor deleted the parent-deprecated-since branch December 14, 2022 21:53
jonpryor pushed a commit to xamarin/xamarin-android that referenced this pull request Dec 16, 2022
Context: 4da2792
Context: xamarin/java.interop@d3ea180
Context: xamarin/java.interop@15c8879

Changes: xamarin/java.interop@149d70f...f8d77fa

  * xamarin/java.interop@f8d77faf: [generator] Better support deprecated property getter/setters. (xamarin/java.interop#1062)
  * xamarin/java.interop@5e6209ea: [generator] Obsolete&SupportedOSPlatform attributes on enum members (xamarin/java.interop#1066)
  * xamarin/java.interop@15c88797: [generator] Use decl type's @deprecated-since if < member's (xamarin/java.interop#1068)
  * xamarin/java.interop@525a45d5: [Java.Interop.Dynamic-Tests] Use Microsoft.CSharp NuGet package (xamarin/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 xamarin/java.interop@d3ea180c, this resulted in the binding:

	[Obsolete]
	partial interface ITabListener {
	    [Obsolete]
	    void OnTabReselected(ActionBar.Tab? tab, FragmentTransaction? ft);
	    // …
	}

Commit xamarin/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 xamarin/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- xamarin/java.interop@d3ea180c,
which means we no longer need to ignore all those
`CannotRemoveAttribute` messages.  xamarin/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
@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.

None yet

2 participants