Bug 49904

Comment 1 Jerome Lanneluc 2004-02-05 06:47:55 EST

I'm not sure to undertand why the else block is not needed in your example.

Comment 2 Mohamed ZERGAOUI 2004-02-05 07:18:13 EST

it is unneeded because the flow is broken (after a <return>, a <throw>, a
<break> or a <continue>) so it obvious that if we remove the else the control
flow is the same

for example :

  if (isAnExitCondition()) {
    // do some finalization stuff
    return false; // for example
  } else {
    // the rest of the method
    ...
  }

there is one level of indentation that is superfluous

it is equivalent 

  if (isAnExitCondition()) {
    // do some finalization stuff
    return false; // for example
  }
  {
    // the rest of the method
    ...
  }

and if it is possible to remove the unneeded brace to have a more READABLE
source (see also #49905)

  if (isAnExitCondition()) {
    // do some finalization stuff
    return false; // for example
  }
  // the rest of the method
  ...

Comment 3 Jerome Lanneluc 2004-02-05 08:55:03 EST

I see. I thought you wanted to remove the whole else block. This would indeed 
be nice to have.

Comment 4 Philipe Mulet 2004-04-28 06:16:41 EDT

I like this suggestion, and it is easy to implement.
Will do right now.

Comment 5 Philipe Mulet 2004-04-28 06:18:46 EDT

public class X {
    void foo() {
        if (this == null) {
            return;
        } else {
            System.out.println("foo");
        }
    }
}

----------
1. WARNING in ...\X.java (at line 6)
	} else {
            System.out.println("foo");
        }
	       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Statement unnecessarily nested within else clause. The corresponding then 
clause does not complete normally
----------

Fixed. Enabled this warning in compiler regression test suites.

Comment 6 Olivier Thomann 2004-05-18 13:43:08 EDT

Verified in 200405180816

Comment 7 Mohamed ZERGAOUI 2004-05-22 06:06:30 EDT

Good job guys, it works well,

But I propose to reopen this bug (may be could you split this demand) because
the initial purpose was to give a Quick Assist

I let you move it to JDT/UI

Comment 8 Philipe Mulet 2004-05-22 08:21:27 EDT

Could you please enter a separate defects against JDT/UI ?
The compiler warning is the basis for subsequent UI actions.

Comment 9 Mohamed ZERGAOUI 2004-06-04 09:57:39 EDT

What about "System.exit()" ?

Comment 10 Mohamed ZERGAOUI 2004-06-04 10:21:58 EDT

Can "indirect flow be detected ?
example

void throwException() throws Exception {
  throw new Exception();
}
void b() {
  if (test()) {
    doSomeStuff();    
    throwException();
  } else {
    doSomeOtherStuff();
  }
}

could "else { doSomeOtherStuff }" be flagged as unneeded else

Comment 11 Mohamed ZERGAOUI 2004-06-04 10:35:55 EDT

Of course i forgot to mention that "throwException()" is either 'final' or
'static' or 'private' (or some combination of these)

Comment 12 Olivier Thomann 2004-06-06 13:58:51 EDT

I don't think that the else should be flagged using your example.
void b() {
  if (test()) {
    doSomeStuff();    
    throwException();
  } else {
    doSomeOtherStuff();
  }
}
Nothing says that throwException(); will throw an exception for sure. It is
possible that an exception is thrown. So when no exception is thrown, the else
statement makes sense.
The fact that your implementation of throwException throws an exception is not
relevant in this example. So I would say that in your case the else should NOT
be flagged as unnecessary.

Comment 13 Philipe Mulet 2004-06-06 18:29:19 EDT

I agree with Olivier, we can only consider local control flow.

Comment 14 Mohamed ZERGAOUI 2004-08-03 06:58:33 EDT

It is may be a philosophical question, but, do you mean "System.exit(int)" as a
java flow breaker like "<return>" ?
In the case, the answer is yes, it must be handled and trigger in unnecessarily
nested for the example
  if (isAnExitCondition()) {
    // do some finalization stuff
    System.exit(0); // for example
  } else {
    // the rest of the method
    ...
  }

Comment 15 Mohamed ZERGAOUI 2004-09-30 07:53:42 EDT

Reopen for System.exit(int) ?

Comment 16 Philipe Mulet 2005-04-07 08:07:34 EDT

Deferring post 3.1

Comment 17 Denis Roy 2009-08-30 02:20:47 EDT

As of now 'LATER' and 'REMIND' resolutions are no longer supported.
Please reopen this bug if it is still valid for you.