search

cushon / issues-import

0 stars 0 forks source link

Precondition.checkFoo() only accepts %s as a placeholders. All others (%d, etc) should be errors or warnings #254

Closed cushon closed 9 years ago

cushon commented 9 years ago

Original issue created by kak@google.com on 2014-05-20 at 08:06 PM

Guava's Precondition class allows users to write error messages w/ placeholders. For example: Preconditions.checkArgument(speed > 0, "speed (%s) must be positive", speed);

Unfortunately (due to GWT compatibility issues), the only placeholder that is allowed is '%s'. Many users assume this works the same as String.format though, and they use %d, etc.

This should probably be an error, or at least a warning? There's dozens of examples of people getting this wrong inside the google3 codebase.

Thanks :-)

cushon commented 9 years ago

Original comment posted by kak@google.com on 2014-05-20 at 08:10 PM

This gsearch should find google3 usages BTW:

Preconditions.check.*\%[^s ]

cushon commented 9 years ago

Original comment posted by eaftan@google.com on 2014-08-22 at 05:34 PM

We have the PreconditionsInvalidPlaceholder check for this, and it's currently on in code review. We should do the cleanup and turn it on as a compiler error. I'll file a separate internal bug to do the cleanup and make this an error, and CC you on it.

Status: Fixed