6

I have a simple method which takes an enum and returns a String:

public static String enumToString(MyEnum type) {
    return switch (type) {
        case Enum1 -> "String_1";
        case Enum2 -> "String_2";
        case Enum3 -> "String_3";
        case Enum4 -> "String_4";
        case Enum5 -> "String_5";
        case Enum6 -> "String_6";
        default -> null;
    };
}

But Sonar gives me this Major error:Unused method parameters should be removed.

as you can see the parameter type is used in the switch. For more details when I use old switch case every thing is OK.

Any idea about the issue, does sonar cover new Java syntaxes?

Hmm, I notice that when I remove default -> null; sonar pass correctly! this is weird.

public static String enumToString(MyEnum type) {
    return switch (type) {
        case Enum1 -> "String_1";
        case Enum2 -> "String_2";
        case Enum3 -> "String_3";
        case Enum4 -> "String_4";
        case Enum5 -> "String_5";
        case Enum6 -> "String_6";
        //default -> null;
    };
}
Jason Aller
  • 3,541
  • 28
  • 38
  • 38
Doesn't Matter
  • 1,061
  • 3
  • 11
  • 29
  • @JohannesKuhn Anyway, there is no case if `type` were `null` – but then I remembered that the switch will cause an NPE to be thrown. – MC Emperor Oct 19 '21 at 14:03
  • In the interest of future readers, you shall mention the Sonar version in use here. Agreed, the reported error looks inappropriate. – Naman Oct 19 '21 at 16:28

2 Answers2

4

This is not a bug, Sonar correctly evaluates that if the listing is exhaustive the switch-expression can never fall into the default branch.

On the other hand, if you decide to not list all possible enum constants, the default branch, however, must be declared. Otherwise, the code would not be compilable, because of the requirement that every enum constant can be matched.

Note: Your code contains a switch expression, not a switch statement.

Nikolas Charalambidis
  • 40,893
  • 16
  • 117
  • 183
  • 3
    But how makes the default clause the parameter "unused"? – Johannes Kuhn Oct 19 '21 at 13:35
  • If you list **all* possible enumerations, the condition can never fall into the `default` branch. Hence such branch is tagged as unused by Sonar (and most IDE's such as IntelliJ Idea do the same). – Nikolas Charalambidis Oct 19 '21 at 13:37
  • 8
    The bug is in the way that Sonar is *describing* the problem. This is a "true positive" with a misleading description. But Java 17 has only been released for a couple of weeks ... so maybe nobody has reported this issue to the SonarTube team. – Stephen C Oct 19 '21 at 13:41
  • @StephenC: [Sonar](https://sonarcloud.io/organizations/default/rules?languages=java&open=java%3AS1172&q=S1172) says: *"Unused parameters are misleading. Whatever the values passed to such parameters, the behavior will be the same."*. I find this description correct. What is misleading? (I ask because I am not an English native so my understanding might be limited.) – Nikolas Charalambidis Oct 19 '21 at 13:45
  • 5
    It is not a parameter. It is a **rule** in a switch expression. That is what is misleading. (This is not an English issue. It is a Java terminology issue.) (The JEP calls them "clauses", but the JLS uses the term "rule". The JLS trumps everything!) – Stephen C Oct 19 '21 at 13:48
  • 1
    I agree with @StephenC at lease the description should be different and more clear to understand the problem – Doesn't Matter Oct 19 '21 at 13:51
  • 1
    Downvoted. Reason - The reported issue is not a compilation issue, neither is an exhaustive list related to the *Unused method parameters should be removed* warning stated by OP. – Naman Oct 19 '21 at 16:31
  • 2
    @Naman: Sorry, but I nowhere stated the reported issue was a compilation issue. – Nikolas Charalambidis Oct 19 '21 at 19:39
  • Can you please rephrase that middle paragraph? I don't quite understand what you're trying to say. – Sotirios Delimanolis Oct 19 '21 at 22:47
  • 1
    @SotiriosDelimanolis I've made my attempt. – Nikolas Charalambidis Oct 19 '21 at 22:55
  • Aha, so if there's isn't a rule for each constant, then we need `default` to capture the remaining possibilities? Otherwise, the whole expression won't compile? – Sotirios Delimanolis Oct 19 '21 at 22:56
  • @SotiriosDelimanolis: Exactly! The compilation error is *java: the switch expression does not cover all possible input values*. – Nikolas Charalambidis Oct 19 '21 at 23:00
  • 1
    I referred to *On the other hand, if you decide to not list all possible enum constants, the default branch, however, must be declared. Otherwise, the code would not be compilable, because of the requirement that every enum constant can be matched.* ... and I meant that OP's former code also aligns to the requirement already. I do second that your suggestion of getting rid of `default` could work, but the way the answer has been phrased it's currently not helpful in my opinion. – Naman Oct 20 '21 at 01:18
  • 1
    There is a behavior compiled into the class file for the case that the enum has a new constant that wasn’t there at compile-time. `default -> null;` changes this behavior from throwing an `IncompatibleClassChangeError` to returning `null`, so it has an impact on the code. – Holger Oct 20 '21 at 12:54
  • @Naman: Would you help to rephrase the paragraph better? As I said, I am not a proficient user of English. – Nikolas Charalambidis Oct 22 '21 at 15:05
  • Stephen's first comment here shall help you with that. – Naman Oct 23 '21 at 04:02
2

Technically the syntax default -> null; is not a "parameter". The JLS refers to various components within a switch block as a "rule", "label", or "expression"; while the relevant JEP also uses the term "clause".

Regardless of what you call its components, Switch Expressions are distinct from the older Switch Statements. In particular, Switch Expressions are exhaustive.

From the JEP,

Exhaustiveness

The cases of a switch expression must be exhaustive; for all possible values there must be a matching switch label. (Obviously switch statements are not required to be exhaustive.)

In practice this normally means that a default clause is required; however, in the case of an enum switch expression that covers all known constants, a default clause is inserted by the compiler to indicate that the enum definition has changed between compile-time and runtime. Relying on this implicit default clause insertion makes for more robust code; now when code is recompiled, the compiler checks that all cases are explicitly handled. Had the developer inserted an explicit default clause (as is the case today) a possible error will have been hidden.

Sonar is smart enough to know when all of the bases are covered, making a default clause not only unreachable, but making it interfere with the behavior described above.

jaco0646
  • 15,303
  • 7
  • 59
  • 83
  • 2
    Downvoted. Reason - An unreachable clause is nowhere related to raising an error stating *"Unused method parameters should be removed"*. – Naman Oct 19 '21 at 16:28