21

I have the following code:

public String testExitPoints() {
    boolean myBoolean = false;
    try {
        if (getBoolean()) {
            return "exit 1";
        }
        if (getBoolean()) {
            throw new RuntimeException();
        }
    } finally {
        myBoolean = true;
    }
    if (getBoolean()) {
        return "exit 2";
    }
    return "exit 3";
}

public static boolean getBoolean() {
    Random rand = new Random();
    return rand.nextInt() > 100;
}

Now IntelliJ idea gives me for the second and third invocation of getBoolean() the following hint:

Condition 'getBoolean()' is always 'false'

Now to my understanding, that is not true, since getBoolean() can either be true or false, depending on the generated random value. Am I missing something here, or is that a bug in IntelliJ Idea?

Mathias Bader
  • 3,585
  • 7
  • 39
  • 61
  • I think not. You either invoke another `getBoolean()` than what you posted, or it's a bug in Intellij Idea (in which case you should file a bug report). What do you get when running your code? – Axel Aug 05 '15 at 10:36
  • There is no other `getBoolean()`, it's just a test project for this method. Running the code does sometimes return `false and sometimes `true`as you would expect. – Mathias Bader Aug 05 '15 at 10:38
  • If the return value of `getBoolean()` wasn't random, IntelliJ would be correct. So I'd guess it is indeed a bug (read as: maybe some kind of incorrect optimization/simplification). – Marvin Aug 05 '15 at 10:39
  • probably because your second and third `if(getBoolean())` conditions are never going to be executed. Since you have a return in first condition. What happens if you remove the return statement from first condition? – Amit.rk3 Aug 05 '15 at 10:39
  • They do get executed, depending on the return value of the first `getBoolean()` invocation. – Mathias Bader Aug 05 '15 at 10:42

4 Answers4

18

It's not a bug. It's a feature :)

If you look carefully in your IDE, it will tell you that the 2nd and 3rd call to getBoolean() are always false, but not the first one.

Idea assumes (in this case incorrectly) that your method, being parameterless and called "get"..., would return always the same value.

If that were the case, and the first call was true, the other would never be accessed (because of the return).

If the first call was false, so would be the others.

IDEA tries to be smart w.r.t. good coding practices, but it's not infallible.

If you change your method to have parameters (or rename it so it doesn't look like a getter)

public  boolean getBoolean(int x) {
    Random rand = new Random();
    return rand.nextInt() > 100;
}

The warnings will go away (even if you invoke with the same argument all times).

(Note that, even if it was a getter, if it's for a non-final field it's still wrong, as it may change in a multithreaded environment!)

Diego Martinoia
  • 4,592
  • 1
  • 17
  • 36
  • Interesting. That would indeed be an error then - calling a method without parameters doesn't mean there can't be side effects or different return values. – Mathias Bader Aug 05 '15 at 10:44
  • Even a change to `public static boolean getBoolean(int ... x) {` does the trick here (which doesn't include the necessity to change the invocations as well). – Mathias Bader Aug 05 '15 at 10:45
  • @MathiasBader you are right. I misexplained it: it's about reading it as a getter. See updated answer (note : it's still wrong, it may be muted in a multithreaded environment even as a getter!, but it shouldn't happen in most cases) IDEA does it's best, but it's not perfect. – Diego Martinoia Aug 05 '15 at 10:47
  • Great - all clarified then. Glad that my intuition was right, that something was wrong here. I filed a bug report at JetBrains - thanks for your answer. – Mathias Bader Aug 05 '15 at 10:54
9

Unfortunately, while the accepted answer gives a good explanation, it is not always possible to rename the triggering methods as they might reside in third party code. For example, I was using a first() function from the MongoDB library, which clearly could return a null value, but triggered warnings when I wanted to test it for nullity.

If you are confident that IDEA is getting it wrong, just put

                //noinspection ConstantConditions

before the statement that is causing the problem.

In general, a handy option to know about is 'Inspect code...' under the 'Analyze' menu. There you can look at the whole project, or just the file you are concerned with. You might find many more areas of concern than you bargained for! The warning in the Question will be listed under 'probable bugs'. Note "probable" - IDEA knows it is not infallible :)

Ger
  • 889
  • 1
  • 12
  • 21
2

IDEA thinks that the getBoolean() call is not changed when it's called for the second (or third time). Normally if you return the same value the second call could be never achieved. That's why IDEA highlight it.

StanislavL
  • 56,971
  • 9
  • 68
  • 98
0

In my case if I you use MY_ACTUAL_CLASS_NAME.getBoolean() it does not complain (since the method is static). May be it is because IntelliJ Idea does not take static into account (a possible bug here)

Rajesh Goel
  • 3,277
  • 1
  • 17
  • 13