Original issue created by jcanizales@google.com on 2013-07-20 at 09:41 PM
Describe the bug pattern this checker should detect and why it is
incorrect.
Some methods of widely-used libraries, when called, look like they're accepting a format string and arguments, but actually aren't. Yes, java.util.logging.Logger#log(Level, String, Throwable), I'm looking at you. If the programmer calls them passing a literal format string, the user ends up seeing stuff like: "Value: {0}". It's difficult for a reviewer to catch this.
The set of methods for which this is a problem is small, because most methods don't look like they're accepting format arguments when called. For example, because their string parameter is last, because none of their overloads accepts format arguments, because no method in their class accepts them, or because it just wouldn't make sense in its specific case.
This check can lead to false positives if the programmer really wants to pass a string that looks like a format string to one such method. Controlling the set of methods for which this check triggers, and how the check recognizes a string as a format string, can reduce false positives to cases in which the programmer should be shot.
Otherwise, just include positive and negative cases below.
Positives cases:
java.util.logging.Logger logger;
Throwable e;
logger.log(Level.SEVERE, "RPC exception: {0}", e); // Not what it looks like!
In general, a method call:
whose signature is (..., String, ...),
being passed a literal string with the substrings {0}...{N-1}, coinciding with the number of arguments after the String in the call,
whose parameters after the String are other than Object or "Object..."
Negatives cases:
// If the string passed isn't a literal string, obviously.
// If the string literal passed doesn't contain the substring "{0}".
caseMethod("Strings with {non-numeric} parts surrounded by braces, even if they include {0}", val)
caseMethod("Strings with parameter indices beyond the number of following arguments, e.g. {0}, {1} and {2}", val0, val1)
caseMethod("Strings with non-consecutive parameter indices like {0} and {2}.", val0, val1, val2)
caseMethod("Looks like format string, but {0} is passed null.", null)
caseMethod("Stings that contain 'single quotes', even if not around {0}", val)
I guess a whitelist and blacklist of methods in widely-used libraries can be used to prevent false positives. But lacking that, or for unlisted methods, one might want to trigger only if the String passed looks like a format string directed to humans. E.g. not triggering to:
caseMethod("{0}", val) // The string doesn't contain words
Original issue created by jcanizales@google.com on 2013-07-20 at 09:41 PM
Some methods of widely-used libraries, when called, look like they're accepting a format string and arguments, but actually aren't. Yes, java.util.logging.Logger#log(Level, String, Throwable), I'm looking at you. If the programmer calls them passing a literal format string, the user ends up seeing stuff like: "Value: {0}". It's difficult for a reviewer to catch this.
The set of methods for which this is a problem is small, because most methods don't look like they're accepting format arguments when called. For example, because their string parameter is last, because none of their overloads accepts format arguments, because no method in their class accepts them, or because it just wouldn't make sense in its specific case.
This check can lead to false positives if the programmer really wants to pass a string that looks like a format string to one such method. Controlling the set of methods for which this check triggers, and how the check recognizes a string as a format string, can reduce false positives to cases in which the programmer should be shot.
Positives cases:
java.util.logging.Logger logger; Throwable e; logger.log(Level.SEVERE, "RPC exception: {0}", e); // Not what it looks like!
In general, a method call:
Negatives cases:
// If the string passed isn't a literal string, obviously.
// If the string literal passed doesn't contain the substring "{0}".
caseMethod("Strings with {non-numeric} parts surrounded by braces, even if they include {0}", val)
caseMethod("Strings with parameter indices beyond the number of following arguments, e.g. {0}, {1} and {2}", val0, val1)
caseMethod("Strings with non-consecutive parameter indices like {0} and {2}.", val0, val1, val2)
caseMethod("Looks like format string, but {0} is passed null.", null)
caseMethod("Stings that contain 'single quotes', even if not around {0}", val)
I guess a whitelist and blacklist of methods in widely-used libraries can be used to prevent false positives. But lacking that, or for unlisted methods, one might want to trigger only if the String passed looks like a format string directed to humans. E.g. not triggering to:
caseMethod("{0}", val) // The string doesn't contain words
caseMethod("doesnt-look-like-a-sentence-{0}", val)