Added allowInSwitchCase property to AvoidNestedBlocksCheck
to allow limiting the scope of variables to one case of a switch statement. Also currently playing with the Clover IDEA plugin - man, this rocks!!
This commit is contained in:
parent
6db0b3effd
commit
59de966a7e
|
|
@ -325,22 +325,13 @@
|
|||
<!-- --> <a name="AvoidNestedBlocks"></a> <h2>AvoidNestedBlocks</h2> <h4>Description</h4>
|
||||
<p class="body">
|
||||
Finds nested blocks, i.e. blocks that are used freely in the code.
|
||||
For example this Check finds the obsolete braces in
|
||||
</p>
|
||||
<pre class="body">
|
||||
switch (a)
|
||||
{
|
||||
case 0:
|
||||
{
|
||||
x = 1;
|
||||
}
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
</pre>
|
||||
<p class="body">
|
||||
and flags confusing code like
|
||||
Rationale: Nested blocks are often leftovers from the debugging process,
|
||||
they confuse the reader.
|
||||
</p>
|
||||
|
||||
<p>
|
||||
For example this Check finds the obsolete braces in
|
||||
</p>
|
||||
<pre class="body">
|
||||
public void guessTheOutput()
|
||||
|
|
@ -353,9 +344,67 @@
|
|||
}
|
||||
</pre>
|
||||
<p class="body">
|
||||
Rationale: Nested blocks are often leftovers from the debugging process, they confuse
|
||||
the reader.
|
||||
and debugging / refactoring leftovers such as
|
||||
</p>
|
||||
|
||||
<pre class="body">
|
||||
// if (conditionThatIsNotUsedAnyLonger)
|
||||
{
|
||||
System.out.println("unconditional");
|
||||
}
|
||||
</pre>
|
||||
|
||||
<p class="body">
|
||||
A case in a switch statement does not implicitly form a block.
|
||||
Thus to be able to introduce local variables that have case scope
|
||||
it is necessary to open a nested block. This is supported, set
|
||||
the allowInSwitchCase property to true and include all statements
|
||||
of the case in the block.
|
||||
</p>
|
||||
|
||||
<pre class="body">
|
||||
switch (a)
|
||||
{
|
||||
case 0:
|
||||
// Never OK, break outside block
|
||||
{
|
||||
x = 1;
|
||||
}
|
||||
break;
|
||||
case 1:
|
||||
// Never OK, statement outside block
|
||||
System.out.println("Hello");
|
||||
{
|
||||
x = 2;
|
||||
break;
|
||||
}
|
||||
case 1:
|
||||
// OK if allowInSwitchCase is true
|
||||
{
|
||||
System.out.println("Hello");
|
||||
x = 2;
|
||||
break;
|
||||
}
|
||||
}
|
||||
</pre>
|
||||
|
||||
|
||||
<h4>Properties</h4>
|
||||
<table width="100%" border="1" cellpadding="5" class="body">
|
||||
<tr class="header">
|
||||
<th>name</th>
|
||||
<th>description</th>
|
||||
<th>type</th>
|
||||
<th>default value</th>
|
||||
</tr>
|
||||
<tr>
|
||||
<td>allowInSwitchCase</td>
|
||||
<td>Allow nested blocks in case statements</td>
|
||||
<td><a href="property_types.html#boolean">boolean</a></td>
|
||||
<td><span class="default">false</span></td>
|
||||
</tr>
|
||||
</table>
|
||||
|
||||
<h4>Examples</h4>
|
||||
<p class="body">
|
||||
To configure the check:
|
||||
|
|
|
|||
|
|
@ -87,14 +87,14 @@
|
|||
|
||||
<li class="body">Added excludes property to AvoidStarImport, contributed
|
||||
by Bill Schneider (request 744955).</li>
|
||||
|
||||
|
||||
<li class="body">Added CyclomaticComplexityCheck from Simon Harris.</li>
|
||||
|
||||
<li class="body">Added check to catch equality comparison with string literals
|
||||
using the == operator (module StringLiteralEquality, request 754835).</li>
|
||||
|
||||
|
||||
<li class="body">Added check for definition of covariant equals() method
|
||||
without overriding method equals(comp.lang.Object)
|
||||
without overriding method equals(comp.lang.Object)
|
||||
(module coding.CovariantEquals).</li>
|
||||
|
||||
<li class="body">Added NestedTryDepthCheck and
|
||||
|
|
@ -104,6 +104,9 @@
|
|||
<li class="body">Added IllegalTokenCheck from Simon Harris
|
||||
(request 750755).</li>
|
||||
|
||||
<li class="body">Added allowInSwitchCase property to AvoidNestedBlocksCheck
|
||||
to allow limiting the scope of variables to one case of a switch statement.</li>
|
||||
|
||||
</ul>
|
||||
|
||||
<p class="body">
|
||||
|
|
|
|||
|
|
@ -24,20 +24,9 @@ import com.puppycrawl.tools.checkstyle.api.DetailAST;
|
|||
|
||||
/**
|
||||
* Finds nested blocks.
|
||||
* For example this Check finds the obsolete braces in
|
||||
* <pre>
|
||||
* switch (a)
|
||||
* {
|
||||
* case 0:
|
||||
* {
|
||||
* x = 1;
|
||||
* }
|
||||
* break;
|
||||
* default:
|
||||
* break;
|
||||
* }
|
||||
* </pre>
|
||||
* and flags confusing code like
|
||||
*
|
||||
* <p>
|
||||
* For example this Check flags confusing code like
|
||||
* <pre>
|
||||
* public void guessTheOutput()
|
||||
* {
|
||||
|
|
@ -49,10 +38,59 @@ import com.puppycrawl.tools.checkstyle.api.DetailAST;
|
|||
* }
|
||||
* </pre>
|
||||
*
|
||||
* and debugging / refactoring leftovers such as
|
||||
*
|
||||
* <pre>
|
||||
* // if (someOldCondition)
|
||||
* {
|
||||
* System.out.println("unconditional");
|
||||
* }
|
||||
* </pre>
|
||||
*
|
||||
* <p>
|
||||
* A case in a switch statement does not implicitly form a block.
|
||||
* Thus to be able to introduce local variables that have case scope
|
||||
* it is necessary to open a nested block. This is supported, set
|
||||
* the allowInSwitchCase property to true and include all statements
|
||||
* of the case in the block.
|
||||
* </p>
|
||||
*
|
||||
* <pre>
|
||||
* switch (a)
|
||||
* {
|
||||
* case 0:
|
||||
* // Never OK, break outside block
|
||||
* {
|
||||
* x = 1;
|
||||
* }
|
||||
* break;
|
||||
* case 1:
|
||||
* // Never OK, statement outside block
|
||||
* System.out.println("Hello");
|
||||
* {
|
||||
* x = 2;
|
||||
* break;
|
||||
* }
|
||||
* case 1:
|
||||
* // OK if allowInSwitchCase is true
|
||||
* {
|
||||
* System.out.println("Hello");
|
||||
* x = 2;
|
||||
* break;
|
||||
* }
|
||||
* }
|
||||
* </pre>
|
||||
*
|
||||
* @author lkuehne
|
||||
*/
|
||||
public class AvoidNestedBlocksCheck extends Check
|
||||
{
|
||||
/**
|
||||
* Whether nested blocks are allowed if they are the
|
||||
* only child of a switch case.
|
||||
*/
|
||||
private boolean mAllowInSwitchCase = false;
|
||||
|
||||
/** @see Check */
|
||||
public int[] getDefaultTokens()
|
||||
{
|
||||
|
|
@ -62,9 +100,20 @@ public class AvoidNestedBlocksCheck extends Check
|
|||
/** @see Check */
|
||||
public void visitToken(DetailAST aAST)
|
||||
{
|
||||
if (aAST.getParent().getType() == TokenTypes.SLIST) {
|
||||
final DetailAST parent = aAST.getParent();
|
||||
if (parent.getType() == TokenTypes.SLIST) {
|
||||
if (mAllowInSwitchCase
|
||||
&& parent.getParent().getType() == TokenTypes.CASE_GROUP
|
||||
&& parent.getNumberOfChildren() == 1)
|
||||
{
|
||||
return;
|
||||
}
|
||||
log(aAST.getLineNo(), aAST.getColumnNo(), "block.nested");
|
||||
}
|
||||
}
|
||||
|
||||
public void setAllowInSwitchCase(boolean aAllowInSwitchCase)
|
||||
{
|
||||
mAllowInSwitchCase = aAllowInSwitchCase;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,14 +6,31 @@ import com.puppycrawl.tools.checkstyle.DefaultConfiguration;
|
|||
public class AvoidNestedBlocksCheckTest
|
||||
extends BaseCheckTestCase
|
||||
{
|
||||
public void testIt()
|
||||
public void testStrictSettings()
|
||||
throws Exception
|
||||
{
|
||||
final DefaultConfiguration checkConfig =
|
||||
createCheckConfig(AvoidNestedBlocksCheck.class);
|
||||
final String[] expected = {
|
||||
"22:9: Avoid nested blocks.",
|
||||
"38:17: Avoid nested blocks.",
|
||||
"44:17: Avoid nested blocks.",
|
||||
"50:17: Avoid nested blocks.",
|
||||
"58:17: Avoid nested blocks.",
|
||||
};
|
||||
verify(checkConfig, getPath("InputNestedBlocks.java"), expected);
|
||||
}
|
||||
|
||||
public void testAllowSwitchInCase()
|
||||
throws Exception
|
||||
{
|
||||
final DefaultConfiguration checkConfig =
|
||||
createCheckConfig(AvoidNestedBlocksCheck.class);
|
||||
checkConfig.addAttribute("allowInSwitchCase", Boolean.TRUE.toString());
|
||||
|
||||
final String[] expected = {
|
||||
"22:9: Avoid nested blocks.",
|
||||
"44:17: Avoid nested blocks.",
|
||||
"58:17: Avoid nested blocks.",
|
||||
};
|
||||
verify(checkConfig, getPath("InputNestedBlocks.java"), expected);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue