jcw-gen should warn about "duplicate" methods

[jonpryor]

Background

(I forget what book I originally read this scenario from circa 30 years ago…)

Consider these two Java interfaces:

public interface Artist {
    public void draw();
}
public interface Gunslinger {
    public void draw();
}

if you have an artist who is also a gunslinger, then they better need to do the same thing for draw():

class Hmmm implements Artist, Gunslinger {
    public void draw() {
        // uh…
    }
}

This is one of those Don't Do That™ scenarios in Java: if the method implementation can't be the same, then the same type can't implement both interfaces:

C#, meanwhile, supports this scenario via explicit interface implementations:

public interface IArtist {
    void Draw();
}
public interface IGunslinger {
    void Draw();
}

class ThisCanReasonablyeWork : IArtist, IGunslinger  {
    void IArtist.Draw() => …
    void IGunslinger.Draw() => …
}

"The Setup"

Explicit interface implementations thus provide one of the "semantic mismatches" between Java and C# (among many, to be fair!).

So what happens if we use them to implement bound Java interfaces?

Consider the following Java types:

package example;
public interface A {
    void m();

    public static void callM(A a) {
        a.m();
    }
}
public interface B {
    void m();

    public static void callM(B b) {
        b.m();
    }
}

These would be bound (abbreviated) in .NET for Android as:

namespace Example {
	// Metadata.xml XPath interface reference: path="/api/package[@name='example']/interface[@name='A']"
	[Register ("example/A", "", "Example.IAInvoker")]
	public partial interface IA : IJavaObject, IJavaPeerable {
		private static readonly JniPeerMembers _members = new XAPeerMembers ("example/A", typeof (IA), isInterface: true);

		// Metadata.xml XPath method reference: path="/api/package[@name='example']/interface[@name='A']/method[@name='m' and count(parameter)=0]"
		[Register ("m", "()V", "GetMHandler:Example.IAInvoker, explicitly-implement-ilist")]
		void M ();

		// Metadata.xml XPath method reference: path="/api/package[@name='example']/interface[@name='A']/method[@name='callM' and count(parameter)=1 and parameter[1][@type='example.A']]"
		[Register ("callM", "(Lexample/A;)V", "")]
		public static unsafe void CallM (global::Example.IA? p0)
		{
			const string __id = "callM.(Lexample/A;)V";
			try {
				JniArgumentValue* __args = stackalloc JniArgumentValue [1];
				__args [0] = new JniArgumentValue ((p0 == null) ? IntPtr.Zero : ((global::Java.Lang.Object) p0).Handle);
				_members.StaticMethods.InvokeVoidMethod (__id, __args);
			} finally {
				global::System.GC.KeepAlive (p0);
			}
		}
	}

	[Register ("example/B", "", "Example.IBInvoker")]
	public partial interface IB : IJavaObject, IJavaPeerable {
		private static readonly JniPeerMembers _members = new XAPeerMembers ("example/B", typeof (IB), isInterface: true);

		// Metadata.xml XPath method reference: path="/api/package[@name='example']/interface[@name='B']/method[@name='m' and count(parameter)=0]"
		[Register ("m", "()V", "GetMHandler:Example.IBInvoker, explicitly-implement-ilist")]
		void M ();

		// Metadata.xml XPath method reference: path="/api/package[@name='example']/interface[@name='B']/method[@name='callM' and count(parameter)=1 and parameter[1][@type='example.B']]"
		[Register ("callM", "(Lexample/B;)V", "")]
		public static unsafe void CallM (global::Example.IB? p0)
		{
			const string __id = "callM.(Lexample/B;)V";
			try {
				JniArgumentValue* __args = stackalloc JniArgumentValue [1];
				__args [0] = new JniArgumentValue ((p0 == null) ? IntPtr.Zero : ((global::Java.Lang.Object) p0).Handle);
				_members.StaticMethods.InvokeVoidMethod (__id, __args);
			} finally {
				global::System.GC.KeepAlive (p0);
			}
		}
	}
}

…and because these are C# interfaces, we can explicitly implement them!

public class Hmmm : Java.Lang.Object, Example.IA, Example.IB {
    void Example.IA.M() {
        Console.WriteLine("IA.M");
    }
    void Example.IB.M() {
        Console.WriteLine("IB.M");
    }
}

"The Concern"

What happens when we invoke A.m() and B.m() from Java?

var t = new Hmmm ();
Example.IA.CallM (t);
Example.IB.CallM (t);

What happens is Hmmm.IA.M is invoked twice:

DOTNET  : IA.M
DOTNET  : IA.M

Why?

Because of the Java Callable Wrapper (abbreviated):

public /* partial */ class Hmmm
	extends java.lang.Object
	implements
		mono.android.IGCUserPeer,
		example.A,
		example.B
{
/** @hide */
	public static final String __md_methods;
	static {
		__md_methods = 
			"n_m:()V:GetMHandler:Example.IAInvoker, explicitly-implement-ilist\n" +
			"";
		mono.android.Runtime.register ("explicitly_implement_ilist.BreakThings, explicitly-implement-ilist", BreakThings.class, __md_methods);
	}
}

There Can Be Only One™ registration for Hmmm.m(), and the one we get is for Example.IA.M(). (Which is order-dependent: if the interface implementation order is IB, IA instead of IA, IB, then Hmmm.IB.M() will be used.) The call to Example.IB.CallM(B) doesn't fail (no exception) because Hmmm ISA B, but it calls Hmmm.IA.M(), which might not be at all understandable to C# developers.

"The Ask"

We should consider updating jcw-gen to warn when this scenario is encountered: that is, when a single type implements the same Java-side method more than once.

…but what about inheritance?

A related scenario to this already exists in .NET for Android, because many interface re-declare methods from their "base" interfaces, e.g. java.util.Collection declares add(Object), and java.util.List also declares add(Object):

package java.util;

public interface Collection<E> {
    public boolean add(E e);
}
public interface List<E> extends Collection<E> {
    public boolean add(E e);
}

Related:

• [Mono.Android] Bind Android 15 DP 2 android#8741 (comment)
• [Mono.Android] Bind Android 15 DP 2 android#8741 (comment)

Which means we can explicitly implement Add() twice, already:

partial class Pondering : Java.Lang.Object, Java.Util.ICollection, Java.Util.IList {
    bool Java.Util.ICollection.Add(Java.Lang.Object? e) => …
    bool Java.Util.IList.Add(Java.Lang.Object? e) => …
}

The problems are the same: There Can Be Only One™ add() method declared, so the marshal method chosen will depend upon the inheritance order (i.e. the marshal method for ICollection.Add() will be used), and the other method is Dead Code. No warning is emitted.

"The Ask" will also handle this scenario.

Another way to handle this scenario is to prevent it from happening in the first place. This scenario happens because we ignore warning CS0114 in our bindings, which allows IList to re-declare Add(), hiding ICollection.Add(). This only isn't a problem because explicit interface implementations are rarely used within bindings.

If we instead didn't ignore CS0114, we could prevent this scenario by either:

1. Removing "identically declared" methods from derived types, e.g. remove the IList.Add() declaration, or
2. "re-abstract" the IList method declarations which have "identical declarations" from base interfaces (22d5687, 73ebad2)

"Sanely" handling this scenario will require generator support, and may also constitute an ABI break (removing Java.Util.IList.Add() could break existing code!). This might be something we only want to consider for Java.Base (#858).