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

[java] WhileLoopWithLiteralBoolean - false negative with complex expressions still occurs in PMD 6.52.0 #4250

Closed
Han-Cui opened this issue Dec 2, 2022 · 4 comments · Fixed by #4264
Labels
a:false-negative PMD doesn't flag a problematic piece of code
Milestone

Comments

@Han-Cui
Copy link

Han-Cui commented Dec 2, 2022

Affects PMD Version: 6.48.0

Rule: WhileLoopWithLiteralBoolean

Please provide the rule name and a link to the rule documentation:
https://pmd.github.io/latest/pmd_rules_java_bestpractices.html#whileloopwithliteralboolean

Description:
This is an old issue from #3455
When I use pmd 6.48.0 and 6.52.0, I still met this False Negative.

Code Sample demonstrating the issue:

public void func() {
    do {
      // Loop Body
    } while(false || false); // This is a false negative
//        } while(false | false); // This is a similar false negative.
}

Expected outcome:

PMD should report a violation at line 4 and 5, but doesn't. This is a false-negative.

Running PMD through: [CLI]

@Han-Cui Han-Cui added the a:false-negative PMD doesn't flag a problematic piece of code label Dec 2, 2022
@adangel
Copy link
Member

adangel commented Dec 2, 2022

Hi @Han-Cui , thanks for the report. However, I can't reproduce it: PMD 6.52.0 detects both cases correctly:

#!/bin/bash

if [ ! -d pmd-bin-6.52.0 ]; then
    wget -c https://github.com/pmd/pmd/releases/download/pmd_releases%2F6.52.0/pmd-bin-6.52.0.zip
    unzip pmd-bin-6.52.0.zip
fi

if [ ! -d src ]; then mkdir src; fi
cat > src/Foo.java <<EOF
public class Foo {
    public void func() {
        do {
          // Loop Body
        } while(false || false); // This is a false negative

        do {
          // Loop Body
        } while(false | false); // This is a similar false negative.
    }
}
EOF

pmd-bin-6.52.0/bin/run.sh pmd --no-cache --short-names -d src -R category/java/bestpractices.xml/WhileLoopWithLiteralBoolean

results in the following report:

Foo.java:3:	WhileLoopWithLiteralBoolean:	The loop can be simplified.
Foo.java:7:	WhileLoopWithLiteralBoolean:	The loop can be simplified.

Note that the beginning of the whole loop is marked, that is line 3 and 7, not as your comment indicates, where the loop condition is.

Did you maybe override the rule instead of referencing it when you included it in your ruleset? Which ruleset are you using?

@adangel adangel added the needs:user-input Maintainers are waiting for feedback from author label Dec 2, 2022
@Han-Cui
Copy link
Author

Han-Cui commented Dec 4, 2022

Oh! Sorry for my carelessness, I made a stupid mistake. A few months ago I set PMD 6.42.0 in the environment for the sake of convenience and I forgot it... Indeed PMD 6.48.0 and the latest version report these cases correctly. Thanks for your prompt and kind reply very much!

Additionally, should PMD consider more complex expressions like this?

The following cases do not get report by PMD,

public class Foo {
    public void func() {
        do {
            // Loop Body
        } while (false || false || false || false); // This is a false negative

        do {
            // Loop Body
        } while (false | false | false | false | false); // This is a similar false negative.
    }
}

while the following cases are both reported correctly.

public class Foo {
    public void func() {
        do {            //reported: WhileLoopWithLiteralBoolean:    The loop can be simplified.
            // Loop Body
        } while (false && false && false && false);

        do {            //reported: WhileLoopWithLiteralBoolean:    The loop can be simplified.
            // Loop Body
        } while (false & false & false & false & false);
    }
}

@adangel adangel changed the title [java] WhileLoopWithLiteralBoolean - false negative with complex expressions still occurs in PMD 6.48.0 [java] WhileLoopWithLiteralBoolean - false negative with complex expressions still occurs in PMD 6.52.0 Dec 17, 2022
@adangel adangel added this to the 6.53.0 milestone Dec 17, 2022
@adangel
Copy link
Member

adangel commented Dec 17, 2022

I could improve the rule to consider also these cases with multiple repeating boolean literals, see #4264.

However, I'm wondering: Did you see this code for real? Or did you just make it up? I can't imagine, that someone would write "false || false || false" in the first place, let alone that someone adds "|| false" to the end of such an expression...

@adangel adangel removed the needs:user-input Maintainers are waiting for feedback from author label Dec 17, 2022
@oowekyala
Copy link
Member

I don't think the problem is with WhileLoopWithLiteralBoolean. The expression false & false is abstruse regardless of whether it is the condition of a while loop or not. I think this kind of expression should be reported by another rule, like SimplifyBooleanExpressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:false-negative PMD doesn't flag a problematic piece of code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants