On Thu, 8 Feb 2024 21:46:34 GMT, Jonathan Gibbons <j...@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

Reply via email to