diff --git a/src/main/java/com/puppycrawl/tools/checkstyle/checks/blocks/NeedBracesCheck.java b/src/main/java/com/puppycrawl/tools/checkstyle/checks/blocks/NeedBracesCheck.java index c9d604c0f..2a4cef906 100644 --- a/src/main/java/com/puppycrawl/tools/checkstyle/checks/blocks/NeedBracesCheck.java +++ b/src/main/java/com/puppycrawl/tools/checkstyle/checks/blocks/NeedBracesCheck.java @@ -49,14 +49,54 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; * <property name="tokens" value="LITERAL_IF, LITERAL_ELSE"/> * </module> * - * Check has an option allowSingleLineIf which allows one line - * if-statements without braces, e.g.: + * Check has an option allowSingleLineStatement which allows single-line + * statements without braces, e.g.: *

* * if (obj.isValid()) return true; * *

- *
+ *

+ * + * while (obj.isValid()) return true; + * + *

+ *

+ * + * do this.notify(); while (o != null); + * + *

+ *

+ * + * for (int i = 0; ; ) this.notify(); + * + *

+ *

+ * To configure the Check to allow case, default single-line statements + * without braces: + *

+ *

+ * 

+ * <module name="NeedBraces">
+ *     <property name="tokens" value="LITERAL_CASE, LITERAL_DEFAULT"/>
+ *     <property name="allowSingleLineStatement" value="true"/>
+ * </module>
+ *
+ *

+ *

+ * Such statements would be allowed: + *

+ *

+ * 

+ * 
+ * switch (num) {
+ *     case 1: counter++; break; // OK
+ *     case 6: counter += 10; break; // OK
+ *     default: counter = 100; break; // OK
+ * }
+ * 
+ *
+ *

* * @author Rick Giles * @author Aleksey Nesterenko @@ -64,9 +104,9 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes; public class NeedBracesCheck extends Check { /** - * Check's option for skipping single-line if-statements. + * Check's option for skipping single-line statements. */ - private boolean allowSingleLineIf; + private boolean allowSingleLineStatement; /** * A key is pointing to the warning message text in "messages.properties" @@ -76,11 +116,11 @@ public class NeedBracesCheck extends Check /** * Setter. - * @param allowSingleLineIf Check's option for skipping single-line if-statements + * @param allowSingleLineStatement Check's option for skipping single-line statements */ - public void setAllowSingleLineIf(boolean allowSingleLineIf) + public void setAllowSingleLineStatement(boolean allowSingleLineStatement) { - this.allowSingleLineIf = allowSingleLineIf; + this.allowSingleLineStatement = allowSingleLineStatement; } @Override @@ -120,45 +160,252 @@ public class NeedBracesCheck extends Check { isElseIf = true; } - boolean skipStatement = false; - if (ast.getType() == TokenTypes.LITERAL_IF) { - skipStatement = isSkipIfBlock(ast); - } + + final boolean skipStatement = isSkipStatement(ast); + if ((slistAST == null) && !isElseIf && !skipStatement) { log(ast.getLineNo(), MSG_KEY_NEED_BRACES, ast.getText()); } } /** - * Checks if current if-block can be skipped by "need braces" warning. - * @param literalIf {@link TokenTypes#LITERAL_IF LITERAL_IF} - * @return true if current if block can be skipped by Check + * Checks if current statement can be skipped by "need braces" warning. + * @param statement if, for, while, do-while, lambda, else, case, default statements. + * @return true if current statement can be skipped by Check. */ - private boolean isSkipIfBlock(DetailAST literalIf) + private boolean isSkipStatement(DetailAST statement) { - return allowSingleLineIf && isSingleLineIf(literalIf); + return allowSingleLineStatement && isSingleLineStatement(statement); } /** - * Checks if current if-statement is single-line statement, e.g.: + * Checks if current statement is single-line statement, e.g.: *

* * if (obj.isValid()) return true; * *

- * @param literalIf {@link TokenTypes#LITERAL_IF LITERAL_IF} - * @return true if current if-statement is single-line statement + *

+ * + * while (obj.isValid()) return true; + * + *

+ * @param statement if, for, while, do-while, lambda, else, case, default statements. + * @return true if current statement is single-line statement. + */ + private static boolean isSingleLineStatement(DetailAST statement) + { + boolean result = false; + switch (statement.getType()) { + case TokenTypes.LITERAL_IF: + result = isSingleLineIf(statement); + break; + case TokenTypes.LITERAL_FOR: + result = isSingleLineFor(statement); + break; + case TokenTypes.LITERAL_DO: + result = isSingleLineDoWhile(statement); + break; + case TokenTypes.LITERAL_WHILE: + result = isSingleLineWhile(statement); + break; + case TokenTypes.LAMBDA: + result = isSingleLineLambda(statement); + break; + case TokenTypes.LITERAL_CASE: + result = isSingleLineCase(statement); + break; + case TokenTypes.LITERAL_DEFAULT: + result = isSingleLineDefault(statement); + break; + case TokenTypes.LITERAL_ELSE: + result = isSingleLineElse(statement); + break; + default: + final String exceptionMsg = statement.getText(); + throw new IllegalArgumentException("Unsupported token: " + exceptionMsg); + } + return result; + } + + /** + * Checks if current while statement is single-line statement, e.g.: + *

+ * + * while (obj.isValid()) return true; + * + *

+ * @param literalWhile {@link TokenTypes#LITERAL_WHILE while statement}. + * @return true if current while statement is single-line statement. + */ + private static boolean isSingleLineWhile(DetailAST literalWhile) + { + boolean result = false; + if (literalWhile.getParent().getType() == TokenTypes.SLIST + && literalWhile.getLastChild().getType() != TokenTypes.SLIST) + { + final DetailAST block = literalWhile.getLastChild().getPreviousSibling(); + result = literalWhile.getLineNo() == block.getLineNo(); + } + return result; + } + + /** + * Checks if current do-while statement is single-line statement, e.g.: + *

+ * + * do this.notify(); while (o != null); + * + *

+ * @param literalDo {@link TokenTypes#LITERAL_DO do-while statement}. + * @return true if current do-while statement is single-line statement. + */ + private static boolean isSingleLineDoWhile(DetailAST literalDo) + { + boolean result = false; + if (literalDo.getParent().getType() == TokenTypes.SLIST + && literalDo.getFirstChild().getType() != TokenTypes.SLIST) + { + final DetailAST block = literalDo.getFirstChild(); + result = block.getLineNo() == literalDo.getLineNo(); + } + return result; + } + + /** + * Checks if current for statement is single-line statement, e.g.: + *

+ * + * for (int i = 0; ; ) this.notify(); + * + *

+ * @param literalFor {@link TokenTypes#LITERAL_FOR for statement}. + * @return true if current for statement is single-line statement. + */ + private static boolean isSingleLineFor(DetailAST literalFor) + { + boolean result = false; + if (literalFor.getLastChild().getType() == TokenTypes.EMPTY_STAT) { + result = true; + } + else if (literalFor.getParent().getType() == TokenTypes.SLIST + && literalFor.getLastChild().getType() != TokenTypes.SLIST) + { + final DetailAST block = literalFor.findFirstToken(TokenTypes.EXPR); + if (block != null) { + result = literalFor.getLineNo() == block.getLineNo(); + } + } + return result; + } + + /** + * Checks if current if statement is single-line statement, e.g.: + *

+ * + * if (obj.isValid()) return true; + * + *

+ * @param literalIf {@link TokenTypes#LITERAL_IF if statement}. + * @return true if current if statement is single-line statement. */ private static boolean isSingleLineIf(DetailAST literalIf) { boolean result = false; - final DetailAST ifBlock = literalIf.getLastChild(); - final DetailAST lastElementInIfBlock = ifBlock.getLastChild(); - if (lastElementInIfBlock != null - && lastElementInIfBlock.getFirstChild() == null - && literalIf.getLineNo() == lastElementInIfBlock.getLineNo()) - { - result = true; + final DetailAST ifCondition = literalIf.findFirstToken(TokenTypes.EXPR); + if (literalIf.getParent().getType() == TokenTypes.SLIST) { + DetailAST block = literalIf.getLastChild(); + if (block.getType() != TokenTypes.LITERAL_RETURN) { + block = literalIf.getLastChild().getPreviousSibling(); + } + result = ifCondition.getLineNo() == block.getLineNo(); + } + return result; + } + + /** + * Checks if current lambda statement is single-line statement, e.g.: + *

+ * + * Runnable r = () -> System.out.println("Hello, world!"); + * + *

+ * @param lambda {@link TokenTypes#LAMBDA lambda statement}. + * @return true if current lambda statement is single-line statement. + */ + private static boolean isSingleLineLambda(DetailAST lambda) + { + boolean result = false; + final DetailAST block = lambda.getLastChild(); + if (block.getType() != TokenTypes.SLIST) { + result = lambda.getLineNo() == block.getLineNo(); + } + return result; + } + + /** + * Checks if current case statement is single-line statement, e.g.: + *

+ * + * case 1: dosomeStuff(); break; + * case 2: dosomeStuff(); break; + * + *

+ * @param literalCase {@link TokenTypes#LITERAL_CASE case statement}. + * @return true if current case statement is single-line statement. + */ + private static boolean isSingleLineCase(DetailAST literalCase) + { + boolean result = false; + final DetailAST slist = literalCase.getNextSibling(); + final DetailAST block = slist.getFirstChild(); + if (block.getType() != TokenTypes.SLIST) { + final DetailAST caseBreak = slist.findFirstToken(TokenTypes.LITERAL_BREAK); + final boolean atOneLine = literalCase.getLineNo() == block.getLineNo(); + if (caseBreak != null) { + result = atOneLine && (block.getLineNo() == caseBreak.getLineNo()); + } + } + return result; + } + + /** + * Checks if current default statement is single-line statement, e.g.: + *

+ * + * default: doSomeStuff(); + * + *

+ * @param literalDefault {@link TokenTypes#LITERAL_DEFAULT default statement}. + * @return true if current default statement is single-line statement. + */ + private static boolean isSingleLineDefault(DetailAST literalDefault) + { + boolean result = false; + final DetailAST slist = literalDefault.getNextSibling(); + final DetailAST block = slist.getFirstChild(); + if (block.getType() != TokenTypes.SLIST) { + result = literalDefault.getLineNo() == block.getLineNo(); + } + return result; + } + + /** + * Checks if current else statement is single-line statement, e.g.: + *

+ * + * else doSomeStuff(); + * + *

+ * @param literalElse {@link TokenTypes#LITERAL_ELSE else statement}. + * @return true if current else statement is single-line statement. + */ + private static boolean isSingleLineElse(DetailAST literalElse) + { + boolean result = false; + final DetailAST block = literalElse.getFirstChild(); + if (block.getType() != TokenTypes.SLIST) { + result = literalElse.getLineNo() == block.getLineNo(); } return result; } diff --git a/src/test/java/com/puppycrawl/tools/checkstyle/checks/blocks/NeedBracesCheckTest.java b/src/test/java/com/puppycrawl/tools/checkstyle/checks/blocks/NeedBracesCheckTest.java index 527de006c..b02bfc1fe 100644 --- a/src/test/java/com/puppycrawl/tools/checkstyle/checks/blocks/NeedBracesCheckTest.java +++ b/src/test/java/com/puppycrawl/tools/checkstyle/checks/blocks/NeedBracesCheckTest.java @@ -18,8 +18,11 @@ //////////////////////////////////////////////////////////////////////////////// package com.puppycrawl.tools.checkstyle.checks.blocks; +import java.io.File; + import com.puppycrawl.tools.checkstyle.BaseCheckTestSupport; import com.puppycrawl.tools.checkstyle.DefaultConfiguration; + import org.junit.Test; import static com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck.MSG_KEY_NEED_BRACES; @@ -54,15 +57,47 @@ public class NeedBracesCheckTest extends BaseCheckTestSupport } @Test - public void testSigleLineIfBlock() throws Exception + public void testSigleLineStatements() throws Exception { final DefaultConfiguration checkConfig = createCheckConfig(NeedBracesCheck.class); - checkConfig.addAttribute("allowSingleLineIf", "true"); + checkConfig.addAttribute("allowSingleLineStatement", "true"); final String[] expected = { "23: " + getCheckMessage(MSG_KEY_NEED_BRACES, "if"), "29: " + getCheckMessage(MSG_KEY_NEED_BRACES, "if"), + "38: " + getCheckMessage(MSG_KEY_NEED_BRACES, "if"), + "46: " + getCheckMessage(MSG_KEY_NEED_BRACES, "while"), + "53: " + getCheckMessage(MSG_KEY_NEED_BRACES, "do"), + "59: " + getCheckMessage(MSG_KEY_NEED_BRACES, "for"), }; - verify(checkConfig, getPath("InputBracesSingleLineIfBlock.java"), expected); + verify(checkConfig, getPath("InputBracesSingleLineStatements.java"), expected); + } + + @Test + public void testSigleLineLambda() throws Exception + { + final DefaultConfiguration checkConfig = + createCheckConfig(NeedBracesCheck.class); + checkConfig.addAttribute("tokens", "LAMBDA"); + checkConfig.addAttribute("allowSingleLineStatement", "true"); + final String[] expected = { + }; + verify(checkConfig, new File("src/test/resources-noncompilable/com/puppycrawl/" + + "tools/checkstyle/blocks/InputSingleLineLambda.java").getCanonicalPath(), + expected); + } + + @Test + public void testSigleLineCaseDefault() throws Exception + { + final DefaultConfiguration checkConfig = + createCheckConfig(NeedBracesCheck.class); + checkConfig.addAttribute("tokens", "LITERAL_CASE, LITERAL_DEFAULT"); + checkConfig.addAttribute("allowSingleLineStatement", "true"); + final String[] expected = { + "69: " + getCheckMessage(MSG_KEY_NEED_BRACES, "case"), + "72: " + getCheckMessage(MSG_KEY_NEED_BRACES, "case"), + }; + verify(checkConfig, getPath("InputBracesSingleLineStatements.java"), expected); } } diff --git a/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/blocks/InputSingleLineLambda.java b/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/blocks/InputSingleLineLambda.java new file mode 100644 index 000000000..0ed454e45 --- /dev/null +++ b/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/blocks/InputSingleLineLambda.java @@ -0,0 +1,7 @@ +//Compilable with Java8 +package com.puppycrawl.tools.checkstyle.blocks; +public class InputSingleLineLambda { + + static Runnable r1 = ()->System.out.println("Hello world one!"); + static Runnable r2 = () -> System.out.println("Hello world two!"); +} \ No newline at end of file diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/InputBracesSingleLineIfBlock.java b/src/test/resources/com/puppycrawl/tools/checkstyle/InputBracesSingleLineIfBlock.java deleted file mode 100644 index 95403705c..000000000 --- a/src/test/resources/com/puppycrawl/tools/checkstyle/InputBracesSingleLineIfBlock.java +++ /dev/null @@ -1,32 +0,0 @@ -package com.puppycrawl.tools.checkstyle; - -public class InputBracesSingleLineIfBlock -{ - private static class SomeClass { - boolean flag = true; - private static boolean test(boolean k) { - return k; - } - } - - private int foo() { - if (SomeClass.test(true) == true) return 4; //No warning if 'mAllowSingleLineIf' is true - return 0; - } - - private int foo1() { - if (SomeClass.test(true) == true) return 4; int k = 3; //No warning if 'mAllowSingleLineIf' is true - return 0; - } - - private int foo2() { - if (SomeClass.test(true) == true) //Warning, not single-line if-statement - return 4; - return 0; - } - - private int foo3() { - if (SomeClass.test(true) == true) if (true) return 4; //Warning, complex block - return 0; - } -} diff --git a/src/test/resources/com/puppycrawl/tools/checkstyle/InputBracesSingleLineStatements.java b/src/test/resources/com/puppycrawl/tools/checkstyle/InputBracesSingleLineStatements.java new file mode 100644 index 000000000..1c2b54387 --- /dev/null +++ b/src/test/resources/com/puppycrawl/tools/checkstyle/InputBracesSingleLineStatements.java @@ -0,0 +1,86 @@ +package com.puppycrawl.tools.checkstyle; + +public class InputBracesSingleLineStatements +{ + private static class SomeClass { + boolean flag = true; + private static boolean test(boolean k) { + return k; + } + } + + private int foo() { + if (SomeClass.test(true) == true) return 4; //No warning if 'mAllowSingleLineIf' is true + return 0; + } + + private int foo1() { + if (SomeClass.test(true) == true) return 4; int k = 3; //No warning if 'mAllowSingleLineIf' is true + return 0; + } + + private int foo2() { + if (SomeClass.test(true) == true) //Warning, not single-line if-statement + return 4; + return 0; + } + + private int foo3() { + if (SomeClass.test(true) == true) if (true) return 4; //Warning, complex block + return 0; + } + + private void foo(Object o) { + if (o != null) this.notify(); + } + + private void foo2(Object o) { + if (o != null) + this.notify(); + } + + private void loopTest(Object o) { + while (o != null) { + this.notify(); + } + while (o != null) + this.notify(); + while (o != null) this.notify(); + do { + this.notify(); + } while (o != null); + do this.notify(); while (o != null); + do + this.notify(); + while (o != null); + for (int i = 0; i < 10; i++) { + this.notify(); + } + for (int i = 0; i < 10; i++) + this.notify(); + for (int i = 0; ; ) this.notify(); + } + + private int getSmth(int num) + { + int counter = 0; + switch (num) { + case 1: counter++; break; + case 2: + counter += 2; + break; + case 3: + counter += 3; + break; + case 6: counter += 10; break; + default: counter = 100; break; + } + return counter; + } + + private void testElse(int k) { + if (k == 4) System.out.println("yes"); + else System.out.println("no"); + for (;;); + } +} diff --git a/src/xdocs/config_blocks.xml b/src/xdocs/config_blocks.xml index 8ad21fd22..35ef9eefc 100644 --- a/src/xdocs/config_blocks.xml +++ b/src/xdocs/config_blocks.xml @@ -337,8 +337,8 @@ try { all tokens - allowSingleLineIf - allow one line if statements + allowSingleLineStatement + allows single-line statements without braces boolean false @@ -362,13 +362,46 @@ try {

- To configure the check to allow one line if statements without braces: + To configure the check to allow single-line statements + (if, while, do-while, for) without braces:

<module name="NeedBraces"> - <property name="allowSingleLineIf" value="FALSE"/> + <property name="allowSingleLineStatement" value="true"/> </module> + +

+ Next statements won't be violated by Check: +

+ +if (obj.isValid()) return true; // OK +while (obj.isValid()) return true; // OK +do this.notify(); while (o != null); // OK +for (int i = 0; ; ) this.notify(); // OK + + +

+ To configure the Check to allow case, default single-line statements + without braces: +

+ +<module name="NeedBraces"> + <property name="tokens" value="LITERAL_CASE, LITERAL_DEFAULT"/> + <property name="allowSingleLineStatement" value="true"/> +</module> + + +

+ Next statements won't be violated by Check: +

+ +switch (num) { + case 1: counter++; break; // OK + case 6: counter += 10; break; // OK + default: counter = 100; break; // OK +} +