Eclipse - Sonar S2629 possible false positive with new String

Question

I'm using latest Eclipse and Sonar plugin

In answer for logging there's the following line:

log.debug("Request body: {}", new String(body, "UTF-8"));

Which should create String only if in DEBUG level:

/**
 * Log a message at the DEBUG level according to the specified format
 * and argument.
 * <p/>
 * <p>This form avoids superfluous object creation when the logger
 * is disabled for the DEBUG level. </p>
 *
 * @param format the format string
 * @param arg    the argument
 */
public void debug(String format, Object arg);

But Sonar is marking it as squid:S2629:

"Preconditions" and logging arguments should not require evaluation (squid:S2629)

And give examples with concatenation

logger.log(Level.DEBUG, "Something went wrong: " + message); // Noncompliant; string concatenation performed even when log level too high to show DEBUG messages

Is it a false positive sonar warning or am I missing something?

This isn't a duplicate of this question which is generally asking about the rule concept, which is concatenation, but not formatting with creating object as new String

Also link of an answer says creating new Date() isn't creating an issue with built in format:

public static void main(String[] args) {
    LOGGER.info("The program started at {}", new Date());
}

}
Logging this way, you avoid performance overhead for strings concatenation when nothing should be actually logged.

score 10 · Accepted Answer · edited Apr 27 '22 at 08:01

In non-DEBUG mode the line

log.debug("Request body: {}", new String(body, "UTF-8"));

instead of

log.debug(MessageFormatter.format("Request body: {}", new String(body, "UTF-8")));

avoids the creation of the string that is created via MessageFormatter.format(String messagePattern, Object arg), but not the creation of the other string that is created by new String(body, "UTF-8").

This means it is not a false positive, because the arguments are calculated first before the logging method is called.

As long as SLF4J does not support lambda expression to lazy evaluate arguments, the following utility method can be used as a workaround:

private static Object lazyToString(final Supplier<String> stringSupplier) {
    return new Object() {
        @Override
        public String toString() {
            return stringSupplier.get();
        }
    };
}

This can be used to limit the converting of the byte array into a string to the DEBUG mode only:

log.debug("Request body: {}", lazyToString(() -> new String(body, StandardCharsets.UTF_8)));

score 3 · Answer 2 · answered Oct 28 '18 at 18:14

While the use of lambda or lazy lambda is fine, there is still the good old isDebugEnabled way:

if (log.isDebugEnabled()) {
  log.debug("Request body: {}", new String(body, StandardCharsets.UTF_8));
}

That's won't fix the String being created (because you want to display it after all), but that would not consume memory when debug mode is disabled.

Eclipse - Sonar S2629 possible false positive with new String

2 Answers2