Skip to content

Commit

Permalink
Issue checkstyle#15159: RedundantModifierCheck should violate final m…
Browse files Browse the repository at this point in the history
…odifier on unnamed variables
  • Loading branch information
mahfouz72 authored and rnveach committed Aug 24, 2024
1 parent d32c27b commit d010bda
Show file tree
Hide file tree
Showing 8 changed files with 287 additions and 10 deletions.
3 changes: 2 additions & 1 deletion config/pmd.xml
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,8 @@
| //ClassDeclaration[@SimpleName='SarifLogger']//MethodDeclaration[@Name='escape']
| //ClassDeclaration[@SimpleName='LeftCurlyCheck'
or @SimpleName='FinalLocalVariableCheck'
or @SimpleName='NPathComplexityCheck']
or @SimpleName='NPathComplexityCheck'
or @SimpleName='RedundantModifierCheck']
//MethodDeclaration[@Name='visitToken']
| //ClassDeclaration[@SimpleName='FallThroughCheck']
//MethodDeclaration[@Name='isTerminated']
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@
* {@code strictfp} modifier when using JDK 17 or later. See reason at
* <a href="https://openjdk.org/jeps/306">JEP 306</a>
* </li>
* <li>
* {@code final} modifier on unnamed variables when using JDK 22 or later.
* </li>
* </ol>
* <p>
* interfaces by definition are abstract so the {@code abstract} modifier is redundant on them.
Expand Down Expand Up @@ -152,7 +155,7 @@
* Old JDK version numbering is supported (e.g. 1.8 for Java 8)
* as well as just the major JDK version alone (e.g. 8) is supported.
* Type is {@code java.lang.String}.
* Default value is {@code "21"}.
* Default value is {@code "22"}.
* </li>
* <li>
* Property {@code tokens} - tokens to check
Expand All @@ -178,7 +181,13 @@
* <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#ANNOTATION_DEF">
* ANNOTATION_DEF</a>,
* <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#RECORD_DEF">
* RECORD_DEF</a>.
* RECORD_DEF</a>,
* <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#PATTERN_VARIABLE_DEF">
* PATTERN_VARIABLE_DEF</a>,
* <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LITERAL_CATCH">
* LITERAL_CATCH</a>,
* <a href="https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/api/TokenTypes.html#LAMBDA">
* LAMBDA</a>.
* </li>
* </ul>
* <p>
Expand Down Expand Up @@ -214,9 +223,9 @@ public class RedundantModifierCheck
};

/**
* Constant for jdk 21 version number.
* Constant for jdk 22 version number.
*/
private static final int JDK_21 = 21;
private static final int JDK_22 = 22;

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

/**
* Setter to set the JDK version that you are using.
Expand Down Expand Up @@ -275,6 +284,9 @@ public int[] getAcceptableTokens() {
TokenTypes.RESOURCE,
TokenTypes.ANNOTATION_DEF,
TokenTypes.RECORD_DEF,
TokenTypes.PATTERN_VARIABLE_DEF,
TokenTypes.LITERAL_CATCH,
TokenTypes.LAMBDA,
};
}

Expand All @@ -300,8 +312,17 @@ public void visitToken(DetailAST ast) {
case TokenTypes.RECORD_DEF:
checkForRedundantModifier(ast, TokenTypes.FINAL, TokenTypes.LITERAL_STATIC);
break;
case TokenTypes.CLASS_DEF:
case TokenTypes.VARIABLE_DEF:
case TokenTypes.PATTERN_VARIABLE_DEF:
checkUnnamedVariables(ast);
break;
case TokenTypes.LITERAL_CATCH:
checkUnnamedVariables(ast.findFirstToken(TokenTypes.PARAMETER_DEF));
break;
case TokenTypes.LAMBDA:
processLambdaParameters(ast);
break;
case TokenTypes.CLASS_DEF:
case TokenTypes.ANNOTATION_FIELD_DEF:
break;
default:
Expand All @@ -317,6 +338,44 @@ public void visitToken(DetailAST ast) {
}
}

/**
* Process lambda parameters.
*
* @param lambdaAst node of type {@link TokenTypes#LAMBDA}
*/
private void processLambdaParameters(DetailAST lambdaAst) {
final DetailAST lambdaParameters = lambdaAst.findFirstToken(TokenTypes.PARAMETERS);
if (lambdaParameters != null) {
TokenUtil.forEachChild(lambdaParameters, TokenTypes.PARAMETER_DEF,
this::checkUnnamedVariables);
}
}

/**
* Check if the variable is unnamed and has redundant final modifier.
*
* @param ast node of type {@link TokenTypes#VARIABLE_DEF}
* or {@link TokenTypes#PATTERN_VARIABLE_DEF}
* or {@link TokenTypes#PARAMETER_DEF}
*/
private void checkUnnamedVariables(DetailAST ast) {
if (jdkVersion >= JDK_22 && isUnnamedVariable(ast)) {
checkForRedundantModifier(ast, TokenTypes.FINAL);
}
}

/**
* Check if the variable is unnamed.
*
* @param ast node of type {@link TokenTypes#VARIABLE_DEF}
* or {@link TokenTypes#PATTERN_VARIABLE_DEF}
* or {@link TokenTypes#PARAMETER_DEF}
* @return true if the variable is unnamed
*/
private static boolean isUnnamedVariable(DetailAST ast) {
return "_".equals(ast.findFirstToken(TokenTypes.IDENT).getText());
}

/**
* Check modifiers of constructor.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
{@code strictfp} modifier when using JDK 17 or later. See reason at
&lt;a href="https://openjdk.org/jeps/306"&gt;JEP 306&lt;/a&gt;
&lt;/li&gt;
&lt;li&gt;
{@code final} modifier on unnamed variables when using JDK 22 or later.
&lt;/li&gt;
&lt;/ol&gt;
&lt;p&gt;
interfaces by definition are abstract so the {@code abstract} modifier is redundant on them.
Expand Down Expand Up @@ -120,12 +123,12 @@
}
&lt;/pre&gt;</description>
<properties>
<property default-value="21" name="jdkVersion" type="java.lang.String">
<property default-value="22" name="jdkVersion" type="java.lang.String">
<description>Set the JDK version that you are using.
Old JDK version numbering is supported (e.g. 1.8 for Java 8)
as well as just the major JDK version alone (e.g. 8) is supported.</description>
</property>
<property default-value="METHOD_DEF,VARIABLE_DEF,ANNOTATION_FIELD_DEF,INTERFACE_DEF,CTOR_DEF,CLASS_DEF,ENUM_DEF,RESOURCE,ANNOTATION_DEF,RECORD_DEF"
<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"
name="tokens"
type="java.lang.String[]"
validation-type="tokenSet">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ public void testGetAcceptableTokens() {
TokenTypes.RESOURCE,
TokenTypes.ANNOTATION_DEF,
TokenTypes.RECORD_DEF,
TokenTypes.PATTERN_VARIABLE_DEF,
TokenTypes.LITERAL_CATCH,
TokenTypes.LAMBDA,
};
assertWithMessage("Invalid acceptable tokens")
.that(actual)
Expand Down Expand Up @@ -422,4 +425,34 @@ public void testStrictfpWithDefaultVersion() throws Exception {
getNonCompilablePath("InputRedundantModifierStrictfpWithDefaultVersion.java"),
expected);
}

@Test
public void testFinalUnnamedVariablesWithDefaultVersion() throws Exception {
final String[] expected = {
"18:26: " + getCheckMessage(MSG_KEY, "final"),
"24:9: " + getCheckMessage(MSG_KEY, "final"),
"34:18: " + getCheckMessage(MSG_KEY, "final"),
"44:14: " + getCheckMessage(MSG_KEY, "final"),
"51:14: " + getCheckMessage(MSG_KEY, "final"),
"54:18: " + getCheckMessage(MSG_KEY, "final"),
"65:53: " + getCheckMessage(MSG_KEY, "final"),
"69:53: " + getCheckMessage(MSG_KEY, "final"),
"69:70: " + getCheckMessage(MSG_KEY, "final"),
};
verifyWithInlineConfigParser(
getNonCompilablePath("InputRedundantModifierFinalUnnamedVariables.java"),
expected);
}

@Test
public void testFinalUnnamedVariablesWithOldVersion() throws Exception {
final String[] expected = {
"40:14: " + getCheckMessage(MSG_KEY, "final"),
"47:14: " + getCheckMessage(MSG_KEY, "final"),
};
verifyWithInlineConfigParser(
getNonCompilablePath(
"InputRedundantModifierFinalUnnamedVariablesWithOldVersion.java"),
expected);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
RedundantModifier
tokens = (default)METHOD_DEF, VARIABLE_DEF, ANNOTATION_FIELD_DEF, INTERFACE_DEF, \
CTOR_DEF, CLASS_DEF, ENUM_DEF, RESOURCE, PATTERN_VARIABLE_DEF, LITERAL_CATCH, LAMBDA
jdkVersion = (default)22
*/

//non-compiled with javac: Compilable with Java21
package com.puppycrawl.tools.checkstyle.checks.modifier.redundantmodifier;

import java.util.function.BiFunction;

public class InputRedundantModifierFinalUnnamedVariables {
void m(Object o) {
// violation below, 'Redundant 'final' modifier'
if (o instanceof final String _) { }
if (o instanceof final String __) { }
if (o instanceof final String s)
if (o instanceof final String _s) { }

// violation below, 'Redundant 'final' modifier'
final int _ = sideEffect();
final int __ = sideEffect();
final int x = sideEffect();
final int _x = sideEffect();

}

void m2(Object o) {
switch (o) {
// violation below, 'Redundant 'final' modifier'
case final String _ -> { }
case Float _ -> { }
case final Integer __ -> { }
case final Double s -> { }
default -> { }
}
}

void m3() {
// violation below, 'Redundant 'final' modifier'
try (final var a = lock();) {

} catch (final Exception e) {

}

// violation below, 'Redundant 'final' modifier'
try (final var _ = lock();) {

// violation below, 'Redundant 'final' modifier'
} catch (final Exception _) {

}
}

void m4() {
BiFunction<Integer, Integer, Integer> f = (final Integer a, final Integer b) -> {
return 5;
};

// violation below, 'Redundant 'final' modifier'
BiFunction<Integer, Integer, Integer> f2 = (final Integer _, final Integer b) -> {
return 5;
};

BiFunction<Integer, Integer, Integer> f3 = (final Integer _, final Integer _) -> {
// 2 violations above:
// 'Redundant 'final' modifier'
// 'Redundant 'final' modifier'
return 5;
};
}

int sideEffect() {
return 0;
}

AutoCloseable lock() {
return null;
}

record Point(int x, int y) { }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
RedundantModifier
tokens = (default)METHOD_DEF, VARIABLE_DEF, ANNOTATION_FIELD_DEF, INTERFACE_DEF, \
CTOR_DEF, CLASS_DEF, ENUM_DEF, RESOURCE, PATTERN_VARIABLE_DEF, LITERAL_CATCH, LAMBDA
jdkVersion = 8
*/

//non-compiled with javac: Compilable with Java21
package com.puppycrawl.tools.checkstyle.checks.modifier.redundantmodifier;

import java.util.function.BiFunction;

public class InputRedundantModifierFinalUnnamedVariablesWithOldVersion {
void m(Object o) {
if (o instanceof final String _) { }
if (o instanceof final String __) { }
if (o instanceof final String s)
if (o instanceof final String _s) { }

final int _ = sideEffect();
final int __ = sideEffect();
final int x = sideEffect();
final int _x = sideEffect();
}

void m2(Object o) {
switch (o) {
case final String _ -> { }
case Float _ -> { }
case final Integer __ -> { }
case final Double s -> { }
default -> { }
}
}

void m3() {
// violation below, 'Redundant 'final' modifier'
try (final var a = lock();) {

} catch (final Exception e) {

}

// violation below, 'Redundant 'final' modifier'
try (final var _ = lock();) {

} catch (final Exception _) {

}
}

void m4() {
BiFunction<Integer, Integer, Integer> f = (final Integer a, final Integer b) -> {
return 5;
};

BiFunction<Integer, Integer, Integer> f2 = (final Integer _, final Integer b) -> {
return 5;
};

BiFunction<Integer, Integer, Integer> f3 = (final Integer _, final Integer _) -> {
return 5;
};
}

int sideEffect() {
return 0;
}

AutoCloseable lock() {
return null;
}

record Point(int x, int y) { }
}
Loading

0 comments on commit d010bda

Please sign in to comment.