[java-source-utils] Better support orphaned Javadocs (dotnet#757)

Context: dotnet#687 (comment)

What happens when there's a "regular" Java comment in between a
Javadoc comment and a member?

	/* partial */ class Object {
	    /** Create and return a copy of this object… */
	    // BEGIN Android-changed: Use native local helper for clone()
	    // …
	    protected Object clone() throws CloneNotSupportedException {…}
	}

What happens is that the Javadoc becomes *orphaned*.

Commit 69e1b80 attempted to handle such orphaned Javadocs via
heuristic, using the first orphaned Javadoc comment in the parent
scope.  This didn't work reliably, as the parent scope could contain
multiple "*unrelated*" orphaned Javadoc comments:

	class Outer {
	    /** Orphaned dotnet#1 */
	    // cause orphaning
	    class Inner {}

	    void m() {}
	}

Because containing types are fully processed before contained types,
`Outer.m()` would grab the Javadoc for `Outer.Inner` before
`Outer.Inner` would have a chance to grab it.

Re-work the logic to associate orphaned Javadocs with their members,
by requiring that the Javadoc comment begin *before* the member of
interest, and *after* any preceding members.  This should prevent
incorrect correlation of orphaned Javadoc comment blocks.

Additionally, update gradle to use javaparser 3.18.0, from 3.16.1:

  * javaparser/javaparser@javaparser-parent-3.16.1...javaparser-parent-3.18.0

Author: jonpryor

tools/java-source-utils/build.gradle

@@ -30,8 +30,8 @@ repositories {
 
 dependencies {
     // This dependency is used by the application.
-    implementation 'com.github.javaparser:javaparser-core:3.16.1'
-    implementation 'com.github.javaparser:javaparser-symbol-solver-core:3.16.1'
+    implementation 'com.github.javaparser:javaparser-core:3.18.0'
+    implementation 'com.github.javaparser:javaparser-symbol-solver-core:3.18.0'
 
     // Use JUnit test framework
     testImplementation 'junit:junit:4.12'

tools/java-source-utils/src/main/java/com/microsoft/android/JniPackagesInfoFactory.java

@@ -1,6 +1,7 @@
 package com.microsoft.android;
 
 import java.io.File;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
@@ -35,8 +36,6 @@
 
 import com.microsoft.android.ast.*;
 
-import javassist.compiler.ast.FieldDecl;
-
 import static com.microsoft.android.util.Parameter.*;
 
 public final class JniPackagesInfoFactory {
@@ -90,6 +89,8 @@ private void parse(final JniPackagesInfo packages, final CompilationUnit unit) t
 			: "";
 		final   JniPackageInfo packageInfo  = packages.getPackage(packageName);
 
+		fixJavadocComments(unit, unit.getTypes());
+
 		for (final TypeDeclaration<?> type : unit.getTypes()) {
 			if (JavaSourceUtilsOptions.verboseOutput && type.getFullyQualifiedName().isPresent()) {
 				System.out.println("Processing: " + type.getFullyQualifiedName().get());
@@ -171,6 +172,7 @@ static ClassOrInterfaceType getTypeParameterBound(TypeParameter typeParameter) {
 	}
 
 	private final void parseType(final JniPackageInfo packageInfo, final JniTypeInfo typeInfo, TypeDeclaration<?> typeDecl) {
+		fixJavadocComments(typeDecl, getUndocumentedBodyMembers(typeDecl.getMembers()));
 		for (final BodyDeclaration<?> body : typeDecl.getMembers()) {
 			if (body.isAnnotationDeclaration()) {
 				final   AnnotationDeclaration       annoDecl    = body.asAnnotationDeclaration();
@@ -215,6 +217,68 @@ private final void parseType(final JniPackageInfo packageInfo, final JniTypeInfo
 		}
 	}
 
+	private final void fixJavadocComments(final Node decl, final Iterable<? extends BodyDeclaration<?>> bodyMembers) {
+		final List<BodyDeclaration<?>>  members             = getUndocumentedBodyMembers(bodyMembers);
+		final List<JavadocComment>      orphanedComments    = getOrphanComments(decl);
+
+		if (members.size() == 0)
+			return;
+
+		final BodyDeclaration<?>        firstMember = members.get(0);
+		JavadocComment                  comment     = orphanedComments.stream()
+			.filter(c -> c.getBegin().get().isBefore(firstMember.getBegin().get()))
+			.reduce((a, b) -> b)
+			.orElse(null);
+		if (comment != null) {
+			((NodeWithJavadoc<?>) firstMember).setJavadocComment(comment);
+		}
+
+		for (int i = 1; i < members.size(); ++i) {
+			BodyDeclaration<?> prevMember   = members.get(i-1);
+			BodyDeclaration<?> member       = members.get(i);
+
+			Optional<JavadocComment> commentOpt = orphanedComments.stream()
+				.filter(c -> c.getBegin().get().isAfter(prevMember.getEnd().get()) &&
+					c.getEnd().get().isBefore(member.getBegin().get()))
+				.findFirst();
+			if (!commentOpt.isPresent())
+				continue;
+			((NodeWithJavadoc<?>)member).setJavadocComment(commentOpt.get());
+		}
+	}
+
+	private final List<BodyDeclaration<?>> getUndocumentedBodyMembers(Iterable<? extends BodyDeclaration<?>> bodyMembers) {
+		final List<BodyDeclaration<?>> members = new ArrayList<BodyDeclaration<?>> ();
+		for (BodyDeclaration<?> member : bodyMembers) {
+			if (!(member instanceof NodeWithJavadoc<?>)) {
+				continue;
+			}
+			final NodeWithJavadoc<?> memberJavadoc = (NodeWithJavadoc<?>) member;
+			if (memberJavadoc.getJavadocComment().isPresent())
+				continue;
+			final Optional<Position> memberBeginOpt = member.getBegin();
+			if (!memberBeginOpt.isPresent())
+				continue;
+			members.add(member);
+		}
+		members.sort((a, b) -> a.getBegin().get().compareTo(b.getBegin().get()));
+		return members;
+	}
+
+	private final List<JavadocComment> getOrphanComments(Node decl) {
+		final List<JavadocComment> orphanedComments = new ArrayList<JavadocComment>(decl.getOrphanComments().size());
+		for (Comment c : decl.getOrphanComments()) {
+			if (!c.isJavadocComment())
+				continue;
+			final Optional<Position> commentBeginOpt = c.getBegin();
+			if (!commentBeginOpt.isPresent())
+				continue;
+			orphanedComments.add(c.asJavadocComment());
+		}
+		orphanedComments.sort((a, b) -> a.getBegin().get().compareTo(b.getBegin().get()));
+		return orphanedComments;
+	}
+
 	private final void parseAnnotationMemberDecl(final JniTypeInfo typeInfo, final AnnotationMemberDeclaration memberDecl) {
 		final JniMethodInfo methodInfo = new JniMethodInfo(typeInfo, memberDecl.getNameAsString());
 		typeInfo.add(methodInfo);
@@ -266,30 +330,8 @@ private static final void fillJavadoc(final HasJavadocComment member, NodeWithJa
 		JavadocComment javadoc = null;
 		if (nodeWithJavadoc.getJavadocComment().isPresent()) {
 			javadoc = nodeWithJavadoc.getJavadocComment().get();
-		} else {
-			Node node = (Node) nodeWithJavadoc;
-			if (!node.getParentNode().isPresent())
-				return;
-
-			/*
-			 * Sometimes `JavaParser` won't associate a Javadoc comment block with
-			 * the AST node we expect it to.  In such circumstances the Javadoc
-			 * comment will become an "orphan" comment, unassociated with anything.
-			 *
-			 * If `nodeWithJavadoc` has no Javadoc comment, use the *first*
-			 * orphan Javadoc comment in the *parent* scope, then *remove* that
-			 * comment so that it doesn't "stick around" for the next member we
-			 * attempt to grab Javadoc comments for.
-			 */
-			Node parent = node.getParentNode().get();
-			for (Comment c : parent.getOrphanComments()) {
-				if (c.isJavadocComment()) {
-					javadoc = c.asJavadocComment();
-					c.remove();
-					break;
-				}
-			}
 		}
+
 		if (javadoc != null) {
 			member.setJavadocComment(javadoc.parse().toText());
 		}

tools/java-source-utils/src/test/java/com/microsoft/android/JavadocXmlGeneratorTest.java

@@ -4,6 +4,7 @@
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.PrintStream;
+import java.io.FileOutputStream;
 import java.io.UnsupportedEncodingException;
 import java.util.Arrays;
 import javax.xml.parsers.ParserConfigurationException;
@@ -120,6 +121,9 @@ private static void testWritePackages(final String resourceJava, final String re
 		final   String                  expected        = JniPackagesInfoTest.getResourceContents(resourceXml);
 
 		generator.writePackages(packagesInfo);
+		// try (FileOutputStream o = new FileOutputStream(resourceXml + "-jonp.xml")) {
+		// 	bytes.writeTo(o);
+		// }
 		assertEquals(resourceJava + " Javadoc XML", expected, bytes.toString());
 	}
 }

tools/java-source-utils/src/test/resources/com/microsoft/android/Outer.java

@@ -9,7 +9,7 @@
  * 
  * JNI sig: Lexample/Outer;
  */
-
+// Cause the above Javadoc block to be orphaned from the below class declaration
  @Outer.MyAnnotation(keys={"a", "b", "c"})
 public class Outer<T extends Object & Runnable, U extends Error> {