TouchInstinct
/
checkstyle
306
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Applied patch 1348873 which implements rfe 1345691

This commit is contained in:
Oleg Sukhodolsky 2005-11-10 13:38:44 +00:00
parent ab1e308913
commit 9d8134f4e8
6 changed files with 529 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

156
src/checkstyle/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheck.java
View File

@ -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.
*
* <p>
* 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.
* </p>
* <p>
* The following fragment of code will NOT trigger the check,
* because of the comment "fallthru".
* </p>
* <pre>
* case 3:
* x = 2;
* // fallthru
* case 4:
* </pre>
* <p>
* The recognized relief comment can be configured with the property
* <code>reliefPattern</code>. Default value of this regular expression
* is "fallthru|fall through|fallthrough|falls through".
* </p>
* <p>
* An example of how to configure the check is:
* </p>
* <pre>
* &lt;module name="FallThrough"/&gt;
* &lt;module name="FallThrough"&gt;
* &lt;property name=&quot;reliefPattern&quot;
* value=&quot;Fall Through&quot;/&gt;
* &lt;/module&gt;
* </pre>
*
* @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 <code>aCurrentCase</code> and
* <code>aNextCase</code> 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;
}
}

274
src/testinputs/com/puppycrawl/tools/checkstyle/coding/InputFallThrough.java
View File

@ -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 */ }
}
}
}

58
src/tests/com/puppycrawl/tools/checkstyle/checks/coding/FallThroughCheckTest.java
View File

@ -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);
}
}

47
src/xdocs/config_coding.xml
View File

@ -1851,12 +1851,40 @@ if (&quot;something&quot;.equals(x))
<p>
Checks for fall through in <span class="code">switch</span>
statements Finds locations where a <span class="code">case</span>
contains<br/> Java code - but lacks a <span
contains Java code - but lacks a <span
class="code">break</span>, <span class="code">return</span>, <span
class="code">throw</span> or <span class="code">continue</span>
statement.
</p>
<p>
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
<span class="code">case</span> triggering the warning or on
the same line before the <span class="code">case</span>
(urgly, but possible).
</p>
<source>
switch (i){
case 0:
i++; // fall through
case 1:
i++;
// falls through
case 2: {
i++;
}
// fallthrough
case 3:
i++;
/* fallthru */case 4:
i++
break;
}
</source>
<p>
Note: the check works in assumption that there is no unreachable
code in the <span class="code">case</span>.
@ -1879,6 +1907,15 @@ if (&quot;something&quot;.equals(x))
<td><a href="property_types.html#boolean">Boolean</a></td>
<td><span class="default">false</span></td>
</tr>
<tr>
<td>reliefPattern</td>
<td>
Regulare expression to match the relief comment that supresses
the warning about a fall through.
</td>
<td><a href="property_types.html#regexp">regular expression</a></td>
<td><span class="default">fallthru|falls? ?through</span></td>
</tr>
</table>
</subsection>
@ -1887,6 +1924,14 @@ if (&quot;something&quot;.equals(x))
To configure the check:
</p>
<source>
&lt;module name=&quot;FallThrough&quot;/&gt;
</source>
<p>
or
</p>
<source>
&lt;module name=&quot;FallThrough&quot;&gt;
&lt;property name=&quot;reliefPattern&quot; value=&quot;continue in next case&quot;/&gt;
&lt;module name=&quot;FallThrough&quot;/&gt;
</source>
</subsection>

13
src/xdocs/releasenotes.xml
View File

@ -14,10 +14,10 @@
<p>New features:</p>
<ul>
<li>
Applied excellent patch 1344344 by sjq to consolidate the
regexp checks into one.
</li>
<li>
Applied excellent patch 1344344 by sjq to consolidate the
regexp checks into one.
</li>
<li>
Added support for property <i>ignoreStringsRegexp</i> to
<i>MultipleStringLiterals</i> check. Patch 1254918 from taab.
@ -50,6 +50,11 @@
WhitespaceAround, properties allowEmptyCtors and
allowEmptyMethods).
</li>
<li>
FallThrough check can be configured to ignore fall-thoughs
if there is a relief comment, thanks to Ralf (aka rakus)
for a great patch (rfe 1345691, patch 1348873)
</li>
</ul>
<p>Fixed Bugs:</p>

3
suppressions.xml
View File

@ -11,9 +11,6 @@
<suppress checks="MagicNumber"
files="UnusedPrivateMethodCheck.java"
lines="176"/>
<suppress checks="MagicNumber"
files="JavadocMethodCheck.java"
lines="748,780,805"/>
<suppress checks="ImportControl"
files="NewlineAtEndOfFileCheck.java"
lines="26"/>