On Thu, 5 May 2022 23:39:46 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
> This PR is for an update to the doc comment `{@value}` tag to permit an
> optional [format
> string](https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/Formatter.html#syntax)
> after the name of the tag.
>
> {@value optional-format-string optional-reference}
>
> If given, the format string should either begin with `%` or the string should
> be quoted with `"`. For example, `%4x` or `"0x%4x"`. The format string must
> contain exactly one `%` character.
So far the idea looks okay. My feedback is purely on its implementation.
I'm concerned about backward compatibility. If we believe that it is unlikely
that javadoc can be configured to use a custom implentation of
`DocTreeFactory`, then the proposed implementation looks okay. Otherwise,
consider a case where a new javadoc tool reads old doc comments. Do we expect
it to fail with `UnsupportedOperationException` for every `value` tag in those
doc comments?
My inline comments below are about that latter case; ignore them if you don't
think it's an issue.
src/jdk.compiler/share/classes/com/sun/source/doctree/ValueTree.java line 55:
> 53: default TextTree getFormat() {
> 54: throw new UnsupportedOperationException();
> 55: }
Return `null` and specify this behavior in the `@implSpec` section.
src/jdk.compiler/share/classes/com/sun/tools/javac/parser/DocCommentParser.java
line 1605:
> 1603: if (ch == '}') {
> 1604: nextChar();
> 1605: return m.at(pos).newValueTree(format, ref);
If `format == null` call `newValueTree(ref)`; otherwise call
`newValueTree(format, ref)`.
src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DocTreeMaker.java line
482:
> 480:
> 481: @Override @DefinedBy(Api.COMPILER_TREE)
> 482: public DCValue newValueTree(ReferenceTree ref) {
Revert the implementation of this method.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ValueTaglet.java
line 112:
> 110: text = String.format(configuration.getLocale(), f,
> field.getConstantValue());
> 111: } catch (IllegalFormatException e) {
> 112: messages.warning(holder,
Doesn't CSR say that it is an *error*?
test/langtools/tools/javac/doctree/ValueTest.java line 108:
> 106: */
> 107:
> 108: /**
Consider adding the following cases:
* erroneous format
* valid format and reference followed by the trailing "junk"
-------------
PR: https://git.openjdk.java.net/jdk/pull/8565
