Refactor Java.Interop.Tools.JavaCallableWrappers

[jpobst]

Context: #540

Refactor Java.Interop.Tools.JavaCallableWrappers to be more maintainable and modular. Adopts the same architecture as generator: using a CecilImporter to import data from a compiled assembly into an intermediate model. Java code is then generated from the intermediate model.

Refactoring and updating this code should make it not only more maintainable, but also easier to extend with potential upcoming features we are considering:

• Moving jcw-gen into the ILLink pipeline
• Java Callable Wrappers should be emitted & built as part of Build android#4278
• Possibly creating a Kotlin backend

XA test PR which requires a few XA side updates: dotnet/android#8701

[jonpryor]

An alternate architectural "solution" which needs consideration is to turn Java Callable Wrapper generation into an illink step part of the illink pipeline:

https://github.com/xamarin/xamarin-android/blob/4eb515499d623fd26b636d95db95f241c696f2f4/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.ILLink.targets#L49-L96

We believe (hope?) that this should be possible, could be an "interesting"/"useful" "forcing function" to help split up <GenerateJavaStubs/>, and would mean that we wouldn't need to have changes such as this current PR. (Though the "incremental build" aspect is unknown, and Debug config builds don't currently have an illink pipeline at all, and we don't know what the perf impacts would be if we introduced such a thing, and…)

[jpobst]

Moving the concern of figuring out what to generate from the [Register] attributes will become an illink step in either architectural solution.

The difference would be the concern of "writing out the Java source code". This PR proposes that should be a separate task that runs after the linker. But it could also be done by the linker.

If feels like something the linker shouldn't be doing, but that ship may have already sailed, as we do other "non-linker" tasks in the linker. 😉

[jonpryor]

What I don't immediately understand is the criteria used to determine which "options" go into CallableWrapperWriterOptions and which "options" become properties on the given type.

Consider https://github.com/xamarin/xamarin-android/pull/8701/files, which provides "context" for how these changes would be used within xamarin-android.

Every property set on jcw_type is the same for every instance of jcw_type.

Wouldn't it make more sense for the "invariant" properties to instead be on CallableWrapperWriterOptions, and then "hoist" the var options = new CallableWrapperWriterOptions before the foreach loop? This would make explicit the values which don't depend upon type being emitted, as they would be the same for all types, e.g.

var options = new CallableWrapperWriterOptions {
    ApplicationJavaClass = ApplicationJavaClass,
    CodeGenerationTarget = JavaPeerStyle.XAJavaInterop1,
    GenerateOnCreateOverrides = generateOnCreateOverrides,
    MonoRuntimeInitialization = monoInit,
};
foreach (JavaType jt in newJavaTypes) {
    …
    var jcw_type = CecilImporter.CreateType (t, cache, methodClassifier: classifier);
    jcw_type.Generate (writer, options);
}

As a quick spot-check, CallableWrapperType.cs use ApplicationJavaClass, GenerateOnCreateOverrides, and MonoRuntimeInitialization from methods which already have a CallableWrapperWriterOptions options parameter, so such a change wouldn't be significant.

So why not have these "invariant" properties on CallableWrapperWriterOptions?

[jonpryor]

Counter-argument: CallableWrapperWriterOptions contains options that apply to Types, Fields, Methods, etc., and e.g. ApplicationJavaClass isn't used by CallableWrapperField.

Counter-counter-argument: CecilImporter.CreateType() is the entrypoint. If we ever needed to introduce a global MSBuild option specific to field generation, having a caller iterate over all fields within a CallableWrapperField to set a field-specific value would be incredibly annoying. It would be much easier to add such options to CallableWrapperWriterOptions directly. (Or pass it into CecilImporter.CreateType()? At which point we might instead want a CallableWrapperImportOptions?)

[jpobst]

Created CallableWrapperReaderOptions, as it seemed like the cleanest way to implement this.

[jonpryor]

Instead of these two fields, I think I'd prefer to have a public CallableWrapperType? DeclaringType {get;set;} property, then read CannotRegisterInStaticConstructor from e.g. DeclaringType.CannotRegisterInStaticConstructor. This would help ensure consistency, as these values cannot differ from the declaring type.

[jpobst]

Fixed