Skip to content
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.

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

Deprecated binding constructs #1200

Open
jpobst opened this issue Feb 28, 2024 · 2 comments
Open

Deprecated binding constructs #1200

jpobst opened this issue Feb 28, 2024 · 2 comments
Labels
enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.)

Comments

@jpobst
Copy link
Contributor

jpobst commented Feb 28, 2024

This issue documents the constructs we have marked as [Obsolete] to inform if we want to remove them in the future.

Additionally, we would need to decide if any changes are global (affecting both Mono.Android and outside users) or if we will feature flag them and whether the flag will be opt-in or opt-out.

Scenario 1 - Interface Alternative Classes

When C# 8 added support for static interface members, matching Java's support, it removed the need for many workarounds we had previously employed to make these members available. Previously, we had copied these static members to a similarly named class. As of API-30, we place the static members in their original interface and mark these "alternative" classes as [Obsolete].

Example:

// Metadata.xml XPath interface reference: path="/api/package[@name='java.util.random']/interface[@name='RandomGenerator']"
[Register ("java/util/random/RandomGenerator", "", "Java.Util.RandomGenerators.IRandomGeneratorInvoker", ApiSince = 35)]
public partial interface IRandomGenerator : IJavaObject, IJavaPeerable {
  // Metadata.xml XPath method reference: path="/api/package[@name='java.util.random']/interface[@name='RandomGenerator']/method[@name='of' and count(parameter)=1 and parameter[1][@type='java.lang.String']]"
  [global::System.Runtime.Versioning.SupportedOSPlatformAttribute ("android35.0")]
  [Register ("of", "(Ljava/lang/String;)Ljava/util/random/RandomGenerator;", "", ApiSince = 35)]
  public static unsafe Java.Util.RandomGenerators.IRandomGenerator? Of (string? name)
  { ... Implementation omitted ... }
}

[global::System.Runtime.Versioning.SupportedOSPlatformAttribute ("android35.0")]
[Register ("java/util/random/RandomGenerator", ApiSince = 35, DoNotGenerateAcw=true)]
[global::System.Obsolete (@"Use the 'Java.Util.RandomGenerators.IRandomGenerator' type. This class will be removed in a future release.")]
public abstract class RandomGenerator : Java.Lang.Object {
  internal RandomGenerator ()
  {
  }

  // Metadata.xml XPath method reference: path="/api/package[@name='java.util.random']/interface[@name='RandomGenerator']/method[@name='of' and count(parameter)=1 and parameter[1][@type='java.lang.String']]"
  [global::System.Obsolete (@"Use 'Java.Util.RandomGenerators.IRandomGenerator.Of'. This class will be removed in a future release.")]
  [global::System.Runtime.Versioning.SupportedOSPlatformAttribute ("android35.0")]
  [Register ("of", "(Ljava/lang/String;)Ljava/util/random/RandomGenerator;", "", ApiSince = 35)]
  public static unsafe Java.Util.RandomGenerators.IRandomGenerator? Of (string? name)
  { ... Implementation omitted ... }
}

As of API-35 there are 179 of these "alternative" classes.

Possible next deprecation step(s):

  • Change obsolete warnings to errors: [Obsolete ("...", error: true)]

Scenario 2 - Enumified Constants

When we create an enum from a set of constants as part of enumification, we use to leave the constant in its original location. At some point we decided this was confusing, and as of API-30 we mark these original locations with [Obsolete ("...", error: true)].

Example:

public enum SleepStageType 
{
  [global::Android.Runtime.IntDefinition ("Android.Health.Connect.DataTypes.SleepSessionRecord.StageType.StageTypeUnknown", JniField = "android/health/connect/datatypes/SleepSessionRecord$StageType.STAGE_TYPE_UNKNOWN")]
  [global::System.Runtime.Versioning.SupportedOSPlatformAttribute ("android34.0")]
  Unknown = 0,

  [global::Android.Runtime.IntDefinition ("Android.Health.Connect.DataTypes.SleepSessionRecord.StageType.StageTypeAwake", JniField = "android/health/connect/datatypes/SleepSessionRecord$StageType.STAGE_TYPE_AWAKE")]
  [global::System.Runtime.Versioning.SupportedOSPlatformAttribute ("android34.0")]
  Awake = 1,

  [global::Android.Runtime.IntDefinition ("Android.Health.Connect.DataTypes.SleepSessionRecord.StageType.StageTypeSleeping", JniField = "android/health/connect/datatypes/SleepSessionRecord$StageType.STAGE_TYPE_SLEEPING")]
  [global::System.Runtime.Versioning.SupportedOSPlatformAttribute ("android34.0")]
  Sleeping = 2
}

// Metadata.xml XPath class reference: path="/api/package[@name='android.health.connect.datatypes']/class[@name='SleepSessionRecord.StageType']"
[global::System.Runtime.Versioning.SupportedOSPlatformAttribute ("android34.0")]
[global::Android.Runtime.Register ("android/health/connect/datatypes/SleepSessionRecord$StageType", DoNotGenerateAcw=true, ApiSince = 34)]
public sealed partial class StageType : Java.Lang.Object {
// Metadata.xml XPath field reference: path="/api/package[@name='android.health.connect.datatypes']/class[@name='SleepSessionRecord.StageType']/field[@name='STAGE_TYPE_UNKNOWN']"
  [Register ("STAGE_TYPE_UNKNOWN", ApiSince = 34)]
  [global::System.Runtime.Versioning.SupportedOSPlatformAttribute ("android34.0")]
  [global::System.Obsolete (@"This constant will be removed in the future version. Use Android.Health.Connect.DataTypes.SleepStageType enum directly instead of this field.", error: true)]
  public const Android.Health.Connect.DataTypes.SleepStageType StageTypeUnknown = (Android.Health.Connect.DataTypes.SleepStageType) 0;

  // Metadata.xml XPath field reference: path="/api/package[@name='android.health.connect.datatypes']/class[@name='SleepSessionRecord.StageType']/field[@name='STAGE_TYPE_AWAKE']"
  [Register ("STAGE_TYPE_AWAKE", ApiSince = 34)]
  [global::System.Runtime.Versioning.SupportedOSPlatformAttribute ("android34.0")]
  [global::System.Obsolete (@"This constant will be removed in the future version. Use Android.Health.Connect.DataTypes.SleepStageType enum directly instead of this field.", error: true)]
  public const Android.Health.Connect.DataTypes.SleepStageType StageTypeAwake = (Android.Health.Connect.DataTypes.SleepStageType) 1;

  // Metadata.xml XPath field reference: path="/api/package[@name='android.health.connect.datatypes']/class[@name='SleepSessionRecord.StageType']/field[@name='STAGE_TYPE_SLEEPING']"
  [Register ("STAGE_TYPE_SLEEPING", ApiSince = 34)]
  [global::System.Runtime.Versioning.SupportedOSPlatformAttribute ("android34.0")]
  [global::System.Obsolete (@"This constant will be removed in the future version. Use Android.Health.Connect.DataTypes.SleepStageType enum directly instead of this field.", error: true)]
  public const Android.Health.Connect.DataTypes.SleepStageType StageTypeSleeping = (Android.Health.Connect.DataTypes.SleepStageType) 2;
}

As of API-35 there are 7501 instances of these [Obsolete ("...", error: true)] constants.

Possible next deprecation step(s):

  • Remove these constants from the public API

Scenario 3 - Interface Consts Alternative Classes

Before we created scenario 1, we originally moved interface constants to an alternative type with a Consts suffix where they could be accessed.

Example:

[Register ("androidx/media3/common/SimpleBasePlayer$PositionSupplier", DoNotGenerateAcw=true)]
[global::System.Obsolete (@"Use the 'PositionSupplier' type. This type will be removed in a future release.", error: true)]
public abstract class PositionSupplierConsts : PositionSupplier {
  private PositionSupplierConsts () { }
}

This alternative class is always error: true, and is not generated at all if C# 8's interface constants feature is turned on. (lang-features=interface-constants)

As Mono.Android uses interface-constants, there are 0 of these types in API-35.

@jpobst jpobst added enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.) labels Feb 28, 2024
@jonpryor
Copy link
Member

jonpryor commented Mar 4, 2024

Regarding Scenario 1, I agree that we should [Obsolete(error:true)] those types, and remove them in a future release. We should absolutely prefer static members on interfaces, which better mirrors Java.

Regarding Scenario 2, I like having the [Obsolete] members, as I think they are useful when manually porting Java code to C#, as one may find in e.g. stackoverflow answers. That said, we've stopped emitting these fields for new members ages ago, which means things are now inconsistent. In the interest of consistency, yes, we should also remove these fields as well.

@jonpryor
Copy link
Member

jonpryor commented Mar 5, 2024

On further sleep & contemplation, one issue with Scenario 1 and removing the "Interface Alternative Classes" is that it'll be an ABI break. Yes, new code can't have been using those types since…(insert date here). However, any assembly built before that day may still be attempting to reference those members, and if a new app uses those older assemblies, then the apps will break.

I can think of only two "workarounds":

  1. Keep the types, but make them internal. The types will continue to exist, maintaining ABI, while new code won't be able to see the types, cleaning up API.
  2. Introduce a linker step which fixes the "obsolete" references and replaces them with their new replacements. I imagine this is possible, but I don't know how easy it'll be, and I'm sure it'll involve lots of "inferencing".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Proposed change to current functionality generator Issues binding a Java library (generator, class-parse, etc.)
Projects
None yet
Development

No branches or pull requests

2 participants