On Tue, 14 Sep 2021 16:05:41 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
> 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. src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/DocLint.java line 295: > 293: t.printStackTrace(); > 294: throw t; > 295: } This needs to be reverted. ------------- PR: https://git.openjdk.java.net/jdk/pull/5510
