RFR: JDK-8298405: Support Markdown in Documentation Comments [v28]

[Pavel Rappo]

On Thu, 8 Feb 2024 21:46:34 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

>> I believe this is substantially covered in line 226-227.  See the third call to `test` in the following group of lines:
>> 
>> 
>> for (String src : sources) {
>>                 test(src);
>>                 test(src.replaceAll("@Deprecated", "/** @deprecated */"));
>>                 test(src.replaceAll("deprecated", "notDeprecated2") // change class name
>>                         .replaceAll("@Deprecated", "/// @deprecated\n"));
>>             }
>> 
>> 
>> * The first call, `test(src)`, runs all the test cases, using the `@Deprecated` annotation by default.  In these test cases, the name of the element encodes whether it is expected that the element is deprecated or not.
>> 
>> * The second call, `test(src.replaceAll("deprecated", "notDeprecated2")`, runs the test cases again, after changing the annotation to a traditional (`/** ...*/`) comment containing the `@deprecated` tag. This is a long-standing call, and tests the legacy behavior of `@deprecated` appearing in a traditional doc comment.  Effectively, it tests whether a `/** @deprecated */` comment has equivalent behavior to a `@Deprecated` comment.
>> 
>> * The third call is new to this PR and the functionality to support Markdown comments.  It makes two changes to the source for the test cases, before running them:
>>    1. as in the previous test, the annotations are replaced by a comment containing `@deprecated` -- except that this time, the comment is a new-style `/// ... ` comment instead of a traditional `/** ... */` comment, and ...
>>    2. because we do not expect `/// @deprecated` to indicate deprecation, we need to change the expected result for each test case, which we do by changing the element name for the test case.  The change is the first call to replace to avoid false positives after changing the doc comment. The change uses a new name, `notDeprecated2`, in which `notDeprecated` encodes the expected deprecation status, and the `2` is to differentiate the declarations from other declarations in the case case that already use the name `notDeprecated`.
>
> While the underlying mechanism in javac for indicating deprecation is the same for all, I accept this is primarily a test for generated class files. I can add a javadoc test for basic behavior of `@deprecated` in Markdown comments.

Thanks for confirming that there's a test to check that `/// @deprecated` is a no-op and for patiently explaining how that test works.

I confirm that test/langtools/tools/javac/classfiles/attributes/deprecated/DeprecatedTest.java starts failing if I introduce the following change:


--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavaTokenizer.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavaTokenizer.java
@@ -1638,7 +1638,7 @@ protected void scanDocComment() {
                             ? trimJavadocLineComment(line, indent)
                             : trimJavadocComment(line);
 
-                    if (cs == CommentStyle.JAVADOC_BLOCK) {
+//                    if (cs == CommentStyle.JAVADOC_BLOCK) {
                         // If standalone @deprecated tag
                         int pos = line.position();
                         line.skipWhitespace();
@@ -1652,7 +1652,7 @@ protected void scanDocComment() {
                         }
 
                         line.reset(pos);
-                    }
+//                    }
 
                     putLine(line);
                 }


I can also see that you have introduced a dedicated test/langtools/jdk/javadoc/doclet/testMarkdown/TestMarkdown.java#testDeprecated which also starts failing with that change of mine.

Please update the copyright year in test/langtools/jdk/javadoc/doclet/testMarkdown/TestMarkdown.java.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16388#discussion_r1484410402