google / google/error-prone

StatementSwitchToExpressionSwitch shows misleading message

Open
#5,066 4 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
7.2k
Forks
820
Avg merge
5h 9m
Merged PRs (30d)
50

Description

Consider the following switch statement:

``` java
switch (mode) {
case "alfa":
return Result.A;
case "bravo":
return Result.B;
case "delta":
return Result.D;
default:
log.warn("Ignoring unknown mode indicator '{}'", mode);
break;
}
```

Error Prone 2.38.0 shows this error:
```
Example.java:[21,12] [StatementSwitchToExpressionSwitch] This statement switch can be converted to an equivalent expression switch
(see https://errorprone.info/bugpattern/StatementSwitchToExpressionSwitch)
Did you mean 'switch (mode) {'?
```

I see several issues with StatementSwitchToExpressionSwitch here:
* The "did you mean" message is not actually suggesting a change.
* The claim that this can be "converted to an equivalent expression switch" is wrong as the default branch has no `return` statement. Maybe in this case, it was the intention that the bug pattern only suggests using arrow case labels to eliminate the potential for unintended fall through?
* Indeed, when changed as below the error message disappears. However, at least in my view that change is not really for the better, as the version above is already safe (due to each branch having either `return` or `break`).
``` java
switch (mode) {
case "alfa" -> {
return Result.A;
}
case "bravo" -> {
return Result.B;
}
case "delta" -> {
return Result.D;
}
default -> log.warn("Ignoring unknown mode indicator '{}'", mode);
}
```

Here's the full example:
``` java
package com.example;

import lombok.extern.slf4j.Slf4j;

@Slf4j
final class Example {
private enum Result {
A,
B,
C,
D,
E
}

public static void main(String[] args) {
log.info("Result: {}", analyze(args[0], args[1]));
}

private static Result analyze(String mode, String extraData) {
if (mode != null && !mode.isEmpty()) {
switch (mode) {
case "alfa":
return Result.A;
case "bravo":
return Result.B;
case "delta":
return Result.D;
default:
log.warn("Ignoring unknown mode indicator '{}'", mode);
break;
}
}

if (extraData.contains("*")) {
return Result.C;
}

if (extraData.length() % 2 == 0) {
return Result.E;
}

return Result.B;
}
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.