Skip to content

Commit 6bbb00a

Browse files
authored
[generator] Use an intermediate C# model (#674)
`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: #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.
1 parent 69e1b80 commit 6bbb00a

File tree

113 files changed

+6715
-741
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

113 files changed

+6715
-741
lines changed

Java.Interop.sln

+14
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Java.Interop.Tools.Generato
9393
EndProject
9494
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "java-source-utils", "tools\java-source-utils\java-source-utils.csproj", "{F46EDFA5-C52A-4F0C-B5A2-5BB67E0D8C74}"
9595
EndProject
96+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Xamarin.SourceWriter", "src\Xamarin.SourceWriter\Xamarin.SourceWriter.csproj", "{C5B732C8-7AF3-41D3-B903-AEDFC392E5BA}"
97+
EndProject
98+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Xamarin.SourceWriter-Tests", "tests\Xamarin.SourceWriter-Tests\Xamarin.SourceWriter-Tests.csproj", "{6CF94627-BA74-4336-88CD-7EDA20C8F292}"
99+
EndProject
96100
Global
97101
GlobalSection(SharedMSBuildProjectFiles) = preSolution
98102
src\Java.Interop.NamingCustomAttributes\Java.Interop.NamingCustomAttributes.projitems*{58b564a1-570d-4da2-b02d-25bddb1a9f4f}*SharedItemsImports = 5
@@ -257,6 +261,14 @@ Global
257261
{F46EDFA5-C52A-4F0C-B5A2-5BB67E0D8C74}.Debug|Any CPU.Build.0 = Debug|Any CPU
258262
{F46EDFA5-C52A-4F0C-B5A2-5BB67E0D8C74}.Release|Any CPU.ActiveCfg = Release|Any CPU
259263
{F46EDFA5-C52A-4F0C-B5A2-5BB67E0D8C74}.Release|Any CPU.Build.0 = Release|Any CPU
264+
{C5B732C8-7AF3-41D3-B903-AEDFC392E5BA}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
265+
{C5B732C8-7AF3-41D3-B903-AEDFC392E5BA}.Debug|Any CPU.Build.0 = Debug|Any CPU
266+
{C5B732C8-7AF3-41D3-B903-AEDFC392E5BA}.Release|Any CPU.ActiveCfg = Release|Any CPU
267+
{C5B732C8-7AF3-41D3-B903-AEDFC392E5BA}.Release|Any CPU.Build.0 = Release|Any CPU
268+
{6CF94627-BA74-4336-88CD-7EDA20C8F292}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
269+
{6CF94627-BA74-4336-88CD-7EDA20C8F292}.Debug|Any CPU.Build.0 = Debug|Any CPU
270+
{6CF94627-BA74-4336-88CD-7EDA20C8F292}.Release|Any CPU.ActiveCfg = Release|Any CPU
271+
{6CF94627-BA74-4336-88CD-7EDA20C8F292}.Release|Any CPU.Build.0 = Release|Any CPU
260272
EndGlobalSection
261273
GlobalSection(SolutionProperties) = preSolution
262274
HideSolutionNode = FALSE
@@ -301,6 +313,8 @@ Global
301313
{C2FD2F12-DE3B-4FB9-A0D3-FA3EF597DD04} = {0998E45F-8BCE-4791-A944-962CD54E2D80}
302314
{7F4828AB-3908-458C-B09F-33C74A1368F9} = {271C9F30-F679-4793-942B-0D9527CB3E2F}
303315
{F46EDFA5-C52A-4F0C-B5A2-5BB67E0D8C74} = {C8F58966-94BF-407F-914A-8654F8B8AE3B}
316+
{C5B732C8-7AF3-41D3-B903-AEDFC392E5BA} = {0998E45F-8BCE-4791-A944-962CD54E2D80}
317+
{6CF94627-BA74-4336-88CD-7EDA20C8F292} = {271C9F30-F679-4793-942B-0D9527CB3E2F}
304318
EndGlobalSection
305319
GlobalSection(ExtensibilityGlobals) = postSolution
306320
SolutionGuid = {29204E0C-382A-49A0-A814-AD7FBF9774A5}

Makefile

+2-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ TESTS = \
2828
bin/Test$(CONFIGURATION)/generator-Tests.dll \
2929
bin/Test$(CONFIGURATION)/Xamarin.Android.Tools.ApiXmlAdjuster-Tests.dll \
3030
bin/Test$(CONFIGURATION)/Xamarin.Android.Tools.Bytecode-Tests.dll \
31-
bin/Test$(CONFIGURATION)/Java.Interop.Tools.Generator-Tests.dll
31+
bin/Test$(CONFIGURATION)/Java.Interop.Tools.Generator-Tests.dll \
32+
bin/Test$(CONFIGURATION)/Xamarin.SourceWriter-Tests.dll
3233

3334
PTESTS = \
3435
bin/Test$(CONFIGURATION)/Java.Interop-PerformanceTests.dll

build-tools/automation/azure-pipelines.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ jobs:
5555
inputs:
5656
solution: build-tools/scripts/RunNUnitTests.targets
5757
configuration: $(Build.Configuration)
58-
msbuildArguments: /p:TestAssembly="bin\Test$(Build.Configuration)\generator-Tests.dll;bin\Test$(Build.Configuration)\Java.Interop.Tools.JavaCallableWrappers-Tests.dll;bin\Test$(Build.Configuration)\logcat-parse-Tests.dll;bin\Test$(Build.Configuration)\Xamarin.Android.Tools.ApiXmlAdjuster-Tests.dll;bin\Test$(Build.Configuration)\Xamarin.Android.Tools.Bytecode-Tests.dll;bin\Test$(Build.Configuration)\Java.Interop.Tools.Generator-Tests.dll"
58+
msbuildArguments: /p:TestAssembly="bin\Test$(Build.Configuration)\generator-Tests.dll;bin\Test$(Build.Configuration)\Java.Interop.Tools.JavaCallableWrappers-Tests.dll;bin\Test$(Build.Configuration)\logcat-parse-Tests.dll;bin\Test$(Build.Configuration)\Xamarin.Android.Tools.ApiXmlAdjuster-Tests.dll;bin\Test$(Build.Configuration)\Xamarin.Android.Tools.Bytecode-Tests.dll;bin\Test$(Build.Configuration)\Java.Interop.Tools.Generator-Tests.dll;bin\Test$(Build.Configuration)\Xamarin.SourceWriter-Tests.dll"
5959
condition: succeededOrFailed()
6060

6161
- task: PublishTestResults@2

build-tools/automation/templates/core-tests.yaml

+7
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,13 @@ steps:
5151
arguments: bin/Test$(Build.Configuration)/Java.Interop.Tools.JavaSource-Tests.dll
5252
continueOnError: true
5353

54+
- task: DotNetCoreCLI@2
55+
displayName: 'Tests: Xamarin.SourceWriter'
56+
inputs:
57+
command: test
58+
arguments: bin/Test$(Build.Configuration)/Xamarin.SourceWriter-Tests.dll
59+
continueOnError: true
60+
5461
# Running native Java.Interop tests are not yet supported on .NET Core
5562
#- task: DotNetCoreCLI@2
5663
# displayName: 'Tests: Java.Interop'
+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.IO;
4+
using System.Text;
5+
6+
namespace Xamarin.SourceWriter
7+
{
8+
public class CodeWriter : IDisposable
9+
{
10+
TextWriter stream;
11+
bool owns_stream;
12+
int indent;
13+
bool need_indent = true;
14+
string base_indent;
15+
16+
public CodeWriter (string filename)
17+
{
18+
stream = File.CreateText (filename);
19+
owns_stream = true;
20+
}
21+
22+
public CodeWriter (TextWriter streamWriter, string baseIndent = "")
23+
{
24+
stream = streamWriter;
25+
base_indent = baseIndent;
26+
}
27+
28+
public void Write (string value)
29+
{
30+
WriteIndent ();
31+
stream.Write (value);
32+
}
33+
34+
public void WriteLine ()
35+
{
36+
WriteIndent ();
37+
stream.WriteLine ();
38+
need_indent = true;
39+
}
40+
41+
public void WriteLine (string value)
42+
{
43+
WriteIndent ();
44+
stream.WriteLine (value);
45+
need_indent = true;
46+
}
47+
48+
public void WriteLine (string format, params object[] args)
49+
{
50+
WriteIndent ();
51+
stream.WriteLine (format, args);
52+
need_indent = true;
53+
}
54+
55+
public void Indent (int count = 1) => indent += count;
56+
public void Unindent (int count = 1) => indent -= count;
57+
58+
private void WriteIndent ()
59+
{
60+
if (!need_indent)
61+
return;
62+
63+
stream.Write (base_indent + new string ('\t', indent));
64+
65+
need_indent = false;
66+
}
67+
68+
public void Dispose ()
69+
{
70+
if (owns_stream)
71+
stream?.Dispose ();
72+
}
73+
}
74+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Text;
4+
5+
namespace Xamarin.SourceWriter
6+
{
7+
public enum Visibility
8+
{
9+
Default = 0,
10+
Private = 1,
11+
Public = 2,
12+
Protected = 4,
13+
Internal = 8
14+
}
15+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Text;
4+
5+
namespace Xamarin.SourceWriter
6+
{
7+
public static class StringExtensions
8+
{
9+
public static bool HasValue (this string str) => !string.IsNullOrWhiteSpace (str);
10+
}
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Text;
4+
5+
namespace Xamarin.SourceWriter
6+
{
7+
public abstract class AttributeWriter
8+
{
9+
public virtual void WriteAttribute (CodeWriter writer) { }
10+
}
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Collections.ObjectModel;
4+
using System.Text;
5+
6+
namespace Xamarin.SourceWriter
7+
{
8+
public class ClassWriter : TypeWriter
9+
{
10+
public ObservableCollection<ConstructorWriter> Constructors { get; } = new ObservableCollection<ConstructorWriter> ();
11+
12+
public ClassWriter ()
13+
{
14+
Constructors.CollectionChanged += MemberAdded;
15+
}
16+
17+
public override void WriteConstructors (CodeWriter writer)
18+
{
19+
foreach (var ctor in Constructors) {
20+
ctor.Write (writer);
21+
writer.WriteLine ();
22+
}
23+
}
24+
}
25+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Text;
4+
5+
namespace Xamarin.SourceWriter
6+
{
7+
public class CommentWriter : ISourceWriter
8+
{
9+
public string Value { get; set; }
10+
public int Priority { get; set; }
11+
12+
public CommentWriter (string value)
13+
{
14+
Value = value;
15+
}
16+
17+
public virtual void Write (CodeWriter writer)
18+
{
19+
writer.WriteLine (Value);
20+
}
21+
}
22+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Text;
4+
5+
namespace Xamarin.SourceWriter
6+
{
7+
public class ConstructorWriter : MethodWriter
8+
{
9+
public string BaseCall { get; set; }
10+
11+
protected override void WriteReturnType (CodeWriter writer)
12+
{
13+
}
14+
15+
protected override void WriteConstructorBaseCall (CodeWriter writer)
16+
{
17+
if (BaseCall.HasValue ())
18+
writer.Write ($" : {BaseCall}");
19+
}
20+
}
21+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Text;
4+
5+
namespace Xamarin.SourceWriter
6+
{
7+
public class DelegateWriter : ISourceWriter, ITakeParameters
8+
{
9+
Visibility visibility;
10+
11+
public string Name { get; set; }
12+
public List<MethodParameterWriter> Parameters { get; } = new List<MethodParameterWriter> ();
13+
public TypeReferenceWriter Type { get; set; }
14+
public List<string> Comments { get; } = new List<string> ();
15+
public List<AttributeWriter> Attributes { get; } = new List<AttributeWriter> ();
16+
public bool IsPublic { get => visibility.HasFlag (Visibility.Public); set => visibility = value ? Visibility.Public : Visibility.Default; }
17+
public bool UseExplicitPrivateKeyword { get; set; }
18+
public bool IsInternal { get => visibility.HasFlag (Visibility.Internal); set => visibility = value ? Visibility.Internal : Visibility.Default; }
19+
public bool IsConst { get; set; }
20+
public string Value { get; set; }
21+
public bool IsStatic { get; set; }
22+
public bool IsReadonly { get; set; }
23+
public bool IsPrivate { get => visibility.HasFlag (Visibility.Private); set => visibility = value ? Visibility.Private : Visibility.Default; }
24+
public bool IsProtected { get => visibility.HasFlag (Visibility.Protected); set => visibility = value ? Visibility.Protected : Visibility.Default; }
25+
public int Priority { get; set; }
26+
public bool IsShadow { get; set; }
27+
28+
public void SetVisibility (string visibility)
29+
{
30+
switch (visibility?.ToLowerInvariant ()) {
31+
case "public":
32+
IsPublic = true;
33+
break;
34+
case "internal":
35+
IsInternal = true;
36+
break;
37+
case "protected":
38+
IsProtected = true;
39+
break;
40+
case "private":
41+
IsPrivate = true;
42+
break;
43+
}
44+
}
45+
46+
public virtual void Write (CodeWriter writer)
47+
{
48+
WriteComments (writer);
49+
WriteAttributes (writer);
50+
WriteSignature (writer);
51+
}
52+
53+
public virtual void WriteComments (CodeWriter writer)
54+
{
55+
foreach (var c in Comments)
56+
writer.WriteLine (c);
57+
}
58+
59+
public virtual void WriteAttributes (CodeWriter writer)
60+
{
61+
foreach (var att in Attributes)
62+
att.WriteAttribute (writer);
63+
}
64+
65+
public virtual void WriteSignature (CodeWriter writer)
66+
{
67+
if (IsPublic)
68+
writer.Write ("public ");
69+
else if (IsInternal)
70+
writer.Write ("internal ");
71+
else if (IsPrivate)
72+
writer.Write ("private ");
73+
74+
if (IsStatic)
75+
writer.Write ("static ");
76+
if (IsReadonly)
77+
writer.Write ("readonly ");
78+
if (IsConst)
79+
writer.Write ("const ");
80+
81+
if (IsShadow)
82+
writer.Write ("new ");
83+
84+
writer.Write ("delegate ");
85+
86+
WriteType (writer);
87+
88+
writer.Write (Name + " ");
89+
writer.Write ("(");
90+
91+
WriteParameters (writer);
92+
93+
writer.Write (")");
94+
95+
writer.Write (";");
96+
}
97+
98+
protected virtual void WriteParameters (CodeWriter writer)
99+
{
100+
for (var i = 0; i < Parameters.Count; i++) {
101+
var p = Parameters [i];
102+
p.WriteParameter (writer);
103+
104+
if (i < Parameters.Count - 1)
105+
writer.Write (", ");
106+
}
107+
}
108+
109+
protected virtual void WriteType (CodeWriter writer)
110+
{
111+
Type.WriteTypeReference (writer);
112+
}
113+
}
114+
}

0 commit comments

Comments
 (0)