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