diff --git a/.ci/pitest-suppressions/pitest-coding-1-suppressions.xml b/.ci/pitest-suppressions/pitest-coding-1-suppressions.xml index 6d6c4cabebb..4b4a19c52d9 100644 --- a/.ci/pitest-suppressions/pitest-coding-1-suppressions.xml +++ b/.ci/pitest-suppressions/pitest-coding-1-suppressions.xml @@ -126,51 +126,6 @@ switch (examineNode.getType()) { - - VariableDeclarationUsageDistanceCheck.java - com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck - getFirstNodeInsideIfBlock - org.pitest.mutationtest.engine.gregor.mutators.experimental.NakedReceiverMutator - replaced call to com/puppycrawl/tools/checkstyle/api/DetailAST::getLastChild with receiver - currentNode = currentNode.getLastChild(); - - - - VariableDeclarationUsageDistanceCheck.java - com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck - getFirstNodeInsideIfBlock - org.pitest.mutationtest.engine.gregor.mutators.NonVoidMethodCallMutator - removed call to com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheck::isChild - else if (isChild(currentNode, variable)) { - - - - VariableDeclarationUsageDistanceCheck.java - com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck - getFirstNodeInsideIfBlock - org.pitest.mutationtest.engine.gregor.mutators.RemoveConditionalMutator_EQUAL_ELSE - removed conditional - replaced equality check with false - else if (isChild(currentNode, variable)) { - - - - VariableDeclarationUsageDistanceCheck.java - com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck - getFirstNodeInsideIfBlock - org.pitest.mutationtest.engine.gregor.mutators.NonVoidMethodCallMutator - removed call to com/puppycrawl/tools/checkstyle/api/DetailAST::getType - if (currentNode.getType() == TokenTypes.LITERAL_IF) { - - - - VariableDeclarationUsageDistanceCheck.java - com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck - getFirstNodeInsideIfBlock - org.pitest.mutationtest.engine.gregor.mutators.RemoveConditionalMutator_EQUAL_ELSE - removed conditional - replaced equality check with false - if (currentNode.getType() == TokenTypes.LITERAL_IF) { - - VariableDeclarationUsageDistanceCheck.java com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck @@ -228,7 +183,7 @@ VariableDeclarationUsageDistanceCheck.java com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck - lambda$getFirstCaseGroupOrSwitchRule$1 + lambda$getFirstCaseGroupOrSwitchRule$2 org.pitest.mutationtest.engine.gregor.mutators.experimental.NakedReceiverMutator replaced call to com/puppycrawl/tools/checkstyle/api/DetailAST::findFirstToken with receiver .orElseGet(() -> block.findFirstToken(TokenTypes.SWITCH_RULE)); diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheck.java index 34fb64fe233..893fa3fe31b 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheck.java @@ -761,37 +761,26 @@ private static DetailAST getFirstNodeInsideIfBlock( DetailAST firstNodeInsideBlock = null; if (!isVariableInOperatorExpr(block, variable)) { - DetailAST currentNode = block.getLastChild(); - final List 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 slistToken = TokenUtil + .findFirstTokenByPredicate(block, token -> token.getType() == TokenTypes.SLIST); + final DetailAST lastNode = block.getLastChild(); + DetailAST previousNode = lastNode.getPreviousSibling(); - // Looking into ELSE block, get its first child and analyze it. - currentNode = currentNode.getFirstChild(); + if (slistToken.isEmpty() + && lastNode.getType() == TokenTypes.LITERAL_ELSE) { - if (currentNode.getType() == TokenTypes.LITERAL_IF) { - currentNode = currentNode.getLastChild(); - } - else if (isChild(currentNode, variable)) { - variableUsageExpressions.add(currentNode); - currentNode = null; - } + // Is if statement without '{}' and has a following else branch, + // then change previousNode to the if statement body. + previousNode = previousNode.getPreviousSibling(); + } + + final List 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)) { - variableUsageExpressions.add(currentNode); + if (isChild(lastNode, variable)) { + variableUsageExpressions.add(lastNode); } // If variable usage exists in several related blocks, then diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheckTest.java index bf2f3250dcf..eb9e37e0517 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheckTest.java @@ -91,6 +91,23 @@ public void testGeneralLogic2() throws Exception { getPath("InputVariableDeclarationUsageDistanceGeneral2.java"), expected); } + @Test + public void testIfStatements() throws Exception { + final String[] expected = { + "18:9: " + getCheckMessage(MSG_KEY, "a", 4, 1), + "28:9: " + getCheckMessage(MSG_KEY, "a", 2, 1), + "32:9: " + getCheckMessage(MSG_KEY, "b", 2, 1), + "38:9: " + getCheckMessage(MSG_KEY, "c", 3, 1), + "49:9: " + getCheckMessage(MSG_KEY, "b", 2, 1), + "50:9: " + getCheckMessage(MSG_KEY, "c", 3, 1), + "51:9: " + getCheckMessage(MSG_KEY, "d", 4, 1), + "63:9: " + getCheckMessage(MSG_KEY, "a", 4, 1), + + }; + verifyWithInlineConfigParser( + getPath("InputVariableDeclarationUsageDistanceIfStatements.java"), expected); + } + @Test public void testDistance() throws Exception { final String[] expected = { diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/variabledeclarationusagedistance/InputVariableDeclarationUsageDistanceIfStatements.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/variabledeclarationusagedistance/InputVariableDeclarationUsageDistanceIfStatements.java new file mode 100644 index 00000000000..bc08d547ffc --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/variabledeclarationusagedistance/InputVariableDeclarationUsageDistanceIfStatements.java @@ -0,0 +1,72 @@ +/* +VariableDeclarationUsageDistance +allowedDistance = 1 +ignoreVariablePattern = (default) +validateBetweenScopes = true +ignoreFinal = false + + +*/ + +package com.puppycrawl.tools.checkstyle.checks.coding.variabledeclarationusagedistance; + +import java.util.HashSet; +import java.util.Set; + +public class InputVariableDeclarationUsageDistanceIfStatements { + void method2() { + int a = 12; // violation + if (true) { + method2(); + checkIfStatementWithoutParen(); + method2(); + a++; + } + } + + void checkIfStatementWithoutParen() { + int a = 12; // violation + method2(); + if (true) + a++; + int b = 12; // violation + method2(); + if (false) + method2(); + else if(true) + b++; + int c = 12; // violation + method2(); + checkIfStatementWithoutParen(); + if (true) + c++; + else + method2(); + } + + void testConsecutiveIfStatements() { + int a = 12; // ok + int b = 13; // violation + int c = 14; // violation + int d = 15; // violation + if (true) + a++; + if (true) + b++; + if (false) + c++; + if (true) + d++; + } + + int testReturnStatement() { + int a = 1; // violation + testConsecutiveIfStatements(); + testConsecutiveIfStatements(); + testConsecutiveIfStatements(); + if (true) + return a; + + return 0; + } +}