JDK-8304433: cleanup sentence breaker code in DocTreeMaker

[jonathan-gibbons]

Please review a localized cleanup of the code in DocTreeMaker to split a description into the first sentence and the rest.

There are two main aspects to the refactoring:

1. All the related code is moved into a new nested class, SentenceBreaker, within DocTreeMaker. This provides a stronger distinction between that code and the other, primary methods in DocTreeMaker that implement DocTreeFactory.

2. The use of casts is improved to reduce the overall number required. In particular, the cast method is used early on to convert List<DocTree> to List<DCTree>. This is similar to existing usage of that method elsewhere.

There are no significant changes in functionality. There is just one small observable difference, detected by one of the tests. The refactored code uses List.of() for an empty list; the older code used empty javac List objects. This was detected in a test that uses reflection to verify the result of scanning a tree, and the reflective check was for javac List objects (only). The fix in the test is to just remove the import for javac.util.List and rely on the import for List in java.util.*, which picks up java.util.List, and which is a supertype of javac List.

There is followup work that could be done, but is deferred to keep this change simpler and cleaner, with no significant change in functionality.

1. The handling of HTML tags in the first sentence is "less than ideal" and could be improved. In particular, most block tags, including lists and tables, do not terminate the first sentence, and could even lead to incorrect partitioning and downstream HTML generation. Presumably, this does not occur in practice. Related: the code that ignores EndElement tags in the first position also seems suspect.

2. When a List<DocTree> is split, the two partial lists use javac Lists, not standard lists. This does have the benefit that such lists are immutable by design. That being said, it might be worth looking at the possibility of using unmodifiable "standard" lists instead.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8304433: cleanup sentence breaker code in DocTreeMaker

Reviewers

• Hannes Wallnöfer (@hns - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/13091/head:pull/13091
$ git checkout pull/13091

Update a local copy of the PR:
$ git checkout pull/13091
$ git pull https://git.openjdk.org/jdk pull/13091/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 13091

View PR using the GUI difftool:
$ git pr show -t 13091

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/13091.diff

[bridgekeeper]

👋 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.

[openjdk]

@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 pull request command.

[mlbridge]

Webrevs

• 00: Full (a9c61f0d)

[hns]

Looks good to me. Took some time to step through old and new code because of many subtle changes, but that's the purpose of a cleanup I guess. 👍

[openjdk]

@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 details.

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

8304433: cleanup sentence breaker code in DocTreeMaker

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 7 new commits pushed to the master branch:

• c396f1e: 8304443: bootcycle builds fail after JDK-8015831
• ded6a81: 8303742: CompletableFuture.orTimeout leaks if the future completes exceptionally
• 652bda0: 8304411: Remove unused CardTable::clear
• 4505670: 8304174: Remove delays from httpserver tests
• c09f83e: 8304293: RISC-V: JDK-8276799 missed atomic intrinsic support for C1
• e339e18: 7016187: javac -h could generate conflict .h for inner class and class name with '_'
• 033c0b1: 8304437: ProblemList com/sun/jdi/ThreadMemoryLeadTest.java with ZGC

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

[jonathan-gibbons]

Looks good to me. Took some time to step through old and new code because of many subtle changes, but that's the purpose of a cleanup I guess. 👍

Thank you

[jonathan-gibbons]

/integrate

[openjdk]

Going to push as commit 80e9797.
Since your change was applied there have been 7 commits pushed to the master branch:

• c396f1e: 8304443: bootcycle builds fail after JDK-8015831
• ded6a81: 8303742: CompletableFuture.orTimeout leaks if the future completes exceptionally
• 652bda0: 8304411: Remove unused CardTable::clear
• 4505670: 8304174: Remove delays from httpserver tests
• c09f83e: 8304293: RISC-V: JDK-8276799 missed atomic intrinsic support for C1
• e339e18: 7016187: javac -h could generate conflict .h for inner class and class name with '_'
• 033c0b1: 8304437: ProblemList com/sun/jdi/ThreadMemoryLeadTest.java with ZGC

Your commit was automatically rebased without conflicts.

[openjdk]

@jonathan-gibbons Pushed as commit 80e9797.

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