Skip to content

Commit d010bda

Browse files
mahfouz72rnveach
mahfouz72
authored and
rnveach
committed
Issue #15159: RedundantModifierCheck should violate final modifier on unnamed variables
1 parent d32c27b commit d010bda

File tree

8 files changed

+287
-10
lines changed

8 files changed

+287
-10
lines changed

‎config/pmd.xml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,8 @@
372372
| //ClassDeclaration[@SimpleName='SarifLogger']//MethodDeclaration[@Name='escape']
373373
| //ClassDeclaration[@SimpleName='LeftCurlyCheck'
374374
or @SimpleName='FinalLocalVariableCheck'
375-
or @SimpleName='NPathComplexityCheck']
375+
or @SimpleName='NPathComplexityCheck'
376+
or @SimpleName='RedundantModifierCheck']
376377
//MethodDeclaration[@Name='visitToken']
377378
| //ClassDeclaration[@SimpleName='FallThroughCheck']
378379
//MethodDeclaration[@Name='isTerminated']

‎src/main/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheck.java

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@
6464
* {@code strictfp} modifier when using JDK 17 or later. See reason at
6565
* <a href="https://openjdk.org/jeps/306">JEP 306</a>
6666
* </li>
67+
* <li>
68+
* {@code final} modifier on unnamed variables when using JDK 22 or later.
69+
* </li>
6770
* </ol>
6871
* <p>
6972
* interfaces by definition are abstract so the {@code abstract} modifier is redundant on them.
@@ -152,7 +155,7 @@
152155
* Old JDK version numbering is supported (e.g. 1.8 for Java 8)
153156
* as well as just the major JDK version alone (e.g. 8) is supported.
154157
* Type is {@code java.lang.String}.
155-
* Default value is {@code "21"}.
158+
* Default value is {@code "22"}.
156159
* </li>
157160
* <li>
158161
* Property {@code tokens} - tokens to check
@@ -178,7 +181,13 @@
178181
* <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#ANNOTATION_DEF">
179182
* ANNOTATION_DEF</a>,
180183
* <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#RECORD_DEF">
181-
* RECORD_DEF</a>.
184+
* RECORD_DEF</a>,
185+
* <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#PATTERN_VARIABLE_DEF">
186+
* PATTERN_VARIABLE_DEF</a>,
187+
* <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LITERAL_CATCH">
188+
* LITERAL_CATCH</a>,
189+
* <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LAMBDA">
190+
* LAMBDA</a>.
182191
* </li>
183192
* </ul>
184193
* <p>
@@ -214,9 +223,9 @@ public class RedundantModifierCheck
214223
};
215224

216225
/**
217-
* Constant for jdk 21 version number.
226+
* Constant for jdk 22 version number.
218227
*/
219-
private static final int JDK_21 = 21;
228+
private static final int JDK_22 = 22;
220229

221230
/**
222231
* Constant for jdk 17 version number.
@@ -230,7 +239,7 @@ public class RedundantModifierCheck
230239
* as well as just the major JDK version alone (e.g. 8) is supported.
231240
*
232241
*/
233-
private int jdkVersion = JDK_21;
242+
private int jdkVersion = JDK_22;
234243

235244
/**
236245
* Setter to set the JDK version that you are using.
@@ -275,6 +284,9 @@ public int[] getAcceptableTokens() {
275284
TokenTypes.RESOURCE,
276285
TokenTypes.ANNOTATION_DEF,
277286
TokenTypes.RECORD_DEF,
287+
TokenTypes.PATTERN_VARIABLE_DEF,
288+
TokenTypes.LITERAL_CATCH,
289+
TokenTypes.LAMBDA,
278290
};
279291
}
280292

@@ -300,8 +312,17 @@ public void visitToken(DetailAST ast) {
300312
case TokenTypes.RECORD_DEF:
301313
checkForRedundantModifier(ast, TokenTypes.FINAL, TokenTypes.LITERAL_STATIC);
302314
break;
303-
case TokenTypes.CLASS_DEF:
304315
case TokenTypes.VARIABLE_DEF:
316+
case TokenTypes.PATTERN_VARIABLE_DEF:
317+
checkUnnamedVariables(ast);
318+
break;
319+
case TokenTypes.LITERAL_CATCH:
320+
checkUnnamedVariables(ast.findFirstToken(TokenTypes.PARAMETER_DEF));
321+
break;
322+
case TokenTypes.LAMBDA:
323+
processLambdaParameters(ast);
324+
break;
325+
case TokenTypes.CLASS_DEF:
305326
case TokenTypes.ANNOTATION_FIELD_DEF:
306327
break;
307328
default:
@@ -317,6 +338,44 @@ public void visitToken(DetailAST ast) {
317338
}
318339
}
319340

341+
/**
342+
* Process lambda parameters.
343+
*
344+
* @param lambdaAst node of type {@link TokenTypes#LAMBDA}
345+
*/
346+
private void processLambdaParameters(DetailAST lambdaAst) {
347+
final DetailAST lambdaParameters = lambdaAst.findFirstToken(TokenTypes.PARAMETERS);
348+
if (lambdaParameters != null) {
349+
TokenUtil.forEachChild(lambdaParameters, TokenTypes.PARAMETER_DEF,
350+
this::checkUnnamedVariables);
351+
}
352+
}
353+
354+
/**
355+
* Check if the variable is unnamed and has redundant final modifier.
356+
*
357+
* @param ast node of type {@link TokenTypes#VARIABLE_DEF}
358+
* or {@link TokenTypes#PATTERN_VARIABLE_DEF}
359+
* or {@link TokenTypes#PARAMETER_DEF}
360+
*/
361+
private void checkUnnamedVariables(DetailAST ast) {
362+
if (jdkVersion >= JDK_22 && isUnnamedVariable(ast)) {
363+
checkForRedundantModifier(ast, TokenTypes.FINAL);
364+
}
365+
}
366+
367+
/**
368+
* Check if the variable is unnamed.
369+
*
370+
* @param ast node of type {@link TokenTypes#VARIABLE_DEF}
371+
* or {@link TokenTypes#PATTERN_VARIABLE_DEF}
372+
* or {@link TokenTypes#PARAMETER_DEF}
373+
* @return true if the variable is unnamed
374+
*/
375+
private static boolean isUnnamedVariable(DetailAST ast) {
376+
return "_".equals(ast.findFirstToken(TokenTypes.IDENT).getText());
377+
}
378+
320379
/**
321380
* Check modifiers of constructor.
322381
*

‎src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/modifier/RedundantModifierCheck.xml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@
3737
{@code strictfp} modifier when using JDK 17 or later. See reason at
3838
&lt;a href="https://openjdk.org/jeps/306"&gt;JEP 306&lt;/a&gt;
3939
&lt;/li&gt;
40+
&lt;li&gt;
41+
{@code final} modifier on unnamed variables when using JDK 22 or later.
42+
&lt;/li&gt;
4043
&lt;/ol&gt;
4144
&lt;p&gt;
4245
interfaces by definition are abstract so the {@code abstract} modifier is redundant on them.
@@ -120,12 +123,12 @@
120123
}
121124
&lt;/pre&gt;</description>
122125
<properties>
123-
<property default-value="21" name="jdkVersion" type="java.lang.String">
126+
<property default-value="22" name="jdkVersion" type="java.lang.String">
124127
<description>Set the JDK version that you are using.
125128
Old JDK version numbering is supported (e.g. 1.8 for Java 8)
126129
as well as just the major JDK version alone (e.g. 8) is supported.</description>
127130
</property>
128-
<property default-value="METHOD_DEF,VARIABLE_DEF,ANNOTATION_FIELD_DEF,INTERFACE_DEF,CTOR_DEF,CLASS_DEF,ENUM_DEF,RESOURCE,ANNOTATION_DEF,RECORD_DEF"
131+
<property default-value="METHOD_DEF,VARIABLE_DEF,ANNOTATION_FIELD_DEF,INTERFACE_DEF,CTOR_DEF,CLASS_DEF,ENUM_DEF,RESOURCE,ANNOTATION_DEF,RECORD_DEF,PATTERN_VARIABLE_DEF,LITERAL_CATCH,LAMBDA"
129132
name="tokens"
130133
type="java.lang.String[]"
131134
validation-type="tokenSet">

‎src/test/java/com/puppycrawl/tools/checkstyle/checks/modifier/RedundantModifierCheckTest.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,9 @@ public void testGetAcceptableTokens() {
154154
TokenTypes.RESOURCE,
155155
TokenTypes.ANNOTATION_DEF,
156156
TokenTypes.RECORD_DEF,
157+
TokenTypes.PATTERN_VARIABLE_DEF,
158+
TokenTypes.LITERAL_CATCH,
159+
TokenTypes.LAMBDA,
157160
};
158161
assertWithMessage("Invalid acceptable tokens")
159162
.that(actual)
@@ -422,4 +425,34 @@ public void testStrictfpWithDefaultVersion() throws Exception {
422425
getNonCompilablePath("InputRedundantModifierStrictfpWithDefaultVersion.java"),
423426
expected);
424427
}
428+
429+
@Test
430+
public void testFinalUnnamedVariablesWithDefaultVersion() throws Exception {
431+
final String[] expected = {
432+
"18:26: " + getCheckMessage(MSG_KEY, "final"),
433+
"24:9: " + getCheckMessage(MSG_KEY, "final"),
434+
"34:18: " + getCheckMessage(MSG_KEY, "final"),
435+
"44:14: " + getCheckMessage(MSG_KEY, "final"),
436+
"51:14: " + getCheckMessage(MSG_KEY, "final"),
437+
"54:18: " + getCheckMessage(MSG_KEY, "final"),
438+
"65:53: " + getCheckMessage(MSG_KEY, "final"),
439+
"69:53: " + getCheckMessage(MSG_KEY, "final"),
440+
"69:70: " + getCheckMessage(MSG_KEY, "final"),
441+
};
442+
verifyWithInlineConfigParser(
443+
getNonCompilablePath("InputRedundantModifierFinalUnnamedVariables.java"),
444+
expected);
445+
}
446+
447+
@Test
448+
public void testFinalUnnamedVariablesWithOldVersion() throws Exception {
449+
final String[] expected = {
450+
"40:14: " + getCheckMessage(MSG_KEY, "final"),
451+
"47:14: " + getCheckMessage(MSG_KEY, "final"),
452+
};
453+
verifyWithInlineConfigParser(
454+
getNonCompilablePath(
455+
"InputRedundantModifierFinalUnnamedVariablesWithOldVersion.java"),
456+
expected);
457+
}
425458
}

‎src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/modifier/redundantmodifier/InputRedundantModifierFinalUnnamedVariables.java

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
RedundantModifier
3+
tokens = (default)METHOD_DEF, VARIABLE_DEF, ANNOTATION_FIELD_DEF, INTERFACE_DEF, \
4+
CTOR_DEF, CLASS_DEF, ENUM_DEF, RESOURCE, PATTERN_VARIABLE_DEF, LITERAL_CATCH, LAMBDA
5+
jdkVersion = (default)22
6+
7+
8+
*/
9+
10+
//non-compiled with javac: Compilable with Java21
11+
package com.puppycrawl.tools.checkstyle.checks.modifier.redundantmodifier;
12+
13+
import java.util.function.BiFunction;
14+
15+
public class InputRedundantModifierFinalUnnamedVariables {
16+
void m(Object o) {
17+
// violation below, 'Redundant 'final' modifier'
18+
if (o instanceof final String _) { }
19+
if (o instanceof final String __) { }
20+
if (o instanceof final String s)
21+
if (o instanceof final String _s) { }
22+
23+
// violation below, 'Redundant 'final' modifier'
24+
final int _ = sideEffect();
25+
final int __ = sideEffect();
26+
final int x = sideEffect();
27+
final int _x = sideEffect();
28+
29+
}
30+
31+
void m2(Object o) {
32+
switch (o) {
33+
// violation below, 'Redundant 'final' modifier'
34+
case final String _ -> { }
35+
case Float _ -> { }
36+
case final Integer __ -> { }
37+
case final Double s -> { }
38+
default -> { }
39+
}
40+
}
41+
42+
void m3() {
43+
// violation below, 'Redundant 'final' modifier'
44+
try (final var a = lock();) {
45+
46+
} catch (final Exception e) {
47+
48+
}
49+
50+
// violation below, 'Redundant 'final' modifier'
51+
try (final var _ = lock();) {
52+
53+
// violation below, 'Redundant 'final' modifier'
54+
} catch (final Exception _) {
55+
56+
}
57+
}
58+
59+
void m4() {
60+
BiFunction<Integer, Integer, Integer> f = (final Integer a, final Integer b) -> {
61+
return 5;
62+
};
63+
64+
// violation below, 'Redundant 'final' modifier'
65+
BiFunction<Integer, Integer, Integer> f2 = (final Integer _, final Integer b) -> {
66+
return 5;
67+
};
68+
69+
BiFunction<Integer, Integer, Integer> f3 = (final Integer _, final Integer _) -> {
70+
// 2 violations above:
71+
// 'Redundant 'final' modifier'
72+
// 'Redundant 'final' modifier'
73+
return 5;
74+
};
75+
}
76+
77+
int sideEffect() {
78+
return 0;
79+
}
80+
81+
AutoCloseable lock() {
82+
return null;
83+
}
84+
85+
record Point(int x, int y) { }
86+
}

‎src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/modifier/redundantmodifier/InputRedundantModifierFinalUnnamedVariablesWithOldVersion.java

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
/*
2+
RedundantModifier
3+
tokens = (default)METHOD_DEF, VARIABLE_DEF, ANNOTATION_FIELD_DEF, INTERFACE_DEF, \
4+
CTOR_DEF, CLASS_DEF, ENUM_DEF, RESOURCE, PATTERN_VARIABLE_DEF, LITERAL_CATCH, LAMBDA
5+
jdkVersion = 8
6+
7+
8+
*/
9+
10+
//non-compiled with javac: Compilable with Java21
11+
package com.puppycrawl.tools.checkstyle.checks.modifier.redundantmodifier;
12+
13+
import java.util.function.BiFunction;
14+
15+
public class InputRedundantModifierFinalUnnamedVariablesWithOldVersion {
16+
void m(Object o) {
17+
if (o instanceof final String _) { }
18+
if (o instanceof final String __) { }
19+
if (o instanceof final String s)
20+
if (o instanceof final String _s) { }
21+
22+
final int _ = sideEffect();
23+
final int __ = sideEffect();
24+
final int x = sideEffect();
25+
final int _x = sideEffect();
26+
}
27+
28+
void m2(Object o) {
29+
switch (o) {
30+
case final String _ -> { }
31+
case Float _ -> { }
32+
case final Integer __ -> { }
33+
case final Double s -> { }
34+
default -> { }
35+
}
36+
}
37+
38+
void m3() {
39+
// violation below, 'Redundant 'final' modifier'
40+
try (final var a = lock();) {
41+
42+
} catch (final Exception e) {
43+
44+
}
45+
46+
// violation below, 'Redundant 'final' modifier'
47+
try (final var _ = lock();) {
48+
49+
} catch (final Exception _) {
50+
51+
}
52+
}
53+
54+
void m4() {
55+
BiFunction<Integer, Integer, Integer> f = (final Integer a, final Integer b) -> {
56+
return 5;
57+
};
58+
59+
BiFunction<Integer, Integer, Integer> f2 = (final Integer _, final Integer b) -> {
60+
return 5;
61+
};
62+
63+
BiFunction<Integer, Integer, Integer> f3 = (final Integer _, final Integer _) -> {
64+
return 5;
65+
};
66+
}
67+
68+
int sideEffect() {
69+
return 0;
70+
}
71+
72+
AutoCloseable lock() {
73+
return null;
74+
}
75+
76+
record Point(int x, int y) { }
77+
}

0 commit comments

Comments
 (0)