Skip to content

Commit

Permalink
[api-merge] Correctly compute //method/@deprecated-since (#7645)
Browse files Browse the repository at this point in the history
Fixes: #7558

Context: xamarin/monodroid@16338f9
Context: a073d99
Context: 938c349

Currently when `api-merge` loops through type members, its first check
is to see if there is any existing member with the same name.  If there
isn't, it can short-circuit, add the new member, and move to the next
one.  If there is, we compare the existing API level member to the new
API level one to see if it has been marked deprecated in this level,
and if so we can add the `deprecated-since` attribute.

However, this initial existing member check was solely done on member
*name*, not signature.  😱

If the member is a method, a later check finds the exact matching
method with the same signature.  We were performing the
`deprecated-since` logic against the first method with a matching name
rather than the method with the same signature.

Consider [`PackageManager.getPackageInfo(String, int)`][0], which
was deprecated in *API-33*.  Yet our binding emitted `[Obsolete]`,
meaning it has been deprecated since API-21 or before!

	namespace Android.Content.PM;
	partial class PackageManager {
	    [global::System.Obsolete (@"deprecated")]
	    [Register ("getPackageInfo", "(Ljava/lang/String;I)Landroid/content/pm/PackageInfo;", "GetGetPackageInfo_Ljava_lang_String_IHandler")]
	    public override unsafe Android.Content.PM.PackageInfo? GetPackageInfo (string packageName, [global::Android.Runtime.GeneratedEnum] Android.Content.PM.PackageInfoFlags flags) => …
	}

This was wrong; it should instead have used `[ObsoletedOSPlatform]`:

	namespace Android.Content.PM;
	partial class PackageManager {
	    [global::System.Runtime.Versioning.ObsoletedOSPlatform ("android33.0")]
	    [Register ("getPackageInfo", "(Ljava/lang/String;I)Landroid/content/pm/PackageInfo;", "GetGetPackageInfo_Ljava_lang_String_IHandler")]
	    public override unsafe Android.Content.PM.PackageInfo? GetPackageInfo (string packageName, [global::Android.Runtime.GeneratedEnum] Android.Content.PM.PackageInfoFlags flags) => …
	}


Move the `UpdateDeprecatedSince()` invocations to be called *after*
we have the method with the matching signature.

The `acceptable-breakages` changes reflect additional `[Obsolete]`
attributes that are now being converted to
`[ObsoletedOSPlatform ("android-XX.0")]` attributes.

[0]: https://developer.android.com/reference/android/content/pm/PackageManager#getPackageInfo(java.lang.String,%20int)
  • Loading branch information
jpobst authored Jan 5, 2023
1 parent dc3ccf2 commit c339d1f
Show file tree
Hide file tree
Showing 2 changed files with 469 additions and 1 deletion.
7 changes: 6 additions & 1 deletion build-tools/api-merge/ApiDescription.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ public void Merge (XDocument n, string apiLocation)
continue;
}

UpdateDeprecatedSince (smember, nmember, platform);
if (nmember.Name.LocalName == "field") {
// FIXME: enable this to get the latest field attributes precisely.
/*
Expand All @@ -140,6 +139,9 @@ public void Merge (XDocument n, string apiLocation)
sa.SetValue (a.Value);
}
*/

UpdateDeprecatedSince (smember, nmember, platform);

#if KEEP_OLD_WRONG_COMPATIBILITY
var isDeprecatedS = smember.Attribute ("deprecated");
var isDeprecatedN = nmember.Attribute ("deprecated");
Expand All @@ -154,6 +156,9 @@ public void Merge (XDocument n, string apiLocation)
AddNewMember (stype, nmember, apiLocation, platform);
continue;
}

UpdateDeprecatedSince (smember, nmember, platform);

foreach (var a in nmember.Attributes ()) {
var sa = smember.Attribute (a.Name);
if (sa == null)
Expand Down
Loading

0 comments on commit c339d1f

Please sign in to comment.