From 9d8134f4e8f2400519d4097a96a87c1b533dc9d2 Mon Sep 17 00:00:00 2001 From: Oleg Sukhodolsky Date: Thu, 10 Nov 2005 13:38:44 +0000 Subject: [PATCH] Applied patch 1348873 which implements rfe 1345691 --- .../checks/coding/FallThroughCheck.java | 156 +++++++++- .../checkstyle/coding/InputFallThrough.java | 274 ++++++++++++++++++ .../checks/coding/FallThroughCheckTest.java | 58 +++- src/xdocs/config_coding.xml | 47 ++- src/xdocs/releasenotes.xml | 13 +- suppressions.xml | 3 - 6 files changed, 529 insertions(+), 22 deletions(-) diff --git a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java index 3f523c4ea..7ffcd49b2 100644 --- a/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java +++ b/src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java @@ -18,9 +18,13 @@ //////////////////////////////////////////////////////////////////////////////// package com.puppycrawl.tools.checkstyle.checks.coding; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + import com.puppycrawl.tools.checkstyle.api.Check; -import com.puppycrawl.tools.checkstyle.api.TokenTypes; import com.puppycrawl.tools.checkstyle.api.DetailAST; +import com.puppycrawl.tools.checkstyle.api.TokenTypes; +import com.puppycrawl.tools.checkstyle.api.Utils; /** * Checks for fall through in switch statements @@ -28,10 +32,33 @@ import com.puppycrawl.tools.checkstyle.api.DetailAST; * but lacks a break, return, throw or continue statement. * *

+ * The check honors special comments to suppress warnings about + * the fall through. By default the comments "fallthru", + * "fall through", "falls through" and "fallthrough" are recognized. + *

+ *

+ * The following fragment of code will NOT trigger the check, + * because of the comment "fallthru". + *

+ * 
+ * case 3:
+ *     x = 2;
+ *     // fallthru
+ * case 4:
+ *
+ *

+ * The recognized relief comment can be configured with the property + * reliefPattern. Default value of this regular expression + * is "fallthru|fall through|fallthrough|falls through". + *

+ *

* An example of how to configure the check is: *

* 
- * <module name="FallThrough"/>
+ * <module name="FallThrough">
+ *     <property name="reliefPattern"
+ *                  value="Fall Through"/>
+ * </module>
  *
* * @author o_sukhodolsky @@ -41,6 +68,12 @@ public class FallThroughCheck extends Check /** Do we need to check last case group. */ private boolean mCheckLastGroup; + /** Relief pattern to allow fall throught to the next case branch. */ + private String mReliefPattern = "fallthru|falls? ?through"; + + /** Relief regexp. */ + private Pattern mRegExp; + /** Creates new instance of the check. */ public FallThroughCheck() { @@ -59,6 +92,17 @@ public class FallThroughCheck extends Check return getDefaultTokens(); } + /** + * Set the relief pattern. + * + * @param aPattern + * The regular expression pattern. + */ + public void setReliefPattern(String aPattern) + { + mReliefPattern = aPattern; + } + /** * Configures whether we need to check last case group or not. * @param aValue new value of the property. @@ -68,6 +112,13 @@ public class FallThroughCheck extends Check mCheckLastGroup = aValue; } + /** {@inheritDoc} */ + public void init() + { + super.init(); + mRegExp = Utils.getPattern(mReliefPattern); + } + /** {@inheritDoc} */ public void visitToken(DetailAST aAST) { @@ -82,11 +133,13 @@ public class FallThroughCheck extends Check final DetailAST slist = aAST.findFirstToken(TokenTypes.SLIST); if (!isTerminated(slist, true, true)) { - if (!isLastGroup) { - log(nextGroup, "fall.through"); - } - else { - log(aAST, "fall.through.last"); + if (!hasFallTruComment(aAST, nextGroup)) { + if (!isLastGroup) { + log(nextGroup, "fall.through"); + } + else { + log(aAST, "fall.through.last"); + } } } } @@ -192,7 +245,7 @@ public class FallThroughCheck extends Check } /** - * Checks if a given try/cath/finally block terminated by return, throw or, + * Checks if a given try/catch/finally block terminated by return, throw or, * if allowed break, continue. * @param aAST loop to check * @param aUseBreak should we consider break as terminator. @@ -243,4 +296,91 @@ public class FallThroughCheck extends Check return isTerminated; } + /** + * Determines if the fall through case between aCurrentCase and + * aNextCase is reliefed by a appropriate comment. + * + * @param aCurrentCase AST of the case that falls through to the next case. + * @param aNextCase AST of the next case. + * @return True if a relief comment was found + */ + private boolean hasFallTruComment(DetailAST aCurrentCase, + DetailAST aNextCase) + { + + final int startLineNo = aCurrentCase.getLineNo(); + final int startColNo = aCurrentCase.getColumnNo(); + final int endLineNo = aNextCase.getLineNo(); + final int endColNo = aNextCase.getColumnNo(); + + /* + * Remember: The lines number returned from the AST is 1-based, but + * the lines number in this array are 0-based. So you will often + * see a "lineNo-1" etc. + */ + final String[] lines = getLines(); + + /* + * Handle: + * case 1: + * /+ FALLTHRU +/ case 2: + * .... + * and + * switch(i) { + * default: + * /+ FALLTHRU +/} + */ + String linepart = lines[endLineNo - 1].substring(0, endColNo); + if (commentMatch(mRegExp, linepart, endLineNo, 0)) { + return true; + } + + /* + * Handle: + * case 1: + * ..... + * // FALLTHRU + * case 2: + * .... + * and + * switch(i) { + * default: + * // FALLTHRU + * } + */ + for (int i = endLineNo - 2; i > startLineNo - 1; i--) { + if (lines[i].trim().length() != 0) { + return commentMatch(mRegExp, lines[i], i + 1, 0); + } + } + + // Well -- no relief comment found. + return false; + } + + /** + * Does a regular expression match on the given line and checks that a + * possible match is within a comment. + * @param aPattern The regular expression pattern to use. + * @param aLine The line of test to do the match on. + * @param aLineNo The line number in the file. + * @param aStartColNo Column on that the regexp matching starts. + * @return True if a match was found inside a comment. + */ + private boolean commentMatch(Pattern aPattern, String aLine, int aLineNo, + int aStartColNo) + { + Matcher matcher = aPattern.matcher(aLine); + + boolean hit = matcher.find(); + + if (hit) { + final int startMatch = matcher.start(); + // -1 because it returns the char position beyond the match + final int endMatch = matcher.end() - 1; + return getFileContents().hasIntersectionWithComment(aLineNo, + startMatch, aLineNo, endMatch); + } + return false; + } } diff --git a/src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputFallThrough.java b/src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputFallThrough.java index ed071eb88..ed3647d9b 100644 --- a/src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputFallThrough.java +++ b/src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputFallThrough.java @@ -124,4 +124,278 @@ public class InputFallThrough } } } + + + + /* Like above, but all fall throughs with relief comment */ + void methodFallThru(int i, int j, boolean cond) { + while (true) { + switch (i) { + case -1: // FALLTHRU + + case 0: // no problem + case 1: + i++; + break; + case 2: + i++; + // fallthru + case 3: + i++; + break; + case 4: + return; + case 5: + throw new RuntimeException(""); + case 6: + continue; + case 7: { + break; + } + case 8: { + return; + } + case 9: { + throw new RuntimeException(""); + } + case 10: { + continue; + } + case 11: { + i++; + } + // fallthru + case 12: + if (false) + break; + else + break; + case 13: + if (true) { + return; + } + case 14: + if (true) { + return; + } else { + //do nothing + } + // fallthru + case 15: + do { + System.out.println("something"); + return; + } while(true); + case 16: + for (int j1 = 0; j1 < 10; j1++) { + System.err.println("something"); + return; + } + case 17: + while (cond) + throw new RuntimeException(""); + case 18: + while(cond) { + break; + } + // fallthru + case 19: + try { + i++; + break; + } catch (RuntimeException e) { + break; + } catch (Error e) { + return; + } + case 20: + try { + i++; + break; + } catch (RuntimeException e) { + } catch (Error e) { + return; + } + // fallthru + case 21: + try { + i++; + } catch (RuntimeException e) { + i--; + } finally { + break; + } + case 22: + try { + i++; + break; + } catch (RuntimeException e) { + i--; + break; + } finally { + i++; + } + /* fallthru */ + case 23: + switch (j) { + case 1: + continue; + case 2: + return; + default: + return; + } + case 24: + i++; + /* fallthru */ case 25: + i++; + break; + + case 26: + switch (j) { + case 1: + continue; + case 2: + break; + default: + return; + } + // fallthru + default: + // this is the last label + i++; + // fallthru + } + } + } + + /* Test relief comment. */ + void methodFallThruCC(int i, int j, boolean cond) { + while (true) { + switch (i){ + case 0: + i++; // fallthru + + case 1: + i++; + // fallthru + case 2: { + i++; + } + // fallthru + case 3: + i++; + /* fallthru */case 4: + break; + case 5: + i++; + // fallthru + } + } + } + + /* Like above, but C-style comments. */ + void methodFallThruC(int i, int j, boolean cond) { + while (true) { + switch (i){ + case 0: + i++; /* fallthru */ + + case 1: + i++; + /* fallthru */ + case 2: + i++; + /* fallthru */case 3: + break; + case 4: + i++; + /* fallthru */ + } + } + } + + /* Like above, but C-style comments with no spaces. */ + void methodFallThruC2(int i, int j, boolean cond) { + while (true) { + switch (i){ + case 0: + i++; /*fallthru*/ + + case 1: + i++; + /*fallthru*/ + case 2: + i++; + /*fallthru*/case 3: + break; + case 4: + i++; + /*fallthru*/ + } + } + } + + /* C-style comments with other default fallthru-comment. */ + void methodFallThruCOtherWords(int i, int j, boolean cond) { + while (true) { + switch (i){ + case 0: + i++; /* falls through */ + + case 1: + i++; + /* falls through */ + case 2: + i++; + /* falls through */case 3: + break; + case 4: + i++; + /* falls through */ + } + } + } + + /* C-style comments with custom fallthru-comment. */ + void methodFallThruCCustomWords(int i, int j, boolean cond) { + while (true) { + switch (i){ + case 0: + i++; /* Continue with next case */ + + case 1: + i++; + /* Continue with next case */ + case 2: + i++; + /* Continue with next case */case 3: + break; + case 4: + i++; + /* Continue with next case */ + } + } + } + + void methodFallThruLastCaseGroup(int i, int j, boolean cond) { + while (true) { + switch (i){ + case 0: + i++; // fallthru + } + switch (i){ + case 0: + i++; + // fallthru + } + switch (i){ + case 0: + i++; + /* fallthru */ } + } + } + + + } diff --git a/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheckTest.java b/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheckTest.java index f2329a813..3ca4d8459 100644 --- a/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheckTest.java +++ b/src/tests/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheckTest.java @@ -7,15 +7,30 @@ import java.io.File; public class FallThroughCheckTest extends BaseCheckTestCase { - private DefaultConfiguration checkConfig; - public void setUp() + public void testDefault() throws Exception { - checkConfig = createCheckConfig(FallThroughCheck.class); + DefaultConfiguration checkConfig = createCheckConfig(FallThroughCheck.class); + final String[] expected = { + "12:13: Fall through from previous branch of the switch statement.", + "36:13: Fall through from previous branch of the switch statement.", + "51:13: Fall through from previous branch of the switch statement.", + "68:13: Fall through from previous branch of the switch statement.", + "85:13: Fall through from previous branch of the switch statement.", + "103:13: Fall through from previous branch of the switch statement.", + "121:13: Fall through from previous branch of the switch statement.", + "367:11: Fall through from previous branch of the switch statement.", + "370:11: Fall through from previous branch of the switch statement.", + "372:40: Fall through from previous branch of the switch statement.", + }; + verify(checkConfig, + getPath("coding" + File.separator + "InputFallThrough.java"), + expected); } - - public void testIt() throws Exception + + public void testLastCaseGroup() throws Exception { + DefaultConfiguration checkConfig = createCheckConfig(FallThroughCheck.class); checkConfig.addAttribute("checkLastCaseGroup", "true"); final String[] expected = { "12:13: Fall through from previous branch of the switch statement.", @@ -26,14 +41,23 @@ public class FallThroughCheckTest extends BaseCheckTestCase "103:13: Fall through from previous branch of the switch statement.", "121:13: Fall through from previous branch of the switch statement.", "121:13: Fall through from the last branch of the switch statement.", + "367:11: Fall through from previous branch of the switch statement.", + "370:11: Fall through from previous branch of the switch statement.", + "372:40: Fall through from previous branch of the switch statement.", + "374:11: Fall through from the last branch of the switch statement.", }; verify(checkConfig, getPath("coding" + File.separator + "InputFallThrough.java"), expected); } - public void testDefault() throws Exception + public void testOwnPattern() throws Exception { + final String ownPattern = "Continue with next case"; + final DefaultConfiguration checkConfig = + createCheckConfig(FallThroughCheck.class); + checkConfig.addAttribute("reliefPattern", ownPattern); + final String[] expected = { "12:13: Fall through from previous branch of the switch statement.", "36:13: Fall through from previous branch of the switch statement.", @@ -42,9 +66,31 @@ public class FallThroughCheckTest extends BaseCheckTestCase "85:13: Fall through from previous branch of the switch statement.", "103:13: Fall through from previous branch of the switch statement.", "121:13: Fall through from previous branch of the switch statement.", + "143:11: Fall through from previous branch of the switch statement.", + "168:11: Fall through from previous branch of the switch statement.", + "184:11: Fall through from previous branch of the switch statement.", + "202:11: Fall through from previous branch of the switch statement.", + "220:11: Fall through from previous branch of the switch statement.", + "239:11: Fall through from previous branch of the switch statement.", + "250:26: Fall through from previous branch of the switch statement.", + "264:11: Fall through from previous branch of the switch statement.", + "279:11: Fall through from previous branch of the switch statement.", + "282:11: Fall through from previous branch of the switch statement.", + "286:11: Fall through from previous branch of the switch statement.", + "288:25: Fall through from previous branch of the switch statement.", + "304:11: Fall through from previous branch of the switch statement.", + "307:11: Fall through from previous branch of the switch statement.", + "309:25: Fall through from previous branch of the switch statement.", + "325:11: Fall through from previous branch of the switch statement.", + "328:11: Fall through from previous branch of the switch statement.", + "330:23: Fall through from previous branch of the switch statement.", + "346:11: Fall through from previous branch of the switch statement.", + "349:11: Fall through from previous branch of the switch statement.", + "351:30: Fall through from previous branch of the switch statement.", }; verify(checkConfig, getPath("coding" + File.separator + "InputFallThrough.java"), expected); + } } diff --git a/src/xdocs/config_coding.xml b/src/xdocs/config_coding.xml index 5ffdf8294..cd90519f4 100755 --- a/src/xdocs/config_coding.xml +++ b/src/xdocs/config_coding.xml @@ -1851,12 +1851,40 @@ if ("something".equals(x))

Checks for fall through in switch statements Finds locations where a case - contains
Java code - but lacks a break, return, throw or continue statement.

+

+ The check honores special comments to supress the warning. By + default the text "fallthru", "fall through", "fallthrough", + "falls through" and "fallsthrough" are recognized (case + sensitive). The comment containing this words must be a + one-liner and must be on the last none-empty line before the + case triggering the warning or on + the same line before the case + (urgly, but possible). +

+ +switch (i){ +case 0: + i++; // fall through +case 1: + i++; + // falls through +case 2: { + i++; +} +// fallthrough +case 3: + i++; +/* fallthru */case 4: + i++ + break; +} +

Note: the check works in assumption that there is no unreachable code in the case. @@ -1879,6 +1907,15 @@ if ("something".equals(x)) Boolean false + + reliefPattern + + Regulare expression to match the relief comment that supresses + the warning about a fall through. + + regular expression + fallthru|falls? ?through + @@ -1887,6 +1924,14 @@ if ("something".equals(x)) To configure the check:

+<module name="FallThrough"/> + +

+ or +

+ +<module name="FallThrough"> + <property name="reliefPattern" value="continue in next case"/> <module name="FallThrough"/> diff --git a/src/xdocs/releasenotes.xml b/src/xdocs/releasenotes.xml index 36f7f2ae1..092f8d089 100755 --- a/src/xdocs/releasenotes.xml +++ b/src/xdocs/releasenotes.xml @@ -14,10 +14,10 @@

New features:

Fixed Bugs:

diff --git a/suppressions.xml b/suppressions.xml index 43338e3ac..480334584 100755 --- a/suppressions.xml +++ b/suppressions.xml @@ -11,9 +11,6 @@ -