From c5e1b92ac1fa95df5bc44202fb5b6528ccb020e9 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 16 Dec 2025 13:57:53 -0600 Subject: [PATCH 1/3] [generator] Fix `UnsupportedOSPlatform` for property setters when base has getter-only Context: https://github.com/dotnet/android/pull/10510#issuecomment-3325250701 When a derived class has a property setter with `ApiRemovedSince`, but the base class only has a getter (no setter), clear the setter's `ApiRemovedSince` if the base getter is not removed. Also handle standalone `setXxx` methods that correspond to base class properties. Fixes `CA1416` warning for `ListView.Adapter.set` being incorrectly marked as unsupported on `android15.0`. --- .../Unit-Tests/CodeGeneratorTests.cs | 37 +++++++++++++++++++ .../GenBase.cs | 23 ++++++++++++ 2 files changed, 60 insertions(+) diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs index bc7adf68e..bd952f6f4 100644 --- a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs +++ b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs @@ -1563,6 +1563,43 @@ 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 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..215ff1df0 100644 --- a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs +++ b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs @@ -366,6 +366,10 @@ public void FixupMethodOverrides (CodeGenerationOptions opt) 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 +377,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) @@ -419,6 +438,10 @@ public void FixupMethodOverrides (CodeGenerationOptions opt) 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; From 25e6a9aaaf2d7acb945e08f6e0819f4da2d83c77 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 16 Dec 2025 15:59:09 -0600 Subject: [PATCH 2/3] Update CodeGeneratorTests.cs --- .../Unit-Tests/CodeGeneratorTests.cs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs index bd952f6f4..11f0219c5 100644 --- a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs +++ b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs @@ -1600,6 +1600,43 @@ public void UnsupportedOSPlatformIgnoresPropertySetterOverridesWhenBaseHasGetter 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 StringPropertyOverride ([Values ("true", "false")] string final) { From bca132539f368c4baeb4203f60891361b4298a41 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 6 Jan 2026 16:55:03 -0600 Subject: [PATCH 3/3] Handle covariant property overrides for ApiRemovedSince Updated property override logic to match base properties by name only, allowing for covariant return and contravariant parameter types. This prevents incorrectly marking overridden getters/setters as removed when the base property is not removed, even if the return or parameter types differ. Added a unit test to verify that [UnsupportedOSPlatform] is not applied in these covariant override scenarios. --- .../Unit-Tests/CodeGeneratorTests.cs | 40 ++++++++++++++ .../GenBase.cs | 52 +++++++++---------- 2 files changed, 64 insertions(+), 28 deletions(-) diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs index 11f0219c5..242edc28e 100644 --- a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs +++ b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs @@ -1637,6 +1637,46 @@ public void UnsupportedOSPlatformIgnoresStandaloneSetterMethodWhenBaseHasGetterO 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 215ff1df0..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,26 @@ 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; @@ -419,25 +415,25 @@ 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;