How can I detect dead code after always-throwing methods?

Question

consider the following code:

@Test 
public void testDeadCode() {
    letsThrow();
    System.out.println("will never be reached");
}

private final void letsThrow() {
    throw new RuntimeException("guess you didnt see this one coming");
}

To me it seems absolutely impossible that the println() would ever be executed - as the call to letsThrow() will always throw an exception.

Thus I am

a) surprised that the compiler can't tell me "this is dead code"

b) wondering if there are some compiler flags (or eclipse settings) that would result in telling me: you got dead code there.

Indefatigable

Imagine the same situation but with much more complex code in letsThrow() method with lots of inner method calls. Do you ask the compiler to check the dead code in this situation for you too? Compiler is not almighty. – Torbert

The best way to know is thorough unit testing and checking on the clover code coverage which will tell you what code is never executed – Disdainful

@Vwin Not really. The code that made me asking looks like catch(Whatever w) { tuneAndRethrow(w); throw Bla(w); } so there is only one line of dead code. That would mean that I need close to 100% coverage for all our classes to find those places. Doesn't sound like a real plan to me. – Indefatigable

This is why Scala has the type Nothing, so the compiler can check this kind of thing. – Wonderland

I find your question interesting. When inlining the RuntimeException Eclipse won't compile the class and give Unreachable Code error message. Not even Find Bugs can track this. – Kenaz

@Kenaz What do you mean by inlining? – Indefatigable

The refactoring 'inline'. Eclipse copies/removes the separat method call and put the code in place. LIke: @Test public void testDeadCode() { throw new RuntimeException("guess you didnt see this one coming"); System.out.println("will never be reached"); } – Kenaz

@Kenaz I see; but well, that matches perfectly to what Compass answered: the java compiler doesn't care what methods you are calling is doing. If method A calls method B, and B throws ... you don't notice in A. Whereas when you inline, anything after the throws becomes "dead". – Indefatigable

Answer 1

Dead code compile-time errors are defined by the compiler and not the IDE. While it is true the code will never get executed, it doesn't violate any of the rules for unreachable statements from the Oracle Docs.

From Unreachable Statements

This section is devoted to a precise explanation of the word "reachable." The idea is that there must be some possible execution path from the beginning of the constructor, method, instance initializer, or static initializer that contains the statement to the statement itself. The analysis takes into account the structure of statements. Except for the special treatment of while, do, and for statements whose condition expression has the constant value true, the values of expressions are not taken into account in the flow analysis.

The rules specifically for this case are related to whether or not the blocks you've created are reachable. (iff = if and only if)

An empty block that is not a switch block can complete normally iff it is reachable.

A non-empty block that is not a switch block can complete normally iff the last statement in it can complete normally.

The first statement in a non-empty block that is not a switch block is reachable iff the block is reachable.

Every other statement S in a non-empty block that is not a switch block is reachable iff the statement preceding S can complete normally.

The letsThrow method meets the criteria for a working block of code and technically completes normally. It throws an exception, but it completes. Whether or not it throws a guaranteed exception is not taken into account in determining whether that block of code in its actual use, just whether or not it can be reached. In most cases, dead code will only be found when it involves try/catch/returns, which are the bulk of the rules.

Consider the following, even more concise version:

@Test 
public void testDeadCode() {
    System.exit(0);
    System.out.println("will never be reached");
}

There's no real counter for this aside from diligent use of coverage tools, but the bright side in your example is you'll see guaranteed exceptions every time you run the code.

Leena

Answer 2

Aim to have comprehensive unit tests and measure test coverage of the test. The dead code will be noticeable because none of your tests cause it to be executed.

Seawright

See my comment to Vwins comment above. – Indefatigable

Answer 3

Declare your method as returning a throwable type:

private final RuntimeException letsThrow() {
  throw new RuntimeException("guess you didnt see this one coming");
}

Then you can throw when you call it:

throw letsThrow();

Now, any code which follows the call to letsThrow() will be detectable as dead.

You can enforce this by checking for cases where the return value of letsThrow() isn't used using your static analysis tool. For example, Google's errorprone has a checker for the @CheckReturnValue annotation which ensures that you use the result.

(For a poor man's version, search for the regex ^\s*letsThrow();).

Paquito

Interesting idea. But I guess I would then prefer letsThrow() to not throw at all - anything else seems like asking for a "wtf code quality metric" numbers ... – Indefatigable

I don't understand - if you don't want letsThrow() not to throw, why does it? – Paquito

I mean: only the "inner throw" will actually happen. But you need to dive into that method to understand that. It seems like misleading the reader to write throw someCall()... if you already know that someCall() itself will throw. I understand that this construct resolves my problem; but if I go for that, I would rather rework letsThrow() to only return, and not throw! – Indefatigable

@Jägermeister does it really matter which throw throws? What you can see is that normal execution stops at that line. Note that this is the approach used in Guava's Throwables.propagate(), specifically "to signal to the compiler that statements after the call are unreachable". – Paquito