From 2466ba7163a61274ccc3e7ca5b495e421166151d Mon Sep 17 00:00:00 2001 From: Matias Comercio Date: Sat, 28 Oct 2017 19:50:06 -0300 Subject: [PATCH] Add remove & removeChildAtIndex to Node interface - Implement these methods at AbstractNode. - Add AbstractNodeTest class with test cases for the implemented methods. - Add JUnitParams dependency: https://github.com/Pragmatists/JUnitParams - Implement these test cases using JUnitParams features --- pmd-core/pom.xml | 5 + .../pmd/lang/ast/AbstractNode.java | 33 +++ .../net/sourceforge/pmd/lang/ast/Node.java | 15 ++ .../pmd/lang/ast/AbstractNodeTest.java | 233 ++++++++++++++++++ pom.xml | 5 + 5 files changed, 291 insertions(+) create mode 100644 pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java diff --git a/pmd-core/pom.xml b/pmd-core/pom.xml index 9262c56110..ebf2654132 100644 --- a/pmd-core/pom.xml +++ b/pmd-core/pom.xml @@ -125,6 +125,11 @@ junit test + + pl.pragmatists + JUnitParams + test + org.apache.ant ant-testutil diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AbstractNode.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AbstractNode.java index f7bbd8e178..693b4096ad 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AbstractNode.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AbstractNode.java @@ -12,6 +12,7 @@ import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import org.apache.commons.lang3.ArrayUtils; import org.jaxen.BaseXPath; import org.jaxen.JaxenException; import org.w3c.dom.Document; @@ -418,4 +419,36 @@ public abstract class AbstractNode implements Node { public void jjtSetLastToken(GenericToken token) { this.lastToken = token; } + + @Override + public void remove() { + // Detach current node of all its children, if any + if (children != null) { + for (Node child : children) { + child.jjtSetParent(null); + } + children = new Node[0]; + } + + // Detach current node of its parent, if any + final Node parent = jjtGetParent(); + if (parent != null) { + parent.removeChildAtIndex(jjtGetChildIndex()); + jjtSetParent(null); + } + + // TODO [autofix]: Notify action for handling text edition + } + + @Override + public void removeChildAtIndex(final int childIndex) { + if (0 <= childIndex && childIndex < jjtGetNumChildren()) { + // Remove the child at the given index + children = ArrayUtils.remove(children, childIndex); + // Update the remaining children indexes + for (int i = 0; i < jjtGetNumChildren(); i++) { + jjtGetChild(i).jjtSetChildIndex(i); + } + } + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/Node.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/Node.java index 1461eba2d2..691e6aa0f7 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/Node.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/Node.java @@ -268,4 +268,19 @@ public interface Node { * The data to set on this node. */ void setUserData(Object userData); + + /** + * Remove the current node from its parent & the association it has with all its children. + *

+ * This last type of removal makes it possible to avoid the visitor to visit children of removed nodes. + */ + void remove(); + + /** + * This method tells the node to remove the child node at the given index from the node's list of + * children, if any; if not, no changes are done. + * @param childIndex + * The index of the child to be removed + */ + void removeChildAtIndex(int childIndex); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java new file mode 100644 index 0000000000..c23fb9fe00 --- /dev/null +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/AbstractNodeTest.java @@ -0,0 +1,233 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.ast; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import junitparams.JUnitParamsRunner; +import junitparams.Parameters; + +/** + * Unit test for {@link AbstractNode}. + */ +@RunWith(JUnitParamsRunner.class) +public class AbstractNodeTest { + private static final int NUM_CHILDREN = 3; + private static final int NUM_GRAND_CHILDREN = 3; + + // Note that in order to successfully run JUnitParams, we need to explicitly use `Integer` instead of `int` + + private Integer[] childrenIndexes() { + return getIntRange(NUM_CHILDREN); + } + + private Integer[] grandChildrenIndexes() { + return getIntRange(NUM_GRAND_CHILDREN); + } + + private static Integer[] getIntRange(final int exclusiveLimit) { + final Integer[] childIndexes = new Integer[exclusiveLimit]; + for (int i = 0; i < exclusiveLimit; i++) { + childIndexes[i] = i; + } + return childIndexes; + } + + public Object childrenAndGrandChildrenIndexes() { + final Integer[] childrenIndexes = childrenIndexes(); + final Integer[] grandChildrenIndexes = grandChildrenIndexes(); + final Object[] indexes = new Object[childrenIndexes.length * grandChildrenIndexes.length]; + int i = 0; + for (final int childIndex : childrenIndexes) { + for (final int grandChildIndex : grandChildrenIndexes) { + indexes[i++] = new Integer[] { childIndex, grandChildIndex }; + } + } + return indexes; + } + + private int id; + private Node rootNode; + + private int nextId() { + return id++; + } + + private Node newDummyNode() { + return new DummyNode(nextId()); + } + + private static void addChild(final Node parent, final Node child) { + parent.jjtAddChild(child, parent.jjtGetNumChildren()); // Append child at the end + child.jjtSetParent(parent); + } + + @Before + public void setUpSampleNodeTree() { + id = 0; + rootNode = newDummyNode(); + + for (int i = 0; i < NUM_CHILDREN; i++) { + final Node child = newDummyNode(); + for (int j = 0; j < NUM_GRAND_CHILDREN; j++) { + final Node grandChild = newDummyNode(); + addChild(child, grandChild); + } + addChild(rootNode, child); + } + } + + /** + * Explicitly tests the {@code remove} method, and implicitly the {@code removeChildAtIndex} method + */ + @Test + @Parameters(method = "childrenIndexes") + public void testRemoveChildOfRootNode(final int childIndex) { + final Node child = rootNode.jjtGetChild(childIndex); + + // Check that the child has the expected properties + assertEquals(NUM_CHILDREN, rootNode.jjtGetNumChildren()); + assertEquals(rootNode, child.jjtGetParent()); + assertEquals(NUM_GRAND_CHILDREN, child.jjtGetNumChildren()); + final Node[] grandChildren = new Node[child.jjtGetNumChildren()]; + for (int i = 0; i < grandChildren.length; i++) { + final Node grandChild = child.jjtGetChild(i); + grandChildren[i] = grandChild; + assertEquals(child, grandChild.jjtGetParent()); + } + + // Do the actual removal + child.remove(); + + // Check that conditions have been successfully changed + assertEquals(NUM_CHILDREN - 1, rootNode.jjtGetNumChildren()); + assertNull(child.jjtGetParent()); + assertEquals(0, child.jjtGetNumChildren()); + for (final Node grandChild : grandChildren) { + assertNull(grandChild.jjtGetParent()); + } + } + + /** + * Explicitly tests the {@code remove} method, and implicitly the {@code removeChildAtIndex} method. + * This is a border case as the root node does not have any parent. + */ + @Test + public void testRemoveRootNode() { + // Check that the root node has the expected properties + assertEquals(NUM_CHILDREN, rootNode.jjtGetNumChildren()); + assertNull(rootNode.jjtGetParent()); + final Node[] children = new Node[rootNode.jjtGetNumChildren()]; + for (int i = 0; i < children.length; i++) { + final Node child = rootNode.jjtGetChild(i); + children[i] = child; + assertEquals(rootNode, child.jjtGetParent()); + } + + // Do the actual removal + rootNode.remove(); + + // Check that conditions have been successfully changed + assertEquals(0, rootNode.jjtGetNumChildren()); + assertNull(rootNode.jjtGetParent()); + for (final Node aChild : children) { + assertNull(aChild.jjtGetParent()); + } + } + + /** + * Explicitly tests the {@code remove} method, and implicitly the {@code removeChildAtIndex} method. + * These are border cases as grandchildren nodes do not have any child. + */ + @Test + @Parameters(method = "childrenAndGrandChildrenIndexes") + public void testRemoveGrandChildNode(final Integer childIndex, final Integer grandChildIndex) { + final Node child = rootNode.jjtGetChild(childIndex); + final Node grandChild = child.jjtGetChild(grandChildIndex); + + // Check that the child has the expected properties + assertEquals(NUM_GRAND_CHILDREN, child.jjtGetNumChildren()); + assertEquals(0, grandChild.jjtGetNumChildren()); + assertEquals(child, grandChild.jjtGetParent()); + + // Do the actual removal + grandChild.remove(); + + // Check that conditions have been successfully changed + assertEquals(NUM_GRAND_CHILDREN - 1, child.jjtGetNumChildren()); + assertEquals(0, grandChild.jjtGetNumChildren()); + assertNull(grandChild.jjtGetParent()); + } + + /** + * Explicitly tests the {@code removeChildAtIndex} method. + */ + @Test + @Parameters(method = "childrenIndexes") + public void testRemoveRootNodeChildAtIndex(final Integer childIndex) { + final Node[] originalChildren = new Node[rootNode.jjtGetNumChildren()]; + + // Check that prior conditions are OK + for (int i = 0; i < originalChildren.length; i++) { + originalChildren[i] = rootNode.jjtGetChild(i); + assertEquals(i, originalChildren[i].jjtGetChildIndex()); + if (i > 0) { + assertNotEquals(originalChildren[i - 1], originalChildren[i]); + } + } + assertEquals(NUM_CHILDREN, rootNode.jjtGetNumChildren()); + + // Do the actual removal + rootNode.removeChildAtIndex(childIndex); + + // Check that conditions have been successfully changed + assertEquals(NUM_CHILDREN - 1, rootNode.jjtGetNumChildren()); + int j = 0; + for (int i = 0; i < rootNode.jjtGetNumChildren(); i++) { + if (j == childIndex) { // Skip the removed child + j++; + } + // Check that the nodes have been rightly shifted + assertEquals(originalChildren[j], rootNode.jjtGetChild(i)); + // Check that the child index has been updated + assertEquals(i, rootNode.jjtGetChild(i).jjtGetChildIndex()); + j++; + } + } + + /** + * Explicitly tests the {@code removeChildAtIndex} method. + * Test how invalid indexes cases are handled. + */ + @Test + public void testRemoveChildAtIndexWithInvalidIndex() { + // No assert as the test is considered passed if no exception is thrown + rootNode.removeChildAtIndex(-1); + rootNode.removeChildAtIndex(rootNode.jjtGetNumChildren()); + } + + /** + * Explicitly tests the {@code removeChildAtIndex} method. + * This is a border case as the method invocation should do nothing. + */ + @Test + public void testRemoveChildAtIndexOnNodeWithNoChildren() { + final Node grandChild = rootNode.jjtGetChild(0).jjtGetChild(0); + // Check that this node does not have any children + assertEquals(0, grandChild.jjtGetNumChildren()); + + grandChild.removeChildAtIndex(0); + + // If here, no exception has been thrown + // Check that this node still does not have any children + assertEquals(0, grandChild.jjtGetNumChildren()); + } +} diff --git a/pom.xml b/pom.xml index 19a01e76fe..91d65d54e5 100644 --- a/pom.xml +++ b/pom.xml @@ -799,6 +799,11 @@ Additionally it includes CPD, the copy-paste-detector. CPD finds duplicated code junit 4.12 + + pl.pragmatists + JUnitParams + 1.1.0 + net.java.dev.javacc javacc