checkstyle / checkstyle

Checkstyle is a development tool to help programmers write Java code that adheres to a coding standard. By default it supports the Google Java Style Guide and Sun Code Conventions, but is highly configurable. It can be invoked with an ANT task and a command line program.
https://checkstyle.org
GNU Lesser General Public License v2.1
8.32k stars 3.66k forks source link

MissingJavadocType - Incorrect Location Being Reported #15723

Open carldenigma opened 2 weeks ago

carldenigma commented 2 weeks ago

I have been working on upgrading an old Checkstyle configuration to the latest version. During this process, I noticed an issue related to missing Javadoc type for CLASS_DEF. The missing javadoc is now being incorrectly associated with a different line. After reviewing previous versions, I have identified that this change occurred in version 8.34.

Simple Test Java File:

package models;

import javax.persistence.Entity;

@Entity
public class Example {

}

Test Checksum Config:

<?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="fileExtensions" value="java"/>
    <module name="TreeWalker">
        <module name="MissingJavadocType">
          <property name="tokens" value="INTERFACE_DEF, CLASS_DEF"/>
        </module>
    </module>
</module>

All versions show the correct tree in that the CLASS_DEF is on [5:0]

xxxx@xxxx:~/test$ java -jar checkstyle-10.18.2-all.jar -T Example.java | grep 5:
`--CLASS_DEF -> CLASS_DEF [5:0]
    |--MODIFIERS -> MODIFIERS [5:0]
    |   |--ANNOTATION -> ANNOTATION [5:0]
    |   |   |--AT -> @ [5:0]
    |   |   `--IDENT -> Entity [5:1]

Expected Result: MissingJavdocType will be reported for [5:] CLASS_DEF like it is in checksum 8.33

Output from 8.33 last known working version:

xxxx@xxxx:~/test$ java -jar checkstyle-8.33-all.jar -c checkstyle-config-javadocs.xml Example.java
Starting audit...
[ERROR] /PATH_TO_FILE/Example.java:5: Missing a Javadoc comment. [MissingJavadocType]

Actual Result: In all versions since 8.34 the ERROR line for MissingJavadocType gets reported as being on the annotation indent for Entity [5:1] instead of [5:]

8.34 output

xxxx@xxxx:~/test$ java -jar checkstyle-8.34-all.jar -c checkstyle-config-javadocs.xml Example.java
Starting audit...
[ERROR] /PATH_TO_FILE/Example.java:5:1: Missing a Javadoc comment. [MissingJavadocType]

latest 10.18.2 output

xxxx@xxxx:~/test$ java -jar checkstyle-10.18.2-all.jar -c checkstyle-config-javadocs.xml Example.java
Starting audit...
[ERROR] /PATH_TO_FILE/Example.java:5:1: Missing a Javadoc comment. [MissingJavadocType]
romani commented 2 weeks ago

https://checkstyle.org/releasenotes_old_8-0_8-34.html#Release_8.34

Update AbstractChecks to log DetailAST - MissingJavadocType. Author: HuGanghui #7747

romani commented 2 weeks ago

Nuance is that public class Example is on line 6 `--CLASS_DEF -> CLASS_DEF [5:0] beginning of class declaration is on line 5.

So this is visual problem, humans naturally expects violation to be on line where class keyword is placed.

We started to report violations on AST nodes, to allow xpath suppression to work.

User will need to put javadoc comment above line 5 , not above line 6. Javadoc tool of jdk demands comments to be above annotations.

I see point of confusion, but in same time can not approve issue. Lets collect more opinions on this problem.

carldenigma commented 2 weeks ago

Thanks for the explanation

romani commented 2 weeks ago

@carldenigma, consider closing issue if you agree with reasoning from me.