[java] UnconditionalIfStatement: False negative when true/false is not literal but local variable

[csrma]

Affects PMD Version: 7.15.0

Rule: UnconditionalIfStatement

Description: Currently the rule can only detect literal if (false)/if (true). However, in real code it is more likely that the if condition checks a variable or method return value that is always true/false instead of having literal true/false inside if.

Update: Removed "method return value", as the method might do some real work which makes the condition not unconditional anymore. We don't know whether the method would return always the literal true/false or whether it takes more information into consideration.

-> The rule should only find the following example, where the local variable is effectively final and initialized with a boolean literal.

Code Sample demonstrating the issue:

class UnconditionalIfStatementBug {
    public static void main(String[] args) {
        boolean condition = true;
        if (condition) {        // should raise a flag.
            System.out.println("This will always print.");
        }
    }
}

Expected outcome:

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

Running PMD through: [CLI]

[zbynek]

FTR it's been like that since introducing the rule in d532b57e
and there is one test veryfying that it does not trigger a violation with the following code:

 public class Foo {
     private static final boolean DEBUG = false;
     void bar() {
      if (DEBUG) {}
     }
  }

which makes me wonder what's the right solution here, should this be configurable to allow toggles like this?

[oowekyala]

I think it is normal not to trigger the rule if the boolean is named, especially if it is a compile-time constant (ie, static final field with a constant initializer). There is no cost at runtime, and it is a common pattern if eg you have a debug flag that you want to enable manually when you are debugging.

Maybe we could report when it is a local variable as that may be something that was forgotten, but compile-time constants should be whitelisted IMO.