Skip to content

False Positive: Suppressions and explicit fall-through explanations are still treated as undocumented fall-through. #5634

@Carlson-JLQ

Description

@Carlson-JLQ

False Positive: Suppressions and explicit fall-through explanations are still treated as undocumented fall-through.

Version
2.48.0

Checker

  • Checker id: FallThrough
  • Checker description: Detects switch cases where execution may unintentionally fall through to the next case without a proper "fall through" comment, or where a "fall through" comment exists but fall-through is impossible.

Description of the false positive

diff/output marks 4 false positives for FallThrough. Across these samples, the checker tends to fire on the outer syntax while missing code-level cues that make the pattern harmless, exempted, or at least much weaker than a real bug. The recurring themes are switch control flow.

Affected test cases

NegCase1.java

Source: output/codeql/BreakInSwitchCase/src/main/java/scensct/core/neg/NegCase1.java

The example is built around a switch whose case-group layout is the whole point of the test. A human reader would likely classify this as an allowed or intentionally structured pattern rather than a bug. That matches the benchmark's stated intent: Switch case with break should not be flagged as unintentional fallthrough.

// Switch case with break should not be flagged as unintentional fallthrough.
package scensct.core.neg;

public class NegCase1 {
    public static void main(String[] args) {
        int x = 1;
        switch (x) {
            case 1:
                System.out.println("Case 1");
                break; // Ends with break, preventing fallthrough
            case 2:
                System.out.println("Case 2");
                break;
        }
    }
}

NegCase1.java

Source: output/errorprone/FallThrough/src/main/java/scensct/core/neg/NegCase1.java

The example is built around a switch whose case-group layout is the whole point of the test. A human reader would likely classify this as an allowed or intentionally structured pattern rather than a bug. That matches the benchmark's stated intent: Switch case ending with break should not be flagged as missing fall-through comment.

// Switch case ending with break should not be flagged as missing fall-through comment.
package scensct.core.neg;

public class NegCase1 {
    public static void main(String[] args) {
        int x = 1;
        switch (x) {
            case 1:
                System.out.println("one");
                break; // Ends with break, cannot fall through
            case 2: // [REPORTED LINE]
                ; // trivial statement to avoid checker false positive
                System.out.println("two");
                break;
        }
    }
}

NegCase1.java

Source: output/pmd/ImplicitSwitchFallThrough/src/main/java/scensct/core/neg/NegCase1.java

The example is built around a switch whose control-flow shape is slightly unusual because comments, breaks, or default handling change how a human would read the fall-through. A human reader would likely classify this as an allowed or intentionally structured pattern rather than a bug. That matches the benchmark's stated intent: A switch branch that is not a fall-through branch should not be flagged as implicit fall-through.

// A switch branch that is not a fall-through branch should not be flagged as implicit fall-through.
package scensct.core.neg;

public class NegCase1 {
    public static void main(String[] args) {
        int x = 1;
        switch (x) {
            case 1:
                System.out.println("Case 1");
                break; // Not a fall-through branch due to break
            case 2:
                System.out.println("Case 2");
                return; // Not a fall-through branch due to return
            case 3:
                System.out.println("Case 3");
                throw new RuntimeException(); // Not a fall-through branch due to throw
            case 4:
                throw new RuntimeException(); // Prevents fall-through without infinite loop
            default:
                System.out.println("Default");
        }
    }
}

NegCase3.java

Source: output/pmd/ImplicitSwitchFallThrough/src/main/java/scensct/core/neg/NegCase3.java

The example is built around a switch whose control-flow shape is slightly unusual because comments, breaks, or default handling change how a human would read the fall-through. The report hinges on comment placement more than on a real fall-through bug. That matches the benchmark's stated intent: A fall-through switch branch that dataflow analysis determines cannot fall through to the next branch should not be flagged as implicit fall-through.

// A fall-through switch branch that dataflow analysis determines cannot fall through to the next branch should not be flagged as implicit fall-through.
package scensct.core.neg;

public class NegCase3 {
    public static void main(String[] args) {
        int x = 1;
        switch (x) {
            case 1:
                System.out.println("Case 1");
                return; // Direct return prevents fall-through
            case 2:
                System.out.println("Case 2");
                break;
            default:
                break;
        }
    }
}

Cause analysis

The rule is documented as follows: Detects switch cases where execution may unintentionally fall through to the next case without a proper "fall through" comment, or where a "fall through" comment exists but fall-through is impossible.

In this set the problem shows up around switch control flow. The implementation appears to lean too heavily on surface indicators and not enough on what the code actually does.

That is why these reports read like over-approximation. The code often retains a suspicious signature or control-flow skeleton, but once you read the body, the benchmark's claimed exemption is visible in the source itself.

For FallThrough, the practical improvement would be to model those source-level exemptions more explicitly instead of reporting every syntactic near-match.

References

None known.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions