[generator] Override methods should match base deprecated info (#1130)

jpobst · web-flow · commit d0231c5c7eab · 2023-07-12T12:48:59.000-04:00
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)
diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs
@@ -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
diff --git a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs
@@ -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;
 					}
 				}
