[Xamarin.Android.Build.Tasks] FixAbstractMethodsStep overrides (#861)_wow

Context: https://bugzilla.xamarin.com/show_bug.cgi?id=22074
Context: https://bugzilla.xamarin.com/show_bug.cgi?id=59293
Context: https://developer.xamarin.com/releases/android/xamarin.android_6/xamarin.android_6.1/#Improved_Java_Interface_Versioning_Support

**Background**:

Until [C# gains support for default interface methods][ifaces], there
is an "impedance mismatch" between Java interfaces and C# interfaces:
Java methods can be added to interfaces *without* requiring that
existing classes implementing those interfaces implement the "new"
methods. (*This is not* Java default interface methods!)

[ifaces]: https://github.com/dotnet/csharplang/blob/master/proposals/default-interface-methods.md

Consider [`android.database.Cursor`][cursor]:

[cursor]: https://developer.android.com/reference/android/database/Cursor.html

```java
// API-1
interface Cursor {
    void close();
    // ...
}

class MyCursor implements Cursor {
    public void close() {}
}
```

The `Cursor` interface had several methods added to it over time --
*before* the Android SDK supported default interface methods! --
e.g. the addition of `Cursor.getType()` in API-11:

```java
interface Cursor {
    // in API-1
    void close();
    // ...
    
    // Added in API-11
    int getType(int columnIndex);
}
```

Question: What happens when `MyCursor.getType()` is invoked, if/when
`MyCursor` *has not* been recompiled?

```java
Cursor c = new MyCursor();     // Implements API-1 API, *not* API-11!
c.getType(0);                  // Method added in API-11!
```

What happens is that an [`AbstractMethodError`][ame] is thrown.

[ame]: https://developer.android.com/reference/java/lang/AbstractMethodError.html

Compare to C#, in which *the type cannot be loaded* if an interface
has a method added:

	[ERROR] FATAL UNHANDLED EXCEPTION: System.TypeLoadException: VTable setup of type MyCursor failed

What this meant is [Bug #22074][b22074]: if you have a
Xamarin.Android library, and that library:

  1. Has `$(TargetFrameworkVersion)`=v2.3 (API-10), and
  2. Has a type which implements `Android.Database.ICursor`

and you then use the type (2) in an App which has
`$(TargetFrameworkVersion)`=v4.4, the app would crash with a
`TypeLoadException`.

[b22074]: https://bugzilla.xamarin.com/show_bug.cgi?id=22074

The fix, [introduced in Xamarin.Android 6.1][xa6.1], is the
`FixAbstractMethodsStep` step, which looks at every type in every
assembly to be included in the `.apk`, and checks to see if that type
is "missing" any abstract methods. If any such type is found, then
the "missing" method is *inserted*, and it will throw an
`AbstractMethodError`.

[xa6.1]: https://developer.xamarin.com/releases/android/xamarin.android_6/xamarin.android_6.1/#Improved_Java_Interface_Versioning_Support

For example, if an assembly `Lib.dll` built against API-10 has:

```csharp
public class MyCursor : Java.Lang.Object, Android.Database.ICursor {
    // Implement `ICursor` methods from API-10
}
```

then when `Lib.dll` is packaged as part of an API-11+ app the
`FixAbstractMethodsStep` step will *add* the following method:

```csharp
int ICursor.GetType(int columnIndex)
{
    throw new Java.Lang.AbstractMethodError();
}
```

**The Problem**: The Xamarin.Forms team ran into an issue with
[the debugger in Visual Studio][b59293]. During investigation it
turned out that our `FixAbstractMethodsStep` injects two methods
which were not needed, as the methods were already implemented.

[b59293]: https://bugzilla.xamarin.com/show_bug.cgi?id=59293

**The Fix**: Improve "missing abstract method" detection by looking
in `MethodDefinition.Overrides` to find the interface method and
compare it to the interface method we are looking for, not just
using the method name. This will be more resilient to compiler
changes -- not all compilers emit the same name for explicitly
implemented interface methods -- and prevent us from emitting
explicit interface methods when they're already present.

Author: radekdoulik

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs

@@ -95,8 +95,23 @@ static bool CompareTypes (TypeReference iType, TypeReference tType)
 			return true;
 		}
 
+		bool IsInOverrides (MethodDefinition iMethod, MethodDefinition tMethod)
+		{
+			if (!tMethod.HasOverrides)
+				return false;
+
+			foreach (var o in tMethod.Overrides)
+				if (o != null && iMethod == o.Resolve ())
+					return true;
+
+			return false;
+		}
+
 		bool HaveSameSignature (TypeReference iface, MethodDefinition iMethod, MethodDefinition tMethod)
 		{
+			if (IsInOverrides (iMethod, tMethod))
+				return true;
+
 			if (iMethod.Name != tMethod.Name && (iMethod.DeclaringType == null || (iMethod.DeclaringType.DeclaringType == null ? (string.Format ("{0}.{1}", iface.FullName, iMethod.Name) != tMethod.Name) : (string.Format ("{0}.{1}.{2}", iMethod.DeclaringType.DeclaringType, iface.Name, iMethod.Name) != tMethod.Name))))
 				return false;
 

tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-APIv1Binding/java/test/bindings/Cursor.java

@@ -3,6 +3,7 @@
 public interface Cursor {
     void method();
     int methodWithRT ();
+    int methodWithCursor (Cursor cursor);
     int methodWithParams (int number, String text);
     int methodWithParams (int number, String text, float real);
 }

tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-APIv2Binding/java/test/bindings/Cursor.java

@@ -5,6 +5,7 @@ public interface Cursor {
     void newMethod();
     int methodWithRT ();
     int newMethodWithRT ();
+    int methodWithCursor (Cursor cursor);
     int methodWithParams (int number, String text);
     int newMethodWithParams (int number, String text);
     int methodWithParams (int number, String text, float real);

tests/CodeGen-Binding/Xamarin.Android.FixJavaAbstractMethod-Library/MyClrCursor.cs

@@ -3,7 +3,7 @@
 
 namespace Library
 {
-	public class MyClrCursor : Java.Lang.Object, ICursor
+	public class MyClrCursor : Java.Lang.Object, global::Test.Bindings.ICursor
 	{
 		public void Method ()
 		{
@@ -23,6 +23,13 @@ public int MethodWithRT ()
 		{
 			return 3;
 		}
+
+		int global::Test.Bindings.ICursor.MethodWithCursor (global::Test.Bindings.ICursor cursor)
+		{
+			var a = 2;
+			var b = 2;
+			return a + b;
+		}
 	}
 }
 

tests/CodeGen-Binding/Xamarin.Android.JcwGen-Tests/BindingTests.cs

@@ -179,6 +179,11 @@ public void JavaAbstractMethodTest ()
 				if (e.GetType ().ToString () != "Java.Lang.AbstractMethodError")
 					throw e;
 			}
+
+			var mi = ic.GetType ().GetMethod ("MethodWithCursor");
+
+			if (mi != null && mi.GetMethodBody ().LocalVariables.Count == 0)
+				throw new Exception ("FixAbstractMethodStep broken, MethodWithRT added, while it should not be");
 		}
 
 		// Context https://bugzilla.xamarin.com/show_bug.cgi?id=36036