Issue checkstyle#11720: Kill surviving mutation in VariableDeclaratio…

…nUsageDistanceCheck in getFirstNodeInsideIfBlock
Vyom-Yadav · Jul 29, 2022 · 3fe931a · 3fe931a
1 parent af99318
commit 3fe931a
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 71 deletions.
diff --git a/.ci/pitest-suppressions/pitest-coding-1-suppressions.xml b/.ci/pitest-suppressions/pitest-coding-1-suppressions.xml
@@ -126,51 +126,6 @@
     <lineContent>switch (examineNode.getType()) {</lineContent>
   </mutation>
 
-  <mutation unstable="false">
-    <sourceFile>VariableDeclarationUsageDistanceCheck.java</sourceFile>
-    <mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck</mutatedClass>
-    <mutatedMethod>getFirstNodeInsideIfBlock</mutatedMethod>
-    <mutator>org.pitest.mutationtest.engine.gregor.mutators.experimental.NakedReceiverMutator</mutator>
-    <description>replaced call to com/puppycrawl/tools/checkstyle/api/DetailAST::getLastChild with receiver</description>
-    <lineContent>currentNode = currentNode.getLastChild();</lineContent>
-  </mutation>
-
-  <mutation unstable="false">
-    <sourceFile>VariableDeclarationUsageDistanceCheck.java</sourceFile>
-    <mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck</mutatedClass>
-    <mutatedMethod>getFirstNodeInsideIfBlock</mutatedMethod>
-    <mutator>org.pitest.mutationtest.engine.gregor.mutators.NonVoidMethodCallMutator</mutator>
-    <description>removed call to com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheck::isChild</description>
-    <lineContent>else if (isChild(currentNode, variable)) {</lineContent>
-  </mutation>
-
-  <mutation unstable="false">
-    <sourceFile>VariableDeclarationUsageDistanceCheck.java</sourceFile>
-    <mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck</mutatedClass>
-    <mutatedMethod>getFirstNodeInsideIfBlock</mutatedMethod>
-    <mutator>org.pitest.mutationtest.engine.gregor.mutators.RemoveConditionalMutator_EQUAL_ELSE</mutator>
-    <description>removed conditional - replaced equality check with false</description>
-    <lineContent>else if (isChild(currentNode, variable)) {</lineContent>
-  </mutation>
-
-  <mutation unstable="false">
-    <sourceFile>VariableDeclarationUsageDistanceCheck.java</sourceFile>
-    <mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck</mutatedClass>
-    <mutatedMethod>getFirstNodeInsideIfBlock</mutatedMethod>
-    <mutator>org.pitest.mutationtest.engine.gregor.mutators.NonVoidMethodCallMutator</mutator>
-    <description>removed call to com/puppycrawl/tools/checkstyle/api/DetailAST::getType</description>
-    <lineContent>if (currentNode.getType() == TokenTypes.LITERAL_IF) {</lineContent>
-  </mutation>
-
-  <mutation unstable="false">
-    <sourceFile>VariableDeclarationUsageDistanceCheck.java</sourceFile>
-    <mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck</mutatedClass>
-    <mutatedMethod>getFirstNodeInsideIfBlock</mutatedMethod>
-    <mutator>org.pitest.mutationtest.engine.gregor.mutators.RemoveConditionalMutator_EQUAL_ELSE</mutator>
-    <description>removed conditional - replaced equality check with false</description>
-    <lineContent>if (currentNode.getType() == TokenTypes.LITERAL_IF) {</lineContent>
-  </mutation>
-
   <mutation unstable="false">
     <sourceFile>VariableDeclarationUsageDistanceCheck.java</sourceFile>
     <mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck</mutatedClass>

diff --git a/.../com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheck.java b/.../com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheck.java
@@ -762,36 +762,25 @@ private static DetailAST getFirstNodeInsideIfBlock(
         DetailAST firstNodeInsideBlock = null;
 
         if (!isVariableInOperatorExpr(block, variable)) {
-            DetailAST currentNode = block.getLastChild();
-            final List<DetailAST> variableUsageExpressions =
-                    new ArrayList<>();
-
-            while (currentNode != null
-                    && currentNode.getType() == TokenTypes.LITERAL_ELSE) {
-                final DetailAST previousNode =
-                        currentNode.getPreviousSibling();
-
-                // Checking variable usage inside IF block.
-                if (isChild(previousNode, variable)) {
-                    variableUsageExpressions.add(previousNode);
-                }
+            final Optional<DetailAST> slistToken = TokenUtil
+                .findFirstTokenByPredicate(block, token -> token.getType() == TokenTypes.SLIST);
+            final DetailAST currentNode = block.getLastChild();
+            DetailAST previousNode = currentNode.getPreviousSibling();
 
-                // Looking into ELSE block, get its first child and analyze it.
-                currentNode = currentNode.getFirstChild();
+            // Is if statement without '{}'
+            if (slistToken.isEmpty()
+                && currentNode.getType() == TokenTypes.LITERAL_ELSE) {
 
-                if (currentNode.getType() == TokenTypes.LITERAL_IF) {
-                    currentNode = currentNode.getLastChild();
-                }
-                else if (isChild(currentNode, variable)) {
-                    variableUsageExpressions.add(currentNode);
-                    currentNode = null;
-                }
+                // Change previousNode to the if statement body
+                previousNode = previousNode.getPreviousSibling();
+            }
+
+            final List<DetailAST> variableUsageExpressions = new ArrayList<>();
+            if (isChild(previousNode, variable)) {
+                variableUsageExpressions.add(previousNode);
             }
 
-            // If IF block doesn't include ELSE then analyze variable usage
-            // only inside IF block.
-            if (currentNode != null
-                    && isChild(currentNode, variable)) {
+            if (isChild(currentNode, variable)) {
                 variableUsageExpressions.add(currentNode);
             }
 

diff --git a/.../puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheckTest.java b/.../puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheckTest.java
@@ -86,6 +86,9 @@ public void testGeneralLogic2() throws Exception {
             "17:9: " + getCheckMessage(MSG_KEY, "first", 5, 1),
             "29:9: " + getCheckMessage(MSG_KEY, "allInvariants", 2, 1),
             "59:9: " + getCheckMessage(MSG_KEY, "a", 2, 1),
+            "68:9: " + getCheckMessage(MSG_KEY, "a", 4, 1),
+            "78:9: " + getCheckMessage(MSG_KEY, "a", 2, 1),
+            "83:9: " + getCheckMessage(MSG_KEY, "b", 2, 1),
         };
         verifyWithInlineConfigParser(
                 getPath("InputVariableDeclarationUsageDistanceGeneral2.java"), expected);

diff --git a/...oding/variabledeclarationusagedistance/InputVariableDeclarationUsageDistanceGeneral2.java b/...oding/variabledeclarationusagedistance/InputVariableDeclarationUsageDistanceGeneral2.java
@@ -63,4 +63,28 @@ void method() throws Exception {
             a.equals("");
         }
     }
+
+    void method2() {
+        int a = 12; // violation
+        if (true) {
+            method2();
+            p();
+            method2();
+            a++;
+        }
+    }
+
+    void checkIfStatementWithoutParen() {
+        int a = 12; // violation
+        method2();
+        if (true)
+            a++;
+
+        int b = 12; // violation
+        method2();
+        if (false)
+            method2();
+        else if(true)
+            b++;
+    }
 }