Skip to content

Commit

Permalink
bugfix: default中包含return, throw时漏报
Browse files Browse the repository at this point in the history
  • Loading branch information
@fw8899
fw8899 committed Mar 15, 2018
1 parent 10f3848 commit 42ce06f
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 134 deletions.
26 changes: 15 additions & 11 deletions ...pmd/src/main/java/com/alibaba/p3c/pmd/lang/java/rule/flowcontrol/SwitchStatementRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public Object visit(ASTSwitchStatement node, Object data) {
* @param data
*/
private void checkDefault(ASTSwitchStatement node, Object data) {
final String switchCheckXpath = "SwitchLabel[@Default='true']";
final String switchCheckXpath = "SwitchLabel[@Default = 'true']";
if (!node.hasDescendantMatchingXPath(switchCheckXpath)) {
addViolationWithMessage(data, node, MESSAGE_KEY_PREFIX + ".nodefault");
}
Expand All @@ -60,16 +60,20 @@ private void checkDefault(ASTSwitchStatement node, Object data) {
*/
private void checkFallThrough(ASTSwitchStatement node, Object data) {
// refer the rule MissingBreakInSwitch of PMD
final String xpath = "../SwitchStatement[(count(.//BreakStatement)" +
" + count(BlockStatement//Statement/ReturnStatement)" +
" + count(BlockStatement//Statement/ThrowStatement)" +
" + count(BlockStatement//Statement/IfStatement[@Else='true' and "
+ "Statement[2][ReturnStatement|ThrowStatement]]"
+
"/Statement[1][ReturnStatement|ThrowStatement])" +
" + count(SwitchLabel[name(following-sibling::node()) = 'SwitchLabel'])" +
" + count(SwitchLabel[count(following-sibling::node()) = 0])" +
" < count (SwitchLabel[@Default != 'true']))]";
final String xpath = "../SwitchStatement[(count(.//BreakStatement)"
+ " + count(BlockStatement//Statement/ReturnStatement)"
+ " + count(BlockStatement//Statement/ContinueStatement)"
+ " + count(BlockStatement//Statement/ThrowStatement)"
+ " + count(BlockStatement//Statement/IfStatement[@Else='true'"
+ " and Statement[2][ReturnStatement|ContinueStatement|ThrowStatement]]"
+ "/Statement[1][ReturnStatement|ContinueStatement|ThrowStatement])"
+ " + count(SwitchLabel[name(following-sibling::node()) = 'SwitchLabel'])"
+ " + count(SwitchLabel[count(following-sibling::node()) = 0])"
+ " < count (SwitchLabel[@Default != 'true'])"
+ " + count(SwitchLabel[@Default = 'true']/following-sibling::BlockStatement//Statement/ReturnStatement)"
+ " + count(SwitchLabel[@Default = 'true']/following-sibling::BlockStatement//Statement/ContinueStatement)"
+ " + count(SwitchLabel[@Default = 'true']/following-sibling::BlockStatement//Statement/ThrowStatement)"
+ ")]";

if (node.hasDescendantMatchingXPath(xpath)) {
addViolationWithMessage(data, node, MESSAGE_KEY_PREFIX + ".notermination");
Expand Down
247 changes: 124 additions & 123 deletions ...test/resources/com/alibaba/p3c/pmd/lang/java/rule/flowcontrol/xml/SwitchStatementRule.xml
View file
Original file line number Diff line number Diff line change
@@ -1,146 +1,147 @@
<?xml version="1.0" encoding="UTF-8"?>
<test-data>
<code-fragment id="switch-no-default-1">
<![CDATA[
public class Example {
public void fn() {
int i;
switch (i) {
case 0:
break;
case 1:
int j;
switch (j) {
case 0:
break;
default: // nested switch has default blocks
return;
}
break;
// missing default statement
}
}
}
]]>
</code-fragment>
<test-code>
<description>switch statement in outer class has no default block</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code-ref id="switch-no-default-1" />
</test-code>
<code-fragment id="switch-no-default-1">
<![CDATA[
public class Example {
public void fn() {
int i;
switch (i) {
case 0:
break;
case 1:
int j;
switch (j) {
case 0:
break;
default: // nested switch has default blocks
return;
}
break;
// missing default statement
}
}
}
]]>
</code-fragment>
<test-code>
<description>switch statement in outer class has no default block</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code-ref id="switch-no-default-1" />
</test-code>

<!-- ====================================================================== -->

<code-fragment id="switch-no-default-2">
<![CDATA[
public class Foo {
public void bar() {
int i;
switch (i) {
case 0:
break;
case 1:
int j;
switch (j) {
case 0:
break;
// nested switch has no default block
}
break;
default:
return;
}
}
}
]]>
</code-fragment>
<test-code>
<description>nested switch has no default block</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>9</expected-linenumbers>
<code-ref id="switch-no-default-2" />
</test-code>
<code-fragment id="switch-no-default-2">
<![CDATA[
public class Foo {
public void bar() {
int i;
switch (i) {
case 0:
break;
case 1:
int j;
switch (j) {
case 0:
break;
// nested switch has no default block
}
break;
default:
return;
}
}
}
]]>
</code-fragment>
<test-code>
<description>nested switch has no default block</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>9</expected-linenumbers>
<code-ref id="switch-no-default-2" />
</test-code>

<!-- ====================================================================== -->

<code-fragment id="switch-case-no-break">
<![CDATA[
public class Foo {
public void bar() {
int i;
switch (i) {
case 0:
int j =1;
// missing break,return and continue
case 1:
break;
case 2:
continue;
case 3: // OK
default:
return;
}
}
}
]]>
</code-fragment>
<test-code>
<description>case statement without break</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code-ref id="switch-case-no-break" />
</test-code>
<code-fragment id="switch-case-no-break">
<![CDATA[
public class Foo {
public void bar() {
int i;
switch (i) {
case 0:
int j =1;
// missing break,return and continue
case 1:
break;
case 2:
continue;
case 3: // OK
default:
int k = 1;
return;
}
}
}
]]>
</code-fragment>
<test-code>
<description>case statement without break</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<code-ref id="switch-case-no-break" />
</test-code>

<!-- ====================================================================== -->

<code-fragment id="default-no-break">
<![CDATA[
public class Foo {
public void bar() {
int i;
switch (i) {
case 0:
int j =1;
break;
case 1:
case 2:
continue;
default: // OK
}
}
}
]]>
</code-fragment>
<test-code>
<description>default statement has no break</description>
<expected-problems>0</expected-problems>
<code-ref id="default-no-break" />
</test-code>
<code-fragment id="default-no-break">
<![CDATA[
public class Foo {
public void bar() {
int i;
switch (i) {
case 0:
int j =1;
break;
case 1:
case 2:
continue;
default: // OK
}
}
}
]]>
</code-fragment>
<test-code>
<description>default statement has no break</description>
<expected-problems>0</expected-problems>
<code-ref id="default-no-break" />
</test-code>

<!-- ====================================================================== -->
<!-- ====================================================================== -->


<code-fragment id="multi-case-no-statement">
<![CDATA[
public class Foo {
public void bar() {
int i;
switch (i) {
case 0:
case 1:
case 2:
// do something
break;
}
}
}
]]>
public class Foo {
public void bar() {
int i;
switch (i) {
case 0:
case 1:
case 2:
// do something
break;
}
}
}
]]>
</code-fragment>
<test-code>
<description>multiple continuous blank case</description>
<expected-problems>1</expected-problems>
<expected-linenumbers>4</expected-linenumbers>
<expected-linenumbers>4</expected-linenumbers>
<code-ref id="multi-case-no-statement" />
</test-code>
</test-data>

0 comments on commit 42ce06f

Please sign in to comment.