Skip to content

Commit

Permalink
Issue #11973: Fix NPE on empty switch statements in VariableDeclarati…
Browse files Browse the repository at this point in the history
…onUsageDistance
  • Loading branch information
stoyanK7 committed Sep 15, 2022
1 parent d02f5e7 commit 4dab5af
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 26 deletions.
9 changes: 0 additions & 9 deletions .ci/pitest-suppressions/pitest-coding-1-suppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,4 @@
<description>replaced call to com/puppycrawl/tools/checkstyle/api/DetailAST::getNextSibling with receiver</description>
<lineContent>DetailAST exprBetweenBrackets = openingBracket.getNextSibling();</lineContent>
</mutation>

<mutation unstable="false">
<sourceFile>VariableDeclarationUsageDistanceCheck.java</sourceFile>
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.VariableDeclarationUsageDistanceCheck</mutatedClass>
<mutatedMethod>lambda$getFirstCaseGroupOrSwitchRule$2</mutatedMethod>
<mutator>org.pitest.mutationtest.engine.gregor.mutators.experimental.NakedReceiverMutator</mutator>
<description>replaced call to com/puppycrawl/tools/checkstyle/api/DetailAST::findFirstToken with receiver</description>
<lineContent>.orElseGet(() -&gt; block.findFirstToken(TokenTypes.SWITCH_RULE));</lineContent>
</mutation>
</suppressedMutations>
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.util.AbstractMap.SimpleEntry;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map.Entry;
import java.util.Objects;
Expand Down Expand Up @@ -809,18 +810,8 @@ private static DetailAST getFirstNodeInsideIfBlock(
*/
private static DetailAST getFirstNodeInsideSwitchBlock(
DetailAST block, DetailAST variable) {
final DetailAST currentNode = getFirstCaseGroupOrSwitchRule(block);
final List<DetailAST> variableUsageExpressions =
new ArrayList<>();

// Checking variable usage inside all CASE_GROUP and SWITCH_RULE ast's.
TokenUtil.forEachChild(block, currentNode.getType(), node -> {
final DetailAST lastNodeInCaseGroup =
node.getLastChild();
if (isChild(lastNodeInCaseGroup, variable)) {
variableUsageExpressions.add(lastNodeInCaseGroup);
}
});
getVariableUsageExpressionsInsideSwitchBlock(block, variable);

// If variable usage exists in several related blocks, then
// firstNodeInsideBlock = null, otherwise if variable usage exists
Expand All @@ -835,15 +826,29 @@ private static DetailAST getFirstNodeInsideSwitchBlock(
}

/**
* Helper method for getFirstNodeInsideSwitchBlock to return the first CASE_GROUP or
* SWITCH_RULE ast.
* Helper method for getFirstNodeInsideSwitchBlock to return all variable
* usage expressions inside a given switch block.
*
* @param block the switch block to check.
* @return DetailAST of the first CASE_GROUP or SWITCH_RULE.
* @param variable variable which is checked for in switch block.
* @return List of usages or empty list if none are found.
*/
private static DetailAST getFirstCaseGroupOrSwitchRule(DetailAST block) {
return Optional.ofNullable(block.findFirstToken(TokenTypes.CASE_GROUP))
.orElseGet(() -> block.findFirstToken(TokenTypes.SWITCH_RULE));
private static List<DetailAST> getVariableUsageExpressionsInsideSwitchBlock(DetailAST block,
DetailAST variable) {
return Optional
.ofNullable(block.findFirstToken(TokenTypes.CASE_GROUP))
.or(() -> Optional.ofNullable(block.findFirstToken(TokenTypes.SWITCH_RULE)))
.map(blockStatement -> {
final List<DetailAST> variableUsageExpressions = new ArrayList<>();
TokenUtil.forEachChild(block, blockStatement.getType(), child -> {
final DetailAST lastNodeInCaseGroup = child.getLastChild();
if (isChild(lastNodeInCaseGroup, variable)) {
variableUsageExpressions.add(lastNodeInCaseGroup);
}
});
return variableUsageExpressions;
})
.orElseGet(Collections::emptyList);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,4 +309,15 @@ public void testVariableDeclarationUsageDistanceSwitchExpressions() throws Excep
verifyWithInlineConfigParser(getNonCompilablePath(filename), expected);
}

@Test
public void testVariableDeclarationUsageDistanceSwitchExpressions2() throws Exception {
final int maxDistance = 1;
final String[] expected = {
"16:9: " + getCheckMessage(MSG_KEY, "i", 2, maxDistance),
};

final String filename = "InputVariableDeclarationUsageDistanceCheckSwitchExpressions2.java";
verifyWithInlineConfigParser(getNonCompilablePath(filename), expected);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
VariableDeclarationUsageDistance
allowedDistance = 1
ignoreVariablePattern = (default)
validateBetweenScopes = true
ignoreFinal = false
*/

//non-compiled with javac: Compilable with Java14
package com.puppycrawl.tools.checkstyle.checks.coding.variabledeclarationusagedistance;

public class InputVariableDeclarationUsageDistanceCheckSwitchExpressions2 {
void issue11973() {
int i = -1; // violation 'Distance between.*'i'.*and.*first usage is 2, but allowed 1.'
int x = -1; // ok
switch (i) {
case 1 -> {
x++;
i++;
}
};

int a = -1; // ok
int a123 = switch (a) {
case 1 -> {
yield 2;
}
default -> {
yield a;
}
};

int b = -1; // ok
int b123 = switch (b) {
case 1 -> {
yield b;
}
default -> {
yield 1;
}
};

int day = 1; // ok
int c = -1;
int c123 = switch (c) {
case 1:
case 2:
default:
c++;
c--;
yield day++;
};

int day1 = 1; // ok
int cc = -1;
int d123 = switch (cc) {
case 1 -> {
cc++;
cc--;
yield day1;
}
default -> {
yield 0;
}
};

int day2 = 1; // ok
int ccc = -1;
int e123 = switch (ccc) {
case 1 -> {
ccc++;
ccc--;
yield ccc;
}
case 2 -> {
yield 2;
}
default -> {
ccc++;
ccc--;
ccc++;
day2++;
yield 3;
}
};

int u = 0;
switch (u) { };
}
}

0 comments on commit 4dab5af

Please sign in to comment.