asciidoctor / asciidoclet

:clipboard: A Javadoc Doclet based on Asciidoctor that lets you write Javadoc in the AsciiDoc syntax.
https://github.com/asciidoctor/asciidoclet
Apache License 2.0
133 stars 40 forks source link

Handle type parameters (pointy brackets) correctly. #50

Closed msgilligan closed 8 years ago

msgilligan commented 8 years ago

JavaDoc can be written for type parameters such as <R> in the following example:

/**
 * JSON-RPC remote method call that returns 'response.result`
 *
 * @param method JSON RPC method call to send
 * @param params JSON RPC params
 * @param <R> Type of result object
 * @return the 'response.result' field of the JSON RPC response converted to type R
 */
protected <R> R send(String method, List<Object> params, Class<R> resultType) throws IOException, JsonRPCStatusException {

When processing with Asciidoclet I get the following error:

RPCClient.java:144: warning - @param argument "&lt;R&gt;" is not a parameter name

It looks like Asciidoclet is escaping the pointy brackets before passing them through to JavaDoc. If I remove the pointy brackets, JavaDoc doesn't recognize R as a parameter.

msgilligan commented 8 years ago

Workaround:

* @param pass:[<R>] Type of result object

Seems to work for me.

johncarl81 commented 8 years ago

Hmm, how does the result look? I've used pointy brackets with success:

https://oss.sonatype.org/service/local/repositories/releases/archive/org/parceler/parceler-api/1.0.3/parceler-api-1.0.3-javadoc.jar/!/org/parceler/Parcels.html#unwrap-android.os.Parcelable-

Source:

https://github.com/johncarl81/parceler/blob/master/parceler-api/src/main/java/org/parceler/Parcels.java#L95

johncarl81 commented 8 years ago

Ah, and I bet Asciidoctor is trying to find callouts for these items http://asciidoctor.org/docs/asciidoc-syntax-quick-reference/#source-code

msgilligan commented 8 years ago

With the passthrough workaround (above) the type parameter R is listed in a separate section labeled Type parameters: as R (pointy-brackets removed.)

Without the workaround the not a parameter name warning is output, the processor can't tell R is a type parameter, so it is listed in the regular Parameters: section as <R>.

Not a huge problem, but hopefully one that can be fixed.

johncarl81 commented 8 years ago

I see the options being the following: Either we escape the javadoc to accomodate Asciidoctor (The [<R>] escaping) or escape or disallow Asciidoctor callouts to avoid the conflict with <R>.

I'm prone to leave it as it is.

@mojavelinux, care to weigh in on this one?

mojavelinux commented 8 years ago

What would make the most sense is to have an inline macro for this:

@param type:R[] Type of result object

Another option is to disable special character subs for content in @param. Ultimately, you need to do what feels right for the Javadoc author (and we'll make Asciidoclet / Asciidoctor do what it has to do to make that a good experience).

msgilligan commented 8 years ago

Another option is to disable special character subs for content in @param

That sounds like the nicest option, I think. (But the workaround is not bad once you know about it.)

johncarl81 commented 8 years ago

Playing around with this a bit I am unable to get these warnings, even with your direct example @msgilligan... could you share a buildable code example, perhaps a fork of Asciidoclet, that triggers this?

msgilligan commented 8 years ago

Weird. I just removed the pass:[<R>] workaround from my code and am not seeing the warning message anymore, either. Looking through my Git history since Sept 30th and don't see anything obvious that I changed. Maybe it is because I upgraded to Java 1.8.0_60 since then?

However the output is still incorrect without the workaround.

The project is on Github, it's msgilligan/bitcoinj-addons and it uses Gradle for the build, so you can just clone and then test it with:

./gradlew clean jenkinsBuild

The file that shows the issue is RPCClient.java line 160

You can find the output in build/alljavadoc/com/msgilligan/bitcoinj/rpc/RPCClient.html

msgilligan commented 8 years ago

Here are screenshots of the output.

with workaround: asciidoclet_issue50_workaround

Without: asciidoclet_issue50_error

mojavelinux commented 8 years ago

My understanding is that the primary problem is that the type parameters don't get extracted by default. A warning may or may not be issued depending on the JVM version.

johncarl81 commented 8 years ago

Finally looking at this problem more in depth... I'm able to reproduce this easily.

It seems if we perform the following replacements after asciidoctor renders the text then Javadoc recognizes the type parameters properly:

asciidoctor.render(cleanJavadocInput(input), options)
    .replace("&lt;", "<")
    .replace("&gt;", ">");

I'm a little concerned that this has other side effects though... @mojavelinux, is there a better way to perform this replacement within Asciidoctor and are you aware of any negative side effects from this replacement?

johncarl81 commented 8 years ago

Ah, and as soon as I think about the drawbacks, this affects how xml is rendered within code blocks.

To avoid this, we can just perform this replacement during tag blocks, which we're already treating specially (inline).

johncarl81 commented 8 years ago

Even better, we can use the pass:[<R>] technique mentioned above:

clean.replaceAll("\\<(.*)\\>", "pass:[<$1>]");
johncarl81 commented 8 years ago

One issue that this has is xml code blocks within tags are completely removed... should we check for the various blocks or just accept this as a drawback?

johncarl81 commented 8 years ago

Ugh, which has its own difficulties... like summoning Cthulhu

There must be a better way... like directing asciidoctor to no escape < and > in certain cases.

mojavelinux commented 8 years ago

First of all, we can assume that a @param has an optional leading type, so we could look for and escape that specifically as a special case. Then, we don't need to do a global search and replace since we are merely detecting and escaping a known structure.

The other option, as I mentioned in an earlier comment, is to disable the specialcharacters substitution when invoking Asciidoctor on the @param content. That has the downside that any angled brackets anywhere in the line will get passed through literally.

johncarl81 commented 8 years ago

I was goign to try that next... how do you disable special character substitution with asciidoctorj?

mojavelinux commented 8 years ago

The question, of course, is what is the pattern for a type parameter? From the documentation for the @param tag (http://docs.oracle.com/javase/1.5.0/docs/tooldocs/windows/javadoc.html#@param):

The parameter-name can be the name of a parameter in a method or constructor, or the name of a type parameter of a class, method or constructor. Use angle brackets around this parameter name to specify the use of a type parameter.

mojavelinux commented 8 years ago

So the pattern is something like:

/^<[A-Z]>(?= )/

(That limits the name to a single upper case letter, which I think is reasonable for now).

mojavelinux commented 8 years ago

I think this is the most viable approach.

mojavelinux commented 8 years ago

You could trim it, convert the remainder, then prepend it to the result to avoid having to use a passthrough.

mojavelinux commented 8 years ago

You'd obviously put in a special condition in renderTag to handle the "param" tag name.

johncarl81 commented 8 years ago

right, ok, that does narrow down this special case significantly... ill put that in place.

mojavelinux commented 8 years ago

Btw, that documentation I linked to turns out to be pretty useful :)

mojavelinux commented 8 years ago

That's the only mention of angle brackets outside of a code or literal block, so I think this covers the scenario sufficiently.

johncarl81 commented 8 years ago

Yup, I was just checking for that too. :+1:

johncarl81 commented 8 years ago

Implemented on #54. Asciidoclet will now pass-through the bracketed parameter name during processing. Thanks @msgilligan for the keen eye for details.

msgilligan commented 8 years ago

Thanks for fixing this! I'd love to see asciidoclet become more widely adopted or even become the standard. :)