Skip to content

Commit

Permalink
Issue #13167: Fix NPE in UnusedLocalVariable check
Browse files Browse the repository at this point in the history
  • Loading branch information
Vyom-Yadav authored and romani committed May 25, 2024
1 parent 8605dd8 commit c15242f
Show file tree
Hide file tree
Showing 6 changed files with 202 additions and 6 deletions.
39 changes: 39 additions & 0 deletions config/pitest-suppressions/pitest-coding-2-suppressions.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?xml version="1.0" encoding="UTF-8"?>
<suppressedMutations>
<!-- until https://github.com/checkstyle/checkstyle/issues/14894 -->
<mutation unstable="false">
<sourceFile>UnusedLocalVariableCheck.java</sourceFile>
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.UnusedLocalVariableCheck</mutatedClass>
<mutatedMethod>getBlockContainingLocalAnonInnerClass</mutatedMethod>
<mutator>org.pitest.mutationtest.engine.gregor.mutators.NegateConditionalsMutator</mutator>
<description>negated conditional</description>
<lineContent>if (currentAst.getType() == TokenTypes.LAMBDA) {</lineContent>
</mutation>

<mutation unstable="false">
<sourceFile>UnusedLocalVariableCheck.java</sourceFile>
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.UnusedLocalVariableCheck</mutatedClass>
<mutatedMethod>getBlockContainingLocalAnonInnerClass</mutatedMethod>
<mutator>org.pitest.mutationtest.engine.gregor.mutators.RemoveConditionalMutator_EQUAL_IF</mutator>
<description>removed conditional - replaced equality check with true</description>
<lineContent>if (currentAst.getType() == TokenTypes.LAMBDA) {</lineContent>
</mutation>

<mutation unstable="false">
<sourceFile>UnusedLocalVariableCheck.java</sourceFile>
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.UnusedLocalVariableCheck</mutatedClass>
<mutatedMethod>isInsideLocalAnonInnerClass</mutatedMethod>
<mutator>org.pitest.mutationtest.engine.gregor.mutators.NegateConditionalsMutator</mutator>
<description>negated conditional</description>
<lineContent>if (currentAst.getType() == TokenTypes.SLIST) {</lineContent>
</mutation>

<mutation unstable="false">
<sourceFile>UnusedLocalVariableCheck.java</sourceFile>
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.UnusedLocalVariableCheck</mutatedClass>
<mutatedMethod>isInsideLocalAnonInnerClass</mutatedMethod>
<mutator>org.pitest.mutationtest.engine.gregor.mutators.RemoveConditionalMutator_EQUAL_IF</mutator>
<description>removed conditional - replaced equality check with true</description>
<lineContent>if (currentAst.getType() == TokenTypes.SLIST) {</lineContent>
</mutation>
</suppressedMutations>
7 changes: 7 additions & 0 deletions config/spotbugs-exclude.xml
Original file line number Diff line number Diff line change
Expand Up @@ -315,4 +315,11 @@
</Or>
<Bug pattern="CT_CONSTRUCTOR_THROW"/>
</Match>
<Match>
<!-- variable is just being saved for further analysis and isn't the main decider
of the loop if it is done or not -->
<Class name="com.puppycrawl.tools.checkstyle.checks.coding.UnusedLocalVariableCheck"/>
<Method name="getBlockContainingLocalAnonInnerClass"/>
<Bug pattern="SLS_SUSPICIOUS_LOOP_SEARCH"/>
</Match>
</FindBugsFilter>
Original file line number Diff line number Diff line change
Expand Up @@ -413,16 +413,18 @@ private static boolean isNonLocalTypeDeclaration(DetailAST typeDeclAst) {
private static DetailAST getBlockContainingLocalAnonInnerClass(DetailAST literalNewAst) {
DetailAST currentAst = literalNewAst;
DetailAST result = null;
while (!TokenUtil.isOfType(currentAst, CONTAINERS_FOR_ANON_INNERS)) {
if (currentAst.getType() == TokenTypes.LAMBDA
&& currentAst.getParent()
.getParent().getParent().getType() == TokenTypes.OBJBLOCK) {
result = currentAst;
break;
DetailAST topMostLambdaAst = null;
while (currentAst != null && !TokenUtil.isOfType(currentAst, CONTAINERS_FOR_ANON_INNERS)) {
if (currentAst.getType() == TokenTypes.LAMBDA) {
topMostLambdaAst = currentAst;
}
currentAst = currentAst.getParent();
result = currentAst;
}

if (currentAst == null) {
result = topMostLambdaAst;
}
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,36 @@ public void testUnusedLocalVarEnum() throws Exception {
expected);
}

@Test
public void testUnusedLocalVarLambdas() throws Exception {
final String[] expected = {
"14:9: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "hoo"),
"20:17: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "j"),
"31:9: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "hoo2"),
"32:9: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "hoo3"),
"33:15: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "myComponent"),
"34:19: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "myComponent3"),
"40:25: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "j"),
"52:21: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "j"),
"65:17: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "ja"),
"73:9: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "k"),
};
verifyWithInlineConfigParser(
getPath("InputUnusedLocalVariableLambdaExpression.java"),
expected);
}

@Test
public void testUnusedLocalVariableLocalClasses() throws Exception {
final String[] expected = {
"14:9: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "a"),
"15:9: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "ab"),
};
verifyWithInlineConfigParser(
getPath("InputUnusedLocalVariableLocalClasses.java"),
expected);
}

@Test
public void testUnusedLocalVarRecords() throws Exception {
final String[] expected = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/*
UnusedLocalVariable
*/

package com.puppycrawl.tools.checkstyle.checks.coding.unusedlocalvariable;

import java.util.function.Supplier;

public class InputUnusedLocalVariableLambdaExpression {
private final LambdaTest<String> myComponent = LambdaTest.lazy(() -> {
String foo = "";
String hoo = ""; // violation
new Runnable() {
String hoo = "";

@Override
public void run() {
String j = hoo; // violation
String ja = foo;
ja += "asd";
}
};
return "";
});

final LambdaTest<String> nestedLambdas = LambdaTest.lazy(() -> {
String foo = "";
String hoo = "";
String hoo2 = ""; // violation
String hoo3 = ""; // violation
final LambdaTest<String> myComponent = LambdaTest.lazy(() -> { // violation
final LambdaTest<String> myComponent3 = LambdaTest.lazy(() -> { // violation
new Runnable() {
String hoo2 = "";

@Override
public void run() {
String j = hoo; // violation
String ja = foo;
ja += hoo2;
}
};
return "";
});
new Runnable() {
String hoo3 = "";

@Override
public void run() {
String j = hoo3; // violation
String ja = foo;
ja += "asd";
}
};
return "";
});
new Runnable() {
String hoo2 = "";

@Override
public void run() {
String j = hoo2;
String ja = foo; // violation
j += hoo2;
}
};
return "";
});

final LambdaTest<String> nestedLambdas2 = LambdaTest.lazy(() -> {
String k = ""; // violation
final LambdaTest<String> nestedLambdas = LambdaTest.lazy(() -> {
new LambdaTest<String>() {
void foo() {
String j = k;
j += "asd";
}
};
return "";
});
return nestedLambdas.toString();
});
}

class LambdaTest<T> {
String k = "";
public static <T> LambdaTest<T> lazy(Supplier<T> supplier) {
return new LambdaTest<>();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
UnusedLocalVariable
*/

package com.puppycrawl.tools.checkstyle.checks.coding.unusedlocalvariable;

public class InputUnusedLocalVariableLocalClasses {

int a = 12;

void foo() {
int a = 12; // violation
int ab = 1; // violation

class asd {
InputUnusedLocalVariableLocalClasses a = new InputUnusedLocalVariableLocalClasses() {
void asd() {
System.out.println(a);
}
};
}
}

}

0 comments on commit c15242f

Please sign in to comment.