diff --git a/java-checks-test-sources/default/src/main/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheckSample.java new file mode 100644 index 00000000000..bf7a2d2a00d --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheckSample.java @@ -0,0 +1,176 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package checks; + +class LocalVariablesShouldNotSpanSwitchCaseGroupsCheckSample { + + void statementWithSeveralLaterGroups(int selector) { + switch (selector) { + case 0: + case 1: + int value = 0; // Noncompliant {{Declare a separate variable in each case group; sharing this local variable across groups obscures its scope.}} [[sc=13;ec=18;flows=statementCase2,statementDefault]] + consume(value); + break; + case 2: + value = 2; // flow@statementCase2 [[sc=9;ec=14]] {{Accessed from this later case group.}} + value++; + consume(value); + break; + default: + value = 3; // flow@statementDefault [[sc=9;ec=14]] {{Accessed from this later case group.}} + consume(value); + } + } + + int switchExpression(int selector) { + return switch (selector) { + case 0: + int priority = 1; // Noncompliant {{Declare a separate variable in each case group; sharing this local variable across groups obscures its scope.}} [[sc=13;ec=21;secondary=+3]] + yield priority; + case 1: + priority = 2; + yield priority; + default: + yield 0; + }; + } + + void multipleDeclarations(int selector) { + switch (selector) { + case 0: + int first = 0; // Noncompliant {{Declare a separate variable in each case group; sharing this local variable across groups obscures its scope.}} [[sc=13;ec=18;secondary=+5]] + int second = 0; // Noncompliant {{Declare a separate variable in each case group; sharing this local variable across groups obscures its scope.}} [[sc=13;ec=19;secondary=+5]] + break; + default: + first = 1; + second = 2; + } + } + + void accessInsideNestedBlock(int selector) { + switch (selector) { + case 0: + int value = 0; // Noncompliant {{Declare a separate variable in each case group; sharing this local variable across groups obscures its scope.}} [[sc=13;ec=18;secondary=+4]] + break; + default: { + value = 1; + break; + } + } + } + + void nestedDeclarationsAreExcluded(int selector) { + switch (selector) { + case 0: { + int nested = 0; + consume(nested); + break; + } + default: + break; + } + switch (selector) { + case 0: + if (selector == 0) { + int nested = 0; + consume(nested); + } + break; + default: + break; + } + } + + void consecutiveLabelsAreOneGroup(int selector) { + switch (selector) { + case 0: + case 1: + int value = 0; + consume(value); + break; + default: + break; + } + } + + void arrowRulesAreExcluded(int selector) { + switch (selector) { + case 0 -> { + int value = 0; + consume(value); + } + default -> { + int value = 1; + consume(value); + } + } + } + + void symbolIdentityIsUsed(int selector) { + switch (selector) { + case 0: + int value = 0; + consume(value); + break; + default: + class Local { + int value; + + void increment() { + value++; + } + } + new Local().increment(); + } + } + + void nestedSwitchesAreIndependent(int selector) { + switch (selector) { + case 0: + int outer = 0; // Noncompliant {{Declare a separate variable in each case group; sharing this local variable across groups obscures its scope.}} [[sc=13;ec=18;secondary=+5]] + break; + default: + switch (selector + 1) { + case 1: + outer = 1; + int nested = 0; // Noncompliant {{Declare a separate variable in each case group; sharing this local variable across groups obscures its scope.}} [[sc=17;ec=23;secondary=+3]] + break; + default: + nested = 1; + } + } + } + + void safeDeclarations(int selector) { + switch (selector) { + case 0: + int usedOnlyHere = 0; + consume(usedOnlyHere); + break; + case 1: + int unused = 0; + break; + default: + int declaredInLastGroup = 0; + consume(declaredInLastGroup); + } + } + + private static void consume(int value) { + // Nothing to do. + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheck.java b/java-checks/src/main/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheck.java new file mode 100644 index 00000000000..7896e744823 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheck.java @@ -0,0 +1,109 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import org.sonar.check.Rule; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.tree.BaseTreeVisitor; +import org.sonar.plugins.java.api.tree.CaseGroupTree; +import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.StatementTree; +import org.sonar.plugins.java.api.tree.SwitchTree; +import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.VariableTree; + +@Rule(key = "S9364") +public class LocalVariablesShouldNotSpanSwitchCaseGroupsCheck extends IssuableSubscriptionVisitor { + + static final String PRIMARY_MESSAGE = "Declare a separate variable in each case group; sharing this local variable across groups obscures its scope."; + static final String SECONDARY_MESSAGE = "Accessed from this later case group."; + + @Override + public List nodesToVisit() { + return Arrays.asList(Tree.Kind.SWITCH_STATEMENT, Tree.Kind.SWITCH_EXPRESSION); + } + + @Override + public void visitNode(Tree tree) { + if (context.getSemanticModel() == null) { + return; + } + + List caseGroups = ((SwitchTree) tree).cases(); + for (int index = 0; index < caseGroups.size(); index++) { + CaseGroupTree caseGroup = caseGroups.get(index); + if (!caseGroup.labels().get(0).isFallThrough()) { + continue; + } + for (StatementTree statement : caseGroup.body()) { + if (statement instanceof VariableTree variable) { + reportIfAccessedFromLaterGroup(variable, caseGroups, index + 1); + } + } + } + } + + private void reportIfAccessedFromLaterGroup(VariableTree variable, List caseGroups, int firstLaterGroup) { + Symbol symbol = variable.symbol(); + if (symbol.isUnknown()) { + return; + } + + List secondaries = new ArrayList<>(); + for (int index = firstLaterGroup; index < caseGroups.size(); index++) { + IdentifierTree firstAccess = firstAccessTo(symbol, caseGroups.get(index)); + if (firstAccess != null) { + secondaries.add(new JavaFileScannerContext.Location(SECONDARY_MESSAGE, firstAccess)); + } + } + if (!secondaries.isEmpty()) { + reportIssue(variable.simpleName(), PRIMARY_MESSAGE, secondaries, null); + } + } + + private static IdentifierTree firstAccessTo(Symbol symbol, CaseGroupTree caseGroup) { + FirstSymbolAccessVisitor visitor = new FirstSymbolAccessVisitor(symbol); + visitor.scanBody(caseGroup.body()); + return visitor.firstAccess; + } + + private static class FirstSymbolAccessVisitor extends BaseTreeVisitor { + + private final Symbol symbol; + private IdentifierTree firstAccess; + + private FirstSymbolAccessVisitor(Symbol symbol) { + this.symbol = symbol; + } + + private void scanBody(List trees) { + scan(trees); + } + + @Override + public void visitIdentifier(IdentifierTree tree) { + if (firstAccess == null && tree.symbol().equals(symbol)) { + firstAccess = tree; + } + } + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheckTest.java new file mode 100644 index 00000000000..e0cdbce2fe5 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheckTest.java @@ -0,0 +1,46 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; + +class LocalVariablesShouldNotSpanSwitchCaseGroupsCheckTest { + + private static final String SAMPLE = "checks/LocalVariablesShouldNotSpanSwitchCaseGroupsCheckSample.java"; + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath(SAMPLE)) + .withCheck(new LocalVariablesShouldNotSpanSwitchCaseGroupsCheck()) + .withJavaVersion(21) + .verifyIssues(); + } + + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath(SAMPLE)) + .withCheck(new LocalVariablesShouldNotSpanSwitchCaseGroupsCheck()) + .withJavaVersion(21) + .withoutSemantic() + .verifyNoIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9364.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9364.html new file mode 100644 index 00000000000..16f0b30f3c1 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9364.html @@ -0,0 +1,98 @@ +

In a colon-style switch, case labels do not introduce separate lexical scopes. A local variable declared directly in one case group +remains in scope in later case groups.

+

Why is this an issue?

+

Case groups look like separate branches, so readers commonly expect variables declared in one group to belong only to that group. When a later +group accesses such a variable, its actual scope no longer matches the visual structure of the code. Understanding the variable then requires tracing +its declaration and every later case group, which makes the switch harder to read and maintain.

+

Java’s definite-assignment rules prevent reading a local variable before it has been assigned. This rule does not identify uninitialized reads. It +reports variables that are assigned or otherwise accessed across distinct case groups because sharing them obscures their scope.

+

Consecutive case labels form one case group and do not trigger an issue by themselves. Declarations inside their own block are also excluded +because the block limits the variable’s scope.

+

How to fix it

+

Give each case group its own variable. For colon-style switches, enclose each group’s statements in a block. Alternatively, use arrow-style rules, +which naturally isolate the code for each branch.

+

Code examples

+

Noncompliant code example

+
+void logState(State state) {
+  switch (state) {
+    case NEW:
+    case QUEUED:
+      String message = "Waiting"; // Noncompliant
+      log(message);
+      break;
+    case RETRY:
+      message = "Retrying"; // Accessed from this later case group.
+      log(message);
+      break;
+    case FAILED:
+      message = "Failed"; // Accessed from this later case group.
+      log(message);
+      break;
+  }
+}
+
+

Compliant solution

+
+void logState(State state) {
+  switch (state) {
+    case NEW:
+    case QUEUED: {
+      String message = "Waiting";
+      log(message);
+      break;
+    }
+    case RETRY: {
+      String message = "Retrying";
+      log(message);
+      break;
+    }
+    case FAILED: {
+      String message = "Failed";
+      log(message);
+      break;
+    }
+  }
+}
+
+

Noncompliant code example

+
+int priority(State state) {
+  return switch (state) {
+    case NEW:
+      int priority = 1; // Noncompliant
+      yield priority;
+    case RETRY:
+      priority = 2; // Accessed from this later case group.
+      yield priority;
+    default:
+      yield 0;
+  };
+}
+
+

Compliant solution

+
+int priority(State state) {
+  return switch (state) {
+    case NEW -> {
+      int priority = 1;
+      yield priority;
+    }
+    case RETRY -> {
+      int priority = 2;
+      yield priority;
+    }
+    default -> 0;
+  };
+}
+
+

Resources

+

Documentation

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9364.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9364.json new file mode 100644 index 00000000000..5cbcf1ffae0 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9364.json @@ -0,0 +1,21 @@ +{ + "title": "Local variables should not span switch case groups", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "10min" + }, + "tags": [], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-9364", + "sqKey": "S9364", + "scope": "Main", + "quickfix": "infeasible", + "code": { + "impacts": { + "MAINTAINABILITY": "MEDIUM" + }, + "attribute": "CLEAR" + } +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9364 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9364 new file mode 100644 index 00000000000..e69de29bb2d