RuntimeJavadoc.getJavadoc(Method) fails if parameter has annotation

[bbottema]

I'm on 0.9.0 and since I'm heavily relying on all the api and quirks from that version I'm not ready to upgrade until I'm on Java 8.

However, the following simply stops returning data, Everything works fine when compiled with Java 1.7, but is completely empty when compiled with Java 1.8 (and not just params, but everything):

You can easily reproduce this problem as follows:

1. check out https://github.com/bbottema/simple-java-mail.git (default branch develop)
2. add the following property to the root pom: <java.version>1.8</java.version>
3. run mvn clean test (I'm running with Maven 3.5.0 and jdk1.8.0_241)

It fails on the cli-module's unit tests. I get the same error in IntelliJ. Any idea what might cause this?

[bbottema]

I know the annotation processor is being executed, because the following does yield data:

RuntimeJavadoc.getJavadoc(m.getDeclaringClass())

After digging a little deeper, it looks like matching Method to the right MethodJavadoc doesn't work properly anymore. In RuntimeJavadoc.findMethodJavadoc, methodJavadoc.matches(method) evaluates to false on when it should be true. It seems the parameter types comparison is broken under Java 8:

It seems under Java 8, the NotNull annotations are not included in the toString for the parameter anymore, causing your equals check to fail. Just to be sure, I checked this specific spot in Java 7:

As you can see, it's fine again.

[bbottema]

As a quick fix, I would suggest simply discarding everything except the part after the last ":: " and match on that.

[bbottema]

After some hacking (inlining your methods into my code), I fixed it by as follows:

// explanation here: https://regex101.com/library/Xcb7mJ
private static final Pattern LEGACY_PARAMETER_TYPE_PATTERN = compile("(?:.*:: )?([\\w.]+)\\)?");

/**
 * Inlined, but otherwise untouched original method
 * @deprecated this is a workaround for https://github.com/dnault/therapi-runtime-javadoc/issues/50
 */
@Deprecated
private static MethodJavadoc getMethodJavadoc(final Method m) {
    ClassJavadoc javadoc = RuntimeJavadoc.getJavadoc(m.getDeclaringClass());
    for (MethodJavadoc methodJavadoc : javadoc.getMethods()) {
        if (matches(m, methodJavadoc)) {
            return methodJavadoc;
        }
    }
    return MethodJavadoc.createEmpty(m);
}

/**
 * Inlined, mostrly untouched original method with the fix marked // FIX
 * @deprecated this is a workaround for https://github.com/dnault/therapi-runtime-javadoc/issues/50
 */
@Deprecated
private static boolean matches(final Method m, final MethodJavadoc methodJavadoc) {
    if (!m.getName().equals(methodJavadoc.getName())) {
        return false;
    }
    List<String> methodParamsTypes = new ArrayList<>();
    for (Class<?> aClass : m.getParameterTypes()) {
        methodParamsTypes.add(aClass.getCanonicalName());
    }
    // FIX
    List<String> paramTypesStripped = methodJavadoc.getParamTypes().stream()
            .map(s -> LEGACY_PARAMETER_TYPE_PATTERN.matcher(s).replaceFirst("$1"))
            .collect(toList());
    // /FIX
    return methodParamsTypes.equals(paramTypesStripped);
}

[dnault]

Hi Benny, congrats on the migration to Java 8!

Thank you for the detailed report and workaround. I'll take a look.

[bbottema]

Any ETA known yet? I have a workaround, but it doesn't look very pretty in my code base...

[dnault]

Thanks for the ping. Will look into it over the holiday break and keep you posted.

[dnault]

@bbottema I could not run the simple-java-mail test suite. I tried adding an annotation to one of the methods in the therapi-runtime-javadoc 0.9.0 unit tests, but could not reproduce the problem.

Does the issue remain in therapi-runtime-javadoc 0.12.0? The parameter matching code has changed a bit since 0.9.0.

[bbottema]

I'm not able to test it as the upgrade breaks my ContextualCommentFormatter subclass. Can you point me in the right direction on how to migrate my code?

[bbottema]

If you reset the Simple Java Mail repo to commit 9f6d59eb607bcb0edb136effd2bffc41d00c2eed "Merge remote-tracking branch 'origin/master' into develop", which is on the develop branch), you should be able to reproduce the issue with mvn clean test (running with JDK 1.8 just to be sure) if you revert to the native behavior (removing my workaround).

Just checked it and this reproduces the error: in TherapiJavadocHelper, the method getMethodJavadoc can be changed to revert back to Therapi's native behavior:

/**
 * @deprecated this is a workaround for https://github.com/dnault/therapi-runtime-javadoc/issues/50
 */
@Deprecated
@NotNull
private static MethodJavadoc getMethodJavadoc(final Method m) {
    return RuntimeJavadoc.getJavadoc(m); // <-- this breaks the cli-module junit tests
}

This is all on 0.9.0 of course as upgrading to later versions breaks my code.

[bbottema]

Oops. So anyway I can't upgrade to 11 because of https://github.com/dnault/therapi-runtime-javadoc/issues/53 and I can't upgrade to 12 (yet) because of breaking API changes.

[dnault]

I'm not able to test it as the upgrade breaks my ContextualCommentFormatter subclass. Can you point me in the right direction on how to migrate my code?

If you'd rather not adopt the visitor pattern changes in 0.12.0, I would recommend copying the source code from CommentFormatter code into your source code and extending that version instead. I'm pretty sure the changes to the CommentElement subclasses are minor, and you can ignore the new visit method.

[dnault]

If you're able to reproduce the problem after upgrading to the latest version, please reopen this issue. In the meantime, the workaround you have in place is likely the best solution, given that any bug fixes will only go into the latest version.

Incidentally, I apologize for not doing a better job of documenting the breaking the changes and providing migration tips. I'll try to do a better job of that going forward... and one of these days we'll commit to a stable API with a 1.0 release and semantic versioning.

[bbottema]

You're not getting rid of this so easily ;)

If you'd rather not adopt the visitor pattern changes in 0.12.0, I would recommend copying the source code from CommentFormatter code into your source code and extending that version instead. I'm pretty sure the changes to the CommentElement subclasses are minor, and you can ignore the new visit method.

Ok, this works and my code doesn't break anymore. This bug however persists in 0.12.0. The annotation reflection just works differently in Java 8 and this library doesn't properly pick up on it anymore.

My quickfix at the beginning of the thread looks like a lot of code, but that's because I had to copy a lot in order to do a small tweak (in the 0.9.0 version of the code). The fix itself very small:

As a quick fix, I would suggest simply discarding everything except the part after the last ":: " and match on that.

// explanation here: https://regex101.com/library/Xcb7mJ
private static final Pattern LEGACY_PARAMETER_TYPE_PATTERN = compile("(?:.*:: )?([\\w.]+)\\)?");

// FIX
List<String> paramTypesStripped = methodJavadoc.getParamTypes().stream()
      .map(s -> LEGACY_PARAMETER_TYPE_PATTERN.matcher(s).replaceFirst("$1"))
      .collect(toList());
// /FIX

I can see the code change a bit under 0.12.0, but it didn't solve this issue. I would provide a pull requests, but I was unable to port my tweak to 0.9.0 back to the changed code under 0.12.0 and I'm already spending too much time on this.

[bbottema]

Just to show you what I get under 0.12.0:

The method at debug execution point:

The Therapi result under 0.12.0 without fix:

The Therapi result under 0.12.0 with my fix:

[bbottema]

So I spend some more time after all to reproduce my fix in 0.12.0. Please see pull request https://github.com/dnault/therapi-runtime-javadoc/pull/55.

[dnault]

Thanks for sticking with this. Reopened, and I'll try to reproduce this again.

[dnault]

Symptoms

Depending on the JDK vendor and version, you might see:

• RuntimeJavadoc.getJavadoc(Method) returns an empty MethodJavadoc if the method has an annotated parameter, and the annotation has a target of TYPE_USE.

• When iterating over the methods in a ClassJavadoc, the annotation names can leak into the strings returned by MethodJavadoc.getParamTypes().

Root Cause Analysis

In JsonJavadocBuilder.getParamErasures the annotation processor gets parameter type names using TypeMirror.toString() whose return value is documented as "informative" (meaning the structure is essentially unspecified). The string returned by that method can vary between JDKs, and might or might not include the names of annotations. Additionally, if the result includes an annotation, the delimiter between the annotation name and the parameter class might be " :: " as @bbottema observed, or it might simply be a space.

As a point of reference, AdoptOpenJdk 1.8.0_262 includes the parameter annotations, _but only for annotations whose target includes ElementType.TYPE_USE_. It does not use a " :: " delimiter. Here's a sample TypeMirror.toString() value from this JDK:

"@com.example.SomeAnnotation java.lang.String"

Possible Remediation

Borrow JavaPoet's strategy as referenced in the comments of this StackOverflow post

[dnault]

A quick and safe fix would be to bundle JavaPoet (like we do minimal-json) and use its TypeName class

TypeMirror erasure = typeUtils.erasure(parameter.asType());
- jsonValues.add(Json.value(erasure.toString()));
+ String typeName = TypeName.get(erasure).withoutAnnotations().toString();
+ jsonValues.add(Json.value(erasure.toString()));

[bbottema]

Reminds me of ByteBuddy, which I've used in the past to generate java classes and methods in runtime. It has a very similar builder style api.

I agree, including JavaPoet's class is probably our best bet, rather than trying to solve it again ourselves.

[bbottema]

Looking at JavaPoet's TypeName class, I'm not sure how it solves our problem. Basically I'm unable to understand how it relates to our problem at all, but that's probably entirely on me...

[dnault]

Here's a snapshot with the fix, if you want to try it out:

version: 0.13.0-SNAPSHOT

Maven snapshot repository URL: https://oss.sonatype.org/content/repositories/snapshots/

The release version of 0.13.0 will go out in a day or so, assuming I can navigate the publication process.

[bbottema]

It's 01:00 AM where I live so I'll probably catch the release proper :)

Btw, happy new year!

[dnault]

Happy New Year!

[bbottema]

I just finished testing with the snapshot and all seems to work fine now. Great! Just waiting for the release now.

[dnault]

Version 0.13.0 is live on Maven Central.