Add unit tests for covariant return type override detection

When a derived type overrides a base method with a narrower return type,
the JCW must use the base method's JNI signature:

    // Base: [Register ("getResult", "()Ljava/lang/Object;", "...")]
    public virtual Java.Lang.Object GetResult () => null;

    // User override — narrower return, no [Register]:
    public override Java.Lang.Object GetResult () => myString;

    // JCW must use base's "()Ljava/lang/Object;", not derived's type.

Also addresses review feedback: consolidated tests, JNI-based
ExportField type resolution, consistent signature-based dedup keys.

Part of #10933

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: simonrozsival

src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs

@@ -207,7 +207,7 @@ void ScanAssembly (AssemblyIndex index, Dictionary<string, JavaPeerInfo> results
 			// Override and interface detection is only for user ACW class types:
 			// - MCW types (DoNotGenerateAcw) already have [Register] on every method
 			// - Interface types don't implement other interfaces' methods in JCWs
-			var marshalMethods = CollectMarshalMethods (typeDef, index, detectBaseOverrides: !doNotGenerateAcw && !isInterface);
+			var (marshalMethods, exportFields) = CollectMarshalMethods (typeDef, index, detectBaseOverrides: !doNotGenerateAcw && !isInterface);
 
 			// Resolve activation constructor
 			var activationCtor = ResolveActivationCtor (fullName, typeDef, index);
@@ -232,7 +232,7 @@ void ScanAssembly (AssemblyIndex index, Dictionary<string, JavaPeerInfo> results
 				IsUnconditional = isUnconditional,
 				MarshalMethods = marshalMethods,
 				JavaConstructors = BuildJavaConstructors (marshalMethods),
-				JavaFields = CollectExportFields (typeDef, index),
+				JavaFields = exportFields,
 				ActivationCtor = activationCtor,
 				InvokerTypeName = invokerTypeName,
 				IsGenericDefinition = isGenericDefinition,
@@ -242,14 +242,19 @@ void ScanAssembly (AssemblyIndex index, Dictionary<string, JavaPeerInfo> results
 		}
 	}
 
-	List<MarshalMethodInfo> CollectMarshalMethods (TypeDefinition typeDef, AssemblyIndex index, bool detectBaseOverrides)
+	(List<MarshalMethodInfo>, List<JavaFieldInfo>) CollectMarshalMethods (TypeDefinition typeDef, AssemblyIndex index, bool detectBaseOverrides)
 	{
 		var methods = new List<MarshalMethodInfo> ();
+		var fields = new List<JavaFieldInfo> ();
 		var registeredMethodKeys = new HashSet<string> (StringComparer.Ordinal);
 
-		// Pass 1: collect methods with [Register] or [Export] directly on them
+		// Pass 1: collect methods with [Register], [Export], or [ExportField] directly on them
 		foreach (var methodHandle in typeDef.GetMethods ()) {
 			var methodDef = index.Reader.GetMethodDefinition (methodHandle);
+
+			// Check for [ExportField] — produces both a marshal method AND a field
+			CollectExportField (methodDef, index, fields);
+
 			if (!TryGetMethodRegisterInfo (methodDef, index, out var registerInfo, out var exportInfo) || registerInfo is null) {
 				continue;
 			}
@@ -300,7 +305,7 @@ List<MarshalMethodInfo> CollectMarshalMethods (TypeDefinition typeDef, AssemblyI
 			CollectBaseConstructorChain (typeDef, index, methods);
 		}
 
-		return methods;
+		return (methods, fields);
 	}
 
 	/// <summary>
@@ -380,14 +385,16 @@ void CollectBasePropertyOverrides (TypeDefinition typeDef, AssemblyIndex index,
 			}
 
 			var getterName = index.Reader.GetString (getterDef.Name);
-			if (alreadyRegistered.Contains (getterName)) {
+			var sig = getterDef.DecodeSignature (SignatureTypeProvider.Instance, genericContext: default);
+			var sigKey = $"{getterName}({string.Join (",", sig.ParameterTypes)})";
+			if (alreadyRegistered.Contains (sigKey)) {
 				continue;
 			}
 
 			var baseRegistration = FindBaseRegisteredProperty (typeDef, index, getterName, getterDef);
 			if (baseRegistration is not null) {
 				methods.Add (baseRegistration);
-				alreadyRegistered.Add (getterName);
+				alreadyRegistered.Add (sigKey);
 			}
 		}
 	}
@@ -1413,75 +1420,44 @@ static List<JavaConstructorInfo> BuildJavaConstructors (List<MarshalMethodInfo>
 	}
 
 	/// <summary>
-	/// Collects Java field declarations from [ExportField] attributes on methods.
-	/// Each [ExportField("name")] on a method produces a Java field initialized by
-	/// calling that method.
+	/// Checks a single method for [ExportField] and adds a JavaFieldInfo if found.
+	/// Called inline during Pass 1 to avoid a separate iteration.
 	/// </summary>
-	static List<JavaFieldInfo> CollectExportFields (TypeDefinition typeDef, AssemblyIndex index)
+	static void CollectExportField (MethodDefinition methodDef, AssemblyIndex index, List<JavaFieldInfo> fields)
 	{
-		var fields = new List<JavaFieldInfo> ();
-
-		foreach (var methodHandle in typeDef.GetMethods ()) {
-			var methodDef = index.Reader.GetMethodDefinition (methodHandle);
-
-			foreach (var caHandle in methodDef.GetCustomAttributes ()) {
-				var ca = index.Reader.GetCustomAttribute (caHandle);
-				var attrName = AssemblyIndex.GetCustomAttributeName (ca, index.Reader);
-
-				if (attrName != "ExportFieldAttribute") {
-					continue;
-				}
-
-				var value = index.DecodeAttribute (ca);
-				if (value.FixedArguments.Length == 0) {
-					continue;
-				}
+		foreach (var caHandle in methodDef.GetCustomAttributes ()) {
+			var ca = index.Reader.GetCustomAttribute (caHandle);
+			var attrName = AssemblyIndex.GetCustomAttributeName (ca, index.Reader);
 
-				var fieldName = (string?)value.FixedArguments [0].Value;
-				if (fieldName is null) {
-					continue;
-				}
+			if (attrName != "ExportFieldAttribute") {
+				continue;
+			}
 
-				var managedName = index.Reader.GetString (methodDef.Name);
-				var sig = methodDef.DecodeSignature (SignatureTypeProvider.Instance, genericContext: default);
-				var javaReturnType = ManagedReturnTypeToJava (sig.ReturnType);
-				var access = GetJavaAccess (methodDef.Attributes & MethodAttributes.MemberAccessMask);
-				var isStatic = (methodDef.Attributes & MethodAttributes.Static) != 0;
-
-				fields.Add (new JavaFieldInfo {
-					FieldName = fieldName,
-					JavaTypeName = javaReturnType,
-					InitializerMethodName = managedName,
-					Visibility = access,
-					IsStatic = isStatic,
-				});
+			var value = index.DecodeAttribute (ca);
+			if (value.FixedArguments.Length == 0) {
+				continue;
 			}
-		}
 
-		return fields;
-	}
+			var fieldName = (string?)value.FixedArguments [0].Value;
+			if (fieldName is null) {
+				continue;
+			}
 
-	static string ManagedReturnTypeToJava (string managedType)
-	{
-		switch (managedType) {
-		case "System.String": return "java.lang.String";
-		case "System.Boolean": return "boolean";
-		case "System.Byte":
-		case "System.SByte": return "byte";
-		case "System.Char": return "char";
-		case "System.Int16":
-		case "System.UInt16": return "short";
-		case "System.Int32":
-		case "System.UInt32": return "int";
-		case "System.Int64":
-		case "System.UInt64": return "long";
-		case "System.Single": return "float";
-		case "System.Double": return "double";
-		case "System.Void": return "void";
-		default:
-			// For reference types, use java.lang.Object as fallback.
-			// The exact Java type would require JNI signature resolution.
-			return "java.lang.Object";
+			var managedName = index.Reader.GetString (methodDef.Name);
+			var sig = methodDef.DecodeSignature (SignatureTypeProvider.Instance, genericContext: default);
+			var jniSig = BuildJniSignatureFromManaged (sig);
+			var jniReturnType = JniSignatureHelper.ParseReturnTypeString (jniSig);
+			var javaReturnType = JniSignatureHelper.JniTypeToJava (jniReturnType);
+			var access = GetJavaAccess (methodDef.Attributes & MethodAttributes.MemberAccessMask);
+			var isStatic = (methodDef.Attributes & MethodAttributes.Static) != 0;
+
+			fields.Add (new JavaFieldInfo {
+				FieldName = fieldName,
+				JavaTypeName = javaReturnType,
+				InitializerMethodName = managedName,
+				Visibility = access,
+				IsStatic = isStatic,
+			});
 		}
 	}
 }

tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/TypeDataBuilder.cs

@@ -110,9 +110,8 @@ public static (Dictionary<string, TypeComparisonData> perType, List<TypeMapEntry
 							javaCtorSignatures.Add (ctor.JniSignature);
 						}
 					}
-				} catch {
-					// Some types may fail CecilImporter (e.g., missing base types).
-					// Fall back to direct attribute scanning.
+				} catch (Exception ex) {
+					System.Diagnostics.Debug.WriteLine ($"CecilImporter.CreateType failed for {typeDef.FullName}: {ex.Message}");
 					ExtractDirectRegisterCtors (typeDef, javaCtorSignatures);
 				}
 			} else {

tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/ConstructorSuperArgsTests.cs

@@ -1,3 +1,4 @@
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using Xunit;
@@ -6,58 +7,35 @@ namespace Microsoft.Android.Sdk.TrimmableTypeMap.Tests;
 
 /// <summary>
 /// Tests for constructor super() argument matching.
-/// The legacy pipeline selects super() arguments based on base registered ctors:
-/// - Compatible params → forward all (super(p0, p1, ...))
-/// - No compatible params, parameterless base exists → super()
-/// - [Export(SuperArgumentsString="...")] → use custom string
 /// </summary>
 public class ConstructorSuperArgsTests : FixtureTestBase
 {
-	static string GenerateToString (JavaPeerInfo type)
-	{
-		var generator = new JcwJavaSourceGenerator ();
-		using var writer = new StringWriter ();
-		generator.Generate (type, writer);
-		return writer.ToString ();
-	}
-
 	[Fact]
 	public void MatchingBaseCtor_ForwardsAllParams ()
 	{
-		// UserActivity extends Activity which has [Register(".ctor","()V")]
-		// UserActivity has activation ctor (IntPtr, JniHandleOwnership) — not a user ctor
-		// The base ()V ctor should be inherited as a seed constructor
 		var peer = FindFixtureByJavaName ("my/app/UserActivity");
 		Assert.NotEmpty (peer.JavaConstructors);
 		var ctor = peer.JavaConstructors.First (c => c.JniSignature == "()V");
-		// null SuperArgumentsString means "forward all params"
 		Assert.Null (ctor.SuperArgumentsString);
 	}
 
 	[Fact]
 	public void CustomParamCtor_FallsBackToEmptySuper ()
 	{
-		// CustomParamActivity has ctor(string, int) which doesn't match Activity's ()V
-		// But Activity has parameterless ctor → fallback to super()
 		var peer = FindFixtureByJavaName ("my/app/CustomParamActivity");
 		var customCtor = peer.JavaConstructors.FirstOrDefault (c => c.JniSignature != "()V");
 		Assert.NotNull (customCtor);
 		Assert.Equal ("", customCtor.SuperArgumentsString);
-	}
 
-	[Fact]
-	public void CustomParamCtor_JcwEmitsEmptySuper ()
-	{
-		var peer = FindFixtureByJavaName ("my/app/CustomParamActivity");
-		var java = GenerateToString (peer);
-		// The custom-param ctor should call super() not super(p0, p1)
-		Assert.Contains ("super ();", java);
+		var generator = new JcwJavaSourceGenerator ();
+		using var writer = new StringWriter ();
+		generator.Generate (peer, writer);
+		Assert.Contains ("super ();", writer.ToString ());
 	}
 
 	[Fact]
 	public void ExportSuperArgs_UsesCustomString ()
 	{
-		// From existing test fixtures — uses SuperArgumentsString
 		var type = new JavaPeerInfo {
 			JavaName = "my/app/ExportCtorTest",
 			CompatJniName = "my/app/ExportCtorTest",
@@ -66,7 +44,7 @@ public void ExportSuperArgs_UsesCustomString ()
 			ManagedTypeShortName = "ExportCtorTest",
 			AssemblyName = "App",
 			BaseJavaName = "android/app/Service",
-			JavaConstructors = new System.Collections.Generic.List<JavaConstructorInfo> {
+			JavaConstructors = new List<JavaConstructorInfo> {
 				new JavaConstructorInfo {
 					JniSignature = "(Landroid/content/Context;I)V",
 					ConstructorIndex = 0,
@@ -75,7 +53,10 @@ public void ExportSuperArgs_UsesCustomString ()
 			},
 		};
 
-		var java = GenerateToString (type);
+		var generator = new JcwJavaSourceGenerator ();
+		using var writer = new StringWriter ();
+		generator.Generate (type, writer);
+		var java = writer.ToString ();
 		Assert.Contains ("super (p0);", java);
 		Assert.DoesNotContain ("super (p0, p1);", java);
 	}

tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/ExportAccessModifierTests.cs

@@ -9,34 +9,20 @@ namespace Microsoft.Android.Sdk.TrimmableTypeMap.Tests;
 /// </summary>
 public class ExportAccessModifierTests : FixtureTestBase
 {
-	static string GenerateToString (JavaPeerInfo type)
-	{
-		var generator = new JcwJavaSourceGenerator ();
-		using var writer = new StringWriter ();
-		generator.Generate (type, writer);
-		return writer.ToString ();
-	}
-
-	[Fact]
-	public void Scanner_ExportMethod_HasIsExportTrue ()
-	{
-		var peer = FindFixtureByJavaName ("my/app/ExportAccessTest");
-		var publicMethod = peer.MarshalMethods.First (m => m.JniName == "publicMethod");
-		Assert.True (publicMethod.IsExport);
-	}
-
 	[Fact]
-	public void Scanner_ExportMethod_HasCorrectJavaAccess ()
+	public void Scanner_ExportMethods_HaveCorrectIsExportAndJavaAccess ()
 	{
 		var peer = FindFixtureByJavaName ("my/app/ExportAccessTest");
 		var publicMethod = peer.MarshalMethods.First (m => m.JniName == "publicMethod");
 		var protectedMethod = peer.MarshalMethods.First (m => m.JniName == "protectedMethod");
+		Assert.True (publicMethod.IsExport);
 		Assert.Equal ("public", publicMethod.JavaAccess);
+		Assert.True (protectedMethod.IsExport);
 		Assert.Equal ("protected", protectedMethod.JavaAccess);
 	}
 
 	[Fact]
-	public void Scanner_RegisterMethod_IsExportFalse ()
+	public void Scanner_RegisterMethod_IsNotExport ()
 	{
 		var peer = FindFixtureByJavaName ("my/app/MixedMethods");
 		var customMethod = peer.MarshalMethods.First (m => m.JniName == "customMethod");
@@ -45,18 +31,14 @@ public void Scanner_RegisterMethod_IsExportFalse ()
 	}
 
 	[Fact]
-	public void JcwGenerator_ProtectedExport_UsesProtectedAccess ()
+	public void JcwGenerator_ExportMethods_UseCorrectAccessModifiers ()
 	{
 		var peer = FindFixtureByJavaName ("my/app/ExportAccessTest");
-		var java = GenerateToString (peer);
+		var generator = new JcwJavaSourceGenerator ();
+		using var writer = new StringWriter ();
+		generator.Generate (peer, writer);
+		var java = writer.ToString ();
 		Assert.Contains ("protected void protectedMethod ()", java);
-	}
-
-	[Fact]
-	public void JcwGenerator_PublicExport_UsesPublicAccess ()
-	{
-		var peer = FindFixtureByJavaName ("my/app/ExportAccessTest");
-		var java = GenerateToString (peer);
 		Assert.Contains ("public void publicMethod ()", java);
 	}
 }