Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #11973: Fix NPE on empty switch statements in VariableDeclarati… #12052

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2437,13 +2437,6 @@
</details>
</checkerFrameworkError>

<checkerFrameworkError unstable="false">
<fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheck.java</fileName>
<specifier>introduce.eliminate</specifier>
<message>It is bad style to create an Optional just to chain methods to get a value.</message>
<lineContent>.orElseGet(() -&gt; block.findFirstToken(TokenTypes.SWITCH_RULE));</lineContent>
</checkerFrameworkError>

<checkerFrameworkError unstable="false">
<fileName>checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/VariableDeclarationUsageDistanceCheck.java</fileName>
<specifier>not.interned</specifier>
Expand Down
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 @@ -809,18 +809,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 +825,32 @@ 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) {
final Optional<DetailAST> firstToken = TokenUtil.findFirstTokenByPredicate(block, child -> {
return child.getType() == TokenTypes.SWITCH_RULE
|| child.getType() == TokenTypes.CASE_GROUP;
});

final List<DetailAST> variableUsageExpressions = new ArrayList<>();

firstToken.ifPresent(token -> {
TokenUtil.forEachChild(block, token.getType(), child -> {
final DetailAST lastNodeInCaseGroup = child.getLastChild();
if (isChild(lastNodeInCaseGroup, variable)) {
variableUsageExpressions.add(lastNodeInCaseGroup);
}
});
});

return variableUsageExpressions;
}

/**
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) { };
}
}