Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JDK-8272374: doclint should report missing "body" comments #5106

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion make/modules/java.desktop/Java.gmk
Expand Up @@ -23,7 +23,7 @@
# questions.
#

DOCLINT += -Xdoclint:all/protected,-reference \
DOCLINT += -Xdoclint:all/protected,-reference,-missing \
'-Xdoclint/package:java.*,javax.*'
COPY += .gif .png .wav .txt .xml .css .pf
CLEAN += iio-plugin.properties cursors.properties
Expand Down
Expand Up @@ -182,9 +182,6 @@ public Void scan(DocCommentTree tree, TreePath p) {
reportMissing("dc.missing.comment");
return null;
}



} else {
if (tree == null) {
if (isDefaultConstructor()) {
Expand All @@ -195,6 +192,18 @@ public Void scan(DocCommentTree tree, TreePath p) {
reportMissing("dc.missing.comment");
}
return null;
} else if (tree.getFirstSentence().isEmpty() && !isOverridingMethod) {
if (tree.getBlockTags().isEmpty()) {
reportMissing("dc.empty.comment");
return null;
} else {
DocTree firstTag = tree.getBlockTags().get(0);
// Don't report an empty description if the comment begins with @deprecated,
// because javadoc will use the content of that tag in summary tables.
if (firstTag.getKind() != DocTree.Kind.DEPRECATED) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we only check the first block tag here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Various reasons,

  1. It seems the convention is to simply prefix an existing comment with @deprecated or to replace the existing body description with @deprecated reason-why-deprecated
  2. This is only for the case when there is no body description; it seems hard to imagine that someone would start a comment with @see @param @return etc and then have the @deprecated tag.

That being said, it would be easy enough to change the code to check for any instance of @deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the downstream code does not impose any ordering restrictions on the tags, it is probably with doing the same here.

env.messages.report(MISSING, Kind.WARNING, tree, "dc.empty.description");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to not use reportMissing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't have the right signature. reportMissing reports a missing comment on an element; here, we're reporting a missing description within a DocTree. There's a similar occurrence at line 1214.

}
}
}
}

Expand Down
Expand Up @@ -39,6 +39,8 @@ dc.bad.option = bad option: {0}
dc.bad.value.for.option = bad value for option: {0} {1}
dc.default.constructor = use of default constructor, which does not provide a comment
dc.empty = no description for @{0}
dc.empty.comment = empty comment
dc.empty.description = no initial description
dc.entity.invalid = invalid entity &{0};
dc.exception.not.thrown = exception not thrown: {0}
dc.exists.param = @param "{0}" has already been specified
Expand Down
51 changes: 23 additions & 28 deletions test/langtools/jdk/javadoc/tool/doclint/DocLintTest.java
Expand Up @@ -32,7 +32,6 @@
import java.io.PrintWriter;
import java.io.StringWriter;
import java.net.URI;
import java.util.Arrays;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
Expand Down Expand Up @@ -63,25 +62,21 @@ public static void main(String... args) throws Exception {

final String code =
/* 01 */ "/** Class comment. */\n" +
/* 02 */ "public class Test { /** */ Test() { }\n" +
/* 02 */ "public class Test { /** Constructor comment. */ Test() { }\n" +
/* 03 */ " /** Method comment. */\n" +
/* 04 */ " public void method() { }\n" +
/* 05 */ "\n" +
/* 06 */ " /** Syntax < error. */\n" +
/* 07 */ """
\s private void syntaxError() { }
""" +
/* 07 */ " private void syntaxError() { }\n" +
/* 08 */ "\n" +
/* 09 */ " /** @see DoesNotExist */\n" +
/* 10 */ """
\s protected void referenceError() { }
""" +
/* 11 */ "\n" +
/* 12 */ " /** @return */\n" +
/* 13 */ """
\s public int emptyReturn() { return 0; }
""" +
/* 14 */ "}\n";
/* 09 */ " /** Description. \n" +
/* 10 */ " * @see DoesNotExist */\n" +
/* 11 */ " protected void referenceError() { }\n" +
/* 12 */ "\n" +
/* 13 */ " /** Description. \n" +
/* 14 */ " * @return */\n" +
/* 15 */ " public int emptyReturn() { return 0; }\n" +
/* 16 */ "}\n";

final String p1Code =
/* 01 */ "package p1;\n" +
Expand All @@ -103,21 +98,21 @@ public static void main(String... args) throws Exception {
private enum Message {
// doclint messages
DL_ERR6(ERROR, "Test.java:6:16: compiler.err.proc.messager: malformed HTML"),
DL_ERR9(ERROR, "Test.java:9:14: compiler.err.proc.messager: reference not found"),
DL_WRN12(WARNING, "Test.java:12:9: compiler.warn.proc.messager: no description for @return"),
DL_ERR10(ERROR, "Test.java:10:13: compiler.err.proc.messager: reference not found"),
DL_WRN14(WARNING, "Test.java:14:8: compiler.warn.proc.messager: no description for @return"),

DL_ERR_P1TEST(ERROR, "P1Test.java:3:16: compiler.err.proc.messager: malformed HTML"),
DL_ERR_P2TEST(ERROR, "P2Test.java:3:16: compiler.err.proc.messager: malformed HTML"),
DL_WARN_P1TEST(WARNING, "P1Test.java:2:8: compiler.warn.proc.messager: no comment"),
DL_WARN_P2TEST(WARNING, "P2Test.java:2:8: compiler.warn.proc.messager: no comment"),

// doclint messages when -XDrawDiagnostics is not in effect
DL_ERR9A(ERROR, "Test.java:9: error: reference not found"),
DL_WRN12A(WARNING, "Test.java:12: warning: no description for @return"),
DL_ERR10A(ERROR, "Test.java:10: error: reference not found"),
DL_WRN14A(WARNING, "Test.java:14: warning: no description for @return"),

// javadoc messages about bad content: these should only appear when doclint is disabled
JD_WRN10(WARNING, "Test.java:10: warning: Tag @see: reference not found: DoesNotExist"),
JD_WRN13(WARNING, "Test.java:13: warning: @return tag has no arguments."),
JD_WRN14(WARNING, "Test.java:14: warning: @return tag has no arguments."),

// javadoc messages for bad options
OPT_BADARG(ERROR, "error: Invalid argument for -Xdoclint option"),
Expand Down Expand Up @@ -156,43 +151,43 @@ void run() throws Exception {

test(List.of(htmlVersion),
Main.Result.ERROR,
EnumSet.of(Message.DL_ERR9A, Message.DL_WRN12A));
EnumSet.of(Message.DL_ERR10A, Message.DL_WRN14A));

test(List.of(htmlVersion, rawDiags),
Main.Result.ERROR,
EnumSet.of(Message.DL_ERR9, Message.DL_WRN12));
EnumSet.of(Message.DL_ERR10, Message.DL_WRN14));

// test(List.of("-Xdoclint:none"),
// Main.Result.OK,
// EnumSet.of(Message.JD_WRN10, Message.JD_WRN13));

test(List.of(htmlVersion, rawDiags, "-Xdoclint"),
Main.Result.ERROR,
EnumSet.of(Message.DL_ERR9, Message.DL_WRN12));
EnumSet.of(Message.DL_ERR10, Message.DL_WRN14));

test(List.of(htmlVersion, rawDiags, "-Xdoclint:all/public"),
Main.Result.ERROR,
EnumSet.of(Message.OPT_BADQUAL));

test(List.of(htmlVersion, rawDiags, "-Xdoclint:all", "-public"),
Main.Result.OK,
EnumSet.of(Message.DL_WRN12));
EnumSet.of(Message.DL_WRN14));

test(List.of(htmlVersion, rawDiags, "-Xdoclint:missing"),
Main.Result.OK,
EnumSet.of(Message.DL_WRN12));
EnumSet.of(Message.DL_WRN14));

test(List.of(htmlVersion, rawDiags, "-private"),
Main.Result.ERROR,
EnumSet.of(Message.DL_ERR6, Message.DL_ERR9, Message.DL_WRN12));
EnumSet.of(Message.DL_ERR6, Message.DL_ERR10, Message.DL_WRN14));

test(List.of(htmlVersion, rawDiags, "-Xdoclint:missing,syntax", "-private"),
Main.Result.ERROR,
EnumSet.of(Message.DL_ERR6, Message.DL_WRN12));
EnumSet.of(Message.DL_ERR6, Message.DL_WRN14));

test(List.of(htmlVersion, rawDiags, "-Xdoclint:reference"),
Main.Result.ERROR,
EnumSet.of(Message.DL_ERR9));
EnumSet.of(Message.DL_ERR10));

test(List.of(htmlVersion, rawDiags, "-Xdoclint:badarg"),
Main.Result.ERROR,
Expand Down
8 changes: 4 additions & 4 deletions test/langtools/tools/doclint/AccessTest.java
Expand Up @@ -18,8 +18,8 @@
* @run main DocLintTester -Xmsgs:all,-syntax/private -ref AccessTest.package.out AccessTest.java
*/

/** */
public class AccessTest { /** */ AccessTest() { }
/** . */
public class AccessTest { /** . */ AccessTest() { }
/**
* public a < b
*/
Expand All @@ -41,8 +41,8 @@ void syntax_error() { }
private void private_syntax_error() { }
}

/** */
class AccessTest2 { /** */ AccessTest2() { }
/** Class comment. */
class AccessTest2 { /** Constructor comment. */ AccessTest2() { }
/**
* public a < b
*/
Expand Down
2 changes: 1 addition & 1 deletion test/langtools/tools/doclint/CustomTagTest.java
Expand Up @@ -11,7 +11,7 @@
* @author bpatel
*/

/**
/** .
* @customTag Text for a custom tag.
* @custom.tag Text for another custom tag.
* @unknownTag Text for an unknown tag.
Expand Down
9 changes: 6 additions & 3 deletions test/langtools/tools/doclint/EmptyAuthorTest.java
Expand Up @@ -8,7 +8,10 @@
* @run main DocLintTester -Xmsgs:missing -ref EmptyAuthorTest.out EmptyAuthorTest.java
*/

/** @author */
/**
* .
* @author
*/
public class EmptyAuthorTest {
/** */ EmptyAuthorTest() { }
}
/** . */ EmptyAuthorTest() { }
}
6 changes: 3 additions & 3 deletions test/langtools/tools/doclint/EmptyAuthorTest.out
@@ -1,5 +1,5 @@
EmptyAuthorTest.java:11: warning: no description for @author
/** @author */
^
EmptyAuthorTest.java:13: warning: no description for @author
* @author
^
1 warning

79 changes: 79 additions & 0 deletions test/langtools/tools/doclint/EmptyDescriptionTest.java
@@ -0,0 +1,79 @@

/*
* @test /nodynamiccopyright/
* @bug 8272374
* @summary doclint should report missing "body" comments
* @modules jdk.javadoc/jdk.javadoc.internal.doclint
* @build DocLintTester
* @run main DocLintTester -Xmsgs:-missing EmptyDescriptionTest.java
* @run main DocLintTester -Xmsgs:missing -ref EmptyDescriptionTest.out EmptyDescriptionTest.java
*/

/** . */
public class EmptyDescriptionTest {
// a default constructor triggers its own variant of "no comment"

// no comment
public int f1;

// empty comment
/** */
public int f2;

// empty description
/**
* @since 1.0
*/
public int f3;

// deprecated: no diagnostic
/**
* @deprecated do not use
*/
public int f4;

// no comment
public int m1() { return 0; }

// empty comment
/** */
public int m2() { return 0; }

// empty description
/**
* @return 0
*/
public int m3() { return 0; }

// deprecated: no diagnostic
/**
* @deprecated do not use
* @return 0
*/
public int m4() { return 0; };

/**
* A class containing overriding methods.
* Overriding methods with missing/empty comments do not generate messages
* since they are presumed to inherit descriptions as needed.
*/
public static class Nested extends EmptyDescriptionTest {
/** . */ Nested() { }

@Override
public int m1() { return 1; }

// empty comment
/** */
@Override
public int m2() { return 1; }

// empty description
/**
* @return 1
*/
@Override
public int m3() { return 1; }

}
}
22 changes: 22 additions & 0 deletions test/langtools/tools/doclint/EmptyDescriptionTest.out
@@ -0,0 +1,22 @@
EmptyDescriptionTest.java:13: warning: use of default constructor, which does not provide a comment
public class EmptyDescriptionTest {
^
EmptyDescriptionTest.java:17: warning: no comment
public int f1;
^
EmptyDescriptionTest.java:21: warning: empty comment
public int f2;
^
EmptyDescriptionTest.java:25: warning: no initial description
* @since 1.0
^
EmptyDescriptionTest.java:36: warning: no comment
public int m1() { return 0; }
^
EmptyDescriptionTest.java:40: warning: empty comment
public int m2() { return 0; }
^
EmptyDescriptionTest.java:44: warning: no initial description
* @return 0
^
7 warnings
7 changes: 5 additions & 2 deletions test/langtools/tools/doclint/EmptyExceptionTest.java
Expand Up @@ -9,7 +9,10 @@
*/

/** . */
public class EmptyExceptionTest { /** */ EmptyExceptionTest() { }
/** @exception NullPointerException */
public class EmptyExceptionTest { /** . */ EmptyExceptionTest() { }
/**
* .
* @exception NullPointerException
*/
void emptyException() throws NullPointerException { }
}
6 changes: 3 additions & 3 deletions test/langtools/tools/doclint/EmptyExceptionTest.out
@@ -1,4 +1,4 @@
EmptyExceptionTest.java:13: warning: no description for @exception
/** @exception NullPointerException */
^
EmptyExceptionTest.java:15: warning: no description for @exception
* @exception NullPointerException
^
1 warning
5 changes: 4 additions & 1 deletion test/langtools/tools/doclint/EmptyParamTest.java
Expand Up @@ -10,6 +10,9 @@

/** . */
public class EmptyParamTest { /** . */ EmptyParamTest() { }
/** @param i */
/**
* .
* @param i
*/
void emptyParam(int i) { }
}
6 changes: 3 additions & 3 deletions test/langtools/tools/doclint/EmptyParamTest.out
@@ -1,5 +1,5 @@
EmptyParamTest.java:13: warning: no description for @param
/** @param i */
^
EmptyParamTest.java:15: warning: no description for @param
* @param i
^
1 warning

5 changes: 4 additions & 1 deletion test/langtools/tools/doclint/EmptyReturnTest.java
Expand Up @@ -10,6 +10,9 @@

/** . */
public class EmptyReturnTest { /** . */ EmptyReturnTest() { }
/** @return */
/**
* .
* @return
*/
int emptyReturn() { return 0; }
}
7 changes: 3 additions & 4 deletions test/langtools/tools/doclint/EmptyReturnTest.out
@@ -1,5 +1,4 @@
EmptyReturnTest.java:13: warning: no description for @return
/** @return */
^
EmptyReturnTest.java:15: warning: no description for @return
* @return
^
1 warning

1 change: 1 addition & 0 deletions test/langtools/tools/doclint/EmptySerialFieldTest.java
Expand Up @@ -15,6 +15,7 @@
public class EmptySerialFieldTest implements Serializable { /** . */ EmptySerialFieldTest() { }

/**
* .
* @serialField empty String
*/
private static final ObjectStreamField[] serialPersistentFields = {
Expand Down