Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Java 11 #96

Closed
wants to merge 20 commits into from
Closed

Support for Java 11 #96

wants to merge 20 commits into from

Conversation

chrisvest
Copy link

@chrisvest chrisvest commented Jun 7, 2019

The javadoclet APIs changed a lot in Java 9. And is now much more locked down. Adding support (in this case for Java 11) for asciidoclet unfortunately requires a lot of hacks, but things appear to work stably.

The doclet APIs now work by processing the abstract syntax tree from the java compiler. This is presented to the doclet via the DocletEnvironment interface. In order to avoid duplicating the large body of functionality that exists in the standard doclet, we pass a filtered environment to the standard doclet, which post-processes the javadoc comments, so the standard doclet sees asciidoctor output in place of the javadoc that is written in the source files.

This is fine in principle, but it turns out that the standard doclet is not written to the interfaces, but rather the internal APIs of javac. This immediately forces us to break the module encapsulation via --add-exports. We have to either take the path of hacks in order to reuse the standard doclet, or reimplement all of the standard doclet, or post-process the javadoc HTML that the standard doclet produces. I opted for hacks, since that appeared to be the path of least resistance.

It gets worse, though. Due to a mix of visibility constraints, immutability, memoisation, and lack of extension points, we can't just present a modified view of the AST to the standard doclet. We have to actually dig into, and mutate, the AST to make it look like a person wrote asciidoctor output in their javadoc comments. This messes up error reporting and linting, since javadoc will now be producing warnings for source locations that don't actually exist. This mess is concentrated in LazyDocCommentTableProcessor and AsciidocComment.

The related issue is #76

Also update Java dependency to Java 11, and update all Maven plugins to the latest version.
Also add a requirement that the Maven version must be at least 3.0.5 in order to support the latest plugins.
The StandardAdapter was trashed in the process, though.
… under Java 11.

Many things are broken. This is currently more of an exploratory prototype, than it is working but incomplete code.
There are a lot of ugly hacks here, to work around the new restrictions of the javadoc APIs.
Also update Stylesheets to use new Java APIs, and make sure that input and output streams are closed.
Also make sure that the output directory of the integration test is empty before rendering new doclet output.
@chrisvest
Copy link
Author

What's currently missing is 1) tests to prove that the stylesheets and templates are copied to the correct places and used, 2) bringing back the AsciidoctorRendererTest, and as part of that 3) fix the handling of javadoc tags like @param, @throws, @author, etc.

…void re-rendering asciidoctor javadoc comments that have already been rendered.
@chrisvest
Copy link
Author

chrisvest commented Jul 11, 2019

Javadocs should be parsed correctly now, though some tags are not going through and I'm not sure why. Resources such as stylesheets are still not being copied correctly. Also, attributes such as {project_name} are not being replaced - maybe they are not being loaded correctly.

@chrisvest
Copy link
Author

@msgilligan Have you had a chance to look at this?

.travis.yml Show resolved Hide resolved
@@ -46,70 +46,69 @@
<artifactId>asciidoctorj</artifactId>
<version>1.5.8.1</version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're at it and doing a full 2.x rev, we should really sync with the rest of the asciidoctor projects and use the same versions. That would mean upgrading to the latest asciidoctorj. I know that's a lot for a single pull request though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good version policy to have. If it needs to be done in this PR, then I'd like to get the Java 11 port working first, though, with minimal behavioural changes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

pom.xml Outdated Show resolved Hide resolved
src/main/java/org/asciidoclet/Asciidoclet.java Outdated Show resolved Hide resolved
ATTRIBUTES_FILE( "attributes-file" ),
GEM_PATH( "gem-path" ),
REQUIRE( "r" ),
REQUIRE_LONG( "require" );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming we upgrade to asciidoctorj 2.x, are these all the options?

src/main/java/org/asciidoclet/package-info.java Outdated Show resolved Hide resolved
src/main/java/org/asciidoclet/package-info.java Outdated Show resolved Hide resolved
src/main/java/org/asciidoclet/package-info.java Outdated Show resolved Hide resolved
LightGuard
LightGuard previously approved these changes Jul 15, 2019
@LightGuard
Copy link
Member

@johncarl81 take a look. I think this is a good first step in getting a doclet that works with Java 11.

@johncarl81
Copy link
Member

@LightGuard - I've been following, and yes, this is an excellent first step. I think we should probably make a new repo or a hard version break at least for this next version. Fantastic work @chrisvest!

@LightGuard
Copy link
Member

I think a hard version break is probably best, it doesn't change anything but the version number. Maybe we need to create a new branch for a 2.x version. I think it would at least be good to get this out as a preview and see how it works in the wild.

@LightGuard
Copy link
Member

Well, when I say it doesn't change anything, I mean the rest of the gav.

…ngs, to almost make it possible for asciidoclet to render its own javadoc.
@chrisvest
Copy link
Author

Attributes are being replaced correctly, but resources such as stylesheets are still not being copied correctly to the destination directory, because the javadoc tool does not pass the destination directory argument through to the doclet.

@chrisvest
Copy link
Author

Stylesheets are now copied correctly, so coderay highlighting works again. A Java 11 stylesheet has also been added. There are two problems left, AFAIK: 1) the overview.adoc is somehow not processed through asciidoctor, so javadoc complains about the strange syntax, and 2) asciidoctor (in the version currently integrated) is producing HTML elements that javadoc says is not part of HTML5, such as data-lang and rel attributes, and tables without summary or caption.

Would an asciidoctor upgrade help with the second problem? These HTML5 issues are reported as errors, which makes javadoc fail the build.

@chrisvest
Copy link
Author

Adding <additionalJOption>-Xdoclint:none</additionalJOption> to the javadoc configuration makes the build pass, but the overview is still not rendered properly.

Included in this is also a bunch of cleanup.
Particularly, many inner classes have been extracted.
Reported javadoc errors are now also _slightly_ closer to their cause in the code.
@chrisvest chrisvest changed the title Prototype support for Java 11 Support for Java 11 Jul 21, 2019
@chrisvest
Copy link
Author

Overview files are now processed correctly. The HTML problems have been ignored. So now the javadocs are generated correctly, and without failing the build, given the right options are provided.

@johncarl81 I think this has reached feature and functional parity with the asciidoclet for pre-modular Java versions, so now would be a good time to figure out how and where this gets integrated. And then find a cutting point between this PR and future work.

@chrisvest
Copy link
Author

This PR is an example of the sort of changes people will have to do, to upgrade to an asciidoclet with these changes: chrisvest/stormpot#118

@LightGuard
Copy link
Member

Any further update on this? It would be nice to get a release cut, even if it's a beta.

@chrisvest
Copy link
Author

@LightGuard I'm using this via jitpack to my fork at https://github.com/chrisvest/asciidoclet/tree/chrisvest:

<repositories>
  <repository>
    <!-- Required for the asciidoclet fork. -->
    <id>jitpack.io</id>
    <url>https://jitpack.io</url>
  </repository>
</repositories>
...
<groupId>com.github.chrisvest</groupId>
<artifactId>asciidoclet</artifactId>
<version>2.0.0</version>

@chrisvest
Copy link
Author

It looks like newer versions of the Maven Javadoc plug-in now refuses to download doclet dependencies from anywhere other than Maven Central, which means the JitPack trick above is now ineffective. Unless you use an older version of the plug-in.

Alternatively, you can vendor the binary and refer to it via docletPath.

See https://github.com/chrisvest/asciidoclet/releases/tag/2.0.0 for more information.

@cradloff
Copy link

cradloff commented Jul 10, 2020

The pull request is now more than a year old. Will there ever be support for Java 11 or is this project dead?
And if it's dead, is there an alternative?

@msgilligan
Copy link
Member

I'm interested in getting Java 11 support finished and released. I apologize for not being involved for the last year, but I've just had way too much on my plate. I have more free time now and I have a pressing need to upgrade my asciidoclet-using project ConsensusJ to Java 11, so I have extra motivation and a real-world test case.

I'm not capable of doing this all single-handedly, but maybe some of the there maintainers will be able to help as well. @johncarl81 @mojavelinux ?

@chrisvest Are you still available to help?

@msgilligan
Copy link
Member

@msgilligan Have you had a chance to look at this?

Sorry for not responding until now, but I was spread very thin for the last year. I'll look at the code this weekend.

@msgilligan
Copy link
Member

I think a hard version break is probably best, it doesn't change anything but the version number. Maybe we need to create a new branch for a 2.x version.

Yes. I agree we should make a 2.x version that requires Java 9+. Note that people will still be able to compile code for earlier versions of Java, they'll just need to use JDK 9+ when building the JavaDoc.

I also think the 2.x version should use all the latest upstream AsciiDoctor tooling (though we may want to get this PR merged on a 2.x branch before we try updating the other jars)

I think it would at least be good to get this out as a preview and see how it works in the wild.

I agree and I can create a branch of my ConensusJ project that uses Asciidoclet for some real-world testing.

@chrisvest
Copy link
Author

@chrisvest Are you still available to help?

Yes.

I've been using my own fork for half a year now. I'd also like to see the asciidoctor version upgraded to the latest, and to have asciidoctor-diagram support added. Someone already did that work based off of my fork, but unfortunately not in a way that allow me to apply those patches to my own fork.

@msgilligan
Copy link
Member

Thanks @chrisvest. I'll try to push this forward, but my availability is limited and I don't have the authority to create a release, so I'm definitely hoping others will help as well.

import static javax.tools.StandardLocation.SOURCE_PATH;

class AsciiDocTrees extends DocTrees
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting: We use the java standard "Egyptian brackets" - Would you reformat globally?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't make changes for the time being, because I changed jobs and have to wait (no ETA so far) for approval before making more open source contributions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can handle this one if you're ok with it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static void processComments( DocCommentTable table, Function<Comment,Comment> mapper )
{
// Use heckin' raw-types because LazyDocCommentTable.Entry has private access, so we
// cannot statically express its type here.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dang! Is there truly no other way? This seems like a major shortcoming of the new Javadoc platform.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatives are to do a clean-room reimplementation of standard doclet, or change the license of asciidoclet to be compatible with OpenJDK and then copy-paste the real standard doclet so it can be modified.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I guess I'm just disappointed that the developers didn't allow for more extensibility here.

@bentolor
Copy link

bentolor commented Sep 14, 2020

So much great work has already done in this PR and based on the comments the results are good.

How about just merging this in a 2.0-alpha release & branch or something release so people can actually use it and then trying to get the other things pinpointed/done?

AFAIK mostly upgrade of asciidoctor4j should also go in a 2.0 release, right?

Am I missing further work to be done?

@johncarl81
Copy link
Member

johncarl81 commented Sep 14, 2020

@bentolor, I'd like to review it first... I think it's nearly there, @chrisvest just needs to respond to my comments. In the mean time you're free to use any code you wish.

@chrisvest
Copy link
Author

@johncarl81 Replied to your comments.

@abelsromero
Copy link
Member

Hi 👋 I'd like to continue the work from this PR. Does anyone have any objection?

@LightGuard
Copy link
Member

Go for it.

abelsromero added a commit that referenced this pull request Mar 19, 2024
Implement support for Java 11

From PR #96

* Set version to 2.0.0-SNAPSHOT.
* Set up Java Module and port code to package 'org.asciidoctor.asciidoclet'.
* Removed Google Guava and other third-party dependencies.
* Formatting using default IntellJ.
* Removed --include/--exclude + @asciidoclet filter feature.
  No longer necessary: the way it works now is that the contents always pass to 
  the default Javadoc converter so this is not necessary to do progressive transitions.
* Renamed "render" references by "convert" in code and README.
  For example, AsciidoctorRenderer to AsciidoctorConverter for following recent Asciidoctor naming.
* Removed support and stylesheets for Java versions below 11.
* Fixed locating templates in a Java module runtime (it was causing tests to fail)
* Added Integration (end-2-end) test with maven-invoker-plugin.
  As a downside, this cannot be debugged from IDE, but it's the most realistic real integration test I can think of (previous experiences have mixed results).
* Added .sdkmanrc
@abelsromero
Copy link
Member

Closing since #108 was merged. Many thanks to all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants