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, which is not desirable in many cases.
There should be an exception (no pun intended) in this rule when the last parameter is en exception in a logging function.
First, thanks for the self-contained reproducer, it always helps!
To reformulate a bit your point:
Without the call to toString(), the resolved method is Logger.info(String, Throwable), which will print the full stack. In such case, the formatter {} is useless.
With the call to toString(), it’s Logger.info(String, Object...) which is called, and we lose the context provided by the stack.
From there I see two possible improvement of the rule:
If the method called is the one having Throwable as last argument, we should raise an issue saying that {} is useless.
logger.info("error: {}", e); // False Negative
// ^^^^^^^^^^^ remove the formatter here, it is useless
If the method called is one of the ones not having Throwable as last argument, then the following line should be OK.
