[generator] Fix NRE when interfaces appear to implement classes. (#798)

Fixes: #797

In commit b0d170c we updated `generator` to only apply the
ConstSugar logic to `Mono.Android.dll`.  However, there are other
libraries written by Google that use the same pattern, namely
AndroidX.  I still think removing ConstSugar is the correct move for
libraries other than `Mono.Android` because it causes more problems
than it solves.  (Also, `ConstSugar` is irrelevant in the
Default Interface Method world we want to move the ecosystem to.)

However, there is an issue when other libraries try to create
interfaces that inherit `Mono.Android.dll`'s
`Android.Provider.BaseColumns` type:

`Mono.Android.dll` contains:

	[Register ("android.provider.BaseColumns")] public abstract class BaseColumns
	[Register ("android.provider.BaseColumns")] public interface IBaseColumns

When we build our Java -> C# typemap in `generator`, we only support
a single mapping per key, and it just happens that
`abstract class BaseColumns` wins.  Thus when we look for the Symbol
that matches `implements android.provider.BaseColumns` we end up with
a `class` instead of a `interface`.  We then cast that as an
`InterfaceGen` which is `null`, and throw a `NullReferenceException`:

        System.NullReferenceException: Object reference not set to an instance of an object
           at generator.SourceWriters.BoundInterface.AddInheritedInterfaces (MonoDroid.Generation.InterfaceGen iface, MonoDroid.Generation.CodeGenerationOptions opt)
           at generator.SourceWriters.BoundInterface..ctor (MonoDroid.Generation.InterfaceGen iface, MonoDroid.Generation.CodeGenerationOptions opt, MonoDroid.Generation.CodeGeneratorContext context, MonoDroid.Generation.GenerationInfo genInfo)
           at generator.SourceWriters.SourceWriterExtensions.BuildManagedTypeModel (MonoDroid.Generation.GenBase gen, MonoDroid.Generation.CodeGenerationOptions opt, MonoDroid.Generation.CodeGeneratorContext context, MonoDroid.Generation.GenerationInfo genInfo)
           at generator.SourceWriters.BoundClass.AddNestedTypes (MonoDroid.Generation.ClassGen klass, MonoDroid.Generation.CodeGenerationOptions opt, MonoDroid.Generation.CodeGeneratorContext context, MonoDroid.Generation.GenerationInfo genInfo)
           at generator.SourceWriters.BoundClass..ctor (MonoDroid.Generation.ClassGen klass, MonoDroid.Generation.CodeGenerationOptions opt, MonoDroid.Generation.CodeGeneratorContext context, MonoDroid.Generation.GenerationInfo generationInfo)
           at MonoDroid.Generation.JavaInteropCodeGenerator.WriteType (MonoDroid.Generation.GenBase gen, System.String indent, MonoDroid.Generation.GenerationInfo gen_info)
           at MonoDroid.Generation.ClassGen.Generate (MonoDroid.Generation.CodeGenerationOptions opt, MonoDroid.Generation.GenerationInfo gen_info)
           at Xamarin.Android.Binder.CodeGenerator.Run (Xamarin.Android.Binder.CodeGeneratorOptions options, Java.Interop.Tools.Cecil.DirectoryAssemblyResolver resolver)
           at Xamarin.Android.Binder.CodeGenerator.Run (Xamarin.Android.Binder.CodeGeneratorOptions options)
           at Xamarin.Android.Binder.CodeGenerator.Main (System.String[] args)

There are at least three ways to address this:

 1. We could update `generator` to support more than
    Java -> C# mapping.

 2. *Don't* emit a `[Register]` on the `BaseColumns` class.

 3. Add a `null` check to `BoundInterface.AddInheritedInterfaces()`,
    avoiding the `NullReferenceException`.

While (1) and (2) may address the exception, we're not sure what the
broader ramifications will be.

For now, go with (3).  The bindings will still be fine since these
interfaces are not meant to be implemented, just used to provide
access to constants.

Author: jpobst

tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs

@@ -229,6 +229,44 @@ public void SupportedOSPlatform ()
 
 			StringAssert.DoesNotContain ("[global::System.Runtime.Versioning.SupportedOSPlatformAttribute (\"android30.0\")]", builder.ToString (), "Should contain SupportedOSPlatform!");
 		}
+
+		[Test]
+		public void InheritedInterfaceAsClass ()
+		{
+			// This is a somewhat cheating way to repro a real issue.
+			// The real issue is:
+			// - Binding an interface which implements 'android.provider.BaseColumns'
+			// - android.provider.BaseColumns is an interface in android.jar
+			// - Mono.Android.dll has both:
+			//   - [Register ("android.provider.BaseColumns")] public abstract class BaseColumns
+			//   - [Register ("android.provider.BaseColumns")] public interface IBaseColumns
+			// Our Java type resolution is "last one wins" and happens to pick the class instead of
+			// the interface. So our code is trying to bind an interface that implements a class.
+			var xml = @"<api>
+			  <package name='java.lang' jni-name='java/lang'>
+			    <class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
+			  </package>
+			  <package name='com.xamarin.android' jni-name='com/xamarin/android'>
+			    <class abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' final='false' name='MyConsts' static='false' visibility='public' jni-signature='Ljava/lang/Object;' />
+			    <interface abstract='false' deprecated='not deprecated' extends='java.lang.Object' extends-generic-aware='java.lang.Object' jni-extends='Ljava/lang/Object;' final='false' name='MyClass' static='false' visibility='public' jni-signature='Lcom/xamarin/android/MyClass;'>
+			      <implements name='com.xamarin.android.MyConsts' name-generic-aware='com.xamarin.android.MyConsts' jni-type='Lcom/xamarin/android/MyConsts;'></implements>
+			    </interface>
+			  </package>
+			</api>";
+
+			var gens = ParseApiDefinition (xml);
+
+			// Disable "BuildingCoreAssembly"
+			(gens.Single (g => g.Name == "Object") as ClassGen).FromXml = false;
+
+			var klass = gens.Single (g => g.Name == "IMyClass");
+
+			generator.Context.ContextTypes.Push (klass);
+			generator.WriteType (klass, string.Empty, new GenerationInfo ("", "", "MyAssembly"));
+			generator.Context.ContextTypes.Pop ();
+
+			Assert.Pass ("WriteType did not NRE");
+		}
 	}
 
 	[TestFixture]

tools/generator/SourceWriters/BoundInterface.cs

@@ -137,7 +137,11 @@ void AddInheritedInterfaces (InterfaceGen iface, CodeGenerationOptions opt)
 			foreach (var isym in iface.Interfaces) {
 				var igen = (isym is GenericSymbol ? (isym as GenericSymbol).Gen : isym) as InterfaceGen;
 
-				if (igen.IsConstSugar (opt) || igen.RawVisibility != "public")
+				// igen *should not* be null here because we *should* only be inheriting interfaces, but
+				// in the case of constants on that interface we create a C# *class* that is registered for the
+				// Java *interface*. Thus when we do type resolution, we are actually pointed to the
+				// Foo abstract class instead of the IFoo interface.
+				if (igen is null || igen.IsConstSugar (opt) || igen.RawVisibility != "public")
 					continue;
 
 				Implements.Add (opt.GetOutputName (isym.FullName));