From c5e1b92ac1fa95df5bc44202fb5b6528ccb020e9 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 16 Dec 2025 13:57:53 -0600 Subject: [PATCH 1/4] [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/4] 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/4] 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; From 65b618499dede81466b0722532506939f04691ed Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 16 Jan 2026 10:01:26 -0600 Subject: [PATCH 4/4] [generator] Fix UnsupportedOSPlatform for properties with different C# names When a derived class property has a different C# name than the base class property but the same Java getter/setter names (e.g., ListView.Adapter vs AdapterView.RawAdapter both using getAdapter/setAdapter), the ApiRemovedSince fixup now correctly matches by Java method name instead of only by C# property name. This prevents incorrect [UnsupportedOSPlatformAttribute] generation on property accessors when the base class accessor is still available. --- .../Unit-Tests/CodeGeneratorTests.cs | 44 +++++++++++++++++++ .../GenBase.cs | 38 ++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs index 242edc28e..34844cd33 100644 --- a/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs +++ b/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs @@ -1677,6 +1677,50 @@ public void UnsupportedOSPlatformIgnoresPropertySetterOverridesWhenBaseHasCovari 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 UnsupportedOSPlatformIgnoresPropertySetterWhenBaseHasDifferentPropertyName () + { + // Given: + // public class AdapterView { + // public Object getAdapter () { ... } // becomes RawAdapter property (type Object) + // public void setAdapter (Object value) { ... } + // } + // public class ListView : AdapterView { + // public ListAdapter getAdapter () { ... } // becomes Adapter property (type ListAdapter), removed-since = 15 + // public void setAdapter (ListAdapter) { ... } // removed-since = 15 + // } + // Due to type refinement, AdapterView has 'RawAdapter' property and ListView has 'Adapter' property (different C# names). + // We should not write [UnsupportedOSPlatform] on ListView.Adapter.set because the base property (via Java setAdapter) 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 setter (Java setAdapter) isn't removed, + // even though they have different C# property names (RawAdapter vs Adapter). + StringAssert.DoesNotContain ("[global::System.Runtime.Versioning.UnsupportedOSPlatformAttribute (\"android15.0\")]", actual, "Should not contain UnsupportedOSPlatform on property setter when base has different property name but same Java setter!"); + } + [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 1d5beb961..c7d566a67 100644 --- a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs +++ b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/GenBase.cs @@ -345,6 +345,44 @@ public void FixupMethodOverrides (CodeGenerationOptions opt) // Match by name only (not type) to handle covariant return types var baseProp = bt.Properties.FirstOrDefault (p => p.Name == prop.Name); if (baseProp == null) { + // Check if base has a property with same Java getter/setter names (different C# name due to type refinement) + // This handles cases like ListView.Adapter overriding AdapterView.RawAdapter + if (prop.Getter != null && prop.Getter.ApiRemovedSince > 0) { + var basePropByGetter = bt.Properties.FirstOrDefault (p => + p.Getter?.JavaName == prop.Getter.JavaName && + p.Getter.ApiRemovedSince == 0); + if (basePropByGetter != null) { + prop.Getter.ApiRemovedSince = default; + // Also fix setter if it matches + if (prop.Setter != null && prop.Setter.ApiRemovedSince > 0 && + basePropByGetter.Setter?.JavaName == prop.Setter.JavaName && + basePropByGetter.Setter.ApiRemovedSince == 0) { + prop.Setter.ApiRemovedSince = default; + } + break; + } + } + // Check if base has a method that matches the setter (e.g. manually-defined property in partial class) + // This handles cases like AbsListView.Adapter which is defined in a partial class but has setAdapter method. + if (prop.Setter != null && prop.Setter.ApiRemovedSince > 0) { + var baseSetterMethod = bt.Methods.FirstOrDefault (m => + m.JavaName == prop.Setter.JavaName && + m.Parameters.Count == prop.Setter.Parameters.Count && + m.RetVal.JavaName == "void" && + m.ApiRemovedSince == 0); + if (baseSetterMethod != null) { + prop.Setter.ApiRemovedSince = default; + break; + } + // Also check for base property with same Java setter name (different C# name due to type refinement) + var basePropBySetter = bt.Properties.FirstOrDefault (p => + p.Setter?.JavaName == prop.Setter.JavaName && + p.Setter.ApiRemovedSince == 0); + if (basePropBySetter != null) { + prop.Setter.ApiRemovedSince = default; + break; + } + } continue; }