RFR: JDK-8286101: Support formatting in @value tag
Jonathan Gibbons
jjg at openjdk.java.net
Tue May 17 22:00:20 UTC 2022
On Mon, 16 May 2022 10:07:18 GMT, Pavel Rappo <prappo at openjdk.org> wrote:
> 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.
1. It is very unlikely that you could/would configure mismatched versions of the jdk.javadoc and jdk.compiler modules.
2. Note that a new javadoc tool can read old doc comments (because the format is optional) and should not throw exceptions. The problem will occur if a new javadoc tool uses and old `DocCommentParser` in a mismatched module.
> 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)`.
I guess I don't think it's an issue but I'm OK with the proposed next form.
> 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.
That's not necessary, because we're in the private implementation world here. The important change you're suggesting is to the methods in the `DocTreeFactory` interface.
> 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*?
The CSR doesn't say, but I changed it to an error anyway.
> 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"
done
-------------
PR: https://git.openjdk.java.net/jdk/pull/8565
More information about the compiler-dev
mailing list