@Cleanup and java.util.Collections.singletonList

[Mike Power]

I was looking at my delombok code

I see the following:
Connection connection = source.getConnection();
try {
...
} finally {
if (java.util.Collections.singletonList(connection).get(0) != null) {
connection.close();
}
}

I find the java.util.Collections.singletonList call interesting. Why not
just do:
if (connection != null) {

Is it to fake out some compiler smarts that complains about null checks
for things that can't be null?

Mike Power

[Roel Spilker]

Hi Mike,

Yes, we're preventing warnings since we cannot suppress them. We
actually have a function in the Lombok utility class, called
preventNullAnalysis, that we plan to use in the future, but we can only
start using that one after most people have updated their installation.
Actually, I think it has been in there long enough now, so we'll
probably can already use it. The good thing is that we can actually
remove the call to preventNullAnalysis from the generated bytecode.

Roel

[Mike Power]

What I was trying to do was max out my code coverage with Jacoco. I was
getting 3/4 branch coverage. But I could not identify why there was 4
branches based on the sample code in the documentation. At first I
thought Jacoco was identifying the IndexOutOfBoundsException that the
'get' method could throw and identifying it as a branch that needs to be
tested. Obviously that would never happen and I couldn't even test it
if I wanted to.

Before you read more I have come to the conclusion in this case that
100% coverage is not feasible or valuable.

However as a learned more I found out that Java emits the finally block
twice in the byte code. Thus for the finally block I get one set of
paths for exception versus non-exception and another set of paths for
the if block. As a result I get four paths.

I can always execute three of the four paths. Depending on how I
structure my code, there is always one path I cannot execute.

For example

@Cleanup
Closable closer = Closable.open()
if(closer != null) {
closer.toString()
}

With this block of code I can generate the following paths
1) Non-exception finally when closer != null
2) Non-exception finally when closer == null
3) exception finally when closer != null

But I cannot execute the exception finally when closer == null

If I drop my 'if' guard:
@Cleanup
Closable closer = Closable.open()
closer.toString()

I can execute the following paths
1) Non-exception finally when closer != null
2) exception finally when closer == null
3) exception finally when closer != null

But I cannot execute the nonexception finally when closer == null.

One contributing factor to this problem is the structure of the
resulting generated code:

Closable closer = Closable.open()
try {
closer.toString()
}
finally {
if(Java.util.Collections.singletonList(closer).get(0) != null) {
closer.close();
}
}

This is generally not how I end up writing a cleanup finally block, but
it is high quality code. I tend to write them as such:

Closable closer = null
try {
closer = Closable.open();
if(closer != null) {
closer.toString();
}
}
finally {
if(java.util.Collections.singletonList(closer).get(0) != null) {
closer.close();
}
}

If this were the generated code I could generate the following paths:
1) Non-exception finally when closer != null
2) Non-exception finally when closer == null
3) Exception finally when closer != null
4) Exception finally when closer == null

That being said my original conclusion still stands, unless the change
is trivial it is best all around to let the coverage sink below 100% for
this case.

[Reinier Zwitserloot]

Presumably Jacoco analyses bytecode.

As Roel explained, the preventNullAnalysis solution allows us to remove it from the bytecode. We already do this in the sneakythrows case: If:

throw Lombok.sneakyThrow(e);

is anywhere in your code, lombok post processing reduces this to 'throw e;' in the bytecode - it removes the call (and thus, the dependency!) on lombok.