Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge branch 'pr-1172'
  • Loading branch information
jsotuyod committed Jun 9, 2018
2 parents 26a5dd6 + 2ed1549 commit 3ca4494
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 12 deletions.
2 changes: 2 additions & 0 deletions docs/pages/release_notes.md
Expand Up @@ -23,6 +23,8 @@ This is a minor release.

* ecmascript
* [#861](https://github.com/pmd/pmd/issues/861): \[ecmascript] InnaccurateNumericLiteral false positive with hex literals
* java-bestpractices
* [#651](https://github.com/pmd/pmd/issues/651): \[java] SwitchStmtsShouldHaveDefault should be aware of enum types
* java-codestyle
* [#1158](https://github.com/pmd/pmd/issues/1158): \[java] Fix IdenticalCatchBranches false positive
* xml
Expand Down
Expand Up @@ -5,6 +5,15 @@

package net.sourceforge.pmd.lang.java.ast;

/**
* Represents either a {@code case} or {@code default} label inside
* a {@linkplain ASTSwitchStatement switch statement}.
*
* <pre>
* SwitchLabel ::= "case" {@linkplain ASTExpression Expression} ":"
* | "default" ":"
* </pre>
*/
public class ASTSwitchLabel extends AbstractJavaNode {

private boolean isDefault;
Expand Down
Expand Up @@ -5,7 +5,22 @@

package net.sourceforge.pmd.lang.java.ast;

public class ASTSwitchStatement extends AbstractJavaNode {
import java.util.Iterator;
import java.util.Set;

import org.apache.commons.lang3.EnumUtils;


/**
* Represents a {@code switch} statement.
*
* <pre>
* SwitchStatement ::= "switch" "(" {@linkplain ASTExpression Expression} ")" "{"
* ( {@linkplain ASTSwitchLabel SwitchLabel} {@linkplain ASTBlockStatement BlockStatement}* )*
* "}"
* </pre>
*/
public class ASTSwitchStatement extends AbstractJavaNode implements Iterable<ASTSwitchLabel> {
public ASTSwitchStatement(int id) {
super(id);
}
Expand All @@ -14,10 +29,69 @@ public ASTSwitchStatement(JavaParser p, int id) {
super(p, id);
}

/**
* Accept the visitor. *
*/

public Object jjtAccept(JavaParserVisitor visitor, Object data) {
return visitor.visit(this, data);
}


/**
* Returns true if this switch has a {@code default} case.
*/
public boolean hasDefaultCase() {
for (ASTSwitchLabel label : this) {
if (label.isDefault()) {
return true;
}
}
return false;
}


/**
* Gets the expression tested by this switch.
* This is the expression between the parentheses.
*/
public ASTExpression getTestedExpression() {
return (ASTExpression) jjtGetChild(0);
}


/**
* Returns true if this switch statement tests an expression
* having an enum type and all the constants of this type
* are covered by a switch case. Returns false if the type of
* the tested expression could not be resolved.
*/
public boolean isExhaustiveEnumSwitch() {
ASTExpression expression = getTestedExpression();

if (expression.getType() == null) {
return false;
}

if (Enum.class.isAssignableFrom(expression.getType())) {

@SuppressWarnings("unchecked")
Set<String> constantNames = EnumUtils.getEnumMap((Class<? extends Enum>) expression.getType()).keySet();

for (ASTSwitchLabel label : this) {
// since this is an enum switch, the labels are necessarily
// the simple name of some enum constant.

constantNames.remove(label.getFirstDescendantOfType(ASTName.class).getImage());

}

return constantNames.isEmpty();
}

return false;
}


@Override
public Iterator<ASTSwitchLabel> iterator() {
return new NodeChildrenIterator<>(this, ASTSwitchLabel.class);
}
}
6 changes: 5 additions & 1 deletion pmd-java/src/main/resources/category/java/bestpractices.xml
Expand Up @@ -1017,15 +1017,19 @@ public class Foo {
language="java"
since="1.0"
message="Switch statements should have a default label"
typeResolution="true"
class="net.sourceforge.pmd.lang.rule.XPathRule"
externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#switchstmtsshouldhavedefault">
<description>
All switch statements should include a default option to catch any unspecified values.
</description>
<priority>3</priority>
<properties>
<property name="version" value="2.0"/>
<property name="xpath">
<value>//SwitchStatement[not(SwitchLabel[@Default='true'])]</value>
<value><![CDATA[
//SwitchStatement[@DefaultCase = false() and @ExhaustiveEnumSwitch = false()]
]]></value>
</property>
</properties>
<example>
Expand Down
@@ -0,0 +1,15 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/

package net.sourceforge.pmd.lang.java.rule.bestpractices.switchstmtsshouldhavedefault;

/**
* @author Clément Fournier
* @since 6.5.0
*/
public enum SimpleEnum {
FOO,
BAR,
BZAZ
}
Expand Up @@ -3,10 +3,8 @@
xmlns="http://pmd.sourceforge.net/rule-tests"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd">
<test-code>
<description><![CDATA[
simple failure case
]]></description>
<test-code>
<description>simple failure case</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
Expand All @@ -19,10 +17,9 @@ public class Foo {
}
]]></code>
</test-code>

<test-code>
<description><![CDATA[
simple ok case
]]></description>
<description>simple ok case</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
Expand All @@ -36,4 +33,51 @@ public class Foo {
}
]]></code>
</test-code>

<test-code>
<description>#651 Enum type, not ok</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
import net.sourceforge.pmd.lang.java.rule.bestpractices.switchstmtsshouldhavedefault.SimpleEnum;
public class Foo {
void bar() {
SimpleEnum a;
// This switch is NOT exhaustive
switch (a) {
case BZAZ:
int y = 8;
break;
}
}
}
]]></code>
</test-code>

<test-code>
<description>#651 Enum type, ok</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import net.sourceforge.pmd.lang.java.rule.bestpractices.switchstmtsshouldhavedefault.SimpleEnum;
public class Foo {
void bar() {
SimpleEnum x;
// This switch is exhaustive
switch (x) {
case BZAZ:
int y = 8;
break;
case FOO:
break;
case BAR:
int w = 8;
break;
}
}
}
]]></code>
</test-code>
</test-data>

0 comments on commit 3ca4494

Please sign in to comment.