Skip to content

Commit

Permalink
Issue checkstyle#3311: Added method doesCheckAcceptJavadocWithUnclose…
Browse files Browse the repository at this point in the history
…dHtmlTag in AbstractJavadocCheck which can be overridden in extending checks which are not to process javadoc with unclosed HTML tags
  • Loading branch information
voidfist committed Jul 21, 2017
1 parent c094935 commit 2a58631
Show file tree
Hide file tree
Showing 6 changed files with 366 additions and 2 deletions.
Expand Up @@ -279,7 +279,11 @@ public final void visitToken(DetailAST blockCommentNode) {
}

if (result.getParseErrorMessage() == null) {
processTree(result.getTree());
final DetailNode javadocTree = result.getTree();
if (doesCheckAcceptJavadocWithUnclosedHtmlTag()
|| !JavadocUtils.hasUnclosedTag(javadocTree)) {
processTree(javadocTree);
}
}
else {
final ParseErrorMessage parseErrorMessage = result.getParseErrorMessage();
Expand Down Expand Up @@ -351,4 +355,16 @@ private boolean shouldBeProcessed(DetailNode curNode) {
return javadocTokens.contains(curNode.getType());
}

/**
* This method determines if a check should process javadoc containing unclosed html tags.
* This method must be overridden in checks extending {@code AbstractJavadocCheck} which
* are not supposed to process javadoc containing unclosed html tags.
* @return true if the check should or can process javadoc containing unclosed html tags;
* false otherwise
* @see JavadocUtils#hasUnclosedTag(DetailNode)
*/
public boolean doesCheckAcceptJavadocWithUnclosedHtmlTag() {
return true;
}

}
111 changes: 111 additions & 0 deletions src/main/java/com/puppycrawl/tools/checkstyle/utils/JavadocUtils.java
Expand Up @@ -22,7 +22,9 @@
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;

import com.google.common.collect.ImmutableMap;
Expand Down Expand Up @@ -74,8 +76,37 @@ public enum JavadocTagType {
/** Tab pattern. */
private static final Pattern TAB = Pattern.compile("\t");

/**
* Maps all the supported nested html open tags to their parents in a nested javadoc tree.
* For ex: {@code <p> This is a para</p>}. The javadoc tree for such input will be:
* {@code
* JAVADOC -> JAVADOC [0:0]
* |--HTML_ELEMENT -> HTML_ELEMENT [0:0]
* | `--PARAGRAPH -> PARAGRAPH [0:0]
* | |--P_TAG_OPEN -> P_TAG_OPEN [0:0]
* | | |--OPEN -> < [0:0]
* | | |--P_HTML_TAG_NAME -> p [0:1]
* | | `--CLOSE -> > [0:2]
* | |--TEXT -> This is a para [0:3]
* | `--P_TAG_CLOSE -> P_TAG_CLOSE [0:18]
* | |--OPEN -> < [0:18]
* | |--SLASH -> / [0:19]
* | |--P_HTML_TAG_NAME -> p [0:20]
* | `--CLOSE -> > [0:21]
* `--EOF -> <EOF> [0:22]}
* As can be seen {@code <p>} is having {@code PARAGRAPH} as parent in the javadoc tree.
* This is what is being referred to as nesting. So, {@code <p>} which is
* {@link JavadocTokenTypes#P_TAG_OPEN} will be mapped to {@link JavadocTokenTypes#PARAGRAPH}.
* Closing all html tags as expected produces a completely nested javadoc tree.
* This field isn't lazy initialized because doing so would require a holder class for thread
* safety which seems like an unnecessary optimization given the concise memory footprint
* and light initialization time of this field.
*/
private static final Map<Integer, Integer> NESTED_HTML_CHILD_PARENT_MAPPINGS = new HashMap<>();

// Using reflection gets all token names and values from JavadocTokenTypes class
// and saves to TOKEN_NAME_TO_VALUE and TOKEN_VALUE_TO_NAME collections.
// -@cs[ExecutableStatementCount] overshoots while populating NESTED_HTML_CHILD_PARENT_MAPPINGS
static {
final ImmutableMap.Builder<String, Integer> builder = ImmutableMap.builder();

Expand Down Expand Up @@ -110,6 +141,37 @@ public enum JavadocTagType {

TOKEN_NAME_TO_VALUE = builder.build();
TOKEN_VALUE_TO_NAME = tempTokenValueToName;

NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.P_TAG_OPEN, JavadocTokenTypes.PARAGRAPH);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.LI_TAG_OPEN, JavadocTokenTypes.LI);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.TR_TAG_OPEN, JavadocTokenTypes.TR);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.TD_TAG_OPEN, JavadocTokenTypes.TD);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.TH_TAG_OPEN, JavadocTokenTypes.TH);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.BODY_TAG_OPEN, JavadocTokenTypes.BODY);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.COLGROUP_TAG_OPEN, JavadocTokenTypes.COLGROUP);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.DD_TAG_OPEN, JavadocTokenTypes.DD);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.DT_TAG_OPEN, JavadocTokenTypes.DT);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.HEAD_TAG_OPEN, JavadocTokenTypes.HEAD);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.HTML_TAG_OPEN, JavadocTokenTypes.HTML);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.OPTION_TAG_OPEN, JavadocTokenTypes.OPTION);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.TBODY_TAG_OPEN, JavadocTokenTypes.TBODY);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.THEAD_TAG_OPEN, JavadocTokenTypes.THEAD);
NESTED_HTML_CHILD_PARENT_MAPPINGS
.put(JavadocTokenTypes.TFOOT_TAG_OPEN, JavadocTokenTypes.TFOOT);
}

/** Prevent instantiation. */
Expand Down Expand Up @@ -420,4 +482,53 @@ public static boolean isCorrectJavadocPosition(DetailAST blockComment) {
|| BlockCommentPosition.isOnEnumConstant(blockComment)
|| BlockCommentPosition.isOnAnnotationDef(blockComment);
}

/**
* Checks if the supplied {@code DetailNode} argument or any of its successors
* is an unclosed HTML tag.
* @param root the node at which to start checking for unclosed HTML tags
* @return true if the supplied node or any of its successors is an unclosed HTML tag;
* false otherwise
*/
public static boolean hasUnclosedTag(DetailNode root) {
boolean hasUnclosedTag;
hasUnclosedTag = isUnclosedTag(root);
for (DetailNode child : root.getChildren()) {
if (hasUnclosedTag) {
break;
}
hasUnclosedTag = hasUnclosedTag(child);
}
return hasUnclosedTag;
}

/**
* Checks if the supplied {@code DetailNode} argument is an unclosed HTML tag.
* @param node the node to check
* @return true if the javadoc node is an unclosed HTML tag; false otherwise
*/
public static boolean isUnclosedTag(DetailNode node) {
final boolean isUnclosedTag;
if (node.getText().endsWith("TAG_OPEN")) {
isUnclosedTag = node.getParent().getType() != getParentTypeWhenNested(node);
}
else {
isUnclosedTag = false;
}
return isUnclosedTag;
}

/**
* When all the html tags are properly closed the resultant javadoc tree is nested. This method
* fetches the type of the parent as described by {@link JavadocTokenTypes} of a html tag in
* a nested javadoc tree.
* @param tagOpen the html tag for which the parent type is to be obtained assuming
* proper nesting
* @return the type of the parent when the supplied tag results in proper nesting
* @see #NESTED_HTML_CHILD_PARENT_MAPPINGS
* @see #hasUnclosedTag(DetailNode)
*/
public static int getParentTypeWhenNested(DetailNode tagOpen) {
return NESTED_HTML_CHILD_PARENT_MAPPINGS.get(tagOpen.getType());
}
}
Expand Up @@ -256,6 +256,15 @@ public void testVisitLeaveToken()
Assert.assertEquals(JavadocVisitLeaveCheck.visitCount, JavadocVisitLeaveCheck.leaveCount);
}

@Test
public void testUnclosedTagIntolerantCheck() throws Exception {
final DefaultConfiguration checkConfig =
createCheckConfig(UnclosedTagIntolerantCheck.class);
final String[] expected = CommonUtils.EMPTY_STRING_ARRAY;
verify(checkConfig, getPath("InputUnclosedTag.java"), expected);
Assert.assertTrue(UnclosedTagIntolerantCheck.testPassed);
}

private static class TempCheck extends AbstractJavadocCheck {

@Override
Expand Down Expand Up @@ -366,4 +375,28 @@ public void leaveJavadocToken(DetailNode ast) {
leaveCount++;
}
}

private static class UnclosedTagIntolerantCheck extends AbstractJavadocCheck {
private static boolean testPassed = true;

@Override
public int[] getDefaultJavadocTokens() {
return new int[] {
JavadocTokenTypes.P_TAG_OPEN,
JavadocTokenTypes.LI_TAG_OPEN,
JavadocTokenTypes.BODY_TAG_OPEN,
};
}

@Override
public void visitJavadocToken(DetailNode ast) {
testPassed = false;
}

@Override
public boolean doesCheckAcceptJavadocWithUnclosedHtmlTag() {
return false;
}

}
}
Expand Up @@ -25,10 +25,15 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.lang.reflect.Method;
import java.util.List;

import org.junit.Test;
import org.powermock.reflect.Whitebox;

import com.puppycrawl.tools.checkstyle.AbstractTreeTestSupport;
import com.puppycrawl.tools.checkstyle.DetailNodeTreeStringPrinter;
import com.puppycrawl.tools.checkstyle.JavadocDetailNodeParser;
import com.puppycrawl.tools.checkstyle.api.Comment;
import com.puppycrawl.tools.checkstyle.api.DetailAST;
import com.puppycrawl.tools.checkstyle.api.DetailNode;
Expand All @@ -39,7 +44,15 @@
import com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocTagInfo;
import com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocTags;

public class JavadocUtilsTest {
public class JavadocUtilsTest extends AbstractTreeTestSupport {

private static final Method CREATE_FAKE_BLOCK_COMMENT = Whitebox.getMethod(
DetailNodeTreeStringPrinter.class, "createFakeBlockComment", String.class);

@Override
public String getPackageLocation() {
return "com/puppycrawl/tools/checkstyle/utils/javadocutils";
}

@Test
public void testTags() {
Expand Down Expand Up @@ -337,4 +350,108 @@ public void testGetTokenNames() {
assertEquals("Unexpected token name",
"HTML_COMMENT", JavadocUtils.getTokenName(20073));
}

//-@cs[JavaNCSS] High NCSS count due to the huge switch needed to test every case.
@Test
public void testIsUnclosedTag() throws Exception {
final DetailAST puppetBlockComment = (DetailAST) CREATE_FAKE_BLOCK_COMMENT.invoke(null,
readFile(getPath("InputUnclosedTag.javadoc")));
DetailNode toVisit = new JavadocDetailNodeParser()
.parseJavadocAsDetailNode(puppetBlockComment).getTree();
while (toVisit != null) {
final boolean isNested;
if (toVisit.getParent() == null) {
isNested = true;
}
else {
final int toVisitParentType = toVisit.getParent().getType();
switch (toVisit.getType()) {
case JavadocTokenTypes.P_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.PARAGRAPH;
break;
case JavadocTokenTypes.LI_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.LI;
break;
case JavadocTokenTypes.TR_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.TR;
break;
case JavadocTokenTypes.TD_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.TD;
break;
case JavadocTokenTypes.TH_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.TH;
break;
case JavadocTokenTypes.BODY_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.BODY;
break;
case JavadocTokenTypes.COLGROUP_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.COLGROUP;
break;
case JavadocTokenTypes.DD_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.DD;
break;
case JavadocTokenTypes.DT_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.DT;
break;
case JavadocTokenTypes.HEAD_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.HEAD;
break;
case JavadocTokenTypes.HTML_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.HTML;
break;
case JavadocTokenTypes.OPTION_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.OPTION;
break;
case JavadocTokenTypes.TBODY_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.TBODY;
break;
case JavadocTokenTypes.THEAD_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.THEAD;
break;
case JavadocTokenTypes.TFOOT_TAG_OPEN:
isNested = toVisitParentType == JavadocTokenTypes.TFOOT;
break;
default:
isNested = true;
}
}
assertEquals(JavadocUtils.isUnclosedTag(toVisit), !isNested);

DetailNode currentNode = JavadocUtils.getFirstChild(toVisit);
while (currentNode == null && toVisit != null) {
currentNode = JavadocUtils.getNextSibling(toVisit);
if (currentNode == null) {
toVisit = toVisit.getParent();
}
}
toVisit = currentNode;
}
}

@Test
public void testHasUnclosedTag() throws Exception {
final DetailAST puppetBlockComment = (DetailAST) CREATE_FAKE_BLOCK_COMMENT.invoke(null,
readFile(getPath("InputUnclosedTag.javadoc")));
DetailNode toVisit = new JavadocDetailNodeParser()
.parseJavadocAsDetailNode(puppetBlockComment).getTree();
DetailNode currentNode;
while (toVisit != null) {
if (JavadocUtils.isUnclosedTag(toVisit) || JavadocUtils.hasUnclosedTag(toVisit)) {
currentNode = toVisit.getParent();
while (currentNode != null) {
assertTrue(JavadocUtils.hasUnclosedTag(currentNode));
currentNode = currentNode.getParent();
}
}
currentNode = JavadocUtils.getFirstChild(toVisit);
while (currentNode == null && toVisit != null) {
currentNode = JavadocUtils.getNextSibling(toVisit);
if (currentNode == null) {
toVisit = toVisit.getParent();
}
}
toVisit = currentNode;
}
}

}
@@ -0,0 +1,42 @@
package com.puppycrawl.tools.checkstyle.checks.javadoc.abstractjavadoc;

/**
* <body>
* <p> This class is only meant for testing. </p>
* <p> In html, closing all tags is not necessary.
* <li> neither is opening every tag <p> </li>
* </body>
*
* @see "https://www.w3.org/TR/html51/syntax.html#optional-start-and-end-tags"
*/
public class InputUnclosedTag {
/** <p> <p> paraception </p> </p> */
int field1;

/**<li> paraTags should be opened</p> list isn't nested in parse tree </li>*/
int field2;

/**
* <body> body <p> paragraph <li> list </li> </p> </body>
*
* @return <li> <li> outer list isn't nested in parse tree </li> </li>
*/
int getField1() {return field1;}

/***/
int getField2() {return field2;} //method with empty javadoc

/**
* <tr> <li> list is not going to be nested in parse tree </tr>
*
* @param field1 {@code <p> paraTag will not be recognized} in javadoc tree </p>
*/
void setField1(int field1) {this.field1 = field1;}

/**
* <p>This is a setter method.
* And paraTag shall be nested in parse tree </p>
* @param field2 <p> settter
*/
void setField2(int field2) {this.field2 = field2;}
}

0 comments on commit 2a58631

Please sign in to comment.