Skip to content

Commit

Permalink
Issue checkstyle#4390: Modified error handling of errors that are to …
Browse files Browse the repository at this point in the history 
…be generated when missing HTML tags are encountered while parsing javadoc
  • Loading branch information
@voidfist
voidfist committed Jul 13, 2017
1 parent 5b5fa83 commit d905eb5
Show file tree
Hide file tree
Showing 11 changed files with 208 additions and 54 deletions.
6 changes: 6 additions & 0 deletions config/findbugs-exclude.xml
View file
Expand Up @@ -104,4 +104,10 @@
<Method name="beginTree"/>
<Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD"/>
</Match>
<Match>
<!-- false-positive. Bugs reported even though casting is done only after verification using instanceof -->
<Class name="com.puppycrawl.tools.checkstyle.JavadocDetailNodeParser"/>
<Method name="parseJavadocAsDetailNode"/>
<Bug pattern="BC_UNCONFIRMED_CAST_OF_RETURN_VALUE"/>
</Match>
</FindBugsFilter>
2 changes: 1 addition & 1 deletion src/main/java/com/puppycrawl/tools/checkstyle/DetailNodeTreeStringPrinter.java
View file
Expand Up @@ -88,7 +88,7 @@ private static DetailNode parseJavadocAsDetailNode(String javadocComment) {
* @param parseErrorMessage ParseErrorMessage
* @return error message
*/
private static String getParseErrorMessage(ParseErrorMessage parseErrorMessage) {
public static String getParseErrorMessage(ParseErrorMessage parseErrorMessage) {
final LocalizedMessage lmessage = new LocalizedMessage(
parseErrorMessage.getLineNumber(),
"com.puppycrawl.tools.checkstyle.checks.javadoc.messages",
Expand Down
80 changes: 67 additions & 13 deletions src/main/java/com/puppycrawl/tools/checkstyle/JavadocDetailNodeParser.java
View file
Expand Up @@ -19,14 +19,21 @@

package com.puppycrawl.tools.checkstyle;

import java.util.ArrayDeque;
import java.util.List;

import org.antlr.v4.runtime.ANTLRInputStream;
import org.antlr.v4.runtime.BailErrorStrategy;
import org.antlr.v4.runtime.BaseErrorListener;
import org.antlr.v4.runtime.BufferedTokenStream;
import org.antlr.v4.runtime.CommonTokenStream;
import org.antlr.v4.runtime.FailedPredicateException;
import org.antlr.v4.runtime.NoViableAltException;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.RecognitionException;
import org.antlr.v4.runtime.Recognizer;
import org.antlr.v4.runtime.Token;
import org.antlr.v4.runtime.misc.Interval;
import org.antlr.v4.runtime.misc.ParseCancellationException;
import org.antlr.v4.runtime.tree.ParseTree;
import org.antlr.v4.runtime.tree.TerminalNode;
Expand Down Expand Up @@ -125,10 +132,22 @@ public ParseStatus parseJavadocAsDetailNode(DetailAST javadocCommentAst) {
result.setTree(tree);
}
catch (ParseCancellationException | IllegalArgumentException ex) {
// If syntax error occurs then message is printed by error listener
// and parser throws this runtime exception to stop parsing.
// Just stop processing current Javadoc comment.
ParseErrorMessage parseErrorMessage = errorListener.getErrorMessage();
ParseErrorMessage parseErrorMessage = null;

if (ex.getCause() instanceof FailedPredicateException
|| ex.getCause() instanceof NoViableAltException) {
final RecognitionException recognitionEx = (RecognitionException) ex.getCause();
if (recognitionEx.getCtx() instanceof JavadocParser.HtmlTagContext) {
parseErrorMessage = generateMissedHtmlTagErrorMessage(recognitionEx);
}
}

if (parseErrorMessage == null) {
// If syntax error occurs then message is printed by error listener
// and parser throws this runtime exception to stop parsing.
// Just stop processing current Javadoc comment.
parseErrorMessage = errorListener.getErrorMessage();
}

// There are cases when antlr error listener does not handle syntax error
if (parseErrorMessage == null) {
Expand Down Expand Up @@ -442,6 +461,47 @@ private static String getNodeClassNameWithoutContext(ParseTree node) {
return className.substring(0, className.length() - contextLength);
}

/**
* Method to generate an error message about missing HTML tag if the ANTLR error is indeed
* because of the missing tag.
* @param exception {@code NoViableAltException} object catched while parsing javadoc
* @return returns appropriate {@code ParseErrorMessage} if a HTML close tag is missed;
* null otherwise
*/
private ParseErrorMessage generateMissedHtmlTagErrorMessage(RecognitionException exception) {
ParseErrorMessage parseErrorMessage = null;
final Interval sourceInterval = exception.getCtx().getSourceInterval();
final List<Token> tokenList = ((BufferedTokenStream) exception.getInputStream())
.getTokens(sourceInterval.a, sourceInterval.b);
final ArrayDeque<Token> stack = new ArrayDeque<>();
for (int i = 0; i < tokenList.size(); i++) {
final Token token = tokenList.get(i);
if (token.getType() == JavadocTokenTypes.HTML_TAG_NAME
&& tokenList.get(i - 1).getType() == JavadocTokenTypes.OPEN) {
stack.push(token);
}
else if (token.getType() == JavadocTokenTypes.HTML_TAG_NAME && !stack.isEmpty()) {
final Token htmlTagNameStart = stack.pop();
if (!htmlTagNameStart.getText().equals(token.getText())) {
parseErrorMessage = new ParseErrorMessage(
errorListener.offset + htmlTagNameStart.getLine(),
MSG_JAVADOC_MISSED_HTML_CLOSE,
htmlTagNameStart.getCharPositionInLine(),
htmlTagNameStart.getText());
}
}
}
if (parseErrorMessage == null) {
final Token htmlTagNameStart = stack.pop();
parseErrorMessage = new ParseErrorMessage(
errorListener.offset + htmlTagNameStart.getLine(),
MSG_JAVADOC_MISSED_HTML_CLOSE,
htmlTagNameStart.getCharPositionInLine(),
htmlTagNameStart.getText());
}
return parseErrorMessage;
}

/**
* Custom error listener for JavadocParser that prints user readable errors.
*/
Expand Down Expand Up @@ -494,17 +554,11 @@ public void syntaxError(
int line, int charPositionInLine,
String msg, RecognitionException ex) {
final int lineNumber = offset + line;
final Token token = (Token) offendingSymbol;

if (MSG_JAVADOC_MISSED_HTML_CLOSE.equals(msg)) {
errorMessage = new ParseErrorMessage(lineNumber,
MSG_JAVADOC_MISSED_HTML_CLOSE, charPositionInLine, token.getText());

throw new IllegalArgumentException(msg);
}
else if (MSG_JAVADOC_WRONG_SINGLETON_TAG.equals(msg)) {
if (MSG_JAVADOC_WRONG_SINGLETON_TAG.equals(msg)) {
errorMessage = new ParseErrorMessage(lineNumber,
MSG_JAVADOC_WRONG_SINGLETON_TAG, charPositionInLine, token.getText());
MSG_JAVADOC_WRONG_SINGLETON_TAG, charPositionInLine,
((Token) offendingSymbol).getText());

throw new IllegalArgumentException(msg);
}
Expand Down
12 changes: 1 addition & 11 deletions src/main/resources/com/puppycrawl/tools/checkstyle/grammars/javadoc/JavadocParser.g4
View file
Expand Up @@ -34,7 +34,6 @@ options { tokenVocab=JavadocLexer; }
boolean isSameTagNames(ParserRuleContext htmlTagOpen, ParserRuleContext htmlTagClose) {
String openTag = htmlTagOpen.getToken(HTML_TAG_NAME, 0).getText().toLowerCase();
String closeTag = htmlTagClose.getToken(HTML_TAG_NAME, 0).getText().toLowerCase();
System.out.println(openTag + " - " + closeTag);
return openTag.equals(closeTag);
}
}
Expand Down Expand Up @@ -116,16 +115,7 @@ htmlTag: htmlElementOpen (htmlElement
| text
//{isSameTagNames($htmlElementOpen.ctx, $htmlElementClose.ctx)}?
| javadocInlineTag)* htmlElementClose

| htmlElementOpen (htmlElement
| ({!isNextJavadocTag()}? LEADING_ASTERISK)
| htmlComment
| CDATA
| NEWLINE
| text
| javadocInlineTag)*
{notifyErrorListeners($htmlElementOpen.ctx.getToken(HTML_TAG_NAME, 0).getSymbol()
, "javadoc.missed.html.close", null);}
{isSameTagNames($htmlElementOpen.ctx, $htmlElementClose.ctx)}?
;

//////////////////////////////////////////////////////////////////////////////////////
Expand Down
112 changes: 87 additions & 25 deletions src/test/java/com/puppycrawl/tools/checkstyle/DetailNodeTreeStringPrinterTest.java
View file
Expand Up @@ -19,14 +19,15 @@

package com.puppycrawl.tools.checkstyle;

import static com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter.getParseErrorMessage;
import static com.puppycrawl.tools.checkstyle.JavadocDetailNodeParser.MSG_JAVADOC_MISSED_HTML_CLOSE;
import static com.puppycrawl.tools.checkstyle.JavadocDetailNodeParser.ParseErrorMessage;
import static com.puppycrawl.tools.checkstyle.internal.TestUtils.assertUtilsClassHasPrivateConstructor;
import static org.junit.Assert.assertEquals;

import java.io.File;
import java.io.IOException;
import java.lang.reflect.Method;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Locale;

import org.junit.Assert;
Expand All @@ -37,10 +38,11 @@
import com.puppycrawl.tools.checkstyle.api.LocalizedMessage;
import com.puppycrawl.tools.checkstyle.api.TokenTypes;

public class DetailNodeTreeStringPrinterTest {
public class DetailNodeTreeStringPrinterTest extends BaseCheckTestSupport {

private static String getPath(String filename) {
return "src/test/resources/com/puppycrawl/tools/checkstyle/astprinter/" + filename;
@Override
protected String getPath(String filename) throws IOException {
return super.getPath("astprinter" + File.separator + filename);
}

@Test
Expand All @@ -50,27 +52,20 @@ public void testIsProperUtilsClass() throws ReflectiveOperationException {

@Test
public void testParseFile() throws Exception {
final String actual = DetailNodeTreeStringPrinter.printFileAst(
new File(getPath("InputJavadocComment.javadoc")))
.replaceAll("\\\\r\\\\n", "\\\\n");
final String expected = new String(Files.readAllBytes(Paths.get(
getPath("expectedInputJavadocComment.txt"))), StandardCharsets.UTF_8)
.replaceAll("\\\\r\\\\n", "\\\\n");
assertEquals(expected, actual);
verifyJavadocTree(getPath("expectedInputJavadocComment.txt"),
getPath("InputJavadocComment.javadoc"));
}

@Test
public void testParseFileWithError() throws Exception {
LocalizedMessage.setLocale(Locale.ROOT);
try {
DetailNodeTreeStringPrinter.printFileAst(
new File(getPath("InputJavadocWithError.javadoc")));
Assert.fail("Javadoc parser didn't failed on missing end tag");
Assert.fail("Javadoc parser didn't fail on missing end tag");
}
catch (IllegalArgumentException ex) {
final String expected = "[ERROR:0] Javadoc comment at column 1 has parse error. "
+ "Missed HTML close tag 'qwe'. Sometimes it means that close tag missed "
+ "for one of previous tags.";
final String expected = getParseErrorMessage(new ParseErrorMessage(
0, MSG_JAVADOC_MISSED_HTML_CLOSE, 1, "qwe"));
assertEquals(expected, ex.getMessage());
}
}
Expand Down Expand Up @@ -100,12 +95,79 @@ public void testCreationOfFakeCommentBlock() throws Exception {

@Test
public void testNoUnnecessaryTextinJavadocAst() throws Exception {
final String actual = DetailNodeTreeStringPrinter.printFileAst(
new File(getPath("InputNoUnnecessaryTextInJavadocAst.javadoc")))
.replaceAll("\\\\r\\\\n", "\\\\n");
final String expected = new String(Files.readAllBytes(Paths.get(
getPath("expectedNoUnnecessaryTextInJavadocAst.txt"))), StandardCharsets.UTF_8)
.replaceAll("\\\\r\\\\n", "\\\\n");
assertEquals(expected, actual);
verifyJavadocTree(getPath("expectedNoUnnecessaryTextInJavadocAst.txt"),
getPath("InputNoUnnecessaryTextInJavadocAst.javadoc"));
}

@Test
public void testUnescapedJavaCodeWithGenericsInJavadoc() throws IOException {
try {
DetailNodeTreeStringPrinter.printFileAst(new File(getPath(
"InputUnescapedJavaCodeWithGenericsInJavadoc.javadoc")));
}
catch (IllegalArgumentException ex) {
final String expected = getParseErrorMessage(new ParseErrorMessage(
35, MSG_JAVADOC_MISSED_HTML_CLOSE, 7, "parsing"));
assertEquals(expected, ex.getMessage());
}
}

@Test
public void testNoViableAltException() throws IOException {
LocalizedMessage.setLocale(Locale.ROOT);
try {
DetailNodeTreeStringPrinter.printFileAst(new File(getPath(
"InputNoViableAltException.javadoc")));
}
catch (IllegalArgumentException ex) {
final String expected = "[ERROR:0] Javadoc comment at column 9 has parse error. "
+ "Details: no viable alternative at input ' <<' "
+ "while parsing JAVADOC_TAG";
assertEquals(expected, ex.getMessage());
}
}

@Test
public void testHtmlTagCloseBeforeTagOpen() throws IOException {
LocalizedMessage.setLocale(Locale.ROOT);
try {
DetailNodeTreeStringPrinter.printFileAst(new File(getPath(
"InputHtmlTagCloseBeforeTagOpen.javadoc"
)));
}
catch (IllegalArgumentException ex) {
final String expected = "[ERROR:0] Javadoc comment at column 4 has parse error."
+ " Details: no viable alternative at input '</tag'"
+ " while parsing HTML_ELEMENT";
assertEquals(expected, ex.getMessage());
}
}

@Test
public void testWrongHtmlTagOrder() throws IOException {
try {
DetailNodeTreeStringPrinter.printFileAst(new File(getPath(
"InputWrongHtmlTagOrder.javadoc"
)));
}
catch (IllegalArgumentException ex) {
final String expected = getParseErrorMessage(new ParseErrorMessage(
0, MSG_JAVADOC_MISSED_HTML_CLOSE, 10, "tag2"));
assertEquals(expected, ex.getMessage());
}
}

@Test
public void testOmittedStartTagForHtmlElement() throws IOException {
try {
DetailNodeTreeStringPrinter.printFileAst(new File(getPath(
"InputOmittedStartTagForHtmlElement.javadoc"
)));
}
catch (IllegalArgumentException ex) {
final String expected = getParseErrorMessage(new ParseErrorMessage(
0, MSG_JAVADOC_MISSED_HTML_CLOSE, 3, "a"));
assertEquals(expected, ex.getMessage());
}
}
}
5 changes: 1 addition & 4 deletions src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AbstractJavadocCheckTest.java
View file
Expand Up @@ -20,7 +20,6 @@
package com.puppycrawl.tools.checkstyle.checks.javadoc;

import static com.puppycrawl.tools.checkstyle.checks.javadoc.AbstractJavadocCheck.MSG_JAVADOC_MISSED_HTML_CLOSE;
import static com.puppycrawl.tools.checkstyle.checks.javadoc.AbstractJavadocCheck.MSG_JAVADOC_PARSE_RULE_ERROR;
import static com.puppycrawl.tools.checkstyle.checks.javadoc.AbstractJavadocCheck.MSG_JAVADOC_WRONG_SINGLETON_TAG;
import static com.puppycrawl.tools.checkstyle.checks.javadoc.AbstractJavadocCheck.MSG_KEY_UNRECOGNIZED_ANTLR_ERROR;
import static java.util.Arrays.asList;
Expand Down Expand Up @@ -61,9 +60,7 @@ protected String getPath(String filename) throws IOException {
public void testNumberFormatException() throws Exception {
final DefaultConfiguration checkConfig = createCheckConfig(TempCheck.class);
final String[] expected = {
"3: " + getCheckMessage(MSG_JAVADOC_PARSE_RULE_ERROR, 52, "no viable "
+ "alternative at input '<ul><li>a' {@link EntityEntry} (by way of {@link #;'",
"HTML_TAG"),
"3: " + getCheckMessage(MSG_KEY_UNRECOGNIZED_ANTLR_ERROR, 0, null),
};
verify(checkConfig, getPath("InputTestNumberFormatException.java"), expected);
}
Expand Down
1 change: 1 addition & 0 deletions ...sources/com/puppycrawl/tools/checkstyle/astprinter/InputHtmlTagCloseBeforeTagOpen.javadoc
View file
@@ -0,0 +1 @@
* </tag> <tag>
1 change: 1 addition & 0 deletions ...st/resources/com/puppycrawl/tools/checkstyle/astprinter/InputNoViableAltException.javadoc
View file
@@ -0,0 +1 @@
* @see <<p>
1 change: 1 addition & 0 deletions ...ces/com/puppycrawl/tools/checkstyle/astprinter/InputOmittedStartTagForHtmlElement.javadoc
View file
@@ -0,0 +1 @@
* <a href="http://www.redbooks.ibm.com/Redbooks.nsf/RedbookAbstracts/tips0243.html">IBM website</code>
41 changes: 41 additions & 0 deletions ...uppycrawl/tools/checkstyle/astprinter/InputUnescapedJavaCodeWithGenericsInJavadoc.javadoc
View file
@@ -0,0 +1,41 @@
/**
@Test
public void someTestMethod() {
List<Foo> x = makeAList();
List<Foo> x = makeAList();
Issue #4390: Javadoc comments containing unescaped Java code with
generic types leads to enormous parsing times
<Issue> <#4390:> <javadoc> <comments> <containing> <unescaped> <java> <code> <with>
<generic> <types> <leads> <to> <enormous> <parsing> <times>

List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();
List<Foo> x = makeAList();

List<Foo> x = makeAList();
List<Foo> x = makeAList();
Issue #4390: Javadoc comments containing unescaped Java code with
generic types leads to enormous parsing times
<parsing> <Issue> <#4390:> <javadoc> <comments> <containing> <unescaped> <java> <code> <with>
<generic> <types> <leads> <to> <enormous> <parsing>
</parsing> </enormous> </to> </leads> </types> </generic> </with> </code> </java>
</unescaped> </containing> </comments> </javadoc> </Issue>
}
**/
1 change: 1 addition & 0 deletions src/test/resources/com/puppycrawl/tools/checkstyle/astprinter/InputWrongHtmlTagOrder.javadoc
View file
@@ -0,0 +1 @@
* <tag1> <tag2> </tag1> </tag2>

0 comments on commit d905eb5

Please sign in to comment.