Skip to content

Commit

Permalink
[generator] Override methods should match base deprecated info (#1130)
Browse files Browse the repository at this point in the history
There are several warnings when building `Mono.Android.dll` related
to `[Obsolete]` methods, e.g.:

	warning CS0672: Member 'MediaRouteActionProvider.OnCreateActionView()' overrides obsolete member 'ActionProvider.OnCreateActionView()'.
	Add the Obsolete attribute to 'MediaRouteActionProvider.OnCreateActionView()'

Technically, `MediaRouteActionProvider.onCreateActionView()` is only
marked as `deprecated` in the [docs][0], but not in `android.jar`:

	public class MediaRouteActionProvider extends ActionProvider {
	  public View onCreateActionView() {
	    throw new RuntimeException("Stub!");
	  }
	}

Regardless, any method that overrides a deprecated method should
itself be marked as deprecated.

Another case specific to `Mono.Android.dll` is
[`ContextWrapper.setWallpaper()`][1].  This method is marked as
`deprecated-since`=23, but the base method is marked as
`deprecated-since`=16.  This causes us to generate:

	public class Context {
	  [Obsolete]
	  public virtual void SetWallpaper () { ... }
	}

	public class ContextWrapper : Context {
	  [ObsoletedOSPlatform ("android23.0")]
	  public override void SetWallpaper () { ... }
	}

This causes the same CS0672 warning.

Fix the CS0672 warnings by setting the `override` method's
`deprecated-since` to match the base method's `deprecated-since`.

[0]: https://developer.android.com/reference/android/app/MediaRouteActionProvider?hl=en#onCreateActionView()
[1]: https://developer.android.com/reference/android/content/ContextWrapper?hl=en#setWallpaper(android.graphics.Bitmap)
  • Loading branch information
jpobst authored Jul 12, 2023
1 parent e1121ea commit d0231c5
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 0 deletions.
44 changes: 44 additions & 0 deletions tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1120,6 +1120,50 @@ public void AvoidNREOnInvalidBaseMethod ()
Assert.AreEqual (true, base_class.Methods [0].IsValid);
}

[Test]
public void FixupDeprecatedBaseMethods ()
{
var xml = @"<api>
<package name='java.lang' jni-name='java/lang'>
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;'>
<method abstract='false' deprecated='deprecated' final='false' name='DoStuff' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public'></method>
</class>
</package>
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;'>
<method abstract='false' deprecated='not deprecated' final='false' name='DoStuff' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public'></method>
</class>
</package>
</api>";

var gens = ParseApiDefinition (xml);

// Override method should be marked deprecated because base method is
Assert.AreEqual ("deprecated", gens.Single (g => g.Name == "MyClass").Methods.Single (m => m.Name == "DoStuff").Deprecated);
}

[Test]
public void FixupDeprecatedSinceBaseMethods ()
{
var xml = @"<api>
<package name='java.lang' jni-name='java/lang'>
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;'>
<method abstract='false' deprecated='deprecated' final='false' name='DoStuff' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public' deprecated-since='11'></method>
</class>
</package>
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
<class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;'>
<method abstract='false' deprecated='not deprecated' final='false' name='DoStuff' jni-signature='()I' bridge='false' native='false' return='int' jni-return='I' static='false' synchronized='false' synthetic='false' visibility='public' deprecated-since='21'></method>
</class>
</package>
</api>";

var gens = ParseApiDefinition (xml);

// Override method should match base method's 'deprecated-since'
Assert.AreEqual (11, gens.Single (g => g.Name == "MyClass").Methods.Single (m => m.Name == "DoStuff").DeprecatedSince);
}

static string StripRegisterAttributes (string str)
{
// It is hard to test if the [Obsolete] is on the setter/etc due to the [Register], so remove all [Register]s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,15 @@ public void FixupMethodOverrides (CodeGenerationOptions opt)
var bm = bt.Methods.FirstOrDefault (mm => mm.Name == m.Name && mm.Visibility == m.Visibility && ParameterList.Equals (mm.Parameters, m.Parameters));
if (bm != null && bm.RetVal.FullName == m.RetVal.FullName) { // if return type is different, it could be still "new", not "override".
m.IsOverride = true;

// If method overrides a deprecated method, it also needs to be marked as deprecated
if (bm.Deprecated.HasValue () && !m.Deprecated.HasValue ())
m.Deprecated = bm.Deprecated;

// Fix issue when base method was deprecated before the overriding method, set both both to base method value
if (bm.DeprecatedSince.GetValueOrDefault (0) < m.DeprecatedSince.GetValueOrDefault (0))
m.DeprecatedSince = bm.DeprecatedSince;

break;
}
}
Expand Down

0 comments on commit d0231c5

Please sign in to comment.