-
Notifications
You must be signed in to change notification settings - Fork 725
RIS-496 Implement S8795: Code after unconditional control flow statements should be removed #5683
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,177 @@ | ||
| package checks; | ||
|
|
||
| class UnreachableCodeCheckSample { | ||
|
|
||
| int codeAfterReturn(int a) { | ||
| int i = 10; | ||
| return i + a; | ||
| i++; // Noncompliant {{Remove this unreachable code.}} | ||
| } | ||
|
|
||
| int multipleStatementsAfterReturn() { | ||
| return 1; | ||
| int x = 2; // Noncompliant | ||
| int y = 3; | ||
| } | ||
|
|
||
| void voidReturn() { | ||
| System.out.println("Start"); | ||
| return; | ||
| System.out.println("End"); // Noncompliant | ||
| } | ||
|
|
||
| void codeAfterThrow(String input) { | ||
| if (input == null) { | ||
| throw new IllegalArgumentException(); | ||
| System.out.println("Valid"); // Noncompliant | ||
| } | ||
| } | ||
|
|
||
| void throwInMethod() { | ||
| throw new RuntimeException(); | ||
| System.out.println("After throw"); // Noncompliant | ||
| } | ||
|
|
||
| void breakInLoop() { | ||
| while (true) { | ||
| System.out.println("Loop"); | ||
| break; | ||
| System.out.println("After break"); // Noncompliant | ||
| } | ||
| } | ||
|
|
||
| void breakInSwitch(int val) { | ||
| switch (val) { | ||
| case 1: | ||
| System.out.println("Case 1"); | ||
| break; | ||
| System.out.println("After break"); // Noncompliant | ||
| case 2: | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| void labeledBreak() { | ||
| outer: for (int i = 0; i < 10; i++) { | ||
| break outer; | ||
| System.out.println(i); // Noncompliant | ||
| } | ||
| } | ||
|
|
||
| void codeAfterContinue() { | ||
| for (int i = 0; i < 10; i++) { | ||
| if (i % 2 == 0) { | ||
| continue; | ||
| System.out.println(i); // Noncompliant | ||
| } | ||
| } | ||
| } | ||
|
|
||
| void labeledContinue() { | ||
| outer: for (int i = 0; i < 10; i++) { | ||
| for (int j = 0; j < 10; j++) { | ||
| continue outer; | ||
| System.out.println(j); // Noncompliant | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public UnreachableCodeCheckSample(boolean flag) { | ||
| if (!flag) { | ||
| return; | ||
| System.out.println("Init"); // Noncompliant | ||
| } | ||
| } | ||
|
|
||
| int lastStatement() { | ||
| int x = 10; | ||
| return x; | ||
| } | ||
|
|
||
| int conditionalReturn(boolean flag) { | ||
| if (flag) { | ||
| return 1; | ||
| } | ||
| return 0; | ||
| } | ||
|
|
||
| void breakInConditional() { | ||
| for (int i = 0; i < 10; i++) { | ||
| if (i == 5) { | ||
| break; | ||
| } | ||
| System.out.println(i); | ||
| } | ||
| } | ||
|
|
||
| void continueInConditional() { | ||
| for (int i = 0; i < 10; i++) { | ||
| if (i % 2 == 0) { | ||
| continue; | ||
| } | ||
| System.out.println(i); | ||
| } | ||
| } | ||
|
|
||
| void emptyAfterReturn() { | ||
| return; | ||
| } | ||
|
|
||
| void nestedBlocks() { | ||
| { | ||
| System.out.println("Block 1"); | ||
| } | ||
| System.out.println("Block 2"); | ||
| } | ||
|
|
||
| void switchCases(int val) { | ||
| switch (val) { | ||
| case 1: | ||
| System.out.println("Case 1"); | ||
| break; | ||
| case 2: | ||
| System.out.println("Case 2"); | ||
| break; | ||
| default: | ||
| System.out.println("Default"); | ||
| } | ||
| } | ||
|
|
||
| void tryCatch() { | ||
| try { | ||
| System.out.println("Try"); | ||
| return; | ||
| } catch (Exception e) { | ||
| System.out.println("Catch"); | ||
| } finally { | ||
| System.out.println("Finally"); | ||
| } | ||
| } | ||
|
|
||
| int ifElseReturns(int x) { | ||
| if (x > 0) { | ||
| return 1; | ||
| } else { | ||
| return -1; | ||
| } | ||
| } | ||
|
|
||
| void forLoop() { | ||
| for (int i = 0; i < 10; i++) { | ||
| if (i == 5) { | ||
| continue; | ||
| } | ||
| System.out.println(i); | ||
| } | ||
| } | ||
|
|
||
| void whileLoop(boolean cond) { | ||
| while (cond) { | ||
| System.out.println("Loop"); | ||
| if (cond) { | ||
| break; | ||
| } | ||
| System.out.println("After check"); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| /* | ||
| * 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.Arrays; | ||
| import java.util.List; | ||
| import org.sonar.check.Rule; | ||
| import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; | ||
| import org.sonar.plugins.java.api.tree.BlockTree; | ||
| import org.sonar.plugins.java.api.tree.CaseGroupTree; | ||
| import org.sonar.plugins.java.api.tree.StatementTree; | ||
| import org.sonar.plugins.java.api.tree.Tree; | ||
|
|
||
| @Rule(key = "S8795") | ||
| public class UnreachableCodeCheck extends IssuableSubscriptionVisitor { | ||
|
|
||
| @Override | ||
| public List<Tree.Kind> nodesToVisit() { | ||
| return Arrays.asList(Tree.Kind.BLOCK, Tree.Kind.CASE_GROUP); | ||
| } | ||
|
|
||
| @Override | ||
| public void visitNode(Tree tree) { | ||
| List<StatementTree> statements; | ||
| if (tree.is(Tree.Kind.BLOCK)) { | ||
| statements = ((BlockTree) tree).body(); | ||
| } else { | ||
| statements = ((CaseGroupTree) tree).body(); | ||
| } | ||
|
|
||
| for (int i = 0; i < statements.size() - 1; i++) { | ||
| StatementTree statement = statements.get(i); | ||
| if (isUnconditionalJump(statement)) { | ||
| StatementTree unreachableStatement = statements.get(i + 1); | ||
| reportIssue(unreachableStatement, "Remove this unreachable code."); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static boolean isUnconditionalJump(StatementTree statement) { | ||
| return statement.is( | ||
| Tree.Kind.RETURN_STATEMENT, | ||
| Tree.Kind.THROW_STATEMENT, | ||
| Tree.Kind.BREAK_STATEMENT, | ||
|
Comment on lines
+45
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| Tree.Kind.CONTINUE_STATEMENT | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| /* | ||
| * 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 UnreachableCodeCheckTest { | ||
|
|
||
| @Test | ||
| void test() { | ||
| CheckVerifier.newVerifier() | ||
| .onFile(mainCodeSourcesPath("checks/UnreachableCodeCheckSample.java")) | ||
| .withCheck(new UnreachableCodeCheck()) | ||
| .verifyIssues(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| <h2>Why is this an issue?</h2> | ||
| <p>Unreachable code is a sign of a logical error in your program's control flow. When a statement appears after an unconditional jump (like return statements or exception throws), it can never be executed because the jump statement always transfers control away from that point.</p> | ||
| <p>This creates several problems:</p> | ||
| <ul> | ||
| <li><strong>Misleading intent</strong>: Developers reading the code may think the unreachable statements have some effect, leading to incorrect assumptions about program behavior.</li> | ||
| <li><strong>Wasted effort</strong>: Time spent understanding, debugging, or modifying unreachable code is entirely wasted.</li> | ||
| <li><strong>Hidden bugs</strong>: The unreachable code might have been intended to execute, indicating a logic error where the jump statement was placed incorrectly or should have been conditional.</li> | ||
| <li><strong>Code bloat</strong>: Unreachable code increases the codebase size without providing any value.</li> | ||
| </ul> | ||
| <p>While compilers often warn about unreachable code, such warnings may be overlooked or disabled. Static analysis helps catch these issues early in the development process.</p> | ||
| <p>Common scenarios where unreachable code appears:</p> | ||
| <ul> | ||
| <li>Statements after a return that were meant to execute before returning</li> | ||
| <li>Code after an exception throw that was intended for cleanup (which should instead go in a cleanup or finalization block)</li> | ||
| <li>Statements after loop control transfers that were meant to be outside the conditional block</li> | ||
| <li>Dead code left over from refactoring that was never removed</li> | ||
| </ul> | ||
| <h2>How to fix it</h2> | ||
| <p>Remove the unreachable code if it serves no purpose. If the statements were intended to execute, restructure the control flow to ensure they run before the jump statement.</p> | ||
| <h3>Code examples</h3> | ||
| <h4>Noncompliant code example</h4> | ||
| <pre data-diff-id="1" data-diff-type="noncompliant"> | ||
| public int calculateValue(int a) { | ||
| int i = 10; | ||
| return i + a; | ||
| i++; // Noncompliant | ||
| } | ||
| </pre> | ||
| <h4>Compliant solution</h4> | ||
| <pre data-diff-id="1" data-diff-type="compliant"> | ||
| public int calculateValue(int a) { | ||
| int i = 10; | ||
| i++; | ||
| return i + a; | ||
| } | ||
| </pre> | ||
| <h2>Resources</h2> | ||
| <h3>Documentation</h3> | ||
| <ul> | ||
| <li><a href="https://docs.oracle.com/javase/tutorial/java/nutsandbolts/flow.html">Oracle Java Documentation - Control Flow Statements</a></li> | ||
| </ul> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| { | ||
| "title": "Code after unconditional control flow statements should be removed", | ||
| "type": "CODE_SMELL", | ||
| "code": { | ||
| "impacts": { | ||
| "MAINTAINABILITY": "HIGH" | ||
| }, | ||
| "attribute": "LOGICAL" | ||
| }, | ||
| "status": "ready", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Quality: S8795 not added to Sonar_way profileThe rule metadata marks Was this helpful? React with 👍 / 👎 |
||
| "remediation": { | ||
| "func": "Constant/Issue", | ||
| "constantCost": "5min" | ||
| }, | ||
| "tags": [ | ||
| "confusing", | ||
| "unused" | ||
| ], | ||
| "defaultSeverity": "Major", | ||
| "ruleSpecification": "RSPEC-8795", | ||
| "sqKey": "S8795", | ||
| "scope": "All", | ||
| "quickfix": "unknown" | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨 Bug: Test sample file fails javac compilation, breaking the build
In Java, a statement that is unreachable is a compile-time error (JLS §14.21 "Unreachable Statements"), not just a warning. Every
// Noncompliantline inUnreachableCodeCheckSample.javaplaces a statement after an unconditionalreturn/throw/break/continuein the same block, whichjavacrejects.I verified this by compiling the file directly:
The
java-checks-test-sources/defaultmodule is built withmaven-compiler-plugin(<release>25</release>, nofailOnError=false/maven.main.skip), so this module is compiled during the build. Because the sample does not compile, the module build — and therefore CI — will fail. TheUnreachableCodeCheckTestwould never even reach a green state.Unlike most SonarJava sample files, which contain semantically questionable but compilable code, genuine unreachable code cannot be made to compile. The sample must be restructured so the analyzed file compiles (e.g., make the jump conditional via an opaque condition the compiler can't fold, or move the sample out of a javac-compiled module if such a mechanism exists), otherwise the rule cannot be tested via CheckVerifier in this module.
Was this helpful? React with 👍 / 👎