codeconsole opened a new pull request, #3217:
URL: https://github.com/apache/groovy/pull/3217
On the path of `FormatStringChecker` that checks calls whose arguments are
not all constants, `checkFormatStringTypes` used the explicit argument index of
a format specifier (`%1$s`, `%2$d`) directly as a zero-based position in the
argument list. `java.util.Formatter` numbers explicit indices from 1, so the
checker inspected the argument after the one `Formatter` uses.
`String.format('%1$s', s)` with a `String s` parameter was rejected with
`MissingFormatArgument`, `String.format('%2$s %s %<s %1$s', s, i)` likewise,
and `String.format('%1$d', s, i)` with `Integer i` was accepted and failed at
run time with `IllegalFormatConversionException: d != java.lang.String`,
because the second argument was checked instead of the first. Calls whose
arguments are all constants were not affected, since that path runs `Formatter`
itself.
The change subtracts one from an explicit index before it is used (`int
indexToUse = argIndex ? argIndex - 1 : next`), so explicit, ordinary and
relative (`%<`) specifiers resolve to the same arguments as `Formatter` does.
The ordinary counter and the index remembered for `%<` were already zero-based
and are unchanged, as is the constant-argument path. No public API changes.
Tested with two new tests in `FormatStringCheckerTest`:
`testExplicitArgumentIndexForInferredArgs` asserts that valid calls with
explicit indices, including ones mixed with ordinary and `%<` specifiers and an
`sprintf` call, compile and produce the same output as `Formatter`;
`testExplicitArgumentIndexErrorsForInferredArgs` asserts that
`IllegalFormatConversion` is reported for the argument the index actually names
(including through `%<`) and that an index beyond the argument count is still
reported as `MissingFormatArgument`. Both tests fail on master without the fix
and pass with it. `./gradlew :groovy-typecheckers:test` (394 tests) and
`./gradlew :groovy-typecheckers:lintGate :groovy-typecheckers:rat` pass on JDK
21.0.12. The reproducer scripts were also run against a `installGroovy`
distribution built from this branch and now behave like `Formatter` on every
case, while 6.0.0, 6.0.1-SNAPSHOT and master without the fix all reproduce the
defect.
Relation to #3213 (GROOVY-12692): that pull request changes the same method,
teaching it that `%%` and `%n` take no argument. The two changes are
independent and are based on the same master commit (d86ea7eb85). A three-way
merge (`git merge-file`) of both the checker and the test class merges cleanly
in either order; the new tests here are placed at the end of the test class,
after the ones #3213 adds.
Suggested backport: GROOVY_6_0_X, where the same code is present.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
