FormatStringAnnotation checker is annoyingly stricter than FormatString

[doublep]

In our project we use a simple custom wrapper over String.format().Error-Prone has FormatStringAnnotation that would check for inconsistencies in format string, but unfortunately it is stricter (unwantedly) than FormatString. In other words, certain code that uses String.format() gets accepted, but results in compilation error as soon as that is replaced with @FormatMethod-annotated wrapper, with zero other changes.

Testcase (I can attach a full Gradle project if wanted):

package foo;

import com.google.errorprone.annotations.FormatMethod;
import com.google.errorprone.annotations.FormatString;

public class Test
{
    public static void main (String[] args)
    {
        // No errors or warnings here.
        System.out.println (String.format (args[Integer.parseInt (args[0])], "foo"));
        // But here it says: "Format strings must be either literals or variables. Other expressions are not valid."
        System.out.println (myFormat      (args[Integer.parseInt (args[0])], "foo"));
    }

    @FormatMethod
    public static String myFormat (@FormatString String format, Object... args)
    {
        return String.format (format, args);
    }
}

I don't see any reason why a call to a @FormatMethod-annotated method would be checked differently compared to a call to String.format().

[cushon]

I agree the difference between FormatStringAnnotation and FormatString is surprising and seems arbitrary.

When they were created, there were difference use-cases. FormatStringAnnotation was deliberately stricter, and intended to be used with APIs where a violation might cause more severe problems. FormatString was more about best-effort bug prevention, while being relatively easy to roll out to code bases with many existing calls to String.format.

It might be more principled to have stricter and less strict versions of both checks. But that also adds a fair bit of complexity, and isn't something there are plans to do at this point.

Can you share any more background about what the real world use-case was where you encountered this, where you wanted to pass non-constant strings to a @FormatString-annotated method.

[doublep]

In our application we had several thousands of calls to String.format(). Basically, it is our agreed standard way of building strings out of pieces. And then we have discovered that String.format() is locale-dependent, whereas we never thought about it like that. E.g. we assumed that String.format ("%.2f", 1.23) would always result in "1.23", but it can also be "1,23" if system locale is German, for example.

In 99% of cases it doesn't matter, but occasionally this can cause bugs. To avoid rechecking all 5000 of calls, we just replaced String.format() everywhere with a custom function that does exactly the same, with the only difference being that it uses a fixed locale.

But now we cannot use Error-Prone to autocheck our format strings, because it issues errors on ~50 calls (not too many, but I don't want to rewrite them out of principle, sort of) where it didn't have any complaints when we used String.format(). It can be argued one way or another, but as Error-Prone is capable of accepting non-constant format string for String.format(), I really would expect it to do the same for customly-annotated functions.

As far as usecases go, 1) arguments may be fairly complex expressions, so one wouldn't want to repeat them in every if-branch (or whatever one would use to have constant format strings); 2) occasionally formatting string is built dynamically in our application, so it is outright impossible to make it constant, even with additional branching. This one is really rare, but we do have a few calls like that.