Repository navigation
Parser: attach // and /* */ comments to enum constants, and record where a javadoc starts and ends - #134
Conversation
… written (TestEnumConstantComments)
The comment pre-scan (SourceCodeScan.scanTypeBody) had no case for an enum constant, so a // or /* */ comment
above a constant, or a // comment after it on its line ("B, // the second"), was attached to nothing:
FieldInfo.comments() of every enum constant held at most its javadoc. A tool removing a dead constant could not
tell which comment went with it. Enum constants are now scanned like field declarations; the lookahead for the
comment after the constant looks past the ',' or ';' that follows it, since that comment precedes the next
constant's first token and would otherwise become the next constant's leading comment.
A javadoc's source spans its TEXT, not the comment: "/** the second */ B," gives the javadoc column 9 (the 't'),
not column 5 (the '/'). Consumers count from there to place a word inside the comment (rename in comments), so
that stays. The comment as written, "/**" through "*/", is now on that source as the new detail
DetailedSources.WHOLE_COMMENT, for a consumer that removes or moves the comment itself and cannot infer its
first character when the line also holds code. The unit's text is read once per compilation unit, on the first
javadoc that needs it.
maddi build: 2586 tests, 0 failures; maddi-mod build: 1668 tests, 0 failures (both against this branch).
Signed-off-by: Tom Tourwe <Tom.Tourwe@codelaser.io>
c996750 to
3bfc86b
Compare
bnaudts
left a comment
There was a problem hiding this comment.
Approving. Enum constants now get their comments the way field declarations do, and the change to addComments leaves the field path as it was. One thing should still be fixed, in this PR or right after it: WHOLE_COMMENT gives the wrong columns on lines indented with tabs.
Checked
- I merged the PR onto current devel (it is 114 commits behind; ScanJavaDoc merges cleanly).
:maddi-java-openjdk:test: 689 tests, 0 failures. - No printer (Java, Kotlin or C#) prints an enum constant's comments, so the translators' output and ratchets don't change.
- I tried cases outside the test, and all of them behave correctly:
A, B, // cattaches the comment to B.C { int f() { return 1; } }, // cattaches it to C.- A
// cafter the last constant's;goes to that constant, not to the field after it. That field keeps its own// above xand// after x.
Should fix: WHOLE_COMMENT columns on tab-indented lines
wholeCommentSource takes its columns from LineMap.getColumnNumber, and javac's LineMap expands a tab to the next multiple of 8. The other sources the parser hands out (fields, // comments) count a tab as one character. On the line \t/** tab */ T; // after T:
| column | |
|---|---|
field T |
7:13 |
// after T |
7:16 |
WHOLE_COMMENT |
7:9-7:18 (should be 7:2-7:11) |
A tool that deletes columns 9-18 removes ab */ T; : it deletes the constant and leaves /** t behind. The test passes because it only uses spaces. The source text is already read, so the fix is small: take the line from lineMap, and compute the column as the offset minus the position of the previous \n. A tab-indented case in TestEnumConstantComments would keep it fixed.
Not blocking
- The javadoc's own source has the same tab expansion: in the example above, T's javadoc text reads as 7:13 instead of 7:6. That is older than this PR, and rename-in-comments counts from that position, so it deserves its own issue.
- In
@Deprecated // notefollowed byD,on the next line, the comment is still attached to nothing. It is rare, and not a regression. source.withDetailedSources(...)replaces whatever details the source already had. It has none today, butmergeDetailedSourceswould be the safer call.
Related: #128. That issue is the same defect in statements (if (c) return true; // found, } // found): a trailing // whose next token belongs to another node gets attached to nothing. The addComments(node, nextSibling) overload added here is what the #128 fix needs, so I'll build #128 on top of this PR once it is in devel.
Problem
The parser attached
//and/* */comments to fields, but not to enum constants. ForFieldInfo.comments()returned only the javadoc, and the other comments were attached to nothing:comments()beforecomments()after[]// above A(3:5),// after A(4:8)[]/* above B */(5:5)before C(7:9)before C(7:9) (unchanged)Above D.(9:8)Above D.(9:8),// after D(11:8)Consequence: a tool that removes a dead enum constant could not remove the comments that belong to it. It
left
// above Aand// after Abehind in the source. refactor-base's dead-code removal(
TestEnumConstantRemovalTakesItsOwnJavaDoc) needs this.A comment after a constant on the same line (
A, // after A) comes before the next constant's first token.Without care it would become B's leading comment. The scan now looks past the
,or;after the constantand gives that comment to A. The test checks that B does not get it.
Javadoc position
A javadoc's source starts at its text, not at
/**: for/** before C */ C,the source begins at column 9(the
bofbefore), not at column 5 (the/). Rename-in-comments counts from the text, so that stays.But a tool that deletes the comment needs the position of
/**, and cannot find it when code shares theline. The new detail
DetailedSources.WHOLE_COMMENTon the javadoc's source gives the comment as written:/** before C */is 7:5-7:19/**at 8:5 through*/at 10:7, so 8:5-10:7Changes
SourceCodeScan.scanTypeBody: scans enum constants the way it scans field declarations.ScanJavaDoc: recordsWHOLE_COMMENT. The file's text is read once per compilation unit, the first timea javadoc needs it.
DetailedSources.WHOLE_COMMENT: new constant.TestEnumConstantComments.Tests
maddi: 2586 tests, 0 failures. maddi-mod: 1668 tests, 0 failures. Both were run against this branch.