Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

SonarQube: Change this condition so that it does not always evaluate to "false" (for finally in javax.mail receiving)

Why is SonarQube complaining about this part of the code??

SonarQube says: Change this condition so that it does not always evaluate to "false"

However I can't seem to understand why the condition would always be false? Indeed, actually it's not, I just reran this part again in debug mode and it works perfectly, it does go inside, and the condition is not false most of the time.

Here's the code part:

    } finally {
        if ((inboxFolder != null) && (inboxFolder.isOpen())) {
            try {
                inboxFolder.close(true);
            } catch (MessagingException e) {
                log.error(e.getMessage(), e);
            }
        }
        if ((store != null) && (store.isConnected())) {
            try {
                store.close();
            } catch (MessagingException e) {
                log.error(e.getMessage(), e);
            }
        }
    }

It's the finally part of try-catch when trying to receive email with javax.email, it's complaining about both if conditions.

This is the declaration of those variables, they get instantiated int the try part:

Folder inboxFolder = null;
Store store = null;

So why is SonarQube complaining about this?

like image 986
Arturas M Avatar asked Sep 25 '16 13:09

Arturas M


1 Answers

We experienced a similar false positive producing the error 'Change this condition so that it does not always evaluate to "false"'. The code in question follows:

  public Properties getProperties() {
  Properties properties = new Properties();
  InputStream in = getClass().getResourceAsStream("/my.properties");
  IllegalStateException streamCloseError = null;
  try {
    if (in != null) {
      try {
        properties.load(in);
      } catch (Exception e) {
        //fall through...
      }
    }
  } finally {
    try {
      if (in != null) {
        in.close();
      }
    } catch (IOException e) {
      streamCloseError = new IllegalStateException(e);
    }
  }
  if (streamCloseError != null) {
    throw streamCloseError;
  }
  return properties;
}

The error was thrown on the line if (streamCloseError != null) {. We cleaned up this code after reading about using the above try-with-resources.

Would it be possible with this rule to detect if it was found in conjunction with "closing" and if so provide the hint to use try-with-resources?

Thanks for considering this.

like image 85
Doug Beattie Avatar answered Oct 28 '22 13:10

Doug Beattie