[generator][api-xml-adjuster] Fix managed type resolution in API adjuster. (#86)

Until now, ApiXmlAdjuster was trying to resolve types using
ClassGen.BaseGen property which is in fact assigned only after Validate()
step in generator. API XML analyzer work is done way before that step,
and therefore it failed to resolve types from managed code (DLL references).

Missing type hierarchy in Java type model from managed code means that
any bindings that references Mono.Android (which wouldn't?) had to rely
on insufficient hierarchy. Especially, it failed to resolve method
overrides when the target Java library types overrode methods in android.jar
indirectly.

SherlockExpandableListActivity.addContentView() was such a method.

And it had appeared as a regression from our MSBuild tests[*1].

Fixes:

- removed dependency on BaseGen in ApiXmlAdjuster. Use BaseType instead,
  which doesn't depend on validation step.
  - Though this change uncovered an issue that Ctor and Parameter needed
    to hold Java type information. So, made some changes to them.
- Now that ApiXmlAdjuster is getting valid managed type hierarchy, it
  had uncovered another kind of issues: managed objects need to exist
  to not cause some null crashes. Hence there is ManagedType class now.
  - Type resolution now premises this ManagedType in type resolver code.
- Fixed incorrect Load() invocation for ClassGen and InterfaceGen (that
  had caused missing BaseType information).
- Added (almost irrelevant) ApiXmlAdjuster test to make sure override
  resolution is done for overrides from indirect inheritance.

[*1] that needs to be OSS-ed...

Author: atsushieno

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApi.XmlModel.cs

@@ -108,6 +108,37 @@ public override string ToString ()
 		}
 	}
 
+
+	class ManagedType : JavaType
+	{
+		static JavaPackage dummy_system_package, dummy_system_io_package;
+		static JavaType system_object, system_exception, system_io_stream;
+
+		static ManagedType ()
+		{
+			dummy_system_package = new JavaPackage (null) { Name = "System" };
+			system_object = new ManagedType (dummy_system_package) { Name = "Object" };
+			system_exception = new ManagedType (dummy_system_package) { Name = "Exception" };
+			dummy_system_package.Types.Add (system_object);
+			dummy_system_package.Types.Add (system_exception);
+			dummy_system_io_package = new JavaPackage (null) { Name = "System.IO" };
+			system_io_stream = new ManagedType (dummy_system_package) { Name = "Stream" };
+			dummy_system_io_package.Types.Add (system_io_stream);
+		}
+
+		public static IEnumerable<JavaPackage> DummyManagedPackages {
+			get {
+				yield return dummy_system_package; 
+				yield return dummy_system_io_package;
+			}
+		}
+
+		public ManagedType (JavaPackage package) : base (package) 
+		{
+		}
+	}
+
+
 	public partial class JavaImplements
 	{
 		public string Name { get; set; }

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiOverrideMarkerExtensions.cs

@@ -36,7 +36,7 @@ static void MarkBaseMethod (this JavaClass cls, JavaMethod method)
 		{
 			JavaClass k = cls;
 			while (true) {
-				k = k.ResolvedExtends != null ? (JavaClass) k.ResolvedExtends.ReferencedType : null;
+				k = k.ResolvedExtends != null ? k.ResolvedExtends.ReferencedType as JavaClass : null;
 				if (k == null)
 					break;
 

src/Xamarin.Android.Tools.ApiXmlAdjuster/JavaApiTypeResolverExtensions.cs

@@ -38,9 +38,12 @@ public static JavaType FindNonGenericType (this JavaApi api, string name)
 			var ret = api.Packages
 				.SelectMany (p => p.Types)
 				.FirstOrDefault (t => name.StartsWith (t.Parent.Name, StringComparison.Ordinal) && name == t.Parent.Name + '.' + t.Name);
+			if (ret == null)
+				ret = ManagedType.DummyManagedPackages
+				                 .SelectMany (p => p.Types)
+				                 .FirstOrDefault (t => t.FullName == name);
 			if (ret == null)
 				throw new JavaTypeResolutionException (string.Format ("Type '{0}' was not found.", name));
-			
 			return ret;
 		}
 

src/Xamarin.Android.Tools.ApiXmlAdjuster/Tests/OverrideMarkerTest.cs

@@ -1,5 +1,7 @@
 using System;
+using System.IO;
 using System.Linq;
+using System.Xml;
 using NUnit.Framework;
 
 namespace Xamarin.Android.Tools.ApiXmlAdjuster.Tests
@@ -29,6 +31,32 @@ public void InstantiatedGenericArgumentName ()
 			Assert.AreEqual (method.Parameters.First (), method.Parameters.Last (), "There should be only one parameter.");
 			Assert.AreEqual ("T", para.InstantiatedGenericArgumentName, "InstantiatedGenericArgumentName mismatch");
 		}
+
+		[Test]
+		public void AncestralOverrides ()
+		{
+			string xml = @"<api>
+  <package name='XXX'>
+    <class abstract='true' deprecated='not deprecated' extends='android.app.ExpandableListActivity' extends-generic-aware='android.app.ExpandableListActivity' final='false' name='SherlockExpandableListActivity' static='false' visibility='public'>
+      <method abstract='false' deprecated='not deprecated' final='false' name='addContentView' native='false' return='void' static='false' synchronized='false' visibility='public'>
+        <parameter name = 'view' type='android.view.View'>
+        </parameter>
+        <parameter name = 'params' type='android.view.ViewGroup.LayoutParams'>
+        </parameter>
+      </method>
+    </class>
+  </package>
+</api>";
+			var xapi = JavaApiTestHelper.GetLoadedApi ();
+			using (var xr = XmlReader.Create (new StringReader (xml)))
+				xapi.Load (xr, false);
+			xapi.Resolve ();
+			xapi.CreateGenericInheritanceMapping ();
+			xapi.MarkOverrides ();
+			var t = xapi.Packages.First (_ => _.Name == "XXX").Types.First (_ => _.Name == "SherlockExpandableListActivity");
+			var m = t.Members.OfType<JavaMethod> ().First (_ => _.Name == "addContentView");
+			Assert.IsNotNull (m.BaseMethod, "base method not found");
+		}
 	}
 }
 

tools/generator/Ctor.cs

@@ -27,8 +27,7 @@ public ManagedCtor (GenBase declaringType, MethodDefinition m)
 			// the type of the containing class must be inserted as the first
 			// argument
 			if (IsNonStaticNestedType)
-				Parameters.AddFirst (Parameter.FromManagedType (m.DeclaringType.DeclaringType));
-
+				Parameters.AddFirst (Parameter.FromManagedType (m.DeclaringType.DeclaringType, DeclaringType.JavaName));
 			var regatt = m.CustomAttributes.FirstOrDefault (a => a.AttributeType.FullName == "Android.Runtime.RegisterAttribute");
 			is_acw = regatt != null;
 			foreach (var p in support.GetParameters (regatt))

tools/generator/Field.cs

@@ -76,7 +76,7 @@ public override string Visibility {
 
 		protected override Parameter SetterParameter {
 			get {
-				var p = Parameter.FromManagedType (f.FieldType.Resolve ());
+				var p = Parameter.FromManagedType (f.FieldType.Resolve (), null);
 				p.Name = "value";
 				return p;
 			}

tools/generator/JavaApiDllLoaderExtensions.cs

@@ -19,11 +19,11 @@ public static void LoadReferences (this JavaApi api, GenBase [] gens)
 				if (gen is InterfaceGen) {
 					var iface = new JavaInterface (pkg);
 					pkg.Types.Add (iface);
-					iface.Load (gen);
+					iface.Load ((InterfaceGen) gen);
 				} else if (gen is ClassGen) {
 					var kls = new JavaClass (pkg);
 					pkg.Types.Add (kls);
-					kls.Load (gen);
+					kls.Load ((ClassGen) gen);
 				}
 				else
 					throw new InvalidOperationException ();
@@ -75,11 +75,11 @@ static void Load (this JavaInterface iface, InterfaceGen gen)
 			((JavaType) iface).Load (gen);
 		}
 
-		static string ExpandTypeParameters (GenericParameterDefinitionList tps)
+		static string ExpandTypeParameters (ISymbol [] tps)
 		{
 			if (tps == null)
 				return null;
-			return '<' + string.Join (", ", tps.Select (_ => _.Name)) + '>';
+			return '<' + string.Join (", ", tps.Select (_ => _.JavaName)) + '>';
 		}
 
 		static void Load (this JavaClass kls, ClassGen gen)
@@ -88,10 +88,13 @@ static void Load (this JavaClass kls, ClassGen gen)
 
 			kls.Abstract = gen.IsAbstract;
 			kls.Final = gen.IsFinal;
-			if (gen.BaseGen != null) {
-				kls.Extends = gen.BaseGen.JavaSimpleName;
-				kls.ExtendsGeneric = gen.BaseGen.JavaSimpleName + ExpandTypeParameters (gen.BaseGen.TypeParameters);
-				kls.ExtendedJniExtends = gen.BaseGen.JniName;
+			var baseGen = gen.BaseType != null ? SymbolTable.Lookup (gen.BaseType) : null;
+
+			if (baseGen != null) {
+				kls.Extends = baseGen.JavaName;
+				var gs = baseGen as GenericSymbol;
+				kls.ExtendsGeneric = gs != null ? gs.JavaName + ExpandTypeParameters (gs.TypeParams) : baseGen.JavaName;
+				kls.ExtendedJniExtends = baseGen.JniName;
 			}
 			foreach (var c in gen.Ctors) {
 				var ctor = new JavaConstructor (kls);

tools/generator/Parameter.cs

@@ -291,9 +291,9 @@ public static Parameter FromManagedParameter (ParameterDefinition p, string jnit
 			return new Parameter (SymbolTable.MangleName (p.Name), jnitype ?? p.ParameterType.FullNameCorrected (), null, isEnumType, rawtype);
 		}
 
-		public static Parameter FromManagedType (TypeDefinition t)
+		public static Parameter FromManagedType (TypeDefinition t, string javaType)
 		{
-			return new Parameter ("__self", t.FullName, null, false);
+			return new Parameter ("__self", javaType ?? t.FullName, t.FullName, false);
 		}
 #endif
 	}