Fix bug with switch arrows

pmd · Feb 23, 2022 · a61f1eb · a61f1eb
1 parent a6e5584
commit a61f1eb
Show file tree

Hide file tree

Showing 3 changed files with 152 additions and 2 deletions.
diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTSwitchStatement.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTSwitchStatement.java
@@ -4,7 +4,9 @@
 
 package net.sourceforge.pmd.lang.java.ast;
 
+import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Set;
 
 import org.apache.commons.lang3.EnumUtils;
@@ -99,9 +101,28 @@ public boolean isExhaustiveEnumSwitch() {
         return false;
     }
 
+    /**
+     * Returns true if this a switch which uses fallthrough branches
+     * (old school {@code case label: break;}) and not arrow branches.
+     * If the switch has no branches, returns false.
+     */
+    public boolean isFallthroughSwitch() {
+        return getFirstChildOfType(ASTSwitchLabel.class) != null
+            && getNumChildren() != 1;
+    }
 
     @Override
     public Iterator<ASTSwitchLabel> iterator() {
-        return new NodeChildrenIterator<>(this, ASTSwitchLabel.class);
+        List<ASTSwitchLabel> result = new ArrayList<>(findChildrenOfType(ASTSwitchLabel.class));
+        for (ASTSwitchLabeledBlock labeled : findChildrenOfType(ASTSwitchLabeledBlock.class)) {
+            result.add((ASTSwitchLabel) labeled.getChild(0));
+        }
+        for (ASTSwitchLabeledExpression labeled : findChildrenOfType(ASTSwitchLabeledExpression.class)) {
+            result.add((ASTSwitchLabel) labeled.getChild(0));
+        }
+        for (ASTSwitchLabeledThrowStatement labeled : findChildrenOfType(ASTSwitchLabeledThrowStatement.class)) {
+            result.add((ASTSwitchLabel) labeled.getChild(0));
+        }
+        return result.iterator();
     }
 }
diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTSwitchStatementTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTSwitchStatementTest.java
@@ -17,5 +17,52 @@ public void exhaustiveEnumSwitchWithDefault() {
                 .get(0);
         Assert.assertFalse(switchStatement.isExhaustiveEnumSwitch()); // this should not throw a NPE...
         Assert.assertTrue(switchStatement.hasDefaultCase());
+        Assert.assertTrue(switchStatement.isFallthroughSwitch());
+    }
+
+    @Test
+    public void defaultCaseWithArrowBlock() {
+        ASTSwitchStatement switchStatement = java.parse(
+                "class Foo { void bar(int x) {"
+                + "switch (x) { default -> { } } } }")
+                .getFirstDescendantOfType(ASTSwitchStatement.class);
+        Assert.assertFalse(switchStatement.isExhaustiveEnumSwitch());
+        Assert.assertTrue(switchStatement.iterator().hasNext());
+        Assert.assertTrue(switchStatement.hasDefaultCase());
+        Assert.assertFalse(switchStatement.isFallthroughSwitch());
+    }
+
+    @Test
+    public void emptySwitch() {
+        ASTSwitchStatement switchStatement = java.parse(
+                "class Foo { void bar(int x) {"
+                + "switch (x) { } } }")
+                .getFirstDescendantOfType(ASTSwitchStatement.class);
+        Assert.assertFalse(switchStatement.isExhaustiveEnumSwitch());
+        Assert.assertFalse(switchStatement.iterator().hasNext());
+        Assert.assertFalse(switchStatement.hasDefaultCase());
+        Assert.assertFalse(switchStatement.isFallthroughSwitch());
+    }
+
+    @Test
+    public void defaultCaseWithArrowExprs() {
+        ASTSwitchStatement switchStatement =
+            java.parse(
+                    "import net.sourceforge.pmd.lang.java.rule.bestpractices.switchstmtsshouldhavedefault.SimpleEnum;\n"
+                        + "\n"
+                        + "            public class Foo {\n"
+                        + "                void bar(SimpleEnum x) {\n"
+                        + "                    switch (x) {\n"
+                        + "                    case FOO -> System.out.println(\"it is on\");\n"
+                        + "                    case BAR -> System.out.println(\"it is off\");\n"
+                        + "                    default -> System.out.println(\"it is neither on nor off - should not happen? maybe null?\");\n"
+                        + "                    }\n"
+                        + "                }\n"
+                        + "            }")
+                .getFirstDescendantOfType(ASTSwitchStatement.class);
+        Assert.assertFalse(switchStatement.isExhaustiveEnumSwitch());
+        Assert.assertTrue(switchStatement.iterator().hasNext());
+        Assert.assertFalse(switchStatement.isFallthroughSwitch());
+        Assert.assertTrue(switchStatement.hasDefaultCase());
     }
 }
diff --git a/...ces/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/SwitchStmtsShouldHaveDefault.xml b/...ces/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/SwitchStmtsShouldHaveDefault.xml
@@ -79,6 +79,88 @@ public class Foo {
         }
     }
 }
-        ]]></code>
+            ]]></code>
+    </test-code>
+    <test-code>
+        <description>#3605 switch on enum with default</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) {
+                    switch (x) {
+                    case BZAZ:
+                        int y = 8;
+                        break;
+                    case FOO:
+                        break;
+                    case BAR:
+                        int w = 8;
+                        break;
+                    default:
+                        break;
+                    }
+                }
+            }
+            ]]></code>
+    </test-code>
+    <test-code>
+        <description>#3605 switch on enum with default, nonexhaustive</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) {
+                    switch (x) {
+                    case BZAZ:
+                        int y = 8;
+                        break;
+                    case FOO:
+                        break;
+                    default:
+                        break;
+                    }
+                }
+            }
+            ]]></code>
+    </test-code>
+    <test-code>
+        <description>#3605 switch on enum with default, nonexhaustive, arrow</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) {
+                    switch (x) {
+                    case FOO -> System.out.println("it is on");
+                    case BAR -> System.out.println("it is off");
+                    default -> System.out.println("it is neither on nor off - should not happen? maybe null?");
+                    }
+                }
+            }
+            ]]></code>
+    </test-code>
+    <test-code>
+        <description>#3605 switch on enum with default, exhaustive, arrow</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) {
+                    switch (x) {
+                    case FOO -> System.out.println("it is on");
+                    case BAR -> System.out.println("it is off");
+                    case BZAZ -> System.out.println("it is bzaz");
+                    default -> System.out.println("it is neither on nor off - should not happen? maybe null?");
+                    }
+                }
+            }
+            ]]></code>
     </test-code>
 </test-data>