[
https://issues.apache.org/jira/browse/GROOVY-12696?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18138440#comment-18138440
]
ASF GitHub Bot commented on GROOVY-12696:
-----------------------------------------
codecov-commenter commented on PR #3217:
URL: https://github.com/apache/groovy/pull/3217#issuecomment-6647393307
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/3217?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 74.1550%. Comparing base
([`d86ea7e`](https://app.codecov.io/gh/apache/groovy/commit/d86ea7eb853ec19d069ec99e3ad478f4186b7d54?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`d9a4df8`](https://app.codecov.io/gh/apache/groovy/commit/d9a4df8e1d51b787e640b269bdeb596ce330d42c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/3217?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #3217 +/- ##
==================================================
- Coverage 74.1563% 74.1550% -0.0013%
+ Complexity 43738 43736 -2
==================================================
Files 1692 1692
Lines 149549 149549
Branches 28371 28371
==================================================
- Hits 110900 110898 -2
- Misses 28437 28439 +2
Partials 10212 10212
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/3217?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...ovy/groovy/typecheckers/FormatStringChecker.groovy](https://app.codecov.io/gh/apache/groovy/pull/3217?src=pr&el=tree&filepath=subprojects%2Fgroovy-typecheckers%2Fsrc%2Fmain%2Fgroovy%2Fgroovy%2Ftypecheckers%2FFormatStringChecker.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LXR5cGVjaGVja2Vycy9zcmMvbWFpbi9ncm9vdnkvZ3Jvb3Z5L3R5cGVjaGVja2Vycy9Gb3JtYXRTdHJpbmdDaGVja2VyLmdyb292eQ==)
| `82.9787% <100.0000%> (+2.8369%)` | :arrow_up: |
... and [9 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/3217/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> 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
> Priority: Major
>
> *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)
