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

JDK-8252712: move doclint to jdk.javadoc module #133

Closed

Conversation

jonathan-gibbons
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons commented Sep 11, 2020

doclint is a component that can check documentation comments, that can be invoked from either javac or javadoc, via the -Xdoclint family of options.

When JDK was modularized in JDK9, doclint was left in the jdk.compiler module because of direct references to the code from javac, even though functionally the code more naturally belongs in the jdk.javadoc module.

This change moves the code into the jdk.javadoc, using a service/provider API to make the functionality available to javac. This should be completely transparent, as long as the jdk.javadoc module is available when performing service binding. If it is not available, a default no-op implementation is automatically used instead.

One minor complication: the old code used static methods on the DocLint class to validate options. This is no longer possible when using the service provider mechanism. Instead, the methods are changed to instance methods. The javac Option enum has no way to cleanly cache an instance of the service provider class, and so a new instance is created for each option. However, this is a relatively lightweight operation, and can reasonably be done for the typically few doclint-related options on the command line.

Note: JShell has been making minor use of an internal doclint class enum HtmlTag, which is no longer easily available. A minimal local enum is left behind in JShell's JavadocFomatter class, sufficient to its requirements. Since the dominant use of the enum is in switch statements, an alternative solution, avoiding the local enum, would be to use strings and switch on string instead. However, the use of the local enum is a smaller disturbance to the JavadocFormatter class.


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed

Issue

Reviewers

Download

$ git fetch https://git.openjdk.java.net/jdk pull/133/head:pull/133
$ git checkout pull/133

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Sep 11, 2020

👋 Welcome back jjg! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@jonathan-gibbons jonathan-gibbons marked this pull request as ready for review Sep 11, 2020
@openjdk openjdk bot added the rfr label Sep 11, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Sep 11, 2020

@jonathan-gibbons The following labels will be automatically applied to this pull request: compiler javadoc.

When this pull request is ready to be reviewed, an RFR email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label (add|remove) "label" command.

@openjdk openjdk bot added javadoc compiler labels Sep 11, 2020
@mlbridge
Copy link

@mlbridge mlbridge bot commented Sep 11, 2020

Webrevs

@openjdk
Copy link

@openjdk openjdk bot commented Sep 16, 2020

@jonathan-gibbons this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout move-doclint
git fetch https://git.openjdk.java.net/jdk master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

@openjdk openjdk bot added the merge-conflict label Sep 16, 2020
@openjdk openjdk bot removed the merge-conflict label Sep 17, 2020
Copy link
Member

@hns hns left a comment

Changes look good apart from minor issues addressed in extra comments.

hns
hns approved these changes Sep 23, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Sep 23, 2020

@jonathan-gibbons This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for more details.

After integration, the commit message for the final commit will be:

8252712: move doclint to jdk.javadoc module

Reviewed-by: hannesw

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 74 new commits pushed to the master branch:

  • c21690b: 8253464: ARM32 Zero: atomic_copy64 is incorrect, breaking volatile stores
  • 0bc01da: 8250635: MethodArityHistogram should use Compile_lock in favour of fancy checks
  • 812b39f: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary
  • 5f1d612: 8253397: Ensure LogTag types are sorted
  • b8ea80a: 8253457: Remove unimplemented register stack functions
  • e4d0e5a: 8253516: ZGC: Remove card table functions
  • 3fe5886: 8252696: Loop unswitching may cause out of bound array load to be executed
  • 226faa5: 8253241: Update comment on java_suspend_self_with_safepoint_check()
  • bd67975: 8253349: Remove unimplemented SharedRuntime::native_method_throw_unsupported_operation_exception_entry
  • bddb822: 8253240: No javadoc for DecimalFormatSymbols.hashCode()
  • ... and 64 more: https://git.openjdk.java.net/jdk/compare/3570f5a924876be000891bc5447742812d6b0262...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready label Sep 23, 2020
@jonathan-gibbons
Copy link
Contributor Author

@jonathan-gibbons jonathan-gibbons commented Sep 23, 2020

/integrate

@openjdk openjdk bot closed this Sep 23, 2020
@openjdk openjdk bot added integrated and removed ready rfr labels Sep 23, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Sep 23, 2020

@jonathan-gibbons Since your change was applied there have been 74 commits pushed to the master branch:

  • c21690b: 8253464: ARM32 Zero: atomic_copy64 is incorrect, breaking volatile stores
  • 0bc01da: 8250635: MethodArityHistogram should use Compile_lock in favour of fancy checks
  • 812b39f: 8252739: Deflater.setDictionary(byte[], int off, int len) ignores the starting offset for the dictionary
  • 5f1d612: 8253397: Ensure LogTag types are sorted
  • b8ea80a: 8253457: Remove unimplemented register stack functions
  • e4d0e5a: 8253516: ZGC: Remove card table functions
  • 3fe5886: 8252696: Loop unswitching may cause out of bound array load to be executed
  • 226faa5: 8253241: Update comment on java_suspend_self_with_safepoint_check()
  • bd67975: 8253349: Remove unimplemented SharedRuntime::native_method_throw_unsupported_operation_exception_entry
  • bddb822: 8253240: No javadoc for DecimalFormatSymbols.hashCode()
  • ... and 64 more: https://git.openjdk.java.net/jdk/compare/3570f5a924876be000891bc5447742812d6b0262...master

Your commit was automatically rebased without conflicts.

Pushed as commit f765a7f.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@jonathan-gibbons jonathan-gibbons deleted the move-doclint branch Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler integrated javadoc
2 participants