Skip to content

Commit c15242f

Browse files
Vyom-Yadavromani
Vyom-Yadav
authored and
romani
committed
Issue #13167: Fix NPE in UnusedLocalVariable check
1 parent 8605dd8 commit c15242f

6 files changed

Lines changed: 202 additions & 6 deletions

File tree

‎config/pitest-suppressions/pitest-coding-2-suppressions.xml‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<suppressedMutations>
3+
<!-- until https://github.com/checkstyle/checkstyle/issues/14894 -->
4+
<mutation unstable="false">
5+
<sourceFile>UnusedLocalVariableCheck.java</sourceFile>
6+
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.UnusedLocalVariableCheck</mutatedClass>
7+
<mutatedMethod>getBlockContainingLocalAnonInnerClass</mutatedMethod>
8+
<mutator>org.pitest.mutationtest.engine.gregor.mutators.NegateConditionalsMutator</mutator>
9+
<description>negated conditional</description>
10+
<lineContent>if (currentAst.getType() == TokenTypes.LAMBDA) {</lineContent>
11+
</mutation>
12+
13+
<mutation unstable="false">
14+
<sourceFile>UnusedLocalVariableCheck.java</sourceFile>
15+
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.UnusedLocalVariableCheck</mutatedClass>
16+
<mutatedMethod>getBlockContainingLocalAnonInnerClass</mutatedMethod>
17+
<mutator>org.pitest.mutationtest.engine.gregor.mutators.RemoveConditionalMutator_EQUAL_IF</mutator>
18+
<description>removed conditional - replaced equality check with true</description>
19+
<lineContent>if (currentAst.getType() == TokenTypes.LAMBDA) {</lineContent>
20+
</mutation>
21+
22+
<mutation unstable="false">
23+
<sourceFile>UnusedLocalVariableCheck.java</sourceFile>
24+
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.UnusedLocalVariableCheck</mutatedClass>
25+
<mutatedMethod>isInsideLocalAnonInnerClass</mutatedMethod>
26+
<mutator>org.pitest.mutationtest.engine.gregor.mutators.NegateConditionalsMutator</mutator>
27+
<description>negated conditional</description>
28+
<lineContent>if (currentAst.getType() == TokenTypes.SLIST) {</lineContent>
29+
</mutation>
30+
31+
<mutation unstable="false">
32+
<sourceFile>UnusedLocalVariableCheck.java</sourceFile>
33+
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.UnusedLocalVariableCheck</mutatedClass>
34+
<mutatedMethod>isInsideLocalAnonInnerClass</mutatedMethod>
35+
<mutator>org.pitest.mutationtest.engine.gregor.mutators.RemoveConditionalMutator_EQUAL_IF</mutator>
36+
<description>removed conditional - replaced equality check with true</description>
37+
<lineContent>if (currentAst.getType() == TokenTypes.SLIST) {</lineContent>
38+
</mutation>
39+
</suppressedMutations>

‎config/spotbugs-exclude.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,4 +315,11 @@
315315
</Or>
316316
<Bug pattern="CT_CONSTRUCTOR_THROW"/>
317317
</Match>
318+
<Match>
319+
<!-- variable is just being saved for further analysis and isn't the main decider
320+
of the loop if it is done or not -->
321+
<Class name="com.puppycrawl.tools.checkstyle.checks.coding.UnusedLocalVariableCheck"/>
322+
<Method name="getBlockContainingLocalAnonInnerClass"/>
323+
<Bug pattern="SLS_SUSPICIOUS_LOOP_SEARCH"/>
324+
</Match>
318325
</FindBugsFilter>

‎src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnusedLocalVariableCheck.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -413,16 +413,18 @@ private static boolean isNonLocalTypeDeclaration(DetailAST typeDeclAst) {
413413
private static DetailAST getBlockContainingLocalAnonInnerClass(DetailAST literalNewAst) {
414414
DetailAST currentAst = literalNewAst;
415415
DetailAST result = null;
416-
while (!TokenUtil.isOfType(currentAst, CONTAINERS_FOR_ANON_INNERS)) {
417-
if (currentAst.getType() == TokenTypes.LAMBDA
418-
&& currentAst.getParent()
419-
.getParent().getParent().getType() == TokenTypes.OBJBLOCK) {
420-
result = currentAst;
421-
break;
416+
DetailAST topMostLambdaAst = null;
417+
while (currentAst != null && !TokenUtil.isOfType(currentAst, CONTAINERS_FOR_ANON_INNERS)) {
418+
if (currentAst.getType() == TokenTypes.LAMBDA) {
419+
topMostLambdaAst = currentAst;
422420
}
423421
currentAst = currentAst.getParent();
424422
result = currentAst;
425423
}
424+
425+
if (currentAst == null) {
426+
result = topMostLambdaAst;
427+
}
426428
return result;
427429
}
428430

‎src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnusedLocalVariableCheckTest.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,36 @@ public void testUnusedLocalVarEnum() throws Exception {
327327
expected);
328328
}
329329

330+
@Test
331+
public void testUnusedLocalVarLambdas() throws Exception {
332+
final String[] expected = {
333+
"14:9: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "hoo"),
334+
"20:17: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "j"),
335+
"31:9: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "hoo2"),
336+
"32:9: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "hoo3"),
337+
"33:15: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "myComponent"),
338+
"34:19: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "myComponent3"),
339+
"40:25: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "j"),
340+
"52:21: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "j"),
341+
"65:17: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "ja"),
342+
"73:9: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "k"),
343+
};
344+
verifyWithInlineConfigParser(
345+
getPath("InputUnusedLocalVariableLambdaExpression.java"),
346+
expected);
347+
}
348+
349+
@Test
350+
public void testUnusedLocalVariableLocalClasses() throws Exception {
351+
final String[] expected = {
352+
"14:9: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "a"),
353+
"15:9: " + getCheckMessage(MSG_UNUSED_LOCAL_VARIABLE, "ab"),
354+
};
355+
verifyWithInlineConfigParser(
356+
getPath("InputUnusedLocalVariableLocalClasses.java"),
357+
expected);
358+
}
359+
330360
@Test
331361
public void testUnusedLocalVarRecords() throws Exception {
332362
final String[] expected = {

‎src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unusedlocalvariable/InputUnusedLocalVariableLambdaExpression.java‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/*
2+
UnusedLocalVariable
3+
4+
5+
*/
6+
7+
package com.puppycrawl.tools.checkstyle.checks.coding.unusedlocalvariable;
8+
9+
import java.util.function.Supplier;
10+
11+
public class InputUnusedLocalVariableLambdaExpression {
12+
private final LambdaTest<String> myComponent = LambdaTest.lazy(() -> {
13+
String foo = "";
14+
String hoo = ""; // violation
15+
new Runnable() {
16+
String hoo = "";
17+
18+
@Override
19+
public void run() {
20+
String j = hoo; // violation
21+
String ja = foo;
22+
ja += "asd";
23+
}
24+
};
25+
return "";
26+
});
27+
28+
final LambdaTest<String> nestedLambdas = LambdaTest.lazy(() -> {
29+
String foo = "";
30+
String hoo = "";
31+
String hoo2 = ""; // violation
32+
String hoo3 = ""; // violation
33+
final LambdaTest<String> myComponent = LambdaTest.lazy(() -> { // violation
34+
final LambdaTest<String> myComponent3 = LambdaTest.lazy(() -> { // violation
35+
new Runnable() {
36+
String hoo2 = "";
37+
38+
@Override
39+
public void run() {
40+
String j = hoo; // violation
41+
String ja = foo;
42+
ja += hoo2;
43+
}
44+
};
45+
return "";
46+
});
47+
new Runnable() {
48+
String hoo3 = "";
49+
50+
@Override
51+
public void run() {
52+
String j = hoo3; // violation
53+
String ja = foo;
54+
ja += "asd";
55+
}
56+
};
57+
return "";
58+
});
59+
new Runnable() {
60+
String hoo2 = "";
61+
62+
@Override
63+
public void run() {
64+
String j = hoo2;
65+
String ja = foo; // violation
66+
j += hoo2;
67+
}
68+
};
69+
return "";
70+
});
71+
72+
final LambdaTest<String> nestedLambdas2 = LambdaTest.lazy(() -> {
73+
String k = ""; // violation
74+
final LambdaTest<String> nestedLambdas = LambdaTest.lazy(() -> {
75+
new LambdaTest<String>() {
76+
void foo() {
77+
String j = k;
78+
j += "asd";
79+
}
80+
};
81+
return "";
82+
});
83+
return nestedLambdas.toString();
84+
});
85+
}
86+
87+
class LambdaTest<T> {
88+
String k = "";
89+
public static <T> LambdaTest<T> lazy(Supplier<T> supplier) {
90+
return new LambdaTest<>();
91+
}
92+
}

‎src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unusedlocalvariable/InputUnusedLocalVariableLocalClasses.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
UnusedLocalVariable
3+
4+
5+
*/
6+
7+
package com.puppycrawl.tools.checkstyle.checks.coding.unusedlocalvariable;
8+
9+
public class InputUnusedLocalVariableLocalClasses {
10+
11+
int a = 12;
12+
13+
void foo() {
14+
int a = 12; // violation
15+
int ab = 1; // violation
16+
17+
class asd {
18+
InputUnusedLocalVariableLocalClasses a = new InputUnusedLocalVariableLocalClasses() {
19+
void asd() {
20+
System.out.println(a);
21+
}
22+
};
23+
}
24+
}
25+
26+
}

0 commit comments

Comments
 (0)