Scott Murphy Heiberg created GROOVY-12696:
---------------------------------------------
Summary: FormatStringChecker treats explicit argument index as
zero-based when arguments are not constants
Key: GROOVY-12696
URL: https://issues.apache.org/jira/browse/GROOVY-12696
Project: Groovy
Issue Type: Bug
Affects Versions: 6.0.0
Reporter: Scott Murphy Heiberg
*Environment:* macOS 26.5, JDK 21.0.12 (BellSoft Liberica), Groovy 6.0.0,
6.0.1-SNAPSHOT (GROOVY_6_0_X at 7c0dcf17dd) and master (7.0.0-SNAPSHOT at
d86ea7eb85)
{{groovy.typecheckers.FormatStringChecker}} has two paths. When every argument
of a format call is a constant it runs {{java.util.Formatter}} itself and
reports whatever {{Formatter}} throws. When at least one argument is not a
constant it walks the format specifiers and checks each argument's static type.
On that second path an explicit argument index ({{%1$s}}, {{%2$d}}, ...) is
used directly as a zero-based position in the argument list, while
{{Formatter}} numbers explicit indices from 1. The checker therefore looks at
the argument after the one {{Formatter}} will use: it rejects valid calls and
accepts calls that throw at run time.
h3. Reproducer
{code:groovy}
import groovy.transform.TypeChecked
@TypeChecked(extensions = 'groovy.typecheckers.FormatStringChecker')
class Repro {
static String run(String s, Integer i) {
String.format('%1$s', s)
}
}
println '[' + Repro.run('x', 1) + ']'
{code}
Actual output ({{groovy Repro.groovy}}):
{noformat}
org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed:
Repro.groovy: 6: [Static type checking] - MissingFormatArgument: Format
specifier '%1$s'
@ line 6, column 9.
String.format('%1$s', s)
^
1 error
{noformat}
Expected: the script compiles and prints {{[x]}}. {{String.format('%1$s',
'x')}} is valid and returns {{x}}; with the constant argument {{'x'}} in place
of {{s}} the checker accepts the call.
The opposite case, with the same class but
{code:groovy}
String.format('%1$d', s, i)
{code}
compiles without a type error and then fails at run time:
{noformat}
Caught: java.util.IllegalFormatConversionException: d != java.lang.String
{noformat}
Expected: a compile-time error {{IllegalFormatConversion: d !=
java.lang.String}}, which is what the checker reports for the same call with
constant arguments, {{String.format('%1$d', 'x', 1)}}.
More calls with {{String s}} and {{Integer i}} parameters, run on all three
versions listed above:
|| call || Formatter at run time || FormatStringChecker ||
| {{String.format('%1$s', s)}} | {{x}} | MissingFormatArgument: Format
specifier '%1$s' |
| {{String.format('%2$s', s, i)}} | {{1}} | MissingFormatArgument: Format
specifier '%2$s' |
| {{String.format('%2$d %1$s', s, i)}} | {{1 x}} | MissingFormatArgument:
Format specifier '%2$d' |
| {{String.format('%1$s %2$d', s, i)}} | {{x 1}} | MissingFormatArgument:
Format specifier '%2$d' |
| {{String.format('%2$s %s %<s %1$s', s, i)}} | {{1 x x x}} |
MissingFormatArgument: Format specifier '%2$s' |
| {{String.format('%1$d', i, s)}} | {{1}} | IllegalFormatConversion: d !=
java.lang.String (checks {{s}} instead of {{i}}) |
| {{String.format('%1$d', s, i)}} | IllegalFormatConversionException: d !=
java.lang.String | accepted |
| {{String.format('%3$s', s, i)}} | MissingFormatArgumentException |
MissingFormatArgument: Format specifier '%3$s' (correct only because 3 and 2
both exceed the last index) |
| {{String.format('%1$s', 'x')}} (constants) | {{x}} | accepted |
| {{String.format('%1$d', 'x', 1)}} (constants) |
IllegalFormatConversionException | IllegalFormatConversion: d !=
java.lang.String |
h3. Cause
In {{FormatStringChecker.checkFormatStringTypes}}
(subprojects/groovy-typecheckers/src/main/groovy/groovy/typecheckers/FormatStringChecker.groovy,
line 184 on master):
{code:groovy}
int indexToUse = argIndex ?: next
{code}
{{argIndex}} is the integer parsed from the {{n$}} prefix and is used as is to
index {{args}}, which is zero-based. The ordinary counter {{next}} and the
index remembered for {{%<}} are zero-based, so an explicit index is off by one
relative to them and to {{Formatter}}, whose {{format}} loop uses {{index - 1}}
for an explicit index. The bounds check {{indexToUse >= args.size()}} then
rejects {{%n$}} whenever {{n}} equals the argument count, and the type check
otherwise inspects argument {{n + 1}} instead of {{n}}. The constant-argument
path ({{checkFormatStringConstantArgs}}) is not affected.
The fix is to subtract one from an explicit index before using it: {{int
indexToUse = argIndex ? argIndex - 1 : next}}. With that change every call in
the table above is checked the same way {{Formatter}} resolves it. A pull
request with regression tests follows.
h3. Versions
Reproduces on Groovy 6.0.0, 6.0.1-SNAPSHOT (GROOVY_6_0_X at 7c0dcf17dd) and
master (d86ea7eb85). The line has been unchanged since the checker was added
for GROOVY-11261 (5.0.0-alpha-5), so 5.0.x is presumably affected as well,
though that was not run.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
