Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SONARJAVA-4441 Fix FP on S1301 when mutiple case labels are used in Java 14 switch. #4970

Merged
merged 4 commits into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
class A {
package checks.tests;

static void recordSwitch1(MyRecord object) {
public class SwitchAtLeastThreeCasesCheckSample {
record MyRecord(int x, int y) {}

static void recordSwitch1(Object object) {
switch (object) { // Compliant
case String x when x.length() > 42 -> { }
case Integer i -> { }
default -> { }
}
}

static void recordSwitch1(Object object) {
static void recordSwitch2(Object object) {
switch (object) { // Compliant
case MyRecord(int x, int y) -> { }
case String s -> { }
default -> { }
}
}

Expand All @@ -19,22 +22,25 @@ sealed interface Shape permits Box, Circle {}
record Box() implements Shape { }
record Circle() implements Shape {}

void foo(Shape shape) {
default void foo(Shape shape) {
switch (shape) { // Compliant because of type pattern matching
case Box ignored -> { }
case Circle ignored -> System.out.println();
}
}

void goo(Shape shape) {
default void goo(Shape shape) {
switch (shape) { // Compliant because of type pattern matching
default -> System.out.println();
case Box ignored -> { }
default -> System.out.println();
}
}
}

public void f() {
private static void doSomething() {}
private static void doSomethingElse() {}

public void f(int variable) {
switch (variable) { // Noncompliant {{Replace this "switch" statement by "if" statements to increase readability.}}
// ^^^^^^
case 0:
Expand All @@ -55,6 +61,15 @@ public void f() {
break;
}

switch (variable) {
case 0, 1:
doSomething();
break;
default:
doSomethingElse();
break;
}

switch (variable) { // Noncompliant
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.sonar.check.Rule;
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
import org.sonar.plugins.java.api.tree.CaseGroupTree;
import org.sonar.plugins.java.api.tree.CaseLabelTree;
import org.sonar.plugins.java.api.tree.SwitchStatementTree;
import org.sonar.plugins.java.api.tree.Tree;

Expand All @@ -42,13 +43,32 @@ public void visitNode(Tree tree) {
if (hasLabelWithAllowedPattern(caseGroup)) {
return;
}
count += caseGroup.labels().size();
count += totalLabelCount(caseGroup);
}
if (count < 3) {
reportIssue(switchStatementTree.switchKeyword(), "Replace this \"switch\" statement by \"if\" statements to increase readability.");
}
}

/**
* Count labels, taking into account Java 14 multi-label switch.
* For example, here we have 4 labels:
* <pre>
* case "Monday", "Tuesday":
* case "Wednesday:
tomasz-tylenda-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
* default: // considered 1 label
* </pre>
*/
private static int totalLabelCount(CaseGroupTree caseGroup) {
int total = 0;
for (CaseLabelTree label: caseGroup.labels()) {
int sz = label.expressions().size();
// `default` does not have any expressions, but we consider it 1 label.
total += sz > 0 ? sz : 1;
}
return total;
}

private static boolean hasLabelWithAllowedPattern(CaseGroupTree caseGroupTree) {
return caseGroupTree.labels().stream()
.flatMap(label -> label.expressions().stream())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@
import org.junit.jupiter.api.Test;
import org.sonar.java.checks.verifier.CheckVerifier;

import static org.sonar.java.checks.verifier.TestUtils.testCodeSourcesPath;

class SwitchAtLeastThreeCasesCheckTest {

@Test
void test() {
CheckVerifier.newVerifier()
.onFile("src/test/files/checks/SwitchAtLeastThreeCasesCheck.java")
.onFile(testCodeSourcesPath("checks/tests/SwitchAtLeastThreeCasesCheckSample.java"))
.withCheck(new SwitchAtLeastThreeCasesCheck())
.verifyIssues();
}
Expand Down
Loading