Skip to content

Commit

Permalink
[java-source-utils] Better support orphaned Javadocs (#757)
Browse files Browse the repository at this point in the history
Context: #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 #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
  • Loading branch information
jonpryor committed Dec 10, 2020
1 parent 0cb8e2d commit c1bc5c8
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 28 deletions.
4 changes: 2 additions & 2 deletions tools/java-source-utils/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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> {

Expand Down

0 comments on commit c1bc5c8

Please sign in to comment.