From 61fccd8dea3304f3aff39a0281975f4d75553d83 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Thu, 10 Dec 2020 15:36:27 -0500 Subject: [PATCH] [java-source-utils] Properly represent unresolved types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/xamarin/java.interop/pull/687#issuecomment-668129226 Commit 69e1b80a stated: > We don't want to *require* that everything be resolvable -- it's painful, and > possibly impossible, e.g. w/ internal types -- so instead we should "demark" > the unresolvable types. > > `.params.txt` output will use `.*` as a type prefix, e.g. > > method(.*UnresolvableType foo, int bar); > > `docs.xml` will output `L.*UnresolvableType;`. …except this didn't actually happen: `java-source.utils.jar --output-javadoc` didn't have an `L` prefix and `;` suffix for the JNI type representation. Additionally, it would include generic type parameters! Thus, a declaration such as: static UnknownType method(AnotherUnknownType p); would result in: which was not at all intended, and causes subsequent issued in PR #687 as the JNI signature parser would break when encountering `.`. Update `java-source-utils.jar` & `//*/@jni-signature` so that [unresolved types are properly represented][0]: `//*/@jni-type` and `//*/@jni-signature` shouldn't contain generic type parameters, and should also (somewhat) "conform" to JNI convention, with a leading `L` and trailing `;`, emitting: Additionally, update `JavaType.java` to cause it to have "orphaned" Javadoc comments; this change was forgotten in c1bc5c83. [0]: https://github.com/xamarin/java.interop/pull/687#issuecomment-668129226 --- .../java/com/xamarin/JavaType.java | 2 + .../android/JniPackagesInfoFactory.java | 45 +++++++++++++++++-- .../android/JavadocXmlGeneratorTest.java | 5 +++ .../src/test/resources/UnresolvedTypes.txt | 11 +++++ .../src/test/resources/UnresolvedTypes.xml | 13 ++++++ 5 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 tools/java-source-utils/src/test/resources/UnresolvedTypes.txt create mode 100644 tools/java-source-utils/src/test/resources/UnresolvedTypes.xml diff --git a/tests/Xamarin.Android.Tools.Bytecode-Tests/java/com/xamarin/JavaType.java b/tests/Xamarin.Android.Tools.Bytecode-Tests/java/com/xamarin/JavaType.java index baad63fb1..d87cacb49 100644 --- a/tests/Xamarin.Android.Tools.Bytecode-Tests/java/com/xamarin/JavaType.java +++ b/tests/Xamarin.Android.Tools.Bytecode-Tests/java/com/xamarin/JavaType.java @@ -6,6 +6,7 @@ /** * JNI sig: Lcom/xamarin/JavaEnum; */ +// Disconnective comment? enum JavaEnum { /** FIRST; JNI sig: Lcom/xamarin/JavaEnum; */ FIRST, @@ -161,6 +162,7 @@ public int compareTo (JavaType value) { } /** JNI sig: func.(Ljava/lang/StringBuilder;)Ljava/util/List; */ + // Comment to "disconnect" Javadoc from the member public List func (StringBuilder value) { return null; } diff --git a/tools/java-source-utils/src/main/java/com/microsoft/android/JniPackagesInfoFactory.java b/tools/java-source-utils/src/main/java/com/microsoft/android/JniPackagesInfoFactory.java index b91e6eb37..3886ec8bb 100644 --- a/tools/java-source-utils/src/main/java/com/microsoft/android/JniPackagesInfoFactory.java +++ b/tools/java-source-utils/src/main/java/com/microsoft/android/JniPackagesInfoFactory.java @@ -361,7 +361,7 @@ static String getJavaType(JniTypeInfo typeInfo, JniMethodInfo methodInfo, Type t final ResolvedType rt = type.resolve(); return rt.describe(); } catch (final Throwable thr) { - return ".*" + type.asString(); + return getUnresolvedJavaType(type); } } @@ -389,7 +389,8 @@ static String getJniType(JniTypeInfo typeInfo, JniMethodInfo methodInfo, Type ty } catch (final Exception thr) { } - return ".*" + type.asString(); + + return getUnresolvedJniType(type); } static String getJniType(JniTypeInfo typeInfo, JniMethodInfo methodInfo, ArrayType type) { @@ -412,7 +413,7 @@ static String getPrimitiveJniType(String javaType) { case "short": return "S"; case "void": return "V"; } - throw new Error("Don't know JNI type for `" + javaType + "`!"); + throw new Error("Don't know JNI type for primitive type `" + javaType + "`!"); } static String getJniType(ResolvedType type) { @@ -453,4 +454,42 @@ static String getJniType(ResolvedReferenceType type) { name.append(";"); return name.toString(); } + + static String getUnresolvedJavaType(Type type) { + final StringBuilder jniType = new StringBuilder(); + + jniType.append(".*"); + + if (type.isClassOrInterfaceType()) { + // Special-case class-or-interface declarations so that we skip type parameters. + type.ifClassOrInterfaceType(c -> { + c.getScope().ifPresent(s -> jniType.append(s.asString()).append(".")); + jniType.append(c.getName().asString()); + }); + } else { + jniType.append(type.asString()); + } + + return jniType.toString(); + } + + static String getUnresolvedJniType(Type type) { + final StringBuilder jniType = new StringBuilder(); + + jniType.append("L.*"); + + if (type.isClassOrInterfaceType()) { + // Special-case class-or-interface declarations so that we skip type parameters. + type.ifClassOrInterfaceType(c -> { + c.getScope().ifPresent(s -> jniType.append(s.asString()).append(".")); + jniType.append(c.getName().asString()); + }); + } else { + jniType.append(type.asString()); + } + + jniType.append(";"); + + return jniType.toString(); + } } diff --git a/tools/java-source-utils/src/test/java/com/microsoft/android/JavadocXmlGeneratorTest.java b/tools/java-source-utils/src/test/java/com/microsoft/android/JavadocXmlGeneratorTest.java index e46d8738c..28107fc68 100644 --- a/tools/java-source-utils/src/test/java/com/microsoft/android/JavadocXmlGeneratorTest.java +++ b/tools/java-source-utils/src/test/java/com/microsoft/android/JavadocXmlGeneratorTest.java @@ -109,6 +109,11 @@ public void testWritePackages_JavaType_java() throws Throwable { testWritePackages("../../../com/xamarin/JavaType.java", "JavaType.xml"); } + @Test + public void testWritePackages_UnresolvedTypes_txt() throws Throwable { + testWritePackages("../../../UnresolvedTypes.txt", "../../../UnresolvedTypes.xml"); + } + private static void testWritePackages(final String resourceJava, final String resourceXml) throws Throwable { final JavaParser parser = JniPackagesInfoFactoryTest.createParser(); final JniPackagesInfoFactory factory = new JniPackagesInfoFactory(parser); diff --git a/tools/java-source-utils/src/test/resources/UnresolvedTypes.txt b/tools/java-source-utils/src/test/resources/UnresolvedTypes.txt new file mode 100644 index 000000000..14965966e --- /dev/null +++ b/tools/java-source-utils/src/test/resources/UnresolvedTypes.txt @@ -0,0 +1,11 @@ +package example; + +public class UnresolvedTypes { + /** + * Method using unresolvable types. As such, we make do. + * + * JNI Sig: method.(L.*example.name.UnresolvedParameterType;)L.*UnresolvedReturnType; + */ + public static UnresolvedReturnType method(example.name.UnresolvedParameterType parameter) { + } +} \ No newline at end of file diff --git a/tools/java-source-utils/src/test/resources/UnresolvedTypes.xml b/tools/java-source-utils/src/test/resources/UnresolvedTypes.xml new file mode 100644 index 000000000..b80c679a2 --- /dev/null +++ b/tools/java-source-utils/src/test/resources/UnresolvedTypes.xml @@ -0,0 +1,13 @@ + + + + + + + + + + +