From a61f1eb40499fec40c3ef99e879b9a2a25643499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 23 Feb 2022 22:57:17 +0100 Subject: [PATCH] Fix bug with switch arrows --- .../pmd/lang/java/ast/ASTSwitchStatement.java | 23 ++++- .../lang/java/ast/ASTSwitchStatementTest.java | 47 +++++++++++ .../xml/SwitchStmtsShouldHaveDefault.xml | 84 ++++++++++++++++++- 3 files changed, 152 insertions(+), 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 index 6a019966d9a..7ba203ac7f7 100644 --- 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 iterator() { - return new NodeChildrenIterator<>(this, ASTSwitchLabel.class); + List 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 index 5915a67c3d4..47cc6c770ec 100644 --- 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/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/SwitchStmtsShouldHaveDefault.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/SwitchStmtsShouldHaveDefault.xml index 0ecf6177f15..a6b767069c6 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/SwitchStmtsShouldHaveDefault.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/SwitchStmtsShouldHaveDefault.xml @@ -79,6 +79,88 @@ public class Foo { } } } - ]]> + ]]> + + + #3605 switch on enum with default + 0 + + + + #3605 switch on enum with default, nonexhaustive + 0 + + + + #3605 switch on enum with default, nonexhaustive, arrow + 0 + 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?"); + } + } + } + ]]> + + + #3605 switch on enum with default, exhaustive, arrow + 0 + 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?"); + } + } + } + ]]>