Fix #5249 and #5250: [java] TooFewBranchesForSwitch ignore pattern matching and support switch expressions (#5251)

Merge pull request #5251 from adangel:issue-5249-5250
2024-10-24 14:06:03 +02:00 · 2024-10-24 14:06:03 +02:00 · 3a501a0f6b
commit 3a501a0f6b
parent 93f6279ea7 45b1217485
6 changed files with 163 additions and 77 deletions
--- a/docs/pages/release_notes.md
+++ b/docs/pages/release_notes.md
@ -24,6 +24,8 @@ See [PR #5040](https://github.com/pmd/pmd/pull/5040) for details.
 ### 🌟 Rule Changes

 #### Changed Rules
+* {% rule java/performance/TooFewBranchesForSwitch %} (Java Performance) doesn't report empty switches anymore.
+  To detect these, use {% rule java/codestyle/EmptyControlStatement %}.
 * {% rule java/bestpractices/UnitTestShouldUseAfterAnnotation %} (Java Best Practices) now also considers JUnit 5 and TestNG tests.
 * {% rule java/bestpractices/UnitTestShouldUseBeforeAnnotation %} (Java Best Practices) now also considers JUnit 5 and TestNG tests.

@ -36,6 +38,8 @@ See [PR #5040](https://github.com/pmd/pmd/pull/5040) for details.
  * {% rule java/bestpractices/UnitTestShouldUseAfterAnnotation %} (Java Best Practices) has been renamed from `JUnit4TestShouldUseAfterAnnotation`.
  * {% rule java/bestpractices/UnitTestShouldUseBeforeAnnotation %} (Java Best Practices) has been renamed from `JUnit4TestShouldUseBeforeAnnotation`.
  * {% rule java/bestpractices/UnitTestShouldUseTestAnnotation %} (Java Best Practices) has been renamed from `JUnit4TestShouldUseTestAnnotation`.
+* {% rule java/performance/TooFewBranchesForSwitch %} (Java Performance) has been renamed from `TooFewBranchesForASwitchStatement`,
+  as it now applies to Switch Expressions as well.

 The old rule names still work but are deprecated.

@ -50,6 +54,9 @@ The old rule names still work but are deprecated.
 * java-errorprone
  * [#3362](https://github.com/pmd/pmd/issues/3362): \[java] ImplicitSwitchFallThrough should consider switch expressions
  * [#5067](https://github.com/pmd/pmd/issues/5067): \[java] CloseResource: False positive for FileSystems.getDefault()
+* java-performance
+  * [#5249](https://github.com/pmd/pmd/issues/5249): \[java] TooFewBranchesForASwitchStatement false positive for Pattern Matching
+  * [#5250](https://github.com/pmd/pmd/issues/5250): \[java] TooFewBranchesForASwitchStatement should consider Switch Expressions

 ### 🚨 API Changes
 * java-bestpractices
@ -59,6 +66,8 @@ The old rule names still work but are deprecated.
  * The old rule name `JUnitAssertionsShouldIncludeMessage` has been deprecated. Use the new name {% rule java/bestpractices/UnitTestAssertionsShouldIncludeMessage %} instead.
  * The old rule name `JUnitTestContainsTooManyAsserts` has been deprecated. Use the new name {% rule java/bestpractices/UnitTestContainsTooManyAsserts %} instead.
  * The old rule name `JUnitTestsShouldIncludeAssert` has been deprecated. Use the new name {% rule java/bestpractices/UnitTestShouldIncludeAssert %} instead.
+* java-performance
+  * The old rule name `TooFewBranchesForASwitchStatement` has been deprecated. Use the new name {% rule java/performance/TooFewBranchesForSwitch %} instead.


 ### ✨ Merged pull requests
@ -69,6 +78,7 @@ The old rule names still work but are deprecated.
 * [#5245](https://github.com/pmd/pmd/pull/5245): \[java] Improve UnitTestShouldUse{After,Before}Annotation rules to support JUnit5 and TestNG - [Andreas Dangel](https://github.com/adangel) (@adangel)
 * [#5247](https://github.com/pmd/pmd/pull/5247): Fix #5030: \[java] SwitchDensity false positive with pattern matching - [Andreas Dangel](https://github.com/adangel) (@adangel)
 * [#5248](https://github.com/pmd/pmd/pull/5248): Fix #3362: \[java] ImplicitSwitchFallThrough should consider switch expressions - [Andreas Dangel](https://github.com/adangel) (@adangel)
+* [#5251](https://github.com/pmd/pmd/pull/5251): Fix #5249 and #5250: \[java] TooFewBranchesForSwitch ignore pattern matching and support switch expressions - [Andreas Dangel](https://github.com/adangel) (@adangel)
 * [#5258](https://github.com/pmd/pmd/pull/5258): Ignore generated antlr classes in coverage reports - [Juan Martín Sotuyo Dodero](https://github.com/jsotuyod) (@jsotuyod)
 * [#5264](https://github.com/pmd/pmd/pull/5264): Fix #5261: \[java] Fix NPE with empty pattern list - [Clément Fournier](https://github.com/oowekyala) (@oowekyala)
 * [#5269](https://github.com/pmd/pmd/pull/5269): Fix #5253: \[java] Support Boolean wrapper class for BooleanGetMethodName rule - [Aryant Tripathi](https://github.com/Aryant-Tripathi) (@Aryant-Tripathi)
--- a/pmd-java/src/main/resources/category/java/performance.xml
+++ b/pmd-java/src/main/resources/category/java/performance.xml
@ -607,16 +607,20 @@ private String baz() {
        </example>
    </rule>

-    <rule name="TooFewBranchesForASwitchStatement"
+    <rule name="TooFewBranchesForASwitchStatement" deprecated="true" ref="TooFewBranchesForSwitch"/>
+
+    <rule name="TooFewBranchesForSwitch"
          language="java"
          since="4.2"
          class="net.sourceforge.pmd.lang.rule.xpath.XPathRule"
          message="A switch with less than three branches is inefficient, use a 'if statement' instead."
-          externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_performance.html#toofewbranchesforaswitchstatement">
+          externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_performance.html#toofewbranchesforswitch">
        <description>
 Switch statements are intended to be used to support complex branching behaviour. Using a switch for only a few
 cases is ill-advised, since switches are not as easy to understand as if-else statements. In these cases use the
 if-else statement to increase code readability.
+
+Note: This rule was named TooFewBranchesForASwitchStatement before PMD 7.7.0.
        </description>
        <priority>3</priority>
        <properties>
@ -624,7 +628,10 @@ if-else statement to increase code readability.
            <property name="xpath">
                <value>
 <![CDATA[
-//SwitchStatement[ (count(*) - 1) < $minimumNumberCaseForASwitch ]
+//(SwitchStatement | SwitchExpression)
+  [ (count(*) - 1) < $minimumNumberCaseForASwitch ]
+  (: only consider if no pattern matching is used :)
+  [*/SwitchLabel[@PatternLabel = false()]]
 ]]>
                </value>
            </property>
--- a/pmd-java/src/main/resources/rulesets/java/quickstart.xml
+++ b/pmd-java/src/main/resources/rulesets/java/quickstart.xml
@ -307,7 +307,7 @@
    <!--<rule ref="category/java/performance.xml/RedundantFieldInitializer"/>-->
    <!-- <rule ref="category/java/performance.xml/StringInstantiation" /> -->
    <!-- <rule ref="category/java/performance.xml/StringToString" /> -->
-    <!--<rule ref="category/java/performance.xml/TooFewBranchesForASwitchStatement"/>-->
+    <!-- <rule ref="category/java/performance.xml/TooFewBranchesForSwitch"/> -->
    <!-- <rule ref="category/java/performance.xml/UseArrayListInsteadOfVector" /> -->
    <!-- <rule ref="category/java/performance.xml/UseArraysAsList" /> -->
    <!-- <rule ref="category/java/performance.xml/UseIndexOfChar" /> -->
--- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/TooFewBranchesForASwitchStatementTest.java
+++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/TooFewBranchesForASwitchStatementTest.java
@ -6,6 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.performance;

 import net.sourceforge.pmd.test.PmdRuleTst;

-class TooFewBranchesForASwitchStatementTest extends PmdRuleTst {
+class TooFewBranchesForSwitchTest extends PmdRuleTst {
    // no additional unit tests
 }
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/TooFewBranchesForASwitchStatement.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/TooFewBranchesForASwitchStatement.xml
@ -1,72 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<test-data
-    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>Only one case, this is useless</description>
-        <rule-property name="minimumNumberCaseForASwitch">3</rule-property>
-        <expected-problems>1</expected-problems>
-        <code><![CDATA[
-public class DumbSwitch {
-    public void foo(int i) {
-        switch (i) {
-            case 0:
-            {
-                System.err.println("I am a fish.");
-            }
-        }
-    }
-}
-        ]]></code>
-    </test-code>
-
-    <test-code>
-        <description>Even two branches is not enough for a switch statement</description>
-        <rule-property name="minimumNumberCaseForASwitch">3</rule-property>
-        <expected-problems>1</expected-problems>
-        <code><![CDATA[
-public class DumbSwitch {
-    public void foo(int i) {
-        switch (i) {
-            case 0:
-            {
-                System.err.println("I am a fish.");
-            }
-            case 1:
-            {
-                System.exit();
-            }
-        }
-    }
-}
-        ]]></code>
-    </test-code>
-
-    <test-code>
-        <description>Three branches in a switch statement is ok.</description>
-        <rule-property name="minimumNumberCaseForASwitch">3</rule-property>
-        <expected-problems>0</expected-problems>
-        <code><![CDATA[
-public class ValidSwitch {
-    public void foo(int i) {
-        switch (i) {
-            case 0:
-            {
-                System.err.println("I am a fish.");
-            }
-            case 1:
-            {
-                System.exit();
-            }
-            case 2:
-            {
-                // ...
-            }
-        }
-    }
-}
-        ]]></code>
-    </test-code>
-</test-data>
--- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/TooFewBranchesForSwitch.xml
+++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/TooFewBranchesForSwitch.xml
@ -0,0 +1,141 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<test-data
+    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>Switch Statement with no case, ok</description>
+        <rule-property name="minimumNumberCaseForASwitch">3</rule-property>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class DumbSwitch {
+    public void foo(int i) {
+        switch (i) { } // This is detected by EmptyControlStatement
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Switch Statement with only one case, not ok</description>
+        <rule-property name="minimumNumberCaseForASwitch">3</rule-property>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class DumbSwitch {
+    public void foo(int i) {
+        switch (i) {
+            case 0:
+            {
+                System.err.println("I am a fish.");
+            }
+        }
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Switch Expression with only one case, not ok #5250</description>
+        <rule-property name="minimumNumberCaseForASwitch">3</rule-property>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class DumbSwitch {
+    public String foo(int i) {
+        return switch (i) {
+            case 0:
+            {
+                yield "I am a fish.";
+            }
+        };
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Even two branches is not enough for a switch statement</description>
+        <rule-property name="minimumNumberCaseForASwitch">3</rule-property>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+public class DumbSwitch {
+    public void foo(int i) {
+        switch (i) {
+            case 0:
+            {
+                System.err.println("I am a fish.");
+            }
+            case 1:
+            {
+                System.exit();
+            }
+        }
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Three branches in a switch statement is ok.</description>
+        <rule-property name="minimumNumberCaseForASwitch">3</rule-property>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+public class ValidSwitch {
+    public void foo(int i) {
+        switch (i) {
+            case 0:
+            {
+                System.err.println("I am a fish.");
+            }
+            case 1:
+            {
+                System.exit();
+            }
+            case 2:
+            {
+                // ...
+            }
+        }
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>[java] TooFewBranchesForASwitchStatement false positive for Pattern Matching #5249</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+sealed interface S permits A {}
+final class A implements S {}
+public class Sample {
+    public void simpleSwitchStatment(S s) {
+        switch(s) {
+            case A a -> System.out.println("a");
+        }
+    }
+
+    public void simpleSwitchExpression(S s) {
+        String result = switch(s) {
+            case A a -> "a";
+        };
+    }
+}
+]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Record patterns are ignored, too #5249</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+record R(int i) {}
+
+public class SwitchWithRecordPattern {
+    public void check(R r) {
+        switch(r) {
+            case R(int a) -> System.out.println(a);
+        }
+    }
+}
+]]></code>
+    </test-code>
+</test-data>