Static analysis tool for String.format

[pgomulka]

Description

String.format can throw RuntimeException when a format has more place holders than provided arguments. If more arguments is provided, it silently return a result.
When a String.format is used in logging, more importantly in delayed execution for instance

logger.trace(()->String.format("%s %s", "hello"))

it will not fail when tested without logging level set to trace.

ES should use a static analysis to find out usages of String.format like this (and the Strings.format which is a utility in core)
Sonarqube has a rule for this, maybe it could be adopted?
https://rules.sonarsource.com/java/RSPEC-2275

[elasticmachine]

Pinging @elastic/es-core-infra (Team:Core/Infra)

[elasticmachine]

Pinging @elastic/es-delivery (Team:Delivery)

[mark-vieira]

IntelliJ has an inspection for this, but it's not an error so this can still slip into the codebase. I suspect we could write a custom checkstyle rule for this.

[pugnascotia]

Do we not already have some prior art with ESLoggerUsageChecker? It's doing a similar thing, I think?

[pgomulka]

Do we not already have some prior art with ESLoggerUsageChecker? It's doing a similar thing, I think?

we do, but extending ESLoggerUsageChecker to work with String.format is a lot of work. This is because Logger would just allow for {} but String.format allows for a lot of different % quantifiers https://docs.oracle.com/javase/7/docs/api/java/util/Formatter.html
We would have to duplicate a lot of validation logic that is performed in String.format now and I wanted to avoid this. https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/java/util/Formatter.java#L2806\

ESLoggerUsageChecker in itself is not easy to extend. If there is a easier to adopt tool it would save us a lot of effort

[pugnascotia]

Sorry, I didn't mean to suggest that we extend ESLoggerUsageChecker, just that we have written code to do this sort of checking before.

[joegallo]

Related to #103186

[joegallo]

I suppose the best version of this would handle org.elasticsearch.common.Strings.format and org.elasticsearch.core.Strings.format, too.