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

CLI Javadoc tree printing does not check placement of Javadoc #4405

Closed
rnveach opened this Issue Jun 1, 2017 · 3 comments

Comments

@rnveach
Member

rnveach commented Jun 1, 2017

Identified when talking about it at checkstyle/contribution#237 (comment) and considered an issue from romani's comments at #4390 (comment) .

$ cat TestClass.java
public class TestClass {
    void method() {
/** <html */
    }
}

$ java -jar checkstyle-7.8-all.jar -J TestClass.java
Exception in thread "main" java.lang.IllegalArgumentException: [ERROR:3] Javadoc comment at column 7 has parse error. Details: no viable alternative at input '<html ' while parsing HTML_ELEMENT
    at com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter.parseJavadocAsDetailNode(DetailNodeTreeStringPrinter.java:71)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.parseAndPrintJavadocTree(AstTreeStringPrinter.java:117)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printJavaAndJavadocTree(AstTreeStringPrinter.java:99)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printJavaAndJavadocTree(AstTreeStringPrinter.java:103)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printJavaAndJavadocTree(AstTreeStringPrinter.java:103)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printJavaAndJavadocTree(AstTreeStringPrinter.java:103)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printJavaAndJavadocTree(AstTreeStringPrinter.java:103)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printJavaAndJavadocTree(AstTreeStringPrinter.java:103)
    at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printJavaAndJavadocTree(AstTreeStringPrinter.java:82)
    at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:333)
    at com.puppycrawl.tools.checkstyle.Main.main(Main.java:174)

From @romani 's comments in another issue, we shouldn't be parsing this "javadoc" as it is in an invalid position. We should treat is as a normal comment.

Issue is with the following code:

&& JavadocUtils.isJavadocComment(node.getParent())) {

public static boolean isJavadocComment(String commentContent) {

@rnveach rnveach changed the title from CLI Javadoc printing doesn't check placement of Javadoc to CLI Javadoc tree printing doesn't check placement of Javadoc Jun 1, 2017

@romani romani added the approved label Jun 7, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jun 13, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jun 13, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jun 13, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jun 14, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jun 14, 2017

@Vladlis Vladlis moved this from To Do to In Progress in Javadoc style coverage and parser optimization Jun 14, 2017

ps-sp added a commit to ps-sp/checkstyle that referenced this issue Jun 26, 2017

romani added a commit that referenced this issue Jun 27, 2017

@romani romani added the bug label Jun 27, 2017

@romani romani added this to the 8.0 milestone Jun 27, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 27, 2017

Member

fix is merged.

Member

romani commented Jun 27, 2017

fix is merged.

@romani romani closed this Jun 27, 2017

@Vladlis Vladlis reopened this Jun 27, 2017

@Vladlis

This comment has been minimized.

Show comment
Hide comment
@Vladlis

Vladlis Jun 27, 2017

Member

regression testing is required

Member

Vladlis commented Jun 27, 2017

regression testing is required

@romani romani changed the title from CLI Javadoc tree printing doesn't check placement of Javadoc to CLI Javadoc tree printing does not check placement of Javadoc Jul 2, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 5, 2017

Member

regression reports are in PR, they are fine.

Member

romani commented Jul 5, 2017

regression reports are in PR, they are fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment