dotnet / java-interop

Java.Interop provides open-source bindings of Java's Java Native Interface (JNI) for use with .NET managed languages such as C#
Other
189 stars 48 forks source link

Deprecated binding constructs #1200

Open jpobst opened 4 months ago

jpobst commented 4 months ago

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):

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):

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.

jonpryor commented 4 months ago

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 commented 4 months ago

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".