Skip to content
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

[apex] False positive with IfElseStmtsMustUseBraces #3711

Open
rsoffiatto opened this issue Dec 29, 2021 · 1 comment
Open

[apex] False positive with IfElseStmtsMustUseBraces #3711

rsoffiatto opened this issue Dec 29, 2021 · 1 comment
Labels
a:false-positive PMD flags a piece of code that is not problematic

Comments

@rsoffiatto
Copy link

rsoffiatto commented Dec 29, 2021

Affects PMD Version:
6.41.0+
7.0.0

Rule: IfElseStmtsMustUseBraces
https://pmd.github.io/pmd-6.41.0/pmd_rules_apex_codestyle.html#ifelsestmtsmustusebraces

Description:
PMD provides some rules for finding blocks without braces for Apex. Talking about ifs, it provides different rules for ifs with and without elses. But the rule made to only match if...elses are catching ifs without elses. Even the provided example for the rule states that it will not match an if without an else, but that is not what is happening now.

The problem is on the xpath used for catching the violation. If we use the provided one:

//IfBlockStatement/BlockStatement[@CurlyBrace= false()][count(child::*) > 0]
|
//IfElseBlockStatement/BlockStatement[@CurlyBrace= false()][count(child::*) > 0]

There is not an enforcement that the else must be present. We need something like:

//IfElseBlockStatement[@ElseStatement=true()]/IfBlockStatement/BlockStatement[@CurlyBrace= false()][count(child::*) > 0]
|
//IfElseBlockStatement[@ElseStatement=true()]/BlockStatement[@CurlyBrace= false()][count(child::*) > 0]

Where we make sure that the there is an else statement present.

Code Sample demonstrating the issue:
The example provided with the rule can be used to demonstrate the problem:

// this is OK
if (foo) x++;

// but this is not
if (foo)
    x = x+1;
else
    x = x-1;

Expected outcome:
PMD reports violations on lines 2, 6 and 8. But the violation on line 2 is wrong. Only the lines 6 and 8 should be reported

Running PMD through: CLI

@rsoffiatto rsoffiatto added the a:false-positive PMD flags a piece of code that is not problematic label Dec 29, 2021
@oowekyala
Copy link
Member

Since you already have a fix, you could submit a PR. We'll gladly help if you have trouble

Side note: in the future, it would be nice to have an Apex rule similar to the Java ControlStatementBraces, which was added to merge all rules about braces and avoid this kind of mishaps.

@jsotuyod jsotuyod added the needs:pmd7-revalidation The issue hasn't yet been retested vs PMD 7 and may be stale label Mar 17, 2024
@jsotuyod jsotuyod removed the needs:pmd7-revalidation The issue hasn't yet been retested vs PMD 7 and may be stale label Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:false-positive PMD flags a piece of code that is not problematic
Projects
None yet
Development

No branches or pull requests

3 participants