[Java] False positive for S1481 when using try-with-resources

[ba...@semedy.com]

Hi,

the following code

try (Record r = getRecord()) {
   // Note: "r" is not used inside the try-block
   executeLongRunningOperation();
}

leads to a "Unused local variables should be removed" (S1481) warning because the variable "r" is not used in the try-block.

As the "Record" class is an "AutoCloseable" instance the "unusued variable" is actually used to auto-close the object.

I could not find a similar issue in Jira, so this might be a new issue.

Regards,

Juergen

[Nicolas Peru]

Hi, 

I don't think we have an issue for this indeed, but can you tell me why it is a false positive in your case ? the variable is declared for autocloseable purpose I get it but what's the point of opening a resource if you don't use it at all ?

Cheers, 

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/52c1836d-d6d0-4187-9123-d95037ad391f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Nicolas PERU | SonarSource

Senior Developer
http://sonarsource.com

https://twitter.com/benzonico

[ba...@semedy.com]

Hi,

I don't think we have an issue for this indeed, but can you tell me why it is a false positive in your case ? the variable is declared for autocloseable purpose I get it but what's the point of opening a resource if you don't use it at all ?

this is a trick to record timing information with less code. Instead of

record.start();
try {
    executeLongRunningOperation();
} finally {
    record.stop();
}

we can just write

try (Record record = ...) {
    executeLongRunningOperation();
}

and the AutoCloseable.close() method will automatically call "record.stop()". This is more compact and improves readability.

Regards,

Juergen

[Nicolas Peru]

Hi, 

Ok, you call it a trick but I can call it a hack :) 

For me this is a corner case and as such I don't think we should improve the rule for this one because it will end up with false negatives on real unused resources.

Cheers, 

--

You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/8868546b-914e-443b-92ed-8fd71038ee37%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[ba...@semedy.com]

Hi,

this is okay for me. It is definitely better to have false positives instead of false negatives.

Thanks for your quick reply,

Juergen (I still call it a trick instead of a hack :) )

[johng...@gmail.com]

We use SLF4J's MCD.PutClosable frequently in resource try blocks to get those variables into the logging system in a systematic way.   

This also causes these false positives.

[Nicolas Peru]

Hi,

First, it is always appreciated to use greetings on this mailing list, helps to get things smoother. 

Second : 

 Do you have any issue ? if yes, could you please state it clearly as your message is rather unclear about your intents ? could you provide a small code sample reproducing the issue and explain why it is a false positive if you think it is. 

Thanks.

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/467832d7-46a7-4ae5-8faf-8e28398dc031%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[olivie...@gmail.com]

Hello,

I just ran into a similar problem of "unused variable" in a try-with-resources block.

The code looks like this :

try (FileChannel fileChannel = new RandomAccessFile(file, "rw").getChannel();
     FileLock lock = fileChannel.lock()) {
  fileChannel.write(...);
  ...
}

The only reason the lock variable exists, is because the syntax requires it. As the FileLock class implements the AutoCloseable interface, the call to release() will be performed by the AutoCloseable#close() call.

Do you also consider this as a corner case ?

If it is, what do you suggest ? Declare this as a false positive ? Modify the code and do it the old way (manual try / finally)

Thanks for your help !

Olivier

[Nicolas Peru]

Hi, 

For me the issue raised is valid and it is a perfect fit for the won't fix status. 

Cheers, 

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/bd46eb62-cdae-4311-8f62-7663e4748e33%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Nicolas Peru | SonarSource

Senior Developer

http://twitter.com/benzonico

http://sonarsource.com

[olivie...@gmail.com]

Hi again,

ok, from a pure Sonar perspective, it is :) 

After a little digging, it seems that the FileChannel class takes care of releasing all the acquired FileLocks in its close() method.

So the lock can be acquired without any variable and automatically released by the channel.

Thanks for your quick reply, and keep up the good work :)

Olivier