CLI Javadoc tree printer doesn't show errors #3219

Closed
baratali opened this Issue May 26, 2016 · 9 comments

Comments

Projects
None yet
5 participants
@baratali
Contributor

baratali commented May 26, 2016

myjavadoc.txt:

<audio>

Output:

git\checkstyle>java -jar target\checkstyle-6.18-SNAPSHOT-all.jar -j myjavadoc.txt

git\checkstyle>

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani May 29, 2016

Member

Exception should not be printed if that is not Checkstyle logic failure as we treat Javadoc parse failures as violations.

Printed exception from CLI is scary and users will open issues on this , as it is not clear for them is it their input file problem or Checkstyle internal problem.

So issue is not resolved but we did good step forward.

Member

romani commented May 29, 2016

Exception should not be printed if that is not Checkstyle logic failure as we treat Javadoc parse failures as violations.

Printed exception from CLI is scary and users will open issues on this , as it is not clear for them is it their input file problem or Checkstyle internal problem.

So issue is not resolved but we did good step forward.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Feb 28, 2017

Member

https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/DetailNodeTreeStringPrinter.java#L70

No exception should be thrown, or DetailNodeTreeStringPrinter.printFileAst should catch it and print only error message getParseErrorMessage(status.getParseErrorMessage()).

Member

romani commented Feb 28, 2017

https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/DetailNodeTreeStringPrinter.java#L70

No exception should be thrown, or DetailNodeTreeStringPrinter.printFileAst should catch it and print only error message getParseErrorMessage(status.getParseErrorMessage()).

@romani

This comment has been minimized.

Show comment
Hide comment
@lordozb

This comment has been minimized.

Show comment
Hide comment
@lordozb

lordozb Mar 8, 2017

I'm on it.

lordozb commented Mar 8, 2017

I'm on it.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Apr 28, 2017

Member

@kazachka , please help us this this issue. previous assignments are not valid as there were no actions for more than month

Member

romani commented Apr 28, 2017

@kazachka , please help us this this issue. previous assignments are not valid as there were no actions for more than month

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani May 2, 2017

Member

issue is resolved already:

$ java -jar checkstyle-7.6-all.jar -j myjavadoc.txt
Exception in thread "main" java.lang.IllegalArgumentException: [ERROR:0] Javadoc comment at column 1 has parse error. Missed HTML close tag 'audio'. Sometimes it means that close tag missed for one of previous tags.
	at com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter.parseJavadocAsDetailNode(DetailNodeTreeStringPrinter.java:71)
	at com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter.parseJavadocAsDetailNode(DetailNodeTreeStringPrinter.java:83)
	at com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter.parseFile(DetailNodeTreeStringPrinter.java:175)
	at com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter.printFileAst(DetailNodeTreeStringPrinter.java:59)
	at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:328)
	at com.puppycrawl.tools.checkstyle.Main.main(Main.java:174)

Member

romani commented May 2, 2017

issue is resolved already:

$ java -jar checkstyle-7.6-all.jar -j myjavadoc.txt
Exception in thread "main" java.lang.IllegalArgumentException: [ERROR:0] Javadoc comment at column 1 has parse error. Missed HTML close tag 'audio'. Sometimes it means that close tag missed for one of previous tags.
	at com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter.parseJavadocAsDetailNode(DetailNodeTreeStringPrinter.java:71)
	at com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter.parseJavadocAsDetailNode(DetailNodeTreeStringPrinter.java:83)
	at com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter.parseFile(DetailNodeTreeStringPrinter.java:175)
	at com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter.printFileAst(DetailNodeTreeStringPrinter.java:59)
	at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:328)
	at com.puppycrawl.tools.checkstyle.Main.main(Main.java:174)

@romani romani closed this May 2, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach May 2, 2017

Member

@romani Are you sure this issue is done?
You commented above you don't want it to be displayed as an exception because it would scare users.

#3219 (comment)

Printed exception from CLI is scary and users will open issues on this , as it is not clear for them is it their input file problem or Checkstyle internal problem.

#3219 (comment)

No exception should be thrown ... should catch it and print only error message

Member

rnveach commented May 2, 2017

@romani Are you sure this issue is done?
You commented above you don't want it to be displayed as an exception because it would scare users.

#3219 (comment)

Printed exception from CLI is scary and users will open issues on this , as it is not clear for them is it their input file problem or Checkstyle internal problem.

#3219 (comment)

No exception should be thrown ... should catch it and print only error message

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani May 2, 2017

Member

@rnveach , good point

but we do stacktrace for java grammar error, so for consistency it will be better to print whole stake trace:

$ cat Test.java 

public1 class Test {
}

$ java -jar checkstyle-7.6-all.jar -t Test.java
com.puppycrawl.tools.checkstyle.api.CheckstyleException: NoViableAltException occurred during the analysis of file Test.java.
	at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.parseFileText(AstTreeStringPrinter.java:249)
	at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.parseFile(AstTreeStringPrinter.java:223)
	at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printFileAst(AstTreeStringPrinter.java:69)
	at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:318)
	at com.puppycrawl.tools.checkstyle.Main.main(Main.java:174)
Caused by: Test.java:2:1: unexpected token: public1
	at com.puppycrawl.tools.checkstyle.grammars.GeneratedJavaRecognizer.compilationUnit(GeneratedJavaRecognizer.java:181)
	at com.puppycrawl.tools.checkstyle.TreeWalker.parse(TreeWalker.java:439)
	at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.parseFileText(AstTreeStringPrinter.java:242)
	... 4 more
Checkstyle ends with 1 errors.

Member

romani commented May 2, 2017

@rnveach , good point

but we do stacktrace for java grammar error, so for consistency it will be better to print whole stake trace:

$ cat Test.java 

public1 class Test {
}

$ java -jar checkstyle-7.6-all.jar -t Test.java
com.puppycrawl.tools.checkstyle.api.CheckstyleException: NoViableAltException occurred during the analysis of file Test.java.
	at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.parseFileText(AstTreeStringPrinter.java:249)
	at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.parseFile(AstTreeStringPrinter.java:223)
	at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.printFileAst(AstTreeStringPrinter.java:69)
	at com.puppycrawl.tools.checkstyle.Main.runCli(Main.java:318)
	at com.puppycrawl.tools.checkstyle.Main.main(Main.java:174)
Caused by: Test.java:2:1: unexpected token: public1
	at com.puppycrawl.tools.checkstyle.grammars.GeneratedJavaRecognizer.compilationUnit(GeneratedJavaRecognizer.java:181)
	at com.puppycrawl.tools.checkstyle.TreeWalker.parse(TreeWalker.java:439)
	at com.puppycrawl.tools.checkstyle.AstTreeStringPrinter.parseFileText(AstTreeStringPrinter.java:242)
	... 4 more
Checkstyle ends with 1 errors.

@lordozb

This comment has been minimized.

Show comment
Hide comment
@lordozb

lordozb May 3, 2017

lordozb commented May 3, 2017

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