diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs index bc7adf68e..242edc28e 100644 --- a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs +++ b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs @@ -1563,6 +1563,120 @@ public void UnsupportedOSPlatformIgnoresPropertyOverrides () StringAssert.DoesNotContain ("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android30.0\")]", ifaceActual, "Should not contain UnsupportedOSPlatform on interface property override!"); } + [Test] + public void UnsupportedOSPlatformIgnoresPropertySetterOverridesWhenBaseHasGetterOnly () + { + // Given: + // public class AdapterView { + // public Object getAdapter () { ... } + // } + // public class ListView : AdapterView { + // public Object getAdapter () { ... } // removed-since = 15 + // public void setAdapter (Object value) { ... } // removed-since = 15 + // } + // We should not write [UnsupportedOSPlatform] on ListView.Adapter.set because the base property (via getter) isn't "removed". + var xml = @$" + + + + + + + + + + + + + + + "; + + var gens = ParseApiDefinition (xml); + var klass = gens.Single (g => g.Name == "ListView"); + var actual = GetGeneratedTypeOutput (klass); + + // Neither the getter nor the setter should have [UnsupportedOSPlatform] because the base property (getter) isn't removed + StringAssert.DoesNotContain ("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android15.0\")]", actual, "Should not contain UnsupportedOSPlatform on property setter when base has getter only!"); + } + + [Test] + public void UnsupportedOSPlatformIgnoresStandaloneSetterMethodWhenBaseHasGetterOnly () + { + // Given: + // public class AdapterView { + // public Object getAdapter () { ... } + // } + // public class ListView : AdapterView { + // // no getAdapter override + // public void setAdapter (Object value) { ... } // removed-since = 15 + // } + // We should not write [UnsupportedOSPlatform] on ListView.SetAdapter because the base property (via getter) isn't "removed". + // The setAdapter remains a standalone method because there's no getAdapter to pair with in the derived class. + var xml = @$" + + + + + + + + + + + + + + "; + + var gens = ParseApiDefinition (xml); + var klass = gens.Single (g => g.Name == "ListView"); + var actual = GetGeneratedTypeOutput (klass); + + // The standalone setter method should not have [UnsupportedOSPlatform] because the base property (getter) isn't removed + StringAssert.DoesNotContain ("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android15.0\")]", actual, "Should not contain UnsupportedOSPlatform on standalone setter method when base has getter only!"); + } + + [Test] + public void UnsupportedOSPlatformIgnoresPropertySetterOverridesWhenBaseHasCovariantReturn () + { + // Given: + // public class AdapterView { + // public Object getAdapter () { ... } // returns Object (generic erasure) + // } + // public class ListView : AdapterView { + // public ListAdapter getAdapter () { ... } // returns ListAdapter (covariant), removed-since = 15 + // public void setAdapter (ListAdapter) { ... } // removed-since = 15 + // } + // We should not write [UnsupportedOSPlatform] on ListView.Adapter.set because the base property (via getter) isn't "removed", + // even though the return types differ (covariant return). + var xml = @$" + + + + + + + + + + + + + + + + "; + + var gens = ParseApiDefinition (xml); + var klass = gens.Single (g => g.Name == "ListView"); + var actual = GetGeneratedTypeOutput (klass); + + // Neither the getter nor the setter should have [UnsupportedOSPlatform] because the base property (getter) isn't removed, + // even though the return types differ (covariant return). + StringAssert.DoesNotContain ("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android15.0\")]", actual, "Should not contain UnsupportedOSPlatform on property setter when base has covariant getter only!"); + } + [Test] public void StringPropertyOverride ([Values ("true", "false")] string final) { diff --git a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs index 4f0ca0b8e..1d5beb961 100644 --- a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs +++ b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs @@ -342,30 +342,30 @@ public void FixupMethodOverrides (CodeGenerationOptions opt) // 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); + // Match by name only (not type) to handle covariant return types + var baseProp = bt.Properties.FirstOrDefault (p => p.Name == prop.Name); 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 a "removed" property getter overrides a "not removed" getter, the method was + // likely moved to a base class (or is a covariant override), so don't mark it as removed. + // Note: We don't check return type equality to support covariant return types. + 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 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. + // Note: We don't check parameter types to support contravariant parameter types. + prop.Setter.ApiRemovedSince = default; + shouldBreak = true; + } else if (prop.Setter != null && prop.Setter.ApiRemovedSince > 0 && baseProp.Setter == null && baseProp.Getter != null && baseProp.Getter.ApiRemovedSince == 0) { + // Base has getter-only property; setter in derived should not be marked removed + prop.Setter.ApiRemovedSince = default; + shouldBreak = true; } if (shouldBreak) { break; @@ -373,6 +373,21 @@ public void FixupMethodOverrides (CodeGenerationOptions opt) } } + // Process standalone setter methods (setXxx) that correspond to base class properties. + // If the base property getter isn't removed, the setter shouldn't be either. + foreach (var m in Methods.Where (m => !m.IsStatic && !m.IsInterfaceDefaultMethod && m.ApiRemovedSince > 0)) { + if (!m.JavaName.StartsWith ("set", StringComparison.Ordinal) || m.Parameters.Count != 1 || m.RetVal.JavaName != "void") + continue; + var propertyName = m.JavaName.Substring (3); + for (var bt = GetBaseGen (opt); bt != null; bt = bt.GetBaseGen (opt)) { + var baseProp = bt.Properties.FirstOrDefault (p => p.Getter?.JavaName == "get" + propertyName); + if (baseProp?.Getter != null && baseProp.Getter.ApiRemovedSince == 0) { + m.ApiRemovedSince = default; + break; + } + } + } + // 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) @@ -400,25 +415,29 @@ public void FixupMethodOverrides (CodeGenerationOptions opt) // 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); + // Match by name only (not type) to handle covariant return types + var baseProp = baseIface.Properties.FirstOrDefault (p => p.Name == prop.Name); 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 a "removed" property getter overrides a "not removed" getter, the method was + // likely moved to a base interface (or is a covariant override), so don't mark it as removed. + // Note: We don't check return type equality to support covariant return types. + 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 a "removed" property setter overrides a "not removed" setter, the method was + // likely moved to a base interface, so don't mark it as removed. + // Note: We don't check parameter types to support contravariant parameter types. + prop.Setter.ApiRemovedSince = default; + shouldBreak = true; + } else if (prop.Setter != null && prop.Setter.ApiRemovedSince > 0 && baseProp.Setter == null && baseProp.Getter != null && baseProp.Getter.ApiRemovedSince == 0) { + // Base has getter-only property; setter in derived should not be marked removed + prop.Setter.ApiRemovedSince = default; + shouldBreak = true; } if (shouldBreak) { break;