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

JavadocStyle doesn't work with inline @return #12600

Closed
Juuxel opened this issue Jan 7, 2023 · 5 comments
Closed

JavadocStyle doesn't work with inline @return #12600

Juuxel opened this issue Jan 7, 2023 · 5 comments

Comments

@Juuxel
Copy link

Juuxel commented Jan 7, 2023

I have read check documentation: https://checkstyle.org/config_javadoc.html#JavadocStyle
I have downloaded the latest checkstyle from: https://checkstyle.org/cmdline.html#Download_and_Run
I have executed the cli and showed it below, as cli describes the problem better than 1,000 words

C:\Users\Juuz>javac Test.java

C:\Users\Juuz>type checkstyle.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">
    <module name="TreeWalker">
        <module name="JavadocStyle"/>
    </module>
</module>

C:\Users\Juuz>type Test.java
class Test {
    /**
     * {@return a string}
     */
    String foo() {
        return "Hello, world";
    }
}

C:\Users\Juuz>set RUN_LOCALE="-Duser.language=en -Duser.country=US"

C:\Users\Juuz>java %RUN_LOCALE% -jar Downloads\checkstyle-10.6.0-all.jar -c checkstyle.xml Test.java
Starting audit...
[ERROR] C:\Users\Juuz\Test.java:2: First sentence should end with a period. [JavadocStyle]
Audit done.
Checkstyle ends with 1 errors.

JavadocStyle requires a period after the first sentence, but it doesn't consider inline {@return} tags which insert a period after the contents.

For the example file with {@return a string}, this is what javadoc -package Test.java (JDK 19) generates:

method summary
method javadoc

This seems to be related to #10776, which is a similar issue with a different check.

Note: A workaround is to add a . after the {@return} tag.

@romani
Copy link
Member

romani commented Jan 7, 2023

attention

public class JavadocStyleCheck
extends AbstractCheck {

this module is old and does not use grammar and AST to deal with, I hope fix can be applied to Check, but it might be more challenging as parsing is done by regexp or other custom techniques.

@romani
Copy link
Member

romani commented Apr 30, 2023

We might be on to to extend this Check to support this for a bit.
But conceptually we need to remove from this Check such validation completely.

We already have validation for summary at https://checkstyle.org/config_javadoc.html#SummaryJavadoc there a lot of nuances of this validation, so better to not have super check that does all validations on javadoc.

@rnveach
Copy link
Member

rnveach commented May 1, 2023

The check shouldn't be removed, but the common functionality between it and other checks should be removed like we have done in other areas.

JavadocStyle has scope and java token support while SummaryJavadoc does not. It is not clear how checkHtml compares to violateExecutionOnNonTightHtml.

@Bananeweizen
Copy link
Contributor

I'm not sure if the refactoring of the Javadoc checks already covers this: Besides the mentioned checks, also the JavadocMethod check has the same issue.

0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Jan 16, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Jan 16, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Jan 16, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Jan 16, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Jan 16, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Jan 17, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Jan 17, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Jan 17, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Jan 17, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Jan 24, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Feb 14, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Feb 14, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Feb 15, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Feb 19, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Feb 19, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Feb 19, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Feb 20, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Feb 20, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Feb 21, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Feb 25, 2024
0xbakry added a commit to 0xbakry/checkstyle that referenced this issue Feb 27, 2024
@github-actions github-actions bot added this to the 10.14.2 milestone Mar 12, 2024
@romani
Copy link
Member

romani commented Mar 12, 2024

Fix is merged

@romani romani closed this as completed Mar 12, 2024
@romani romani added the bug label Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants