Issue checkstyle#4563: fixed stackoverflow with DetailAST.setParent

rnveach · Aug 1, 2017 · 54ab89d · 54ab89d
1 parent 6d290ad
commit 54ab89d
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 9 deletions.
diff --git a/config/sevntu_suppressions.xml b/config/sevntu_suppressions.xml
@@ -31,4 +31,8 @@
     and do not have coverage problems due to imports from java.util.stream. -->
     <suppress id="ForbidInterfacesImportFromJavaUtilStream"
               files=".*[\\/]src[\\/](test|it)[\\/]"/>
+
+    <!-- Tone down the checking for test code -->
+    <suppress id="ForbidCertainImports"
+              files="DetailASTTest\.java"/>
 </suppressions>
diff --git a/config/suppressions.xml b/config/suppressions.xml
@@ -32,7 +32,7 @@
     <suppress checks="Javadoc" files=".*[\\/]src[\\/](test|it)[\\/]"/>
     <suppress checks="MagicNumber" files=".*[\\/]src[\\/](test|it)[\\/]"/>
     <suppress checks="AvoidStaticImport" files=".*[\\/]src[\\/](test|it)[\\/]"/>
-    <suppress checks="ClassDataAbstractionCoupling" files="[\\/]IndentationCheckTest.java$|[\\/]SuppressWithNearbyCommentFilterTest.java$|[\\/]SuppressionCommentFilterTest.java$"/>
+    <suppress checks="ClassDataAbstractionCoupling" files="[\\/]IndentationCheckTest.java$|[\\/]SuppressWithNearbyCommentFilterTest.java$|[\\/]SuppressionCommentFilterTest.java|[\\/]DetailASTTest.java$"/>
     <suppress checks="EqualsAvoidNull" files="[\\/]Int.*FilterTest.java$"/>
     <suppress checks="VisibilityModifier" files="[\\/]BaseCheckTestSupport.java$|[\\/]AbstractModuleTestSupport.java$"/>
     <suppress checks="WriteTag" files=".*[\\/]src[\\/](test|it)[\\/]"/>

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/api/DetailAST.java b/src/main/java/com/puppycrawl/tools/checkstyle/api/DetailAST.java
@@ -199,13 +199,17 @@ public int getChildCount(int type) {
      * @param parent the parent token
      */
     private void setParent(DetailAST parent) {
-        clearBranchTokenTypes();
-        this.parent = parent;
-        final DetailAST nextSibling = getNextSibling();
-        if (nextSibling != null) {
-            nextSibling.setParent(parent);
-            nextSibling.previousSibling = this;
-        }
+        DetailAST instance = this;
+        do {
+            instance.clearBranchTokenTypes();
+            instance.parent = parent;
+            final DetailAST nextSibling = instance.getNextSibling();
+            if (nextSibling != null) {
+                nextSibling.previousSibling = instance;
+            }
+
+            instance = nextSibling;
+        } while (instance != null);
     }
 
     /**

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/api/DetailASTTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/api/DetailASTTest.java
@@ -23,7 +23,9 @@
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
 
+import java.io.BufferedWriter;
 import java.io.File;
+import java.io.FileWriter;
 import java.lang.reflect.Method;
 import java.text.MessageFormat;
 import java.util.Arrays;
@@ -32,16 +34,30 @@
 import java.util.Locale;
 import java.util.function.Consumer;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.powermock.reflect.Whitebox;
 
+import com.puppycrawl.tools.checkstyle.AbstractModuleTestSupport;
+import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
 import com.puppycrawl.tools.checkstyle.TreeWalker;
+import com.puppycrawl.tools.checkstyle.checks.TodoCommentCheck;
+import com.puppycrawl.tools.checkstyle.utils.CommonUtils;
 
 /**
  * TestCase to check DetailAST.
  * @author Oliver Burn
  */
-public class DetailASTTest {
+public class DetailASTTest extends AbstractModuleTestSupport {
+    @Rule
+    public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+    @Override
+    protected String getPackageLocation() {
+        return "com/puppycrawl/tools/checkstyle/api";
+    }
+
     private static Method getSetParentMethod() throws Exception {
         final Class<DetailAST> detailAstClass = DetailAST.class;
         final Method setParentMethod =
@@ -199,6 +215,24 @@ public void testAddNextSibling() {
         assertEquals("Invalid child", newSibling, child.getNextSibling());
     }
 
+    @Test
+    public void testManyComments() throws Exception {
+        final File file = temporaryFolder.newFile("InputDetailASTManyComments.java");
+
+        try (BufferedWriter bw = new BufferedWriter(new FileWriter(file))) {
+            bw.write("class C {\n");
+            for (int i = 0; i <= 30000; i++) {
+                bw.write("// " + i + "\n");
+            }
+            bw.write("}\n");
+        }
+
+        final DefaultConfiguration checkConfig = createModuleConfig(TodoCommentCheck.class);
+
+        final String[] expected = CommonUtils.EMPTY_STRING_ARRAY;
+        verify(checkConfig, file.getAbsolutePath(), expected);
+    }
+
     /**
      * There are asserts in checkNode, but idea does not see them
      * @noinspection JUnitTestMethodWithNoAssertions