Regression in 13.9.0 after switching from JavadocStyle to SummaryJavadoc

[AlBundy33]

Since checkstyle 13.9.0 we have to switch from JavadocStyle to SummaryJavadoc due to #20610

After the update to 13.9.0 we removed

        <module name="JavadocStyle">
            <property name="checkHtml" value="false"/>
        </module>

in TreeWalker and replaced it with

        <module name="SummaryJavadoc"/>

Since this change we have more checkstyle errors because with JavadocStyle it was allowed to have an Empty summary if @PARAM and/or @return was documented.

With JavadocStyle it was also possible to end a sentence with . or ?.
With SummaryJavadoc javadocs like

/**
 * Are you sure?
**/
boolean doSomehing = false;

are invalid.

It would be nice if SummaryJavadoc has an compatibility-option for the old behavior.

[romani]

@gianmarcoschifone , please investigate what is going on here

https://checkstyle.org/checks/javadoc/summaryjavadoc.html#period is . .

[gianmarcoschifone]

With JavadocStyle it was also possible to end a sentence with . or ?. With SummaryJavadoc javadocs like

/**
 * Are you sure?
**/
boolean doSomehing = false;

are invalid.

SummaryJavadoc currently supports only one literal period value, so it cannot be configured to accept both . and ?.
To allow both, we’d need a code change.

---

Since this change we have more checkstyle errors because with JavadocStyle it was allowed to have an Empty summary if @PARAM and/or @return was documented.

JavadocStyle was not applying a correct validation. Javadoc summary is actually missing if only @param or @return is present.

@romani

[romani]

@AlBundy33, please report issue as requested in issue template. We need absolute clarity on what you use to have as behavior and has now.

@gianmarcoschifone , please share CLI output on javadoc jdk tool and html rendering on this snippet.

[gianmarcoschifone]

@gianmarcoschifone , please share CLI output on javadoc jdk tool and html rendering on this snippet.

gianmarco@Mac ~/Documents/Projects/testing-checkstyle % cat Test.java
public class Test {

    /**
     * Are you sure?
     **/
    boolean doSomehing = false;
}

gianmarco@Mac ~/Documents/Projects/testing-checkstyle % javadoc -d docs -private Test.java
Loading source file Test.java...
Constructing Javadoc information...
Building index for all the packages and classes...
Standard Doclet version 21.0.8+9-LTS
Building tree for all the packages and classes...
Generating docs/Test.html...
Test.java:1: warning: no comment
public class Test {
       ^
Test.java:1: warning: use of default constructor, which does not provide a comment
public class Test {
       ^
Generating docs/package-summary.html...
Copying file ./doc-files/image.png to directory docs/doc-files...
Generating docs/package-tree.html...
Generating docs/overview-tree.html...
Building index for all classes...
Generating docs/allclasses-index.html...
Generating docs/allpackages-index.html...
Generating docs/index-all.html...
Generating docs/search.html...
Generating docs/index.html...
Generating docs/help-doc.html...
2 warnings

@romani

[AlBundy33]

create Test.java

/**
  * Best class ever.
  */
public class Test {
  /**
    * Print to standard error?
    */
  boolean printToStdErr = false;
  /**
    * @param name the name to say hello to
    */
  public void hello(String name) {
    if (printToStdErr)
      System.err.println(buildHello(name));
    else
      System.out.println(buildHello(name));
  }

  /**
    * @param name the name to say hello to
    * @return the hello-message
    */
  private String buildHello(String name) {
     return "Hello " + name;
  }
}

and compile it

javac Test.java

create javadocstyle.xml

<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
  <property name="charset" value="UTF-8"/>
  <module name="TreeWalker">
    <module name="JavadocStyle">
      <property name="checkHtml" value="false"/>
    </module>
  </module>
</module>

create summaryjavadoc.xml

<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
  <property name="charset" value="UTF-8"/>
  <module name="TreeWalker">
    <module name="SummaryJavadoc"/>
  </module>
</module>

Download versions to test

curl -SLO https://github.com/checkstyle/checkstyle/releases/download/checkstyle-13.8.0/checkstyle-13.8.0-all.jar
curl -SLO https://github.com/checkstyle/checkstyle/releases/download/checkstyle-13.10.0/checkstyle-13.10.0-all.jar

test file with JavadocStyle and checktstyle 13.8

java -Duser.language=en -Duser.country=US -jar checkstyle-13.8.0-all.jar -c javadocstyle.xml Test.java
Starting audit...
Audit done.

test file with SummaryJavadoc and checkstyle 13.8

java -Duser.language=en -Duser.country=US -jar checkstyle-13.8.0-all.jar -c summaryjavadoc.xml Test.java
Starting audit...
[ERROR] ~/checkstyle/Test.java:5:6: First sentence of Javadoc is missing an ending period. [SummaryJavadoc]
[ERROR] ~/checkstyle/Test.java:9:6: Summary javadoc is missing. [SummaryJavadoc]
[ERROR] ~/checkstyle/Test.java:19:6: Summary javadoc is missing. [SummaryJavadoc]
Audit done.
Checkstyle ends with 3 errors.

test file with SummaryJavadoc and checkstyle 13.10

java -Duser.language=en -Duser.country=US -jar checkstyle-13.10.0-all.jar -c summaryjavadoc.xml Test.java
Starting audit...
[ERROR] ~/checkstyle/Test.java:5:6: First sentence of Javadoc is missing an ending period. [SummaryJavadoc]
[ERROR] ~/checkstyle/Test.java:9:6: Summary javadoc is missing. [SummaryJavadoc]
[ERROR] ~/checkstyle/Test.java:19:6: Summary javadoc is missing. [SummaryJavadoc]
Audit done.
Checkstyle ends with 3 errors.

conclusion

SummaryJavadoc needs some more settings to be a drop in replacement for JavadocStyle.

[romani]

Thanks a lot for all details.
Javadoc tool use ? as end of summary sentence.

User use ? make some lazy sentences that are not good English and should be changed if we strive for better code.

Such an situation.

But Check that we removed will not be restored.

Should we extend SummaryJavadoc? This is very weird edge case, that I can not justify.

Please use suppressions until you can improve wording.

We can keep issue open to collect more feedback from user, and possible solutions.

[AlBundy33]

What is wrong with using ?, ! or . in the first sentence?

tbh this does not sound like an edge case to me. 🤔

The whole idea of this issue was to be compatible with the removed JavadocStyle-Checker without the need to change all javadocs of codebases that have grown over the years.

This was also mentioned by another user: #20610 (comment)

[romani]

The whole idea of this issue was to be compatible with the removed JavadocStyle-Checker without the need to change all javadocs of codebases that have grown over the years.

It was not only reason. Amount of defects in this Check was too much. It was easier to drop it completely.
Nuance that we did not counted is that , sentence finished with any sentence separation is technically summary sentence. But it is not recommended end of sentence by style guides: Google, openjdk.

Each version bump for checkstyle imply change in behavior and new violations. Not that aggressive as other tools as we don't enforce new Checks.

If you want consistent behavior, checkstyle should not be updated.

Workaround for this issue is new Check with allowance on not only .. if someone needs it, it easy to create and keep in your own project and just another Checkstyle extension.

@AlBundy33 , I think for now I can recommend you only to suppress violations all legacy code that you don't want to update. Look at our filters, suppression by message and check name should be enough. Let us know if you need example of suppression.

---

Issue will stay open to gather more feedback from users.

[AlBundy33]

The whole idea of this issue was to be compatible with the removed JavadocStyle-Checker...
I meant that since I was the one who created this issue. 😉

I totally understand the reasons for dropping JavadocStyle, but in my opinion, suppressing the checks would not help because it would also suppress legitimate errors. 🤔

But we can wait and see if other users have similar use cases.

[apollox]

For me or better my team is the period not so problematic because we have only a few cases with an ! and these can be fixed easily.

Problematic is the other issue mentioned here with the @return tag.
In the description of SummaryJavadoc stand the following text

Summaries that contain a non-empty @return are allowed. Check also violate Javadoc that does not contain first sentence, though with @return a period is not required as the Javadoc tool adds it.

But Javadoc with only non-empty @return tags are reported as violation with the maven-checkstyle-plugin:3.6.0.
Example:

  /**
   * @return response representing a successful message-consumption
   */

Violation:

[WARNING] src\main\java\de\...:[22,6] (javadoc) SummaryJavadoc: Javadoc-Zusammenfassung fehlt.

Violation comes also when I set explicit a period at the end or use inline tag like

/** @return response representing a successful message-consumption. */

[gianmarcoschifone]

@apollox it refers to the inline tag {@return }, not to the block tag @return

[apollox]

Oh, my fault.

But when I used the inline tag {@return } I get an incompatibility with the check JavadocMethod because it look like that the property allowMissingReturnTag doesn't support the {@return } inline tag.
Code

  /** {@return response representing a successful message-consumption} */

Checkstyle-Rule

<module name="JavadocMethod">
  <property name="accessModifiers" value="public" />
  <property name="allowMissingParamTags" value="false" />
  <property name="allowMissingReturnTag" value="false" />
</module>

Violation

JavadocMethod: @return-Tag sollte vorhanden sein und eine Beschreibung haben.

If this is correct, I can create a specific issue.

[romani]

@apollox, please update your post to share all details as we demand for issues, it share is all details we need to avoid any conversations.

[Atharv3221]

Oh, my fault.

But when I used the inline tag {@return } I get an incompatibility with the check JavadocMethod because it look like that the property allowMissingReturnTag doesn't support the {@return } inline tag. Code

/** {@return response representing a successful message-consumption} */
Checkstyle-Rule

Violation

JavadocMethod: @return-Tag sollte vorhanden sein und eine Beschreibung haben.

If this is correct, I can create a specific issue.

@apollox try using <property name="allowInlineReturn" value="true"/>

for

public class Test {
    int id;

    /** {@return customer id} */
    public int method() {
        return id;
    }
}

with config

    <module name="JavadocMethod">
      <property name="accessModifiers" value="public" />
      <property name="allowMissingParamTags" value="false" />
      <property name="allowMissingReturnTag" value="false" />
    </module>

atharv@atharv-IdeaPad-3-15IIL05:~/test$ java -jar checkstyle-13.10.0-all.jar -c config.xml Test.java
Starting audit...
[ERROR] /home/atharv/test/Test.java:5: @return tag should be present and have description. [JavadocMethod]
Audit done.
Checkstyle ends with 1 errors.

with config

    <module name="JavadocMethod">
      <property name="accessModifiers" value="public" />
      <property name="allowMissingParamTags" value="false" />
      <property name="allowMissingReturnTag" value="false" />
      <property name="allowInlineReturn" value="true" />
    </module>

atharv@atharv-IdeaPad-3-15IIL05:~/test$ java -jar checkstyle-13.10.0-all.jar -c config.xml Test.java
Starting audit...
Audit done.

[apollox]

Thanks @Atharv3221 , that fixed the problem.