[Java:S3457] False positive when string concatenation is used in exception message

[grimsa (Gediminas Rimša)]

Versions used:

• SonarCloud
• Source and target compile level = Java 14
• org.sonarqube Gradle plugin version 2.8
• Slf4J version 1.7.30

Sample:

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
// ...
    private static final Logger log = LoggerFactory.getLogger(PvWattsApi.class);
// ...
        } catch (RestClientException e) {
            log.error("PV Watts API call failed. Reason " + e.getMessage(), e);
            return Optional.empty();
        }

There is no SLF4J API method that takes a format String, args, and a Throwable - here we are using error(String msg, Throwable e), so the comment about format specifiers is a false-positive.

Image:

Possibly somewhat releated thread:

FP on squid:S3457 when last parameter is Exception

version: SonarLint SolarSource 4.4.0.14142 package foo.bar; import java.io.IOException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class Main { public static void main(String[] args) { Logger logger = LoggerFactory.getLogger(Main.class); try { throw new IOException("test"); } catch (Exception e) { logger.info("error: {}", e.toString()); // FP on squid:S3457 here } } } Without the toString() here the full exception stack is logged, whic…

[Quentin (Quentin Jaquier)]

Hello @grimsa,

What is recomended by the rule is to write something like this:

} catch (RestClientException e) {
      log.error("PV Watts API call failed. Reason {}", e.getMessage(), e);
      return Optional.empty();
}

It will compile since error(String msg, Object e1, Object e2) will be called, and will result in a correctly formatted message if slf4j.simple.SimpleLogger is used for example.

That being said, is your concern the fact that this last argument may not be correctly supported depending on the logger used?
Do you have a specific example by any chance?

[grimsa (Gediminas Rimša)]

In general:
The behavior of “taking the last format argument and interpreting it as a Throwable” seems to be an implementation detail of SLF4J SimpleLogger / Logback.
I feel it is wrong to assume this implementation detail is present in all SLF4J API implementations (it is not documented in the API docs after all).

In our case:
We are using Logback, so what you suggested would make the Sonar issues go away. But I’m not sure we want to.

[Quentin (Quentin Jaquier)]

Okay, I understand what you mean.

Ticket created: SONARJAVA-3577

Thanks