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

[generator] When using interface-constants, remove or obsolete interface alternatives. #600

Merged
merged 2 commits into from
Mar 13, 2020

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Mar 11, 2020

Context: #509.

Due to our legacy workarounds for the C# shortcoming of not being able to place constants or members on interfaces, every interface with those members creates two additional "alternative" classes to provide users access to the members originally on the interface.

For example, the Java interface android.os.Parcelable creates:

  • interface IParcelable containing the interface declaration
  • class ParcelableConsts containing the constants that are on Parcelable
  • class Parcelable contains the constants and static members that are on Parcelable

Now that we can support members on interfaces, we would like to start the process of removing these interface alternatives.

ParcelableConsts class

The XXXXConsts classes have been [Obsolete] for over 5 years, and have been [Obsolete (iserror: true)] since 16.4. They haven't been iserror: true for long enough to completely remove, but we can remove them when --lang-features:interface-constants is specified.

In terms of Mono.Android.dll, this means these classes will be removed for the API-R binding, but will still exist in previous bindings.

Parcelable class

The XXXX class is now redundant when interface-constants and default-interface-methods is specified, as all members of the interface can be generated on the interface.

In this case, we [Obsolete] all members in the alternative Parcelable class with a message pointing to the original interface. Note that if the member already had [Obsolete] for another reason (like being deprecated in Java) we preserve that message instead.

In terms of Mono.Android.dll, this means these classes and their members will be [Obsolete] in the API-R binding, but not be obsoleted in previous bindings.


Also fixed a bug where fields that are marked [Obsolete] that are being changed into property getters where not having the obsolete attribute applied.


Here's the diff on Mono.Android.dll: https://gist.github.com/jpobst/717e67d6d21638c0600511ee34469c1d. Note that mono-api-html apparently doesn't show removed classes, so I had to run it backwards. Thus everything reported as "additions" are actually "removals".

@jpobst jpobst marked this pull request as ready for review March 11, 2020 18:49
@jpobst jpobst requested a review from jonpryor March 11, 2020 18:49
@jonpryor jonpryor merged commit 105d544 into master Mar 13, 2020
@jonpryor jonpryor deleted the obsolete-interface-alternatives branch March 13, 2020 19:22
jonpryor pushed a commit that referenced this pull request Mar 16, 2020
Context: #509

When using `generator --lang-features=interface-constants`, remove or
obsolete interface alternatives.

Due to our legacy workarounds for the C# shortcoming of not being able
to place constants or members on interfaces, every interface with
those members creates two additional "alternative" classes to provide
users access to the members originally on the interface.

For example, consider the Java interface [`android.os.Parcelable`][0]:

	// Java
	package android.os;
	public interface Parcelable {
	    public static final int CONTENTS_FILE_DESCRIPTOR      = 1;
	    public static final int PARCELABLE_WRITE_RETURN_VALUE = 1;

	    int describeContents();
	    void writeToParcel(Parcel dest, int flags);

	    // ...
	}

The binding in a C#7 world is:

	// C# Binding
	namespace Android.OS {
	    public partial interface IParcelable {
	        int DescribeContents();
	        void WriteToParcel(Parcel dest, [GeneratedEnum] ParcelableWriteFlags flags);
	    }
	    public abstract partial class Parcelable {
	        internal Parcelable () {}

	        public const int ContentsFileDescriptor = (int) 1;
		public const ParcelableWriteFlags ParcelableWriteReturnValue = (ParcelableWriteFlags) 1;
	    }
	    [Obsolete ("…", error: true)]
	    public abstract partial class ParcelableConsts : Parcelable {
	        private ParcelableConsts () {}

	        public const int ContentsFileDescriptor = (int) 1;
		public const ParcelableWriteFlags ParcelableWriteReturnValue = (ParcelableWriteFlags) 1;
	    }
	}

`Parcelable` contains the interface constants which couldn't exist on
`IParcelable`, and `ParcelableConsts` exists for backward
compatibility (as we used `*Consts` types before we introduced the
non-`*Consts` types), and is `[Obsolete(error:true)]`.

In a C#8 world with `generator --lang-features=interface-constants`
we no longer need to preserve these alternative types.

The `*Consts` types have been `[Obsolete]` for over 5 years, but have
only been `[Obsolete(error:true)]` since d16-4, released 2019-Dec-03.
We feel it hasn't been an error long enough to remove it for *all*
API levels, so we will stop emitting it for API-R.

The `Parcelable` and similar alternative types are now redundant when
`--lang-features=interface-constants,default-interface-methods` is
used, as all members of the Java interface can now exist in the C#
binding of that interface.  Here, we will `[Obsolete]` the class and
all members of the class, with a message referring to the new types
and members to use.

This results in an API-R binding of:

	// C# Binding, API-R
	namespace Android.OS {
	    public partial interface IParcelable {
	        int DescribeContents();
	        void WriteToParcel(Parcel dest, [GeneratedEnum] ParcelableWriteFlags flags);

	        public const int ContentsFileDescriptor = (int) 1;
		public const ParcelableWriteFlags ParcelableWriteReturnValue = (ParcelableWriteFlags) 1;
	    }
	    [Obsolete ("Use 'Android.OS.IParcelable'.  This class will be removed in a future release.")]
	    public abstract partial class Parcelable {
	        internal Parcelable () {}

	        [Obsolete ("Use 'Android.OS.IParcelable.ContentsFileDescriptor'.  This class will be removed in a future release.")]
	        public const int ContentsFileDescriptor = (int) 1;

	        [Obsolete ("Use 'Android.OS.IParcelable.ParcelableWriteReturnValue'.  This class will be removed in a future release.")]
		public const ParcelableWriteFlags ParcelableWriteReturnValue = (ParcelableWriteFlags) 1;
	    }
	}

Additionally, fix a bug where fields that are marked `[Obsolete]` that
are being changed into property getters were not having the obsolete
attribute applied.

[0]: https://developer.android.com/reference/android/os/Parcelable
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants