diff --git a/config/checkstyle-checks.xml b/config/checkstyle-checks.xml index 3f1f64fb371..86aaa4c973e 100644 --- a/config/checkstyle-checks.xml +++ b/config/checkstyle-checks.xml @@ -577,7 +577,7 @@ SL_ASSIGN, SR_ASSIGN, STAR_ASSIGN, LAMBDA, TEXT_BLOCK_LITERAL_BEGIN, LAND, LOR, LITERAL_INSTANCEOF, GT, LT, GE, LE, EQUAL, NOT_EQUAL, UNARY_MINUS, UNARY_PLUS, INC, DEC, LNOT, BNOT, POST_INC, POST_DEC, BOR, BXOR, BAND, - QUESTION"/> + QUESTION, TYPECAST"/> diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheck.java index 39d6e8b1ac9..4ac332dd261 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheck.java @@ -182,7 +182,9 @@ * * POST_INC, * - * POST_DEC. + * POST_DEC, + * + * TYPECAST. * * * @@ -394,6 +396,7 @@ public int[] getDefaultTokens() { TokenTypes.BNOT, TokenTypes.POST_INC, TokenTypes.POST_DEC, + TokenTypes.TYPECAST, }; } @@ -445,6 +448,7 @@ public int[] getAcceptableTokens() { TokenTypes.BOR, TokenTypes.BAND, TokenTypes.QUESTION, + TokenTypes.TYPECAST, }; } @@ -467,42 +471,85 @@ else if (ast.getType() == TokenTypes.QUESTION) { .forEach(unnecessaryChild -> log(unnecessaryChild, MSG_EXPR)); } else if (parent.getType() != TokenTypes.ANNOTATION_MEMBER_VALUE_PAIR) { - final int type = ast.getType(); - final boolean surrounded = isSurrounded(ast); - // An identifier surrounded by parentheses. - if (surrounded && type == TokenTypes.IDENT) { - parentToSkip = ast.getParent(); - log(ast, MSG_IDENT, ast.getText()); - } - // A literal (numeric or string) surrounded by parentheses. - else if (surrounded && TokenUtil.isOfType(type, LITERALS)) { - parentToSkip = ast.getParent(); - if (type == TokenTypes.STRING_LITERAL) { - log(ast, MSG_STRING, - chopString(ast.getText())); - } - else if (type == TokenTypes.TEXT_BLOCK_LITERAL_BEGIN) { - // Strip newline control characters to keep message as single-line, add - // quotes to make string consistent with STRING_LITERAL - final String logString = QUOTE - + NEWLINE.matcher( - ast.getFirstChild().getText()).replaceAll("\\\\n") - + QUOTE; - log(ast, MSG_STRING, chopString(logString)); - } - else { - log(ast, MSG_LITERAL, ast.getText()); - } - } - // The rhs of an assignment surrounded by parentheses. - else if (TokenUtil.isOfType(type, ASSIGNMENTS)) { - assignDepth++; - final DetailAST last = ast.getLastChild(); - if (last.getType() == TokenTypes.RPAREN) { - log(ast, MSG_ASSIGN); - } + findUnnecessaryParenthesesInAst(ast); + } + } + + /** + * Finds and logs if {@code LITERAL},{@code IDENT}, {@code ASSIGNMENT} + * and {@code TYPECAST} are surrounded by parentheses. + * + * @param ast the {@code DetailAST} to check. + */ + private void findUnnecessaryParenthesesInAst(DetailAST ast) { + final int type = ast.getType(); + final boolean surrounded = isSurrounded(ast); + // An identifier surrounded by parentheses. + if (surrounded && type == TokenTypes.IDENT) { + parentToSkip = ast.getParent(); + log(ast, MSG_IDENT, ast.getText()); + } + // A literal (numeric or string) surrounded by parentheses. + else if (surrounded && TokenUtil.isOfType(type, LITERALS)) { + findUnnecessaryParenthesesAroundLiterals(ast); + } + // The rhs of an assignment surrounded by parentheses. + else if (TokenUtil.isOfType(type, ASSIGNMENTS)) { + assignDepth++; + final DetailAST last = ast.getLastChild(); + if (last.getType() == TokenTypes.RPAREN) { + log(ast, MSG_ASSIGN); } } + // A typecast surrounded by parentheses. + else if (surrounded && type == TokenTypes.TYPECAST + && isTypeCastSurrounded(ast)) { + log(ast, MSG_EXPR); + } + } + + /** + * Checks and logs if the given {@code LITERAL} is surrounded by parentheses. + * + * @param ast the {@code DetailAST} to check. + */ + private void findUnnecessaryParenthesesAroundLiterals(DetailAST ast) { + final int type = ast.getType(); + parentToSkip = ast.getParent(); + if (type == TokenTypes.STRING_LITERAL) { + log(ast, MSG_STRING, + chopString(ast.getText())); + } + else if (type == TokenTypes.TEXT_BLOCK_LITERAL_BEGIN) { + // Strip newline control characters to keep message as single-line, add + // quotes to make string consistent with STRING_LITERAL + final String logString = QUOTE + + NEWLINE.matcher( + ast.getFirstChild().getText()).replaceAll("\\\\n") + + QUOTE; + log(ast, MSG_STRING, chopString(logString)); + } + else { + log(ast, MSG_LITERAL, ast.getText()); + } + } + + /** + * Checks if the given {@code TYPECAST} is surrounded by parentheses. + * + * @param ast the {@code DetailAST} to check if it is surrounded by + * parentheses. + * @return {@code true} if is surrounded by + * parentheses. + */ + private static boolean isTypeCastSurrounded(DetailAST ast) { + final DetailAST parent = ast.getParent(); + final int parentType = parent.getType(); + return parentType != TokenTypes.DOT + && parentType != TokenTypes.EXPR + && parentType != TokenTypes.INDEX_OP + && parentType != TokenTypes.METHOD_REF + && !TokenUtil.isOfType(parentType, ASSIGNMENTS); } @Override diff --git a/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/coding/UnnecessaryParenthesesCheck.xml b/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/coding/UnnecessaryParenthesesCheck.xml index 14cb05d78e6..5081364fb73 100644 --- a/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/coding/UnnecessaryParenthesesCheck.xml +++ b/src/main/resources/com/puppycrawl/tools/checkstyle/meta/checks/coding/UnnecessaryParenthesesCheck.xml @@ -68,7 +68,7 @@ } </pre> - diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheckTest.java index 4fbb0405fa3..f60ed878bea 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/coding/UnnecessaryParenthesesCheckTest.java @@ -100,6 +100,37 @@ public void testDefault() throws Exception { getPath("InputUnnecessaryParenthesesOperatorsAndCasts.java"), expected); } + @Test + public void testCasts1() throws Exception { + final String[] expected = { + "20:18: " + getCheckMessage(MSG_EXPR), + "20:43: " + getCheckMessage(MSG_EXPR), + "30:11: " + getCheckMessage(MSG_ASSIGN), + "33:11: " + getCheckMessage(MSG_ASSIGN), + "33:22: " + getCheckMessage(MSG_EXPR), + "48:19: " + getCheckMessage(MSG_EXPR), + "67:15: " + getCheckMessage(MSG_EXPR), + "76:31: " + getCheckMessage(MSG_EXPR), + "85:40: " + getCheckMessage(MSG_EXPR), + }; + verifyWithInlineConfigParser( + getPath("InputUnnecessaryParenthesesCasts1.java"), expected); + } + + @Test + public void testCasts2() throws Exception { + final String[] expected = { + "31:28: " + getCheckMessage(MSG_EXPR), + "44:15: " + getCheckMessage(MSG_EXPR), + "55:32: " + getCheckMessage(MSG_EXPR), + "63:30: " + getCheckMessage(MSG_EXPR), + "73:14: " + getCheckMessage(MSG_EXPR), + "87:23: " + getCheckMessage(MSG_EXPR), + }; + verifyWithInlineConfigParser( + getPath("InputUnnecessaryParenthesesCasts2.java"), expected); + } + @Test public void test15Extensions() throws Exception { final String[] expected = { diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unnecessaryparentheses/InputUnnecessaryParenthesesCasts1.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unnecessaryparentheses/InputUnnecessaryParenthesesCasts1.java new file mode 100644 index 00000000000..60b9df38c8c --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unnecessaryparentheses/InputUnnecessaryParenthesesCasts1.java @@ -0,0 +1,94 @@ +/* +UnnecessaryParentheses +tokens = (default)EXPR, IDENT, NUM_DOUBLE, NUM_FLOAT, NUM_INT, NUM_LONG, \ + STRING_LITERAL, LITERAL_NULL, LITERAL_FALSE, LITERAL_TRUE, ASSIGN, \ + BAND_ASSIGN, BOR_ASSIGN, BSR_ASSIGN, BXOR_ASSIGN, DIV_ASSIGN, \ + MINUS_ASSIGN, MOD_ASSIGN, PLUS_ASSIGN, SL_ASSIGN, SR_ASSIGN, STAR_ASSIGN, \ + LAMBDA, TEXT_BLOCK_LITERAL_BEGIN, LAND, LITERAL_INSTANCEOF, GT, LT, GE, \ + LE, EQUAL, NOT_EQUAL, UNARY_MINUS, UNARY_PLUS, INC, DEC, LNOT, BNOT, \ + POST_INC, POST_DEC, TYPECAST + + +*/ +package com.puppycrawl.tools.checkstyle.checks.coding.unnecessaryparentheses; +public class InputUnnecessaryParenthesesCasts1 { + public void valid1() { + int x = 23; + int y = 44; + float k = 12f; + + int d = ((int) 100f) + 100 * 2 / ((int) 12.5) + (int) 90f; + // 2 violations above: + // 'Unnecessary parentheses around expression' + // 'Unnecessary parentheses around expression' + int p = (int) 110f + 10 * 2 / (int) 10f + (int) 32.2; + + y = (int) (22.2 * 2) / ((int) 8f + 5); + + double arg2 = 23.2; + int i = (int) arg2; + i = ((int) arg2); // violation 'Unnecessary parentheses around assignment right-hand side' + p = (int) arg2; + + x = (2 * 2 /((int) k)); + // 2 violations above: + // 'Unnecessary parentheses around assignment right-hand side' + // 'Unnecessary parentheses around expression' + x = 2 * 2 / (int) k; + + int par = ((int)2f * 2) / 4; + y = ((Integer) x).hashCode(); + + int py = 12; + float xy = 40f; + int yp = 0; + boolean finished = true; + boolean result = false; + + if(py >= ((int)xy) // violation 'Unnecessary parentheses around expression.' + | (yp ==1 | py >=1)) { + xy--; + } + if(yp >= (int)xy + || (py ==1 | py >=1)) { + xy++; + } + + if (!((int) xy > yp) + && py < 20) { + py++; + } + + if (35 + (int) 'a' == 100) { + py++; + } + + boolean checkone = true; + if (!((boolean) checkone)) { + // violation above 'Unnecessary parentheses around expression.' + checkone = false; + } + else if ((boolean) checkone) { + checkone = !checkone; + } + + double limit = 3.2; + for (int j = 0; j >= ((int) limit); j++) { + // violation above 'Unnecessary parentheses around expression.' + yp +=1; + } + for (int j =0; j >= (int) limit; j++) { + py--; + break; + } + + for(int j = 10; !finished && !((boolean) (j > 5)) ; j++){ + // violation above 'Unnecessary parentheses around expression.' + break; + } + for(int jp = 9; !finished || !(boolean) (jp >5); jp++){ + checkone = false; + break; + } + } +} diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unnecessaryparentheses/InputUnnecessaryParenthesesCasts2.java b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unnecessaryparentheses/InputUnnecessaryParenthesesCasts2.java new file mode 100644 index 00000000000..b0a63db61cd --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/checks/coding/unnecessaryparentheses/InputUnnecessaryParenthesesCasts2.java @@ -0,0 +1,97 @@ +/* +UnnecessaryParentheses +tokens = (default)EXPR, IDENT, NUM_DOUBLE, NUM_FLOAT, NUM_INT, NUM_LONG, \ + STRING_LITERAL, LITERAL_NULL, LITERAL_FALSE, LITERAL_TRUE, ASSIGN, \ + BAND_ASSIGN, BOR_ASSIGN, BSR_ASSIGN, BXOR_ASSIGN, DIV_ASSIGN, \ + MINUS_ASSIGN, MOD_ASSIGN, PLUS_ASSIGN, SL_ASSIGN, SR_ASSIGN, STAR_ASSIGN, \ + LAMBDA, TEXT_BLOCK_LITERAL_BEGIN, LAND, LITERAL_INSTANCEOF, GT, LT, GE, \ + LE, EQUAL, NOT_EQUAL, UNARY_MINUS, UNARY_PLUS, INC, DEC, LNOT, BNOT, \ + POST_INC, POST_DEC, TYPECAST + + +*/ + +package com.puppycrawl.tools.checkstyle.checks.coding.unnecessaryparentheses; + +import static org.junit.Assert.assertEquals; +import static java.lang.Math.abs; +import java.util.Arrays; +import java.util.HashSet; +import java.util.function.ToIntFunction; + +public class InputUnnecessaryParenthesesCasts2 { + public void fooConditionals(T element) { + int x = 23; + int y = 44; + float k = 12f; + boolean finished = true; + boolean result = false; + + String filevalue = "FILEVALUE"; + if (!finished || !((boolean) filevalue.contains("O"))) { + // violation above 'Unnecessary parentheses around expression.' + filevalue += "F"; + } + else if (finished || !(boolean) filevalue.contains("O")) { + filevalue += "G"; + } + + if (result && finished || ((int) 23.1 + 21) == 32) { + y--; + } + + // violation below 'Unnecessary parentheses around expression.' + if (!((boolean) filevalue.contains("G")) + || finished) { + x++; + } + else if (!(boolean) filevalue.contains("P") || finished) { + filevalue += "p"; + } + + String[] a = {"s", "a", "1", "2", "3"}; + String[] abr = {"18", "z", "w", "30", "u", "vel"}; + Arrays.stream(a) + .filter(s -> !((boolean) s.isEmpty())) + // violation above 'Unnecessary parentheses around expression.' + .toArray(String[]::new); + + Arrays.stream(abr).filter(s -> !(boolean) s.isEmpty()) + .toArray(String[]::new); + + Arrays.stream(a) // violation below 'Unnecessary parentheses around expression.' + .filter(s -> ((boolean) s.isEmpty())) + .toArray(String[]::new); + Arrays.stream(abr) + .filter(s -> (boolean) s.isEmpty()) + .toArray(String[]::new); + + new HashSet().stream() + .filter(f -> f > ((int) 1.1 + 200)); + + // violation below 'Unnecessary parentheses around expression.' + if (((double) abs(10 - 2)) + / 2 <= 0.01) { + y += 10; + } + else if ((double) abs(10 - 2) / 2 >= 0.02) { + x += 2; + } + + final ToIntFunction comparison = ((Comparable) element)::compareTo; + + Bean bean = new Bean(); + assertEquals(1, ((int[]) bean.array)[0]); + + // violation below 'Unnecessary parentheses around expression.' + float rest = ((float) (50 - System.currentTimeMillis())) / 1000; + float stop = (float) (50 - System.currentTimeMillis()) / 1000; + } + static class T {} + public class Bean { + public Object array; + public Bean() { + this.array = new int[]{1,2,3}; + } + } +} diff --git a/src/xdocs/checks/coding/unnecessaryparentheses.xml b/src/xdocs/checks/coding/unnecessaryparentheses.xml index c380ee723a4..df1df3f31b0 100644 --- a/src/xdocs/checks/coding/unnecessaryparentheses.xml +++ b/src/xdocs/checks/coding/unnecessaryparentheses.xml @@ -179,6 +179,8 @@ if ((++f) > g && a) { // violation, unnecessary paren BAND , QUESTION + , + TYPECAST . @@ -264,6 +266,8 @@ if ((++f) > g && a) { // violation, unnecessary paren POST_INC , POST_DEC + , + TYPECAST . 3.4