[generator] FixupMethodOverrides() checks properties (#1366)

Context: #1313Fixes: dotnet/android#10509
In .NET 10 RC 1, `View.Foreground` is defined in C# as:
[SupportedOSPlatform("android23.0")]
public virtual Drawable? Foreground { get; set; }
But `FrameLayout.Foreground` is defined as:
[UnsupportedOSPlatform("android23.0")]
public override Drawable? Foreground { get; set; }
I think this is the exact same bug that was addressed in #1313 for
methods, but we still have the issue for properties. I was able to
duplicate the test and make a `getThing()` and `setThing()` methods in
Java to see the issue.
I updated the `FixupMethodOverrides()` to iterate over properties
and do the same fixup for property getters and setters.
This also needed to be done for:
* Interfaces
* Fields (think of `static final int` values)

Author: jonathanpeppers

tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs

@@ -1441,13 +1441,23 @@ public void UnsupportedOSPlatformConstFields ()
 publicvoidUnsupportedOSPlatformIgnoresMethodOverrides()
 {
 // Given:
+// Class inheritance scenario:
 // public class TextView {
 // public Object doThing () { ... }
 // }
 // public class TextView2 : TextView {
 // public Object doThing () { ... } // removed-since = 30
 // }
-// We should not write [UnsupportedOSPlatform] on TextView2.doThing (), because the base method isn't "removed".
+// Interface inheritance scenario:
+// public interface IFoo {
+// public Object doSomething () { ... }
+// public static final String DATE_TAKEN = "datetaken";
+// }
+// public interface IBar : IFoo {
+// public Object doSomething () { ... } // removed-since = 30
+// public static final String DATE_TAKEN = "datetaken"; // removed-since = 30
+// }
+// We should not write [UnsupportedOSPlatform] on overriding methods or fields, because the base methods/fields aren't "removed".
 varxml=@$"<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;' />
@@ -1460,13 +1470,97 @@ public void UnsupportedOSPlatformIgnoresMethodOverrides ()
  <method abstract='false' deprecated='not deprecated' final='false' name='doThing' bridge='false' native='false' return='java.lang.Object' static='false' synchronized='false' synthetic='false' visibility='public' removed-since='30' />
  </class>
  </package>
+ <package name='com.example' jni-name='com/example'>
+ <interface abstract='true' deprecated='not deprecated' final='false' name='Foo' static='false' visibility='public' jni-signature='Lcom/example/Foo;'>
+ <method abstract='true' deprecated='not deprecated' final='false' name='doSomething' bridge='false' native='false' return='java.lang.Object' static='false' synchronized='false' synthetic='false' visibility='public' />
+ <field deprecated='not deprecated' final='true' name='DATE_TAKEN' jni-signature='Ljava/lang/String;' static='true' transient='false' type='java.lang.String' type-generic-aware='java.lang.String' value='&quot;datetaken&quot;' visibility='public' volatile='false'></field>
+ </interface>
+ <interface abstract='true' deprecated='not deprecated' final='false' name='Bar' static='false' visibility='public' jni-signature='Lcom/example/Bar;'>
+ <implements name='com.example.Foo' name-generic-aware='com.example.Foo' jni-type='Lcom/example/Foo;'></implements>
+ <method abstract='true' deprecated='not deprecated' final='false' name='doSomething' bridge='false' native='false' return='java.lang.Object' static='false' synchronized='false' synthetic='false' visibility='public' removed-since='30' />
+ <field deprecated='not deprecated' final='true' name='DATE_TAKEN' jni-signature='Ljava/lang/String;' static='true' transient='false' type='java.lang.String' type-generic-aware='java.lang.String' value='&quot;datetaken&quot;' visibility='public' volatile='false' removed-since='30'></field>
+ </interface>
+ </package>
+ </api>";
+
+vargens=ParseApiDefinition(xml);
+
+// Test class inheritance scenario
+varklass=gens.Single(g =>g.Name=="TextView2");
+varactual=GetGeneratedTypeOutput(klass);
+StringAssert.DoesNotContain("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android30.0\")]",actual,"Should not contain UnsupportedOSPlatform on class override!");
+
+// Test interface inheritance scenario 
+variface=gens.OfType<InterfaceGen>().Single(g =>g.Name=="IBar");
+varifaceActual=GetGeneratedTypeOutput(iface);
+StringAssert.DoesNotContain("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android30.0\")]",ifaceActual,"Should not contain UnsupportedOSPlatform on interface override!");
+}
+
+[Test]
+publicvoidUnsupportedOSPlatformIgnoresPropertyOverrides()
+{
+// Given:
+// Class inheritance scenario:
+// public class TextView {
+// public Object getThing () { ... }
+// public void setThing (Object value) { ... }
+// }
+// public class TextView2 : TextView {
+// public Object getThing () { ... } // removed-since = 30
+// public void setThing (Object value) { ... } // removed-since = 30
+// }
+// Interface inheritance scenario:
+// public interface IPropertyProvider {
+// public Object getSomething () { ... }
+// public static final String DATE_TAKEN = "datetaken";
+// }
+// public interface IExtendedProvider : IPropertyProvider {
+// public Object getSomething () { ... } // removed-since = 30
+// public static final String DATE_TAKEN = "datetaken"; // removed-since = 30
+// }
+// We should not write [UnsupportedOSPlatform] on overriding properties or fields, because the base methods/fields aren't "removed".
+varxml=@$"<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;' />
+ </package>
+ <package name='android.widget' jni-name='android/widget'>
+ <class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' final='false' name='TextView' static='false' visibility='public'>
+ <method abstract='false' deprecated='not deprecated' final='false' name='getThing' bridge='false' native='false' return='java.lang.Object' static='false' synchronized='false' synthetic='false' visibility='public' />
+ <method abstract='false' deprecated='not deprecated' final='false' name='setThing' bridge='false' native='false' return='void' static='false' synchronized='false' synthetic='false' visibility='public'>
+ <parameter name='value' type='java.lang.Object' />
+ </method>
+ </class>
+ <class abstract='false' deprecated='not deprecated' extends='android.widget.TextView' extends-generic-aware='java.lang.Object' final='false' name='TextView2' static='false' visibility='public'>
+ <method abstract='false' deprecated='not deprecated' final='false' name='getThing' bridge='false' native='false' return='java.lang.Object' static='false' synchronized='false' synthetic='false' visibility='public' removed-since='30' />
+ <method abstract='false' deprecated='not deprecated' final='false' name='setThing' bridge='false' native='false' return='void' static='false' synchronized='false' synthetic='false' visibility='public' removed-since='30'>
+ <parameter name='value' type='java.lang.Object' />
+ </method>
+ </class>
+ </package>
+ <package name='com.example' jni-name='com/example'>
+ <interface abstract='true' deprecated='not deprecated' final='false' name='PropertyProvider' static='false' visibility='public' jni-signature='Lcom/example/PropertyProvider;'>
+ <method abstract='true' deprecated='not deprecated' final='false' name='getSomething' bridge='false' native='false' return='java.lang.Object' static='false' synchronized='false' synthetic='false' visibility='public' />
+ <field deprecated='not deprecated' final='true' name='DATE_TAKEN' jni-signature='Ljava/lang/String;' static='true' transient='false' type='java.lang.String' type-generic-aware='java.lang.String' value='&quot;datetaken&quot;' visibility='public' volatile='false'></field>
+ </interface>
+ <interface abstract='true' deprecated='not deprecated' final='false' name='ExtendedProvider' static='false' visibility='public' jni-signature='Lcom/example/ExtendedProvider;'>
+ <implements name='com.example.PropertyProvider' name-generic-aware='com.example.PropertyProvider' jni-type='Lcom/example/PropertyProvider;'></implements>
+ <method abstract='true' deprecated='not deprecated' final='false' name='getSomething' bridge='false' native='false' return='java.lang.Object' static='false' synchronized='false' synthetic='false' visibility='public' removed-since='30' />
+ <field deprecated='not deprecated' final='true' name='DATE_TAKEN' jni-signature='Ljava/lang/String;' static='true' transient='false' type='java.lang.String' type-generic-aware='java.lang.String' value='&quot;datetaken&quot;' visibility='public' volatile='false' removed-since='30'></field>
+ </interface>
+ </package>
  </api>";
 
 vargens=ParseApiDefinition(xml);
+
+// Test class inheritance scenario
 varklass=gens.Single(g =>g.Name=="TextView2");
 varactual=GetGeneratedTypeOutput(klass);
+StringAssert.DoesNotContain("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android30.0\")]",actual,"Should not contain UnsupportedOSPlatform on class property override!");
 
-StringAssert.DoesNotContain("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android30.0\")]",actual,"Should contain UnsupportedOSPlatform!");
+// Test interface inheritance scenario 
+variface=gens.OfType<InterfaceGen>().Single(g =>g.Name=="IExtendedProvider");
+varifaceActual=GetGeneratedTypeOutput(iface);
+StringAssert.DoesNotContain("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android30.0\")]",ifaceActual,"Should not contain UnsupportedOSPlatform on interface property override!");
 }
 
 [Test]

tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs

@@ -307,27 +307,137 @@ public virtual void FixupExplicitImplementation ()
 
 publicvoidFixupMethodOverrides(CodeGenerationOptionsopt)
 {
+// Process regular methods (non-static, non-interface default methods)
 foreach(varminMethods.Where(m =>!m.IsStatic&&!m.IsInterfaceDefaultMethod)){
 for(varbt=GetBaseGen(opt);bt!=null;bt=bt.GetBaseGen(opt)){
-varbm=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(opt.FixObsoleteOverrides){
-// 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(default)<m.DeprecatedSince.GetValueOrDefault(default))
-m.DeprecatedSince=bm.DeprecatedSince;
+varbm=bt.Methods.FirstOrDefault(mm =>
+mm.Name==m.Name&&
+mm.Visibility==m.Visibility&&
+mm.RetVal.FullName==m.RetVal.FullName&&// if return type is different, it could be still "new", not "override".
+ParameterList.Equals(mm.Parameters,m.Parameters));
+if(bm==null){
+continue;
+}
+
+m.IsOverride=true;
+if(opt.FixObsoleteOverrides){
+// 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(default)<m.DeprecatedSince.GetValueOrDefault(default))
+m.DeprecatedSince=bm.DeprecatedSince;
+}
+
+// If a "removed" method overrides a "not removed" method, the method was
+// likely moved to a base class, so don't mark it as removed.
+if(m.ApiRemovedSince>0&&bm.ApiRemovedSince==0){
+m.ApiRemovedSince=default;
+}
+break;
+}
+}
+
+// Process property getter/setter methods for ApiRemovedSince fixup
+foreach(varpropinProperties){
+for(varbt=GetBaseGen(opt);bt!=null;bt=bt.GetBaseGen(opt)){
+varbaseProp=bt.Properties.FirstOrDefault(p =>p.Name==prop.Name&&p.Type==prop.Type);
+if(baseProp==null){
+continue;
+}
+
+boolshouldBreak=false;
+if(prop.Getter!=null&&prop.Getter.ApiRemovedSince>0&&baseProp.Getter!=null&&baseProp.Getter.ApiRemovedSince==0){
+if(baseProp.Getter.Visibility==prop.Getter.Visibility&&
+ParameterList.Equals(baseProp.Getter.Parameters,prop.Getter.Parameters)&&
+baseProp.Getter.RetVal.FullName==prop.Getter.RetVal.FullName){
+// If a "removed" property getter overrides a "not removed" getter, the method was
+// likely moved to a base class, so don't mark it as removed.
+prop.Getter.ApiRemovedSince=default;
+shouldBreak=true;
 }
+}
+if(prop.Setter!=null&&prop.Setter.ApiRemovedSince>0&&baseProp.Setter!=null&&baseProp.Setter.ApiRemovedSince==0){
+if(baseProp.Setter.Visibility==prop.Setter.Visibility&&
+ParameterList.Equals(baseProp.Setter.Parameters,prop.Setter.Parameters)){
+// If a "removed" property setter overrides a "not removed" setter, the method was
+// likely moved to a base class, so don't mark it as removed.
+prop.Setter.ApiRemovedSince=default;
+shouldBreak=true;
+}
+}
+if(shouldBreak){
+break;
+}
+}
+}
 
-// If a "removed" method overrides a "not removed" method, the method was
-// likely moved to a base class, so don't mark it as removed.
-if(m.ApiRemovedSince>0&&bm.ApiRemovedSince==0)
+// Process interface inheritance for both regular and default interface methods
+if(thisisInterfaceGencurrentInterface){
+// For interfaces, check all base interfaces (interfaces that this interface implements/extends)
+varbaseInterfaces=currentInterface.GetAllDerivedInterfaces();
+
+foreach(varminMethods.Where(m =>!m.IsStatic)){
+foreach(varbaseIfaceinbaseInterfaces){
+varbm=baseIface.Methods.FirstOrDefault(mm =>
+mm.Name==m.Name&&
+mm.Visibility==m.Visibility&&
+mm.RetVal.FullName==m.RetVal.FullName&&
+ParameterList.Equals(mm.Parameters,m.Parameters));
+if(bm==null){
+continue;
+}
+// If a "removed" interface method overrides a "not removed" method, the method was
+// likely moved to a base interface, so don't mark it as removed.
+if(m.ApiRemovedSince>0&&bm.ApiRemovedSince==0){
 m.ApiRemovedSince=default;
+}
+break;
+}
+}
+
+// Process interface property getter/setter methods for ApiRemovedSince fixup
+foreach(varpropinProperties){
+foreach(varbaseIfaceinbaseInterfaces){
+varbaseProp=baseIface.Properties.FirstOrDefault(p =>p.Name==prop.Name&&p.Type==prop.Type);
+if(baseProp==null)
+continue;
+
+boolshouldBreak=false;
+if(prop.Getter!=null&&prop.Getter.ApiRemovedSince>0&&baseProp.Getter!=null&&baseProp.Getter.ApiRemovedSince==0){
+if(baseProp.Getter.Visibility==prop.Getter.Visibility&&
+ParameterList.Equals(baseProp.Getter.Parameters,prop.Getter.Parameters)&&
+baseProp.Getter.RetVal.FullName==prop.Getter.RetVal.FullName){
+prop.Getter.ApiRemovedSince=default;
+shouldBreak=true;
+}
+}
+if(prop.Setter!=null&&prop.Setter.ApiRemovedSince>0&&baseProp.Setter!=null&&baseProp.Setter.ApiRemovedSince==0){
+if(baseProp.Setter.Visibility==prop.Setter.Visibility&&
+ParameterList.Equals(baseProp.Setter.Parameters,prop.Setter.Parameters)){
+prop.Setter.ApiRemovedSince=default;
+shouldBreak=true;
+}
+}
+if(shouldBreak){
+break;
+}
+}
+}
 
+// Process interface field inheritance for ApiRemovedSince fixup
+foreach(varfieldinFields){
+foreach(varbaseIfaceinbaseInterfaces){
+varbaseField=baseIface.Fields.FirstOrDefault(f =>f.Name==field.Name&&f.TypeName==field.TypeName&&f.Visibility==field.Visibility);
+if(baseField==null){
+continue;
+}
+// If a "removed" interface field overrides a "not removed" field, the field was
+// likely moved to a base interface, so don't mark it as removed.
+if(field.ApiRemovedSince>0&&baseField.ApiRemovedSince==0){
+field.ApiRemovedSince=default;
+}
 break;
 }
 }