Issue checkstyle#3311: Introduced isUnclosedTag and hasUnclosedTag fo…

…r javadoc trees

config/findbugs-exclude.xml

@@ -76,6 +76,8 @@
         <Class name="com.puppycrawl.tools.checkstyle.checks.regexp.RegexpSinglelineCheck" />
         <!--Uses setters to set fields values-->
         <Class name="com.puppycrawl.tools.checkstyle.api.AbstractCheck" />
+        <!--fields `unclosedTag` and `hasUnclosedTags` are initialized only when needed-->
+        <Class name="com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocNodeImpl"/>
       </Or>
       <Bug pattern="UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR" />
     </Match>

src/main/java/com/puppycrawl/tools/checkstyle/api/DetailNode.java

@@ -70,4 +70,19 @@ public interface DetailNode {
      * @return index
      */
     int getIndex();
+
+    /**
+     * Checks if the {@code DetailNode} or any of its successors is an unclosed tag.
+     * @return true if the javadoc node or any of its successors is an unclosed HTML tag;
+     *     false otherwise
+     * @see com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocNodeImpl#hasUnclosedTag
+     */
+    boolean hasUnclosedTag();
+
+    /**
+     * Checks if the {@code DetailNode} is an unclosed tag.
+     * @return true if the javadoc node is an unclosed HTML tag; false otherwise
+     * @see com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocNodeImpl#isUnclosedTag
+     */
+    boolean isUnclosedTag();
 }

src/main/java/com/puppycrawl/tools/checkstyle/checks/javadoc/JavadocNodeImpl.java

@@ -72,6 +72,20 @@ public class JavadocNodeImpl implements DetailNode {
      */
     private DetailNode parent;
 
+    /**
+     * Reflects if any of the successors of this {@code DetailNode} has property
+     * {@link #unclosedTag} set true.
+     */
+    private Boolean containsUnclosedTag;
+
+    /**
+     * Reflects whether the {@code DetailNode} represents a tag which is not closed.
+     * For example consider input: {@code <p> This is a para}, in which {@code <p>} tag is unclosed.
+     * @see <a href=
+     *     "https://www.w3.org/TR/html51/syntax.html#optional-start-and-end-tags">Optional tags</a>
+     */
+    private Boolean unclosedTag;
+
     @Override
     public int getType() {
         return type;
@@ -111,6 +125,33 @@ public int getIndex() {
         return index;
     }
 
+    @Override
+    public boolean hasUnclosedTag() {
+        if (containsUnclosedTag == null) {
+            containsUnclosedTag = isUnclosedTag();
+            for (DetailNode child : children) {
+                if (containsUnclosedTag) {
+                    break;
+                }
+                containsUnclosedTag = child.hasUnclosedTag();
+            }
+        }
+        return containsUnclosedTag;
+    }
+
+    @Override
+    public boolean isUnclosedTag() {
+        if (unclosedTag == null) {
+            if (getText().endsWith("TAG_OPEN")) {
+                unclosedTag = parent.getType() != JavadocUtils.getParentTypeWhenNested(this);
+            }
+            else {
+                unclosedTag = false;
+            }
+        }
+        return unclosedTag;
+    }
+
     /**
      * Sets node's type.
      * @param type Node's type.

src/main/java/com/puppycrawl/tools/checkstyle/utils/JavadocUtils.java

@@ -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;
@@ -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();
 
@@ -110,6 +141,35 @@ 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.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. */
@@ -420,4 +480,18 @@ public static boolean isCorrectJavadocPosition(DetailAST blockComment) {
                 || BlockCommentPosition.isOnEnumConstant(blockComment)
                 || BlockCommentPosition.isOnAnnotationDef(blockComment);
     }
+
+    /**
+     * 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 com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocNodeImpl#hasUnclosedTag
+     */
+    public static int getParentTypeWhenNested(DetailNode tagOpen) {
+        return NESTED_HTML_CHILD_PARENT_MAPPINGS.get(tagOpen.getType());
+    }
 }

src/test/java/com/puppycrawl/tools/checkstyle/checks/javadoc/AbstractJavadocCheckTest.java

@@ -256,6 +256,15 @@ public void testVisitLeaveToken()
         Assert.assertEquals(JavadocVisitLeaveCheck.visitCount, JavadocVisitLeaveCheck.leaveCount);
     }
 
+    @Test
+    public void testHasUnclosedTag() throws Exception {
+        final DefaultConfiguration checkConfig =
+                createCheckConfig(HasUnclosedTagPuppetCheck.class);
+        final String[] expected = CommonUtils.EMPTY_STRING_ARRAY;
+        verify(checkConfig, getPath("InputUnclosedTag.java"), expected);
+        Assert.assertTrue(HasUnclosedTagPuppetCheck.testPassed);
+    }
+
     private static class TempCheck extends AbstractJavadocCheck {
 
         @Override
@@ -366,4 +375,54 @@ public void leaveJavadocToken(DetailNode ast) {
             leaveCount++;
         }
     }
+
+    private static class HasUnclosedTagPuppetCheck 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) {
+            if (testPassed) {
+                final int astType = ast.getType();
+                if (astType == JavadocTokenTypes.P_TAG_OPEN) {
+                    if (ast.getParent().getType() == JavadocTokenTypes.PARAGRAPH) {
+                        testPassed = !ast.isUnclosedTag();
+                    }
+                    else {
+                        testPassed = ast.isUnclosedTag();
+                    }
+                }
+                else if (astType == JavadocTokenTypes.LI_TAG_OPEN) {
+                    if (ast.getParent().getType() == JavadocTokenTypes.LI) {
+                        testPassed = !ast.isUnclosedTag();
+                    }
+                    else {
+                        testPassed = ast.isUnclosedTag();
+                    }
+                }
+                else {
+                    testPassed = !ast.isUnclosedTag();
+                }
+                assertHasUnclosedTagConditionForAllPredecessors(
+                        ast.isUnclosedTag() || ast.hasUnclosedTag(), ast);
+            }
+        }
+
+        private static void assertHasUnclosedTagConditionForAllPredecessors(boolean condition,
+                                                                            DetailNode curNode) {
+            DetailNode parent = curNode.getParent();
+            while (parent != null && testPassed) {
+                testPassed = curNode.hasUnclosedTag() == condition;
+                parent = parent.getParent();
+            }
+        }
+    }
 }

src/test/java/com/puppycrawl/tools/checkstyle/grammars/javadoc/JavadocParseTreeTest.java

@@ -21,13 +21,25 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.lang.reflect.Method;
 
+import org.junit.Assert;
 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.DetailAST;
+import com.puppycrawl.tools.checkstyle.api.DetailNode;
+import com.puppycrawl.tools.checkstyle.api.JavadocTokenTypes;
+import com.puppycrawl.tools.checkstyle.utils.JavadocUtils;
 
 public class JavadocParseTreeTest extends AbstractTreeTestSupport {
 
+    private static final Method CREATE_FAKE_BLOCK_COMMENT = Whitebox.getMethod(
+            DetailNodeTreeStringPrinter.class, "createFakeBlockComment", String.class);
+
     @Override
     protected String getPackageLocation() {
         return "com/puppycrawl/tools/checkstyle/grammars/javadoc/";
@@ -250,4 +262,74 @@ public void testJavadocWithCrAsNewline() throws Exception {
         verifyJavadocTree(getPath("expectedJavadocWithCrAsNewlineAst.txt"),
                 getPath("InputJavadocWithCrAsNewline.javadoc"));
     }
+
+    @Test
+    public void testIsUnclosedTag() throws Exception {
+        final DetailAST puppetBlockComment = (DetailAST) CREATE_FAKE_BLOCK_COMMENT.invoke(
+                null, readFile(getHtmlPath("InputUnclosedTag.javadoc")));
+        DetailNode toVisit = new JavadocDetailNodeParser()
+                .parseJavadocAsDetailNode(puppetBlockComment).getTree();
+        DetailNode currentNode;
+        while (toVisit != null) {
+            final int tokenTypeToVisit = toVisit.getType();
+            if (tokenTypeToVisit == JavadocTokenTypes.P_TAG_OPEN) {
+                if (toVisit.getParent().getType() == JavadocTokenTypes.PARAGRAPH) {
+                    Assert.assertFalse(toVisit.isUnclosedTag());
+                }
+                else {
+                    Assert.assertTrue(toVisit.isUnclosedTag());
+                }
+
+            }
+            else if (tokenTypeToVisit == JavadocTokenTypes.LI_TAG_OPEN) {
+                if (toVisit.getParent().getType() == JavadocTokenTypes.LI) {
+                    Assert.assertFalse(toVisit.isUnclosedTag());
+                }
+                else {
+                    Assert.assertTrue(toVisit.isUnclosedTag());
+                }
+
+            }
+            else if (tokenTypeToVisit == JavadocTokenTypes.TR_TAG_OPEN) {
+                Assert.assertTrue(toVisit.isUnclosedTag());
+            }
+            else {
+                Assert.assertFalse(toVisit.isUnclosedTag());
+            }
+            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(getHtmlPath("InputUnclosedTag.javadoc")));
+        DetailNode toVisit = new JavadocDetailNodeParser()
+                .parseJavadocAsDetailNode(puppetBlockComment).getTree();
+        DetailNode currentNode;
+        while (toVisit != null) {
+            if (toVisit.isUnclosedTag() || toVisit.hasUnclosedTag()) {
+                currentNode = toVisit.getParent();
+                while (currentNode != null) {
+                    Assert.assertTrue(currentNode.hasUnclosedTag());
+                    currentNode = currentNode.getParent();
+                }
+            }
+            currentNode = JavadocUtils.getFirstChild(toVisit);
+            while (currentNode == null && toVisit != null) {
+                currentNode = JavadocUtils.getNextSibling(toVisit);
+                if (currentNode == null) {
+                    toVisit = toVisit.getParent();
+                }
+            }
+            toVisit = currentNode;
+        }
+    }
 }