Issue checkstyle#6320: added REMOVE_CONDITIONALS mutator for treewalker

rnveach · Dec 31, 2018 · 7b999ad · 7b999ad
1 parent 245fc4b
commit 7b999ad
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 72 deletions.
diff --git a/pom.xml b/pom.xml
@@ -2743,6 +2743,7 @@
                 <mutator>INVERT_NEGS</mutator>
                 <mutator>MATH</mutator>
                 <mutator>NEGATE_CONDITIONALS</mutator>
+                <mutator>REMOVE_CONDITIONALS</mutator>
                 <mutator>RETURN_VALS</mutator>
                 <mutator>TRUE_RETURNS</mutator>
                 <mutator>VOID_METHOD_CALLS</mutator>

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/JavadocDetailNodeParser.java b/src/main/java/com/puppycrawl/tools/checkstyle/JavadocDetailNodeParser.java
@@ -221,21 +221,20 @@ private DetailNode convertParseTreeToDetailNode(ParseTree parseTreeNode) {
 
                 ParseTree nextParseTreeSibling = getNextSibling(parseTreeParent);
 
-                if (nextJavadocSibling == null) {
-                    JavadocNodeImpl tempJavadocParent =
+                while (nextJavadocSibling == null) {
+                    currentJavadocParent =
                             (JavadocNodeImpl) currentJavadocParent.getParent();
 
-                    ParseTree tempParseTreeParent = parseTreeParent.getParent();
+                    parseTreeParent = parseTreeParent.getParent();
 
-                    while (nextJavadocSibling == null && tempJavadocParent != null) {
-                        nextJavadocSibling = (JavadocNodeImpl) JavadocUtil
-                                .getNextSibling(tempJavadocParent);
+                    if (currentJavadocParent == null) {
+                        break;
+                    }
 
-                        nextParseTreeSibling = getNextSibling(tempParseTreeParent);
+                    nextJavadocSibling = (JavadocNodeImpl) JavadocUtil
+                            .getNextSibling(currentJavadocParent);
 
-                        tempJavadocParent = (JavadocNodeImpl) tempJavadocParent.getParent();
-                        tempParseTreeParent = tempParseTreeParent.getParent();
-                    }
+                    nextParseTreeSibling = getNextSibling(parseTreeParent);
                 }
                 currentJavadocParent = nextJavadocSibling;
                 parseTreeParent = nextParseTreeSibling;

diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java b/src/main/java/com/puppycrawl/tools/checkstyle/TreeWalker.java
@@ -41,7 +41,6 @@
 import com.puppycrawl.tools.checkstyle.api.FileContents;
 import com.puppycrawl.tools.checkstyle.api.FileText;
 import com.puppycrawl.tools.checkstyle.api.LocalizedMessage;
-import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
 import com.puppycrawl.tools.checkstyle.utils.TokenUtil;
 
 /**
@@ -175,8 +174,7 @@ else if (module instanceof TreeWalkerFilter) {
     @Override
     protected void processFiltered(File file, FileText fileText) throws CheckstyleException {
         // check if already checked and passed the file
-        if (CommonUtil.matchesFileExtension(file, getFileExtensions())
-                && (!ordinaryChecks.isEmpty() || !commentChecks.isEmpty())) {
+        if (!ordinaryChecks.isEmpty() || !commentChecks.isEmpty()) {
             final FileContents contents = new FileContents(fileText);
             final DetailAST rootAST = JavaParser.parse(contents);
             if (!ordinaryChecks.isEmpty()) {
@@ -301,16 +299,14 @@ else if (TokenUtil.isCommentType(token)) {
      * @throws CheckstyleException when validation of default tokens fails
      */
     private static void validateDefaultTokens(AbstractCheck check) throws CheckstyleException {
-        if (check.getRequiredTokens().length != 0) {
-            final int[] defaultTokens = check.getDefaultTokens();
-            Arrays.sort(defaultTokens);
-            for (final int token : check.getRequiredTokens()) {
-                if (Arrays.binarySearch(defaultTokens, token) < 0) {
-                    final String message = String.format(Locale.ROOT, "Token \"%s\" from required "
-                            + "tokens was not found in default tokens list in check %s",
-                            token, check.getClass().getName());
-                    throw new CheckstyleException(message);
-                }
+        final int[] defaultTokens = check.getDefaultTokens();
+        Arrays.sort(defaultTokens);
+        for (final int token : check.getRequiredTokens()) {
+            if (Arrays.binarySearch(defaultTokens, token) < 0) {
+                final String message = String.format(Locale.ROOT, "Token \"%s\" from required "
+                        + "tokens was not found in default tokens list in check %s",
+                        token, check.getClass().getName());
+                throw new CheckstyleException(message);
             }
         }
     }
@@ -324,11 +320,7 @@ private static void validateDefaultTokens(AbstractCheck check) throws Checkstyle
     private void walk(DetailAST ast, FileContents contents,
             AstState astState) {
         notifyBegin(ast, contents, astState);
-
-        // empty files are not flagged by javac, will yield ast == null
-        if (ast != null) {
-            processIter(ast, astState);
-        }
+        processIter(ast, astState);
         notifyEnd(ast, astState);
     }
 
@@ -418,18 +410,14 @@ private void notifyLeave(DetailAST ast, AstState astState) {
      * @return list of visitors
      */
     private Collection<AbstractCheck> getListOfChecks(DetailAST ast, AstState astState) {
-        Collection<AbstractCheck> visitors = null;
+        final Collection<AbstractCheck> visitors;
         final String tokenType = TokenUtil.getTokenName(ast.getType());
 
         if (astState == AstState.WITH_COMMENTS) {
-            if (tokenToCommentChecks.containsKey(tokenType)) {
-                visitors = tokenToCommentChecks.get(tokenType);
-            }
+            visitors = tokenToCommentChecks.get(tokenType);
         }
         else {
-            if (tokenToOrdinaryChecks.containsKey(tokenType)) {
-                visitors = tokenToOrdinaryChecks.get(tokenType);
-            }
+            visitors = tokenToOrdinaryChecks.get(tokenType);
         }
         return visitors;
     }
@@ -503,9 +491,7 @@ private void processIter(DetailAST root, AstState astState) {
             while (curNode != null && toVisit == null) {
                 notifyLeave(curNode, astState);
                 toVisit = curNode.getNextSibling();
-                if (toVisit == null) {
-                    curNode = curNode.getParent();
-                }
+                curNode = curNode.getParent();
             }
             curNode = toVisit;
         }

diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/TreeWalkerTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/TreeWalkerTest.java
@@ -23,6 +23,10 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.times;
+import static org.powermock.api.mockito.PowerMockito.spy;
+import static org.powermock.api.mockito.PowerMockito.verifyPrivate;
 
 import java.io.File;
 import java.io.Writer;
@@ -39,13 +43,18 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
 import org.mockito.internal.util.Checks;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
 import org.powermock.reflect.Whitebox;
 
 import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
 import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
 import com.puppycrawl.tools.checkstyle.api.Configuration;
 import com.puppycrawl.tools.checkstyle.api.Context;
+import com.puppycrawl.tools.checkstyle.api.DetailAST;
+import com.puppycrawl.tools.checkstyle.api.FileContents;
 import com.puppycrawl.tools.checkstyle.api.FileText;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
 import com.puppycrawl.tools.checkstyle.checks.blocks.LeftCurlyCheck;
@@ -62,6 +71,8 @@
 import com.puppycrawl.tools.checkstyle.filters.SuppressionXpathFilter;
 import com.puppycrawl.tools.checkstyle.utils.CommonUtil;
 
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(TreeWalker.class)
 public class TreeWalkerTest extends AbstractModuleTestSupport {
 
     @Rule
@@ -344,52 +355,42 @@ public void testBehaviourWithZeroChecks() throws Exception {
 
     @Test
     public void testBehaviourWithOnlyOrdinaryChecks() throws Exception {
-        final TreeWalker treeWalker = new TreeWalker();
-        treeWalker.configure(createModuleConfig(TypeNameCheck.class));
+        final TreeWalker treeWalkerSpy = spy(new TreeWalker());
+        final Class<?> classAstState =
+                Class.forName("com.puppycrawl.tools.checkstyle.TreeWalker$AstState");
         final PackageObjectFactory factory = new PackageObjectFactory(
                 new HashSet<>(), Thread.currentThread().getContextClassLoader());
-        treeWalker.setModuleFactory(factory);
-        treeWalker.setupChild(createModuleConfig(TypeNameCheck.class));
+        treeWalkerSpy.configure(createModuleConfig(TypeNameCheck.class));
+        treeWalkerSpy.setModuleFactory(factory);
+        treeWalkerSpy.setupChild(createModuleConfig(TypeNameCheck.class));
         final File file = temporaryFolder.newFile("file.java");
         final List<String> lines = new ArrayList<>();
-        lines.add(" class a%$# {} ");
-        final FileText fileText = new FileText(file, lines);
-
-        try {
-            treeWalker.processFiltered(file, fileText);
-            fail("file is not compilable, exception is expected");
-        }
-        catch (CheckstyleException exception) {
-            final String message =
-                    "TokenStreamRecognitionException occurred while parsing file";
-            assertTrue("Error message is unexpected",
-                    exception.getMessage().contains(message));
-        }
+        lines.add("class Test {}");
+        treeWalkerSpy.processFiltered(file, new FileText(file, lines));
+        verifyPrivate(treeWalkerSpy, times(1)).invoke("walk",
+                any(DetailAST.class), any(FileContents.class), any(classAstState));
+        verifyPrivate(treeWalkerSpy, times(0)).invoke("getFilteredMessages",
+                any(String.class), any(FileContents.class), any(DetailAST.class));
     }
 
     @Test
     public void testBehaviourWithOnlyCommentChecks() throws Exception {
-        final TreeWalker treeWalker = new TreeWalker();
-        treeWalker.configure(createModuleConfig(CommentsIndentationCheck.class));
+        final TreeWalker treeWalkerSpy = spy(new TreeWalker());
+        final Class<?> classAstState =
+                Class.forName("com.puppycrawl.tools.checkstyle.TreeWalker$AstState");
         final PackageObjectFactory factory = new PackageObjectFactory(
                 new HashSet<>(), Thread.currentThread().getContextClassLoader());
-        treeWalker.setModuleFactory(factory);
-        treeWalker.setupChild(createModuleConfig(CommentsIndentationCheck.class));
+        treeWalkerSpy.configure(createModuleConfig(CommentsIndentationCheck.class));
+        treeWalkerSpy.setModuleFactory(factory);
+        treeWalkerSpy.setupChild(createModuleConfig(CommentsIndentationCheck.class));
         final File file = temporaryFolder.newFile("file.java");
         final List<String> lines = new ArrayList<>();
-        lines.add(" class a%$# {} ");
-        final FileText fileText = new FileText(file, lines);
-
-        try {
-            treeWalker.processFiltered(file, fileText);
-            fail("file is not compilable, exception is expected");
-        }
-        catch (CheckstyleException exception) {
-            final String message =
-                    "TokenStreamRecognitionException occurred while parsing file";
-            assertTrue("Error message is unexpected",
-                    exception.getMessage().contains(message));
-        }
+        lines.add("class Test {}");
+        treeWalkerSpy.processFiltered(file, new FileText(file, lines));
+        verifyPrivate(treeWalkerSpy, times(1)).invoke("walk",
+                any(DetailAST.class), any(FileContents.class), any(classAstState));
+        verifyPrivate(treeWalkerSpy, times(0)).invoke("getFilteredMessages",
+                any(String.class), any(FileContents.class), any(DetailAST.class));
     }
 
     @Test
@@ -559,6 +560,11 @@ public void testExternalResourceFiltersWithNoExternalResource() throws Exception
         verify(checkerConfig, filePath, expected);
     }
 
+    /**
+     * Non meaningful javadoc just to contain "noinspection" tag.
+     * Till https://youtrack.jetbrains.com/issue/IDEA-187210
+     * @noinspection JUnitTestCaseWithNoTests
+     */
     private static class BadJavaDocCheck extends AbstractCheck {
 
         @Override
@@ -578,6 +584,11 @@ public int[] getRequiredTokens() {
 
     }
 
+    /**
+     * Non meaningful javadoc just to contain "noinspection" tag.
+     * Till https://youtrack.jetbrains.com/issue/IDEA-187210
+     * @noinspection JUnitTestCaseWithNoTests
+     */
     private static class VerifyInitCheck extends AbstractCheck {
 
         private static boolean initWasCalled;
@@ -609,6 +620,11 @@ public static boolean isInitWasCalled() {
 
     }
 
+    /**
+     * Non meaningful javadoc just to contain "noinspection" tag.
+     * Till https://youtrack.jetbrains.com/issue/IDEA-187210
+     * @noinspection JUnitTestCaseWithNoTests
+     */
     private static class VerifyDestroyCheck extends AbstractCheck {
 
         private static boolean destroyWasCalled;
@@ -644,6 +660,11 @@ public static boolean isDestroyWasCalled() {
 
     }
 
+    /**
+     * Non meaningful javadoc just to contain "noinspection" tag.
+     * Till https://youtrack.jetbrains.com/issue/IDEA-187210
+     * @noinspection JUnitTestCaseWithNoTests
+     */
     private static class VerifyDestroyCommentCheck extends VerifyDestroyCheck {
 
         @Override
@@ -653,6 +674,11 @@ public boolean isCommentNodesRequired() {
 
     }
 
+    /**
+     * Non meaningful javadoc just to contain "noinspection" tag.
+     * Till https://youtrack.jetbrains.com/issue/IDEA-187210
+     * @noinspection JUnitTestCaseWithNoTests
+     */
     private static class RequiredTokenIsNotInDefaultsCheck extends AbstractCheck {
 
         @Override
@@ -672,6 +698,11 @@ public int[] getAcceptableTokens() {
 
     }
 
+    /**
+     * Non meaningful javadoc just to contain "noinspection" tag.
+     * Till https://youtrack.jetbrains.com/issue/IDEA-187210
+     * @noinspection JUnitTestCaseWithNoTests
+     */
     private static class RequiredTokenIsEmptyIntArray extends AbstractCheck {
 
         @Override