Please review a medium size update/overhaul to the way that positions and 
diagnostic positions are handled in the `DocTree`/`DCTree` world.

## Previously ...

Previously, most `DCTree` nodes only had an explicit "position" (`pos`). Start 
and end positions were provided by `DocSourcePositions`, but these were not 
directly available in tree nodes, because of the lack of access to the 
`DocSourcePositions` class.  (In contrast, `JCTree` nodes do have position 
info, via static methods in `TreeInfo`.)  The one notable exception to these 
guidelines was `DCErroneous` which (by analogy with `JCTree`) directly 
implemented `JCDiagnostic.DiagnosticPosition` but that was an arguably bad 
implementation because the positions were relative to the start of the comment, 
and not the start of the file. This did not show up in code and tests, because 
diagnostics related to `DocTree` nodes used `DCTree.pos` which returned a 
`SimpleDiagnosticPosition` referencing just the start position -- at least in 
part because more specific information was not easily available.

## Now ... 

All `DCTree` nodes have 4 positions, 3 publicly available.
* the position of the first unique character, `pos`
* the starting position of the range of characters, `getStartPosition()`
* the "preferred" position in the range, used to position the caret in 
diagnostic messages, `getPreferredPosition()`
* the end position of the range of characters, `getEndPosition()`.
These are all relative to the beginning of the comment text, and are converted 
to file positions as needed.

Code to implement the end position is moved from `JavacTrees` to `DCTree`. It's 
still a switch on kind, but could reasonably be converted to using overriding 
methods.

`DCErroneous` no longer implements `JCDiagnostic.DiagnosticPosition`. the 
constructor methods to create a diagnostic are removed, in favor of passing in 
a correctly-formed diagnostic.

`DCTree` has a new improved `pos(DCDocComment)` method which now uses the new 
start/pref/end position methods.

`DocCommentParser.ParseException` and the `erroneous` method now take an 
optional "pos" parameter to allow the position of an error to be more 
accurately specified.

## Testing ...

Up until the point at which `DCTree.pos` was modified, all tests passed without 
change.   When `DCTree.pos()` was modified, a few (3) doclint tests starting 
failing, demonstrating a latent reliance of the old form of `DCTree.pos()`. 
These tests are updated.

Rather than write a single new test, the existing `DocCommentTester` class is 
updated to include a new `Checker`  which, generally, checks the "left to 
right" nature of all positions in a doc comment tree and its subtrees. This 
leverages all the existing good and bad test cases in the `tools/javac/doctree` 
directory, which therefore all get tagged with the bug number for this issue.

## Behavior ...

Apart from fixing generally bad behavior, there is one other tiny behavioral 
change. For an empty `DocCommentTree` the ending position is now the same at 
the starting position, and not `NOPOS`.  This was triggered by the new code in 
`DocCommentTester`, but which affected one `jshell` test, which started getting 
`StringIndexOutOfBounds` exception. This is minimally fixed for now, but the 
code in question would arguably be better to use the new comment-based 
positions, rather than the file-based positions. But that seems out of scope 
for this work.  Also worth investigating is the method 
`JavacTrees.getDocCommentTree(FileObject fileObject)` which uses a fixed offset 
of 0 (JavacTrees, line 1052) when creating doc comments for HTML files.

-------------

Commit messages:
 - JDK-8273244: Improve diagnostic output related to ErroneousTree

Changes: https://git.openjdk.java.net/jdk/pull/5510/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=5510&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8273244
  Stats: 772 lines in 49 files changed: 412 ins; 123 del; 237 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5510.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5510/head:pull/5510

PR: https://git.openjdk.java.net/jdk/pull/5510

Reply via email to