Skip to content

[generator] FixupMethodOverrides() checks properties #1366

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Sep 23, 2025
Merged

[generator] FixupMethodOverrides() checks properties #1366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
86c029c
[generator] `FixupMethodOverrides()` checks properties
jonathanpeppers Sep 22, 2025
e4a4269
Iterate over base properties once
jonathanpeppers Sep 22, 2025
2c0a8e8
New interface test
jonathanpeppers Sep 22, 2025
e96c813
New property interface test
jonathanpeppers Sep 22, 2025
4682436
Fixes for interfaces
jonathanpeppers Sep 22, 2025
130429b
`public static final String DATE_TAKEN = "datetaken"`
jonathanpeppers Sep 22, 2025
bdce347
`removed-since = 30`
jonathanpeppers Sep 22, 2025
9778ba2
Fields!
jonathanpeppers Sep 22, 2025
77b5bfa
Check Visibility on fields
jonathanpeppers Sep 22, 2025
448b00a
Formatting
jonathanpeppers Sep 23, 2025
1138738
Formatting existing code
jonathanpeppers Sep 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 96 additions & 2 deletions tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1441,13 +1441,23 @@ public void UnsupportedOSPlatformConstFields ()
public void UnsupportedOSPlatformIgnoresMethodOverrides ()
{
// 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".
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;' />
Expand All @@ -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>";

var gens = ParseApiDefinition (xml);

// Test class inheritance scenario
var klass = gens.Single (g => g.Name == "TextView2");
var actual = GetGeneratedTypeOutput (klass);
StringAssert.DoesNotContain ("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android30.0\")]", actual, "Should not contain UnsupportedOSPlatform on class override!");

// Test interface inheritance scenario
var iface = gens.OfType<InterfaceGen> ().Single (g => g.Name == "IBar");
var ifaceActual = GetGeneratedTypeOutput (iface);
StringAssert.DoesNotContain ("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android30.0\")]", ifaceActual, "Should not contain UnsupportedOSPlatform on interface override!");
}

[Test]
public void UnsupportedOSPlatformIgnoresPropertyOverrides ()
{
// 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".
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;' />
</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>";

var gens = ParseApiDefinition (xml);

// Test class inheritance scenario
var klass = gens.Single (g => g.Name == "TextView2");
var actual = 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
var iface = gens.OfType<InterfaceGen> ().Single (g => g.Name == "IExtendedProvider");
var ifaceActual = GetGeneratedTypeOutput (iface);
StringAssert.DoesNotContain ("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android30.0\")]", ifaceActual, "Should not contain UnsupportedOSPlatform on interface property override!");
}

[Test]
Expand Down
140 changes: 125 additions & 15 deletions tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,27 +307,137 @@ public virtual void FixupExplicitImplementation ()

public void FixupMethodOverrides (CodeGenerationOptions opt)
{
// Process regular methods (non-static, non-interface default methods)
foreach (var m in Methods.Where (m => !m.IsStatic && !m.IsInterfaceDefaultMethod)) {
for (var bt = GetBaseGen (opt); bt != null; bt = bt.GetBaseGen (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 (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;
var bm = 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 (var prop in Properties) {
for (var bt = GetBaseGen (opt); bt != null; bt = bt.GetBaseGen (opt)) {
var baseProp = bt.Properties.FirstOrDefault (p => p.Name == prop.Name && p.Type == prop.Type);
if (baseProp == null) {
continue;
}

bool shouldBreak = 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 (this is InterfaceGen currentInterface) {
// For interfaces, check all base interfaces (interfaces that this interface implements/extends)
var baseInterfaces = currentInterface.GetAllDerivedInterfaces ();

foreach (var m in Methods.Where (m => !m.IsStatic)) {
foreach (var baseIface in baseInterfaces) {
var bm = 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 (var prop in Properties) {
foreach (var baseIface in baseInterfaces) {
var baseProp = baseIface.Properties.FirstOrDefault (p => p.Name == prop.Name && p.Type == prop.Type);
if (baseProp == null)
continue;

bool shouldBreak = 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 (var field in Fields) {
foreach (var baseIface in baseInterfaces) {
var baseField = 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;
}
}
Expand Down