RIS-496 Implement S8795: Code after unconditional control flow statements should be removed#5683
RIS-496 Implement S8795: Code after unconditional control flow statements should be removed#5683romainbrenguier wants to merge 1 commit into
Conversation
…uld be removed Detects unreachable code after return, throw, break, and continue statements. Related to RIS-493
hashicorp-vault-sonar-prod
Bot
changed the title
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| int codeAfterReturn(int a) { | ||
| int i = 10; | ||
| return i + a; | ||
| i++; // Noncompliant {{Remove this unreachable code.}} |
There was a problem hiding this comment.
🚨 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 // Noncompliant line in UnreachableCodeCheckSample.java places a statement after an unconditional return/throw/break/continue in the same block, which javac rejects.
I verified this by compiling the file directly:
UnreachableCodeCheckSample.java:8: error: unreachable statement
i++; // Noncompliant {{Remove this unreachable code.}}
UnreachableCodeCheckSample.java:13: error: unreachable statement
...
The java-checks-test-sources/default module is built with maven-compiler-plugin (<release>25</release>, no failOnError=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. The UnreachableCodeCheckTest would 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 👍 / 👎
| 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, |
There was a problem hiding this comment.
⚠️ Quality: Rule duplicates a javac compile error and can't fire on valid code
The rule flags statements immediately following return/throw/break/continue within the same BLOCK/CASE_GROUP. In Java this exact situation is already a hard compile-time error (JLS §14.21), so it can never occur in code that compiles. As a result the rule will essentially never raise an issue on real, compilable production code — it only triggers on source that the Java compiler itself already rejects.
This questions the value/scope of the rule as implemented. If the intent (per the RSpec and HTML description) is to catch code that is logically dead due to control flow within branches, the detection likely needs dataflow/CFG-based reachability analysis (e.g. reuse of the existing CFG in sonar-java) rather than a purely syntactic "next sibling statement" check. Recommend confirming with the RSpec whether the syntactic scope is intended, and aligning the implementation (and the test sample) accordingly.
Was this helpful? React with 👍 / 👎
| }, | ||
| "attribute": "LOGICAL" | ||
| }, | ||
| "status": "ready", |
There was a problem hiding this comment.
💡 Quality: S8795 not added to Sonar_way profile
The rule metadata marks "status": "ready", but S8795 is not present in Sonar_way_profile.json. If the rule is intended to be active by default, it must be added to the ruleKeys list of the Sonar way profile; otherwise it will ship deactivated. Confirm whether default activation is intended and update the profile accordingly (there is typically a profile-completeness test that enforces this).
Was this helpful? React with 👍 / 👎
CI failed: The build is failing due to 'unreachable statement' compilation errors in the newly added 'UnreachableCodeCheckSample.java' file. This occurs because the Java compiler detects the intentionally unreachable code intended for testing as actual errors.OverviewThe build failed because the new test source file FailuresCompilation Failure: Unreachable Statements (confidence: high)
Summary
Code Review 🚫 Blocked 0 resolved / 3 findingsImplements rule S8795 for detecting unreachable code, but currently fails build due to compilation errors in test samples and targets code that javac already rejects. Additionally, the rule is missing from the Sonar way profile. 🚨 Bug: Test sample file fails javac compilation, breaking the build📄 java-checks-test-sources/default/src/main/java/checks/UnreachableCodeCheckSample.java:8 📄 java-checks-test-sources/default/src/main/java/checks/UnreachableCodeCheckSample.java:13 📄 java-checks-test-sources/default/src/main/java/checks/UnreachableCodeCheckSample.java:20 📄 java-checks-test-sources/default/src/main/java/checks/UnreachableCodeCheckSample.java:26 📄 java-checks-test-sources/default/src/main/java/checks/UnreachableCodeCheckSample.java:32 📄 java-checks-test-sources/default/src/main/java/checks/UnreachableCodeCheckSample.java:39 📄 java-checks-test-sources/default/src/main/java/checks/UnreachableCodeCheckSample.java:48 📄 java-checks-test-sources/default/src/main/java/checks/UnreachableCodeCheckSample.java:57 📄 java-checks-test-sources/default/src/main/java/checks/UnreachableCodeCheckSample.java:65 📄 java-checks-test-sources/default/src/main/java/checks/UnreachableCodeCheckSample.java:74 📄 java-checks-test-sources/default/src/main/java/checks/UnreachableCodeCheckSample.java:82 In Java, a statement that is unreachable is a compile-time error (JLS §14.21 "Unreachable Statements"), not just a warning. Every I verified this by compiling the file directly: The 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.
|
| Auto-apply | Compact | Unblock |
|
|
|
Was this helpful? React with 👍 / 👎 | Gitar
There was a problem hiding this comment.
Comment gitar unblock to override this block and allow merging.
Configure merge blocking · Maintainers can dismiss this review. Gitar never approves changes.
1 participant
Implements detection of unreachable code after unconditional control flow statements (return, throw, break, continue).
Changes
Related
Generated using vibe-bot new-rule workflow