Skip to content

[generator] Add support for @explicitInterface metadata. #1006

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

Merged
merged 1 commit into from
Jul 10, 2022

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Jul 7, 2022

Fixes #1005.

In order to fully support "reabstraction", we need to emit member names with explicit interface names.

public partial interface ICloseable : global::Java.Lang.IAutoCloseable {
    abstract void IAutoCloseable.Close ();
}

This isn't something we have enough information to figure out automatically, so we should allow it to be specified with metadata using a new explicitInterface attribute.

<attr path="/api/package[@name='java.io']/interface[@name='Closeable']/method[@name='Close']" name="explicitInterface">
    IAutoCloseable
</attr>

Note this is not really a concept that generator "understands", it will simply output whatever is specified in this attribute. Thus the value is the C# interface name, not the Java interface name.

For properties, placing explicitInterface on either the getXXX or setXXX methods will work. The getter value takes precedence if both are specified.

This is implemented for both interface and class members, should there be other use-cases that can benefit from it.

@jpobst jpobst marked this pull request as ready for review July 7, 2022 14:35
@jonpryor
Copy link
Contributor

Fixes: https://github.com/xamarin/java.interop/issues/1005

Context: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/default-interface-methods#reabstraction

Sometimes Java interfaces will "re-declare" methods from implemented
interfaces:

	// Java
	interface /* java.lang. */ AutoCloseable {
	    void close();
	}
	interface /* java.io. */ Closeable {
	    void close();
	}

Historically, this has been bound in C# by "ignoring" that the method
was re-declared:

	// C#
	partial interface /* Java.Lang. */ IAutoCloseable {
	    void Close();
	}
	partial interface /* Java.IO. */ ICloseable : IAutoCloseable {
	    void Close();
	}

This would result in a [CS0108 warning][0]:

	warning CS0108: 'ICloseable.Close()' hides inherited member 'IAutoCloseable.Close()'.
	Use the new keyword if hiding was intended.

*Note*: in .NET for Android, this *particular* scenario doesn't
happen, as [`metadata` is used to remove `AutoCloseable`][1].
This scenario happens for other methods, e.g.:

	warning CS0108: 'IChannel.Close()' hides inherited member 'ICloseable.Close()'.

Consequently, the .NET for Android build ignores CS0108 warnings.

This in turn means that e.g. `IChannel.Close()` is implicitly
declared as `new`.

With the introduction of [Default Interface Members in C# 8][2], it
is possible to ["re-abstract"][3] a method declared in an implemented
interface.  However, in order to re-abstract a member, the member
must be explicitly qualified:

	partial interface ICloseable : IAutoCloseable {
	    abstract void IAutoCloseable.Close ();
	}

The declaring type isn't something we have enough information to
figure out automatically.

Add support for `explicitInterface` metadata which allows the
declaring interface to be explicitly specified.

	<attr
	    path="/api/package[@name='java.io']/interface[@name='Closeable']/method[@name='close']" 
	    name="explicitInterface">IAutoCloseable</attr>

Note this is not really a concept that `generator` "understands" it
will simply output whatever is specified in this attribute.  Thus the
value is the *C#* interface name, not the *Java* interface name.

For properties, placing `explicitInterface` on either the `getFoo` or
`setFoo` methods will work.  The getter value takes precedence if
both are specified.

This is implemented for both `interface` and `class` members, should
there be other use-cases that can benefit from it.

[0]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/cs0108
[1]: https://github.com/xamarin/xamarin-android/blob/59352f833846c563768f18dd72133be101b0ddc6/src/Mono.Android/metadata#L1393
[2]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/default-interface-methods
[3]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/default-interface-methods#reabstraction

@jonpryor
Copy link
Contributor

TODO (while I'm musing over it): what does "re-declaring" do with versioning? "Rewind" history; java.io.Closeable.close() was added in Java 1.5, while java.lang.AutoCloseable.close() was added in Java 1.7, with java.io.Closeabe updated in Java 7 to implement AutoCloseable.

What happens in C# for a similar scenario:

// Lib- V1
public interface ICloseable {
    void Close();
}
// Lib- V2
public interface IAutoCloseable {
    void Close();
}
public interface ICloseable : IAutoCloseable {
    abstract void IAutoCloseable.Close ();
}

In particular, imagine we have Client.dll written against V1:

// Client
class Whatever : ICloseable {
    void ICloseable.Close () {}
}

Is Whatever, Client usable with Lib- V2? Or is this an ABI break? Testing is required.

@jonpryor
Copy link
Contributor

Is Whatever, Client usable with Lib- V2? Or is this an ABI break? Testing is required.

It's an ABI break:

Unhandled exception. System.MissingMethodException: Method not found: 'Void Lib.ICloseable.Close()'.

There is a way to make this work, though:

// Llib- V2:
public interface IAutoCloseable {
    void Close();
}
public interface ICloseable : IAutoCloseable {
    void IAutoCloseable.Close() => Close();
    new void Close();
}

This means that reabstraction is only usable within the same API level. It can't be used for versioning purposes.

@jonpryor jonpryor merged commit fadbb82 into main Jul 10, 2022
@jonpryor jonpryor deleted the explicit-interface branch July 10, 2022 10:54
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 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.

Add @explicitInterface metadata option to generator
2 participants