[generator] Generator refactor

[jpobst]

XA companion PR: dotnet/android#4938

generator is an amazingly powerful tool that has proven to be very versatile over the past decade. However, many of the remaining bugs fall into a category that is hard to fix given generator's current design.

This design is:

• Read in Java model
• Make tweaks to Java model
• Open .cs
• Loop through Java model determining what C# to write

The issue with this is that we are deciding what to generate on-the-fly to a write-only stream. This means that once we've written to the stream we cannot change it if future information suggests we should.

A good example is issue #461.

The Java model is:

interface java.code.AnimatorListener {
  onAnimationEnd (int p0);
  onAnimationEnd (int p0, p1);
}

Looping through this, we see we need to generate an EventArgs class for onAnimationEnd (int p0), so we do:

public partial class OnAnimationEndEventArgs : global::System.EventArgs {
	int p0;

	public OnAnimationEndEventArgs (int p0) { this.p0= p0; }

	public int P0 { get { return p0; } }
}

Then we get to the second method onAnimationEnd (int p0, int p1) and see that we need to add additional parameters to OnAnimationEndEventArgs. However we cannot modify the already written class, so we generate a second OnAnimationEndEventArgs with different parameters, which causes compilation errors in C#.

Another example is we can generate an empty InterfaceConsts class. This is because we write the class opening because we think we have members to place in it (public static class InterfaceConsts {), but as we loop through the members we thought we were going to add they all get eliminated. At that point all we can do is close the class with no members.

Proposal

The solution to both examples above is "simple": we need to first loop through the rest of the Java model and determine what we actually need to generate before we start writing anything. We could then merge the two OnAnimationEndEventArgs classes, or forgo writing an empty InterfaceConsts.

However, rather than adding this additional logic in each instance that needs it, it feels like there is enough benefit to convert the entire generator architecture to work by first building a C# model of what needs to be generated that can be tweaked before writing to a file:

• Read in Java model
• Make tweaks to Java model
• -> Build C# model from Java model
• -> Make tweaks to C# model
• Open .cs
• Loop through C# model, writing code

Having a C# model to tweak should help in a few other tricky cases we fail at today, like determining overrides (#367, #586) and implementing abstract methods for abstract classes inheriting an interface (#470).

Additionally, having distinct logic and source writing steps should make unit testing better. Currently, the only way to test the logic of if we are going to generate something is to write out the source code and compare it to "expected" source code. This means tiny fixes can have many "expected" files that need to change (#625).

With separate steps, we can have a set of unit tests that test what code we are writing, and a set that tests the logic of determining what to generate. To test the above "2 EventArgs classes" example, we could test a fix by looking at the created C# model to ensure that only a single combined OnAnimationEndEventArgs class is created. We would not need to write out the generated source code and compare it to expected source code.

Other Refactors

Xamarin.SourceWriter

A library for writing C# code that handles syntax. This eliminates a lot of hard to read and error prone code like:

writer.WriteLine ("{0}{1}{2}{3}{4} unsafe {5} {6}{7} ({8})",
	indent,
	visibility,
	static_arg,
	virt_ov,
	seal,
	ret,
	is_explicit ? GetDeclaringTypeOfExplicitInterfaceMethod (method.OverriddenInterfaceMethod) + '.' : string.Empty,
	method.AdjustedName,
	method.GetSignature (opt));

Instead you do something like:

var m = new MethodWriter {
  IsPublic = true,
  IsUnsafe = true,
  IsOverride = true,
  Name = method.AdjustedName
};

m.Write (...);

which will write:

public unsafe override void DoSomething ()
{
}

CodeWriter

CodeWriter is a class around a Stream that tracks current indent level rather than needing to pass indent around to every method.

cw.WriteLine ("{");
cw.Indent ();
... write block body ...
cw.Unindent ();
cw.WriteLine ("}");

Testing

• Many existing unit tests call directly into methods like CodeGenerator.WriteProperties. These methods are no longer available when using JavaInterop/XAJavaInterop. These tests were either changed to use CodeGenerator.WriteType or were moved to only run for XamarinAndroid codegen target.
• Tests were updated to ignore whitespace and line endings differences, as the refactor did not preserve 100% identical whitespace.
• Additional comparisons were done against binding generated source codebases to ensure there were no differences between current and refactored generator. These codebases are:
  • Mono.Android
  • https://github.com/xamarin/XamarinComponents/tree/master/Android/ARCore
  • https://github.com/xamarin/XamarinComponents/tree/master/Android/ConstraintLayout
  • https://github.com/xamarin/XamarinComponents/tree/master/Android/JavaxInject
  • https://github.com/xamarin/XamarinComponents/tree/master/Android/ReactiveStreams
  • https://github.com/xamarin/XamarinComponents/tree/master/Android/RenderScriptV8
  • https://github.com/xamarin/XamarinComponents/tree/master/Android/GoogleZXing

[jpobst]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jonpryor]

`generator` is an amazingly powerful tool that has proven to be very
versatile over the past decade.  However, many of the remaining bugs
fall into a category that is hard to fix given `generator`'s current
design, which is:

 1. Read in Java model
 2. Make tweaks to Java model
 3. Open `<TYPE>.cs`
 4. Loop through Java model determining what C# to write

The issue with this is that we are deciding **what** to generate
on-the-fly to a write-only stream.  This means that once we've written
to the stream we cannot change it if future information suggests that
we should.

A good example is issue #461.

The Java model is: https://github.com/xamarin/Java.Interop/issues/461

Given:

	// Java:
	interface AnimatorListener {
	  onAnimationEnd (int p0);
	  onAnimationEnd (int p0, p1);
	}

Looping through this, we see we need to generate an `EventArgs` class for
`AnimatorListener.onAnimationEnd(int p0)`, so we do:

	public partial class OnAnimationEndEventArgs : global::System.EventArgs {
	    int p0;

	    public OnAnimationEndEventArgs (int p0) { this.p0= p0; }

	    public int P0 { get { return p0; } }
	}

Then we get to the second method
`AnimatorListener.onAnimationEnd(int p0, int p1)` and see that we need
to add additional parameters to `OnAnimationEndEventArgs`.  However we
cannot modify the already written class, so we generate a second
`OnAnimationEndEventArgs` with different parameters, which causes
CS0101 compilation errors in C#.

Another example is we can generate an empty `InterfaceConsts` class.
This is because we write the class opening because we think we have
members to place in it (`public static class InterfaceConsts {`), but
as we loop through the members we thought we were going to add they
all get eliminated.  At that point all we can do is close the class
with no members.

The solution to both examples above is "simple": we need to first loop
through the rest of the Java model and determine what we **actually**
need to generate before we start writing anything.  We can then merge
the two `OnAnimationEndEventArgs` classes, or forgo writing an empty
`InterfaceConsts` type.

However, rather than adding this additional logic in each instance
that needs it, there is enough benefit to convert the entire
`generator` architecture to work by first building a C# model of what
needs to be generated that can be tweaked before writing to a file,
resulting in a design of:

 1. Read in Java model
 2. Make tweaks to Java model
 3. ***Build C# model from Java model***
 4. ***Make tweaks to C# model***
 5. Open `<TYPE>.cs`
 6. Loop through C# model, writing code

Having a C# model to tweak should also help in a few other tricky
cases we fail at today, like determining `override`s (#367, #586) and
implementing `abstract` methods for `abstract` classes inheriting an
`interface` (#470).

Additionally, having distinct logic and source writing steps will make
unit testing better.  Currently, the only way to test the *logic* of if
we are going to generate something is to write out the source code and
compare it to "expected" source code.  This means tiny fixes can have
many "expected" files that need to change (#625).

With separate steps, we can have a set of unit tests that test what
code we are writing, and a set that tests the logic of determining what
to generate.  To test the above "2 `EventArgs` classes" example, we can
test a fix by looking at the created C# model to ensure that only a
single combined `OnAnimationEndEventArgs` class is created.  We would
not need to write out the generated source code and compare it to
expected source code.

To assist in this new design, add a new `Xamarin.SourceWriter.dll`
assembly which is responsible for generating the C# source code.
This eliminates a lot of hard to read and error prone code such as:

	writer.WriteLine ("{0}{1}{2}{3}{4} unsafe {5} {6}{7} ({8})",
	        indent,
	        visibility,
	        static_arg,
	        virt_ov,
	        seal,
	        ret,
	        is_explicit ? GetDeclaringTypeOfExplicitInterfaceMethod (method.OverriddenInterfaceMethod) + '.' : string.Empty,
	        method.AdjustedName,
	        method.GetSignature (opt));

and replaces it with:

	var m = new MethodWriter {
	    IsPublic = true,
	    IsUnsafe = true,
	    IsOverride = true,
	    Name = method.AdjustedName
	};

	m.Write (…);

which will emit:

	public unsafe override void DoSomething ()
	{
	}

`Xamarin.SourceWriter.CodeWriter` is a wrapper around a
`System.IO.Stream` that tracks the current indent level, rather than
needing to pass `indent` around to every method.

	cw.WriteLine ("{");
	cw.Indent ();
	// ... write block body ...
	cw.Unindent ();
	cw.WriteLine ("}");

~~ Testing ~~

Many existing unit tests call directly into methods like
`CodeGenerator.WriteProperties()`.  These methods are no longer
available when using `JavaInterop`/`XAJavaInterop`.  These tests were
either changed to use `CodeGenerator.WriteType()` or were moved to only
run for `XamarinAndroid` codegen target.

Tests were updated to ignore whitespace and line endings differences,
as the refactor did not preserve 100% identical whitespace.