[TrimmableTypeMap] Detect overrides and constructors in JavaPeerScanner

[simonrozsival]

Part of #10789

Context

The JavaPeerScanner was missing two categories of methods that the legacy CecilImporter finds:

1. Method overrides without [Register] — User types (e.g., MainActivity) override Activity.OnCreate() without [Register] on the override. The attribute is only on the base class method in Mono.Android.

2. Java constructors via base ctor chaining — User types have non-activation constructors without [Register]. The legacy CecilImporter walks the base type hierarchy, seeds registered ctors, and accepts derived ctors with compatible parameters.

Missing constructors meant no JCW Java constructors, no UCO constructor wrappers, and no nctor_N native declarations.

Changes

Scanner (JavaPeerScanner.cs)

Pass 3 — Override detection: For each virtual override without [Register], walks the base hierarchy to find the registered base method and copies its registration info. Also handles property overrides (e.g., Throwable.Message). FindBaseRegisteredMethodInfo and FindBaseRegisteredProperty walk all base types with no DoNotGenerateAcw boundary — matching legacy GetBaseDefinition/GetBaseRegisteredMethod behavior. (Only constructor chaining has the DoNotGenerateAcw boundary.)

Pass 4 — Constructor chain detection: Mirrors legacy CecilImporter behavior:

• Seeds all base registered ctors directly into the type's marshal methods (e.g., Implementor types get the base ()V ctor from Object)
• Matches derived ctors with compatible parameters against base registered ctors
• Fallback: if a base has a registered ()V ctor, accepts derived ctors with novel params (generates super() call via SuperArgumentsString=""). Rejects ctors with non-Java parameter types (System.Object, System.IntPtr, System.Action, etc.) matching legacy GetJniSignature null behavior.
• Handles [JniConstructorSignature] (Java.Interop-style) in addition to [Register] in TryGetMethodRegisterInfo
• Stops base walk at DoNotGenerateAcw boundary (matching legacy)
• Only runs for ACW types (gated behind detectBaseOverrides), matching legacy which only runs CecilImporter.CreateType for non-interface, non-DoNotGenerateAcw types

Helper methods:

• TryResolveBaseType — encapsulates the 3-step base type resolution pattern (GetBaseTypeInfo → TryResolveType → GetTypeDefinition)
• CollectBaseRegisteredCtors — walks base hierarchy collecting registered ctors
• ManagedTypeToJniDescriptorOrNull — nullable variant of ManagedTypeToJniDescriptor for fallback signature computation; returns null for types without a proper JNI mapping
• BuildJniCtorSignature — computes JNI ctor signature from managed params, returns null when params aren't Java-mappable

Integration tests

ScannerRunner.cs — Rewrote ExtractMethodRegistrations to use the real legacy JCW pipeline (CecilImporter.CreateType) instead of manual [Register] attribute scanning. This catches overrides and ctor-chained methods that the manual approach missed. Key changes:

• Uses CecilImporter.CreateType for non-interface, non-DoNotGenerateAcw types
• Falls back to direct attribute extraction for interfaces and MCW types
• Filters interface method implementations from CecilImporter output (new scanner places these on the interface peer type)
• Parses connectors from CallableWrapperMethod.Method string format
• Excludes constructors from marshal method comparison (constructors are compared separately in ExactJavaConstructors)
• Made HasDoNotGenerateAcw internal for reuse by TypeDataBuilder

TypeDataBuilder.cs — Replaced manual [Register] attribute scanning for constructors with CecilImporter.CreateType to get the accurate legacy constructor list including the full base ctor chain. Uses ExtractDirectRegisterCtors for interfaces and DoNotGenerateAcw types.

Test fixtures

• Added [Register(".ctor", "()V", "")] to Activity fixture (matches real Mono.Android auto-generated bindings)
• Added JniConstructorSignatureAttribute stub in StubAttributes.cs
• Added override/ctor detection types: UserActivity, FullActivity, DeeplyDerived, MixedMethods, NewSlotActivity, CustomException, JiStyleView, ActivityWithCustomCtor, BaseFragment, DerivedFragment, GrandchildFragment, DialogBase, CustomDialog, ActivityWithMultiParamCtor

Unit tests

• OverrideDetectionTests (10 tests) — overrides without [Register], deep inheritance (3 levels), ACW→ACW→MCW hierarchy, property overrides, mixed methods, new-slot exclusion, DeclaringTypeName verification
• ConstructorDetectionTests (9 tests) — base ctor chain, implicit default ctor, base ctor seed, JniConstructorSignature, parameterless fallback with super(), same-arity type mismatch, multi-parameter signature computation, direct [Register] regression guard

Related issue

Filed #10931 — JcwJavaSourceGenerator must skip registerNatives in the static block for Application/Instrumentation types (the static initializer runs before the runtime ContentProvider loads the native library).

Test results

• 247 unit tests pass
• 10 integration tests pass (including ExactJavaConstructors_MonoAndroid and ExactMarshalMethods_MonoAndroid which validate against real Mono.Android.dll with ~8000 types)

Legacy comparison notes

Deep comparison with the legacy CecilImporter / GetBaseRegisteredMethod / CallableWrapperType pipeline:

Scanner behavior (matches legacy)

• Override detection matches legacy GetBaseDefinition → GetBaseRegisteredMethod: walks all base types (no DoNotGenerateAcw boundary), checks Virtual || Abstract, matches by name + parameter compatibility. Only run for user ACW types (gated by detectBaseOverrides), matching how JavaTypeScanner.ShouldSkipJavaCallableWrapperGeneration filters DoNotGenerateAcw types.
• Constructor chaining matches legacy ctorTypes loop (CecilImporter.cs:124-160): stops at DoNotGenerateAcw, reverses to process base-first, seeds registered base ctors, checks AreParametersCompatibleWith, falls back to parameterless super() when baseCtors.Any(m => !m.HasParameters).
• Parameter compatibility uses string comparison of decoded S.R.M. signatures instead of Cecil's richer IsParameterCompatibleWith (which handles generic constraints, modifiers, TypeSpecification). This is equivalent for the non-generic, non-modifier cases that Android APIs use — validated against ~8000 real Mono.Android types with zero divergence.

JCW Java source (intentional differences)

• Constructor body: Legacy calls mono.android.TypeManager.Activate(...), new calls nctor_N(...) (UCO dispatches to TrimmableNativeRegistration.ActivateInstance). Intentional architectural change.
• Static initializer: Legacy builds __md_methods string and calls Runtime.register(...), new calls Runtime.registerNatives(ClassName.class). Intentional architectural change.
• Native method visibility: Legacy emits private native n_*, new emits public native n_* (needed for UCO registerNatives dispatch).
• @Override: Legacy only emits annotations from managed custom attributes. New hard-codes @Override on registered method overrides (more correct for Java compiler).
• Application/Instrumentation: Legacy defers register() to generated onCreate() and emits ApplicationConstructor with MonoPackageManager.setContext(). New doesn't handle this yet — tracked in [TrimmableTypeMap] JcwJavaSourceGenerator must skip registerNatives for Application/Instrumentation types #10931.
• Nested types: Legacy recursively emits nested JCW types with separate __md_N_methods. Not yet implemented in the new generator.
• Type/method annotations: Legacy converts managed custom attributes to Java annotations (@Activity(label=...) etc.). New generator doesn't emit these — they're handled by the AndroidManifest pipeline, not the JCW source files.

[Copilot]

Pull request overview

Updates the TrimmableTypeMap Java peer scanning pipeline to match legacy JCW behavior for (1) overrides without [Register] and (2) constructor discovery via base-ctor chaining, and expands the test suite to validate parity against the real legacy Cecil-based pipeline.

Changes:

• Extend JavaPeerScanner to detect base-registered method/property overrides and derive Java constructors from base registered constructors (including parameterless fallback).
• Rework integration-test legacy extraction to use CecilImporter.CreateType (instead of manual [Register] scanning) for accurate baseline comparison.
• Add new fixtures and unit tests covering override and constructor-chain scenarios (including JniConstructorSignatureAttribute).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs	Adds override detection and constructor chain detection passes; recognizes JniConstructorSignatureAttribute.
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/TestFixtures/TestTypes.cs	Adds new fixture types for override + ctor-chain scenarios; adjusts Activity ctor registration.
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/TestFixtures/StubAttributes.cs	Adds stub JniConstructorSignatureAttribute for unit tests.
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/OverrideDetectionTests.cs	New unit tests validating override detection behavior.
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/ConstructorDetectionTests.cs	New unit tests validating constructor chain detection behavior.
tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/ScannerRunner.cs	Legacy method extraction now uses CecilImporter.CreateType, with interface/MCW fallbacks and interface-impl filtering.
tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/TypeDataBuilder.cs	Legacy ctor extraction now uses CecilImporter.CreateType, with fallback to direct attribute scanning.

[simonrozsival]

🤖 AI Review Summary

Verdict: ✅ LGTM

Found 0 issues in the updated diff.

The previous round of review feedback has been fully addressed:

• Signature! null-forgiving operators replaced with proper null guards
• FindBaseRegisteredMethodInfo and FindBaseRegisteredProperty now stop at DoNotGenerateAcw boundary
• Property override key format made consistent with Pass 1/2
• default! replaced with default in test fixtures
• Bare catch in TypeDataBuilder removed
• Duplicate if (detectBaseOverrides) blocks merged
• Override detection tests consolidated (14→7 Facts, all assertions preserved)

👍 The override detection and constructor chaining logic is well-structured, thoroughly documented, and correctly mirrors legacy CecilImporter behavior. The 4-pass architecture (direct [Register], property [Register], base overrides, base ctor chain) has clear separation of concerns. Test coverage is comprehensive across edge cases (deep inheritance, new-slot exclusion, property overrides, JI-style attributes, parameterless fallback).

⚠️ CI is still pending (build 1334063) — cannot confirm merge-readiness until checks pass.

---

Review generated by android-reviewer from review guidelines.

[jonathanpeppers]

Does this code have some kind of logging infrastructure?

We would need MSBuild tasks to be able to pass in a delegate for logging, similar to:

android/build-tools/Xamarin.Android.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/JdkInfo.cs

Lines 89 to 105 in ff39607

	internal static Action<TraceLevel, string> CreateTaskLogger (Task task)
	{
	Action<TraceLevel, string> logger = (level, value) => {
	switch (level) {
	case TraceLevel.Error:
	task.Log.LogError (value);
	break;
	case TraceLevel.Warning:
	task.Log.LogWarning (value);
	break;
	default:
	task.Log.LogMessage (MessageImportance.Low, "{0}", value);
	break;
	}
	};
	return logger;
	}

We can file an issue to do this in the future.

[simonrozsival]

I think this log is probably unnecessary going forward. I will remove it.