Skip to content

Commit

Permalink
Issue checkstyle#3311: Modified javadoc grammar, JavadocDetailNodePar…
Browse files Browse the repository at this point in the history
…ser, AbstractJavadocCheck to enable selective processing of javadoc with non tight HTML
  • Loading branch information
voidfist committed Aug 27, 2017
1 parent e3d7e02 commit 159eafb
Show file tree
Hide file tree
Showing 7 changed files with 801 additions and 269 deletions.
Expand Up @@ -27,6 +27,7 @@
import org.antlr.v4.runtime.BailErrorStrategy;
import org.antlr.v4.runtime.BaseErrorListener;
import org.antlr.v4.runtime.BufferedTokenStream;
import org.antlr.v4.runtime.CommonToken;
import org.antlr.v4.runtime.CommonTokenStream;
import org.antlr.v4.runtime.FailedPredicateException;
import org.antlr.v4.runtime.InputMismatchException;
Expand Down Expand Up @@ -82,6 +83,11 @@ public class JavadocDetailNodeParser {
*/
public static final String MSG_KEY_PARSE_ERROR = "javadoc.parse.error";

/**
* Message property key for the Unclosed HTML message.
*/
public static final String MSG_UNCLOSED_HTML = "javadoc.unclosedHtml";

/** Symbols with which javadoc starts. */
private static final String JAVADOC_START = "/**";

Expand Down Expand Up @@ -119,14 +125,17 @@ public ParseStatus parseJavadocAsDetailNode(DetailAST javadocCommentAst) {
final ParseStatus result = new ParseStatus();

try {
final ParseTree parseTree = parseJavadocAsParseTree(javadocComment);
final JavadocParser javadocParser = createJavadocParser(javadocComment);

final ParseTree javadocParseTree = javadocParser.javadoc();

final DetailNode tree = convertParseTreeToDetailNode(parseTree);
final DetailNode tree = convertParseTreeToDetailNode(javadocParseTree);
// adjust first line to indent of /**
adjustFirstLineToJavadocIndent(tree,
javadocCommentAst.getColumnNo()
+ JAVADOC_START.length());
result.setTree(tree);
result.firstNonTightHtmlTag = getFirstNonTightHtmlTag(javadocParser);
}
catch (ParseCancellationException | IllegalArgumentException ex) {
ParseErrorMessage parseErrorMessage = null;
Expand Down Expand Up @@ -164,7 +173,7 @@ public ParseStatus parseJavadocAsDetailNode(DetailAST javadocCommentAst) {
* @return parse tree
* @noinspection deprecation
*/
private ParseTree parseJavadocAsParseTree(String blockComment) {
private JavadocParser createJavadocParser(String blockComment) {
final ANTLRInputStream input = new ANTLRInputStream(blockComment);

final JavadocLexer lexer = new JavadocLexer(input);
Expand All @@ -179,11 +188,11 @@ private ParseTree parseJavadocAsParseTree(String blockComment) {
// add custom error listener that logs syntax errors
parser.addErrorListener(errorListener);

// This strategy stops parsing when parser error occurs.
// By default it uses Error Recover Strategy which is slow and useless.
// JavadocParserErrorStrategy stops parsing on first parse error encountered unlike the
// DefaultErrorStrategy used by ANTLR which rather attempts error recovery.
parser.setErrorHandler(new JavadocParserErrorStrategy());

return parser.javadoc();
return parser;
}

/**
Expand Down Expand Up @@ -502,6 +511,32 @@ else if (token.getType() == JavadocTokenTypes.HTML_TAG_NAME && !stack.isEmpty())
return htmlTagNameStart;
}

/**
* This method is used to get the first non-tight HTML tag encountered while parsing javadoc.
* This shall eventually be reflected by the {@link ParseStatus} object returned by
* {@link #parseJavadocAsDetailNode(DetailAST)} method via the instance member
* {@link ParseStatus#firstNonTightHtmlTag}, and checks not supposed to process non-tight HTML
* or the ones which are supposed to log violation for non-tight javadocs can utilize that.
*
* @param javadocParser The ANTLR recognizer instance which has been used to parse the javadoc
* @return First non-tight HTML tag if one exists; null otherwise
*/
private Token getFirstNonTightHtmlTag(JavadocParser javadocParser) {
final CommonToken offendingToken;
final ParserRuleContext nonTightTagStartContext = javadocParser.nonTightTagStartContext;
if (nonTightTagStartContext == null) {
offendingToken = null;
}
else {
final Token token = ((TerminalNode) nonTightTagStartContext.getChild(1))
.getSymbol();
offendingToken = new CommonToken(token);
offendingToken
.setLine(offendingToken.getLine() + errorListener.offset);
}
return offendingToken;
}

/**
* Custom error listener for JavadocParser that prints user readable errors.
*/
Expand Down Expand Up @@ -589,6 +624,15 @@ public static class ParseStatus {
*/
private ParseErrorMessage parseErrorMessage;

/**
* Stores the first non-tight HTML tag encountered while parsing javadoc.
*
* @see <a
* href="http://checkstyle.sourceforge.net/writingjavadocchecks.html#Tight-HTML_rules">
* Tight HTML rules</a>
*/
private Token firstNonTightHtmlTag;

/**
* Getter for DetailNode tree.
* @return DetailNode tree if parsing was successful, null otherwise.
Expand Down Expand Up @@ -621,6 +665,28 @@ public void setParseErrorMessage(ParseErrorMessage parseErrorMessage) {
this.parseErrorMessage = parseErrorMessage;
}

/**
* This method is used to check if the javadoc parsed has non-tight HTML tags.
*
* @return returns true if the javadoc has at least one non-tight HTML tag; false otherwise
* @see <a
* href="http://checkstyle.sourceforge.net/writingjavadocchecks.html#Tight-HTML_rules">
* Tight HTML rules</a>
*/
public boolean isNonTight() {
return firstNonTightHtmlTag != null;
}

/**
* Getter for {@link #firstNonTightHtmlTag}.
*
* @return the first non-tight HTML tag that is encountered while parsing Javadoc,
* if one exists
*/
public Token getFirstNonTightHtmlTag() {
return firstNonTightHtmlTag;
}

}

/**
Expand Down Expand Up @@ -681,6 +747,10 @@ public Object[] getMessageArguments() {
}

/**
* The DefaultErrorStrategy used by ANTLR attempts to recover from parse errors
* which might result in a performance overhead. Also, a parse error indicate
* that javadoc doesn't follow checkstyle Javadoc grammar and the user should be made aware
* of it.
* <a href="http://www.antlr.org/api/Java/org/antlr/v4/runtime/BailErrorStrategy.html">
* BailErrorStrategy</a> is used to make ANTLR generated parser bail out on the first error
* in parser and not attempt any recovery methods but it doesn't report error to the
Expand Down
Expand Up @@ -87,6 +87,19 @@ public abstract class AbstractJavadocCheck extends AbstractCheck {
/** The javadoc tokens the check is interested in. */
private final Set<Integer> javadocTokens = new HashSet<>();

/**
* This property determines if a check should log a violation upon encountering javadoc with
* non-tight html. The default return value for this method is set to false since checks
* generally tend to be fine with non tight html. It can be set through config file if a check
* is to log violation upon encountering non-tight HTML in javadoc.
*
* @see ParseStatus#firstNonTightHtmlTag
* @see ParseStatus#isNonTight()
* @see <a href="http://checkstyle.sourceforge.net/writingjavadocchecks.html#Tight-HTML_rules">
* Tight HTML rules</a>
*/
private boolean violateExecutionOnNonTightHtml;

/**
* Returns the default javadoc token types a check is interested in.
* @return the default javadoc token types
Expand Down Expand Up @@ -125,6 +138,31 @@ public int[] getRequiredJavadocTokens() {
return CommonUtils.EMPTY_INT_ARRAY;
}

/**
* This method determines if a check should process javadoc containing non-tight html tags.
* This method must be overridden in checks extending {@code AbstractJavadocCheck} which
* are not supposed to process javadoc containing non-tight html tags.
*
* @return true if the check should or can process javadoc containing non-tight html tags;
* false otherwise
* @see ParseStatus#isNonTight()
* @see <a href="http://checkstyle.sourceforge.net/writingjavadocchecks.html#Tight-HTML_rules">
* Tight HTML rules</a>
*/
public boolean acceptJavadocWithNonTightHtml() {
return true;
}

/**
* Setter for {@link #violateExecutionOnNonTightHtml}.
* @param shouldReportViolation value to which the field shall be set to
* @see <a href="http://checkstyle.sourceforge.net/writingjavadocchecks.html#Tight-HTML_rules">
* Tight HTML rules</a>
*/
public final void setViolateExecutionOnNonTightHtml(boolean shouldReportViolation) {
violateExecutionOnNonTightHtml = shouldReportViolation;
}

/**
* Adds a set of tokens the check is interested in.
* @param strRep the string representation of the tokens interested in
Expand Down Expand Up @@ -261,12 +299,21 @@ public final void visitToken(DetailAST blockCommentNode) {
result = TREE_CACHE.get().get(treeCacheKey);
}
else {
result = context.get().parser.parseJavadocAsDetailNode(blockCommentNode);
result = context.get().parser
.parseJavadocAsDetailNode(blockCommentNode);
TREE_CACHE.get().put(treeCacheKey, result);
}

if (result.getParseErrorMessage() == null) {
processTree(result.getTree());
if (acceptJavadocWithNonTightHtml() || !result.isNonTight()) {
processTree(result.getTree());
}

if (violateExecutionOnNonTightHtml && result.isNonTight()) {
log(result.getFirstNonTightHtmlTag().getLine(),
JavadocDetailNodeParser.MSG_UNCLOSED_HTML,
result.getFirstNonTightHtmlTag().getText());
}
}
else {
final ParseErrorMessage parseErrorMessage = result.getParseErrorMessage();
Expand Down
Expand Up @@ -30,6 +30,7 @@
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import com.puppycrawl.tools.checkstyle.JavadocDetailNodeParser;
import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.FileContents;
Expand Down Expand Up @@ -63,7 +64,7 @@ public class JavadocStyleCheck
public static final String MSG_INCOMPLETE_TAG = "javadoc.incompleteTag";

/** Message property key for the Unclosed HTML message. */
public static final String MSG_UNCLOSED_HTML = "javadoc.unclosedHtml";
public static final String MSG_UNCLOSED_HTML = JavadocDetailNodeParser.MSG_UNCLOSED_HTML;

/** Message property key for the Extra HTML message. */
public static final String MSG_EXTRA_HTML = "javadoc.extraHtml";
Expand Down

0 comments on commit 159eafb

Please sign in to comment.