Make RSPEC-1751 detect if code block

  • versions used

    • sonarqube-9.2.2.50622
    • sonar-scanner-cli-4.6.2.2472-linux
  • RSPEC-1751 Rule Link

  • error observed
    Hi, I found a false negative, please refer to the following case. I think SonarQube should have reported a warning at line 6 or line 8 because these break statements can stop the loop in the first time execution. However, no warnings. Thanks for you consideration.

public void foo10() {
    for (int i = 0; i < 10; i++) { 
        String s = String.format("i is %d", i);
        if(true) {
            System.out.println(s);
            break; // should report a warning here, but no warnings
        } else {
            break;
        }
    }
}
  • steps to reproduce
    Directly use SonarQube detect the above minimized case and check the issue report.

  • potential workaround
    Make analysis module of this rule perform more analysis in if statement embedded in identical code block.

Hello @Belle

Since the condition of the if is always true, the else part will never be executed, we will therefore not take it into consideration as it is dead code. We introduced this behavior in order to avoid false positives.

If you change the condition to something not constant, the issue will be reported.

Hope it clarifies the situation.
Best,
Quentin

@Quentin Hi Quentin, thanks for your kind and prompt reply. However, it seems a little misunderstanding here. Please refer to the following two code samples. Thanks for your consideration.

First Sample: Two branches are possible to execute, but sitll no warnings.

public void foo(boolean tag) {
    for (int i = 0; i < 10; i++) { 
        String s = String.format("i is %d", i);
        if(tag) {
            System.out.println(s);
            break;  // should report a warning here, because the loop could be terminated by the break here
        } else {
            break;  // should report a warning here
        }
    }
}

Second Sample: True branch definitely can be executed, but still no warnings.

    public void foo() {
        for (int i = 0; i < 10; i++) { 
            String s = String.format("i is %d", i);
            if(true) {
                break; // should report a warning here
            }
        }
    }
1 Like

Indeed, I misread your initial message, for some reason I thought you were speaking about S135 (two break in a loop), and since your code sample does behave strangely for S135 aswell, I did not realize it, sorry for that.

Now it makes sense, I agree that we should report an issue on this code. Ticket created: SONARJAVA-4110.

Thanks,
Quentin

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.