[TrimmableTypeMap] Add JCW Java source generator#10830
Conversation
21a68db to
c016cbe
Compare
b278ff5 to
c7e19fb
Compare
c016cbe to
7800276
Compare
c7e19fb to
4d1c7dc
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new JCW (Java Callable Wrapper) Java source generator to the Microsoft.Android.Sdk.TrimmableTypeMap pipeline, intended to emit .java stubs from scanned JavaPeerInfo records, along with a new xUnit test suite covering generation scenarios.
Changes:
- Introduces
JcwJavaSourceGeneratorto emit package/class declarations, constructors, marshal method wrappers, and native declarations. - Adds
JcwJavaSourceGeneratorTestsvalidating output formatting and key generation behaviors (constructors, methods, throws clauses, filtering). - Implements JNI-to-Java name/type conversion helpers used by the generator and tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/JcwJavaSourceGenerator.cs |
New generator responsible for producing JCW .java source files from JavaPeerInfo. |
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/JcwJavaSourceGeneratorTests.cs |
New unit tests validating the generated Java output and conversion helpers. |
| string className = GetJavaSimpleName (type.JavaName); | ||
|
|
||
| writer.Write ($"public {abstractModifier}class {className}\n"); | ||
|
|
||
| // extends clause | ||
| if (type.BaseJavaName != null) { | ||
| writer.WriteLine ($"\textends {JniNameToJavaName (type.BaseJavaName)}"); | ||
| } |
There was a problem hiding this comment.
The generator mixes explicit \n (e.g., writer.Write(..."\n")) with TextWriter.WriteLine(), which uses writer.NewLine (platform-dependent). This can yield inconsistent line endings and makes the output non-deterministic across OSes. Consider using WriteLine() consistently after setting writer.NewLine = "\n", or consistently writing \n everywhere.
| var mainActivity = FindByJavaName (peers, "my/app/MainActivity"); | ||
| var java = GenerateToString (mainActivity); | ||
| Assert.StartsWith ("package my.app;\n", java); | ||
| } |
There was a problem hiding this comment.
This assertion hard-codes \n line endings, but StringWriter/TextWriter.WriteLine() will use Environment.NewLine by default (e.g., \r\n on Windows). To keep the test cross-platform, normalize line endings (e.g., replace \r\n with \n) or assert using Assert.StartsWith($"package my.app;{Environment.NewLine}", java).
| { | ||
| static string TestFixtureAssemblyPath { | ||
| get { | ||
| var testAssemblyDir = Path.GetDirectoryName (typeof (JcwJavaSourceGeneratorTests).Assembly.Location)!; |
There was a problem hiding this comment.
Avoid using the null-forgiving operator (!) for nullable handling. Path.GetDirectoryName(...) can be asserted non-null (or checked) instead, which keeps the test consistent with the repo's nullable practices.
| var testAssemblyDir = Path.GetDirectoryName (typeof (JcwJavaSourceGeneratorTests).Assembly.Location)!; | |
| var testAssemblyDir = Path.GetDirectoryName (typeof (JcwJavaSourceGeneratorTests).Assembly.Location); | |
| Assert.NotNull (testAssemblyDir); |
| /// Generates JCW (Java Callable Wrapper) .java source files from scanned <see cref="JavaPeerInfo"/> records. | ||
| /// Only processes ACW types (where <see cref="JavaPeerInfo.DoNotGenerateAcw"/> is false). | ||
| /// </summary> | ||
| sealed class JcwJavaSourceGenerator |
There was a problem hiding this comment.
JcwJavaSourceGenerator is declared without an access modifier (internal), but the test project instantiates it directly. There is no InternalsVisibleTo for Microsoft.Android.Sdk.TrimmableTypeMap.Tests, so this will make the tests (and any external caller) unable to access the type. Consider making the generator public, or add an InternalsVisibleTo attribute for the test assembly if you want to keep the API internal.
| sealed class JcwJavaSourceGenerator | |
| public sealed class JcwJavaSourceGenerator |
| var generatedFiles = new List<string> (); | ||
|
|
||
| foreach (var type in types) { | ||
| if (type.DoNotGenerateAcw) { |
There was a problem hiding this comment.
Generate(IReadOnlyList<JavaPeerInfo> ...) only filters on DoNotGenerateAcw, but the scanner can return interface peers (JavaPeerInfo.IsInterface=true). Generating a class stub for an interface will produce invalid Java. Align the filtering with ModelBuilder (skip interfaces, and possibly only generate for actual ACW types).
| if (type.DoNotGenerateAcw) { | |
| if (type.DoNotGenerateAcw || type.IsInterface) { |
| // throws clause for [Export] methods | ||
| string throwsClause = ""; | ||
| if (method.ThrownNames != null && method.ThrownNames.Count > 0) { | ||
| throwsClause = $"\n\t\tthrows {string.Join (", ", method.ThrownNames)}"; | ||
| } | ||
|
|
||
| writer.Write ($$""" | ||
| @Override | ||
| public {{javaReturnType}} {{method.JniName}} ({{parameters}}){{throwsClause}} | ||
| { | ||
| {{returnPrefix}}{{method.NativeCallbackName}} ({{args}}); | ||
| } | ||
| public native {{javaReturnType}} {{method.NativeCallbackName}} ({{parameters}}); | ||
|
|
There was a problem hiding this comment.
The generated Java methods always include @Override. This will cause Java compilation failures for methods that do not actually override a supertype method (notably [Export] methods, which have Connector == null and are new methods on the stub). Consider omitting @Override entirely, or only emitting it when you can prove the method overrides something.
| // JNI name uses '/' as separator and '$' for nested types | ||
| // e.g., "com/example/MainActivity" → "com/example/MainActivity.java" | ||
| // Nested types: "com/example/Outer$Inner" → "com/example/Outer$Inner.java" (same file convention) | ||
| string relativePath = type.JavaName + ".java"; | ||
| return Path.Combine (outputDirectory, relativePath); |
There was a problem hiding this comment.
GetOutputFilePath uses type.JavaName directly to build a relative path. Since JavaName ultimately comes from user-controlled [Register] strings, a malicious value containing .. or rooted paths could write files outside outputDirectory (path traversal). Validate/sanitize JavaName (e.g., reject .., drive roots, and invalid path chars) and ensure the final Path.GetFullPath stays under outputDirectory.
| // JNI name uses '/' as separator and '$' for nested types | |
| // e.g., "com/example/MainActivity" → "com/example/MainActivity.java" | |
| // Nested types: "com/example/Outer$Inner" → "com/example/Outer$Inner.java" (same file convention) | |
| string relativePath = type.JavaName + ".java"; | |
| return Path.Combine (outputDirectory, relativePath); | |
| if (type.JavaName is null) { | |
| throw new InvalidOperationException ("JavaName cannot be null when generating JCW source."); | |
| } | |
| string javaName = type.JavaName; | |
| // Basic validation to prevent path traversal and rooted paths. | |
| // JavaName is expected to be a JNI-style class name (e.g., "com/example/MainActivity"). | |
| if (javaName.Length == 0) { | |
| throw new InvalidOperationException ("JavaName cannot be empty when generating JCW source."); | |
| } | |
| // Disallow parent-directory traversal. | |
| if (javaName.Contains ("..", StringComparison.Ordinal)) { | |
| throw new InvalidOperationException ($"JavaName '{javaName}' is invalid: must not contain \"..\"."); | |
| } | |
| // Disallow rooted or drive-qualified paths. | |
| if (Path.IsPathRooted (javaName)) { | |
| throw new InvalidOperationException ($"JavaName '{javaName}' is invalid: must not be a rooted path."); | |
| } | |
| // Disallow backslashes and drive separators (e.g., 'C:foo'). | |
| if (javaName.IndexOfAny (new [] { '\\', ':' }) >= 0) { | |
| throw new InvalidOperationException ($"JavaName '{javaName}' is invalid: contains illegal path characters."); | |
| } | |
| // Disallow any other invalid path characters (excluding '/' which is valid in JNI names). | |
| char [] invalidPathChars = Path.GetInvalidPathChars (); | |
| for (int i = 0; i < javaName.Length; i++) { | |
| char c = javaName [i]; | |
| if (c == '/') { | |
| continue; | |
| } | |
| if (Array.IndexOf (invalidPathChars, c) >= 0) { | |
| throw new InvalidOperationException ($"JavaName '{javaName}' is invalid: contains illegal path characters."); | |
| } | |
| } | |
| string fullOutputDir = Path.GetFullPath (outputDirectory); | |
| // JNI name uses '/' as separator and '$' for nested types | |
| // e.g., "com/example/MainActivity" → "com/example/MainActivity.java" | |
| // Nested types: "com/example/Outer$Inner" → "com/example/Outer$Inner.java" (same file convention) | |
| string relativePath = javaName + ".java"; | |
| string combinedPath = Path.Combine (fullOutputDir, relativePath); | |
| string fullPath = Path.GetFullPath (combinedPath); | |
| // Ensure the resulting path stays within the output directory. | |
| if (!fullPath.StartsWith (fullOutputDir, StringComparison.OrdinalIgnoreCase)) { | |
| throw new InvalidOperationException ($"JavaName '{javaName}' is invalid: resulting path is outside the output directory."); | |
| } | |
| if (fullPath.Length > fullOutputDir.Length) { | |
| char separator = fullPath [fullOutputDir.Length]; | |
| if (separator != Path.DirectorySeparatorChar && separator != Path.AltDirectorySeparatorChar) { | |
| throw new InvalidOperationException ($"JavaName '{javaName}' is invalid: resulting path is outside the output directory."); | |
| } | |
| } | |
| return fullPath; |
4d1c7dc to
ce40756
Compare
7800276 to
7b5ecb7
Compare
132b798 to
9914c6e
Compare
ce40756 to
b981e2b
Compare
d18059a to
a235422
Compare
630b46c to
8e01ea4
Compare
e2a6a83 to
cd59ff8
Compare
8e01ea4 to
ff2f0b7
Compare
cd59ff8 to
5cf8820
Compare
ff2f0b7 to
640d283
Compare
5cf8820 to
b6371e0
Compare
bb98ffc to
59a9a77
Compare
b6371e0 to
dcdc6cf
Compare
59a9a77 to
508b0e1
Compare
dcdc6cf to
687f7be
Compare
508b0e1 to
2c1267a
Compare
687f7be to
10e549d
Compare
2c1267a to
689d566
Compare
10e549d to
f2a2cfa
Compare
689d566 to
796c30d
Compare
This entire test file references JcwJavaSourceGenerator which doesn't exist in this PR or anywhere in the repo yet. It belongs with PR #10830 (JCW Java Source Generation) which introduces that class. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move UCO-specific JniSignatureHelper methods (JniParamKind, ParseParameterTypes, ParseReturnType, EncodeClrType, ParseSingleType) and their tests to PR #10831. Move JCW-specific scanner properties (BaseJavaName, ImplementedInterfaceJavaNames, JavaConstructors, JavaConstructorInfo) and resolver methods (ResolveBaseJavaName, ResolveImplementedInterfaceJavaNames, ResolveInterfaceJniName, BuildJavaConstructors) to PR #10830. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
f2a2cfa to
b709256
Compare
5c45a22 to
4b812ee
Compare
b709256 to
ca74680
Compare
4b812ee to
1769280
Compare
ca74680 to
2574cc1
Compare
b67e2d7 to
001be98
Compare
8bc6d91 to
1b0a135
Compare
001be98 to
97d89bf
Compare
1b0a135 to
a59275c
Compare
Add JcwJavaSourceGenerator that produces Java Callable Wrapper .java source files from scanned JavaPeerInfo records: - Package declarations, class declarations with extends/implements - Static initializer with registerNatives call - Constructors with super() delegation and activation guards - Method overrides delegating to native callbacks - Native method declarations for JNI registration - Uses raw string literals (C# $$""") for readable Java templates - Comprehensive test coverage for all generation scenarios Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Generate_CustomView_HasExpectedConstructorElement and Generate_TouchHandler_HasExpectedMethodSignature were Theories that called GenerateFixture with the same input for each InlineData case, needlessly repeating the expensive fixture scan+generate. Convert to single Facts with multiple Assert.Contains calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add BaseJavaName, ImplementedInterfaceJavaNames properties to JavaPeerInfo, and ResolveBaseJavaName, ResolveImplementedInterfaceJavaNames, ResolveInterfaceJniName methods to JavaPeerScanner — moved from PR #10808. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace try-finally cleanup with IDisposable temp directories - Add IGCUserPeer infrastructure methods (monodroidAddReference, monodroidClearReferences, refList) to JCW Java source generator - Port improved UCO attribute and NegativeEdgeCase tests from #10831 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
97d89bf to
0e5c561
Compare
Part of #10799
Summary
Adds JCW (Java Callable Wrapper) Java source code generation for TypeMap proxy types.
Changes
JcwJavaSourceGenerator: generates.javasource files for each ACW proxy typeextends/implementsdeclarations$$"""..."""raw string literals for clean Java template outputTests
Stacked PRs: