FP on squid:S3457 when last parameter is Exception

version: SonarLint SolarSource

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.

Hey @mitch0, and welcome in this community forum.

First, thanks for the self-contained reproducer, it always helps! :+1:

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:

  1. 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
  2. If the method called is one of the ones not having Throwable as last argument, then the following line should be OK.
      logger.info("error: {}", e.toString()); // False Positive

Here is the ticket to handle the FP and improvement of the rule: SONARJAVA-3290


Thanks for the elaboration, indeed this is what I meant with my report.