SONARJAVA-7054: Reduce S9397 scope to short format strings without brackets or backticks

[romainbrenguier]

Summary

• Limit S9397 to format strings shorter than 15 characters to avoid flagging longer, more complex templates where String.format is reasonable
• Exclude cases where %s markers appear inside unclosed brackets {}()[] (e.g., "{%s}")
• Exclude cases where %s markers appear between backticks (e.g., "`%s`")

Test plan

• Updated test samples with new compliant cases for long format strings, brackets, parentheses, square brackets, and backticks
• Verified existing noncompliant cases still trigger correctly
• Unit tests pass (both with and without semantic)
• Refactored hasPlaceholderInBracketsOrQuotes to fix SonarQube quality gate violations (S3776, S127, S135)
• Added edge case tests for improved coverage

🤖 Generated with Claude Code

Agent workflow

Tool link: https://github.com/SonarSource/languages-experimental-tooling/tree/romain/my-tickets/personal/romain-brenguier
PR created using uv run create_with_claude.py /tmp/action_plan_romain/sonarjava-7054.txt

PR updated using uv run update_with_claude.py --prompt "We shouldn't just recognize backtick for rule StringFormatCheck but also quotes, simple and double. Fix this." -a "None" -g "claude"

Addressed review comments in pr_report_6227.md using uv run address_reviews.py pr_report_6227.md

Addressed review comments in pr_report_6227.md using uv run address_reviews.py pr_report_6227.md

PR created using uv run create_with_claude.py /tmp/action_plan_romain/sonarjava-7054.txt

PR created using uv run create_with_claude.py /tmp/action_plan_romain/sonarjava-7054.txt

Iterated on the PR with uv run ci_loop.py for 2 iterations.
✔️ The PR is now ready for review.

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6229

Please review and merge it into your branch.

[github-actions]

Ruling Diff Summary

Detected changes in 5 rule files: 396 issues removed, 0 issues added.

S9397 (java) on eclipse-jetty - 32 issues removed, 0 issues added

Removed jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java (line 1972)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java)

Removed jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java (line 1974)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java)

Removed jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java (line 64)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java)

Removed jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MappedResource.java (line 99)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MappedResource.java)

Removed jetty-io/src/main/java/org/eclipse/jetty/io/ConnectionStatistics.java (line 219)

(source file not found at this revision: jetty-io/src/main/java/org/eclipse/jetty/io/ConnectionStatistics.java)

Removed jetty-io/src/main/java/org/eclipse/jetty/io/ConnectionStatistics.java (line 220)

(source file not found at this revision: jetty-io/src/main/java/org/eclipse/jetty/io/ConnectionStatistics.java)

Removed jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MetaData.java (line 249)

(source file not found at this revision: jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MetaData.java)

Removed jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MetaData.java (line 257)

(source file not found at this revision: jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MetaData.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java (line 339)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java (line 931)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java (line 667)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java (line 712)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java (line 714)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java (line 610)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandlerCollection.java (line 374)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandlerCollection.java)

S9397 (java) on eclipse-jetty-similar-to-main - 18 issues removed, 0 issues added

Removed jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java (line 1972)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java)

Removed jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java (line 1974)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java)

Removed jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java (line 64)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java)

Removed jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MappedResource.java (line 99)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/MappedResource.java)

Removed jetty-io/src/main/java/org/eclipse/jetty/io/ConnectionStatistics.java (line 219)

(source file not found at this revision: jetty-io/src/main/java/org/eclipse/jetty/io/ConnectionStatistics.java)

Removed jetty-io/src/main/java/org/eclipse/jetty/io/ConnectionStatistics.java (line 220)

(source file not found at this revision: jetty-io/src/main/java/org/eclipse/jetty/io/ConnectionStatistics.java)

Removed jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MetaData.java (line 249)

(source file not found at this revision: jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MetaData.java)

Removed jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MetaData.java (line 257)

(source file not found at this revision: jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MetaData.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java (line 339)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java (line 931)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java (line 667)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java (line 712)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java (line 714)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java (line 610)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java)

Removed jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandlerCollection.java (line 374)

(source file not found at this revision: jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandlerCollection.java)

S9397 (java) on eclipse-jetty-similar-to-main-small - 1 issues removed, 0 issues added

Removed jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java (line 64)

(source file not found at this revision: jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java)

S9397 (java) on guava - 3 issues removed, 0 issues added

Removed src/com/google/common/eventbus/EventBus.java (line 177)

       172 |     try {
       173 |       exceptionHandler.handleException(e, context);
       174 |     } catch (Throwable e2) {
       175 |       // if the handler threw an exception... well, just log it
       176 |       logger.log(Level.SEVERE,
>>>    177 |           String.format(Locale.ROOT, "Exception %s thrown while handling exception: %s", e2, e),
       178 |           e2);
       179 |     }
       180 |   }
       181 | 
       182 |   /**

Removed src/com/google/common/io/ByteStreams.java (line 394)

       389 |    */
       390 |   public static ByteArrayDataOutput newDataOutput(int size) {
       391 |     // When called at high frequency, boxing size generates too much garbage,
       392 |     // so avoid doing that if we can.
       393 |     if (size < 0) {
>>>    394 |       throw new IllegalArgumentException(String.format("Invalid size: %s", size));
       395 |     }
       396 |     return newDataOutput(new ByteArrayOutputStream(size));
       397 |   }
       398 | 
       399 |   /**

Removed src/com/google/common/util/concurrent/UncaughtExceptionHandlers.java (line 67)

        62 | 
        63 |     @Override public void uncaughtException(Thread t, Throwable e) {
        64 |       try {
        65 |         // cannot use FormattingLogger due to a dependency loop
        66 |         logger.log(SEVERE,
>>>     67 |             String.format(Locale.ROOT, "Caught an exception in %s.  Shutting down.", t), e);
        68 |       } catch (Throwable errorInLogging) {
        69 |         // If logging fails, e.g. due to missing memory, at least try to log the
        70 |         // message and the cause for the failed logging.
        71 |         System.err.println(e.getMessage());
        72 |         System.err.println(errorInLogging.getMessage());

S9397 (java) on sonar-server - 342 issues removed, 0 issues added

Removed src/main/java/org/sonar/ce/http/CeHttpClient.java (line 112)

(source file not found at this revision: src/main/java/org/sonar/ce/http/CeHttpClient.java)

Removed src/main/java/org/sonar/server/app/ProcessCommandWrapperImpl.java (line 89)

(source file not found at this revision: src/main/java/org/sonar/server/app/ProcessCommandWrapperImpl.java)

Removed src/main/java/org/sonar/server/app/ProcessCommandWrapperImpl.java (line 93)

(source file not found at this revision: src/main/java/org/sonar/server/app/ProcessCommandWrapperImpl.java)

Removed src/main/java/org/sonar/server/app/TomcatConnectors.java (line 51)

(source file not found at this revision: src/main/java/org/sonar/server/app/TomcatConnectors.java)

Removed src/main/java/org/sonar/server/app/TomcatContexts.java (line 75)

(source file not found at this revision: src/main/java/org/sonar/server/app/TomcatContexts.java)

Removed src/main/java/org/sonar/server/app/TomcatContexts.java (line 85)

(source file not found at this revision: src/main/java/org/sonar/server/app/TomcatContexts.java)

Removed src/main/java/org/sonar/server/authentication/AuthenticationError.java (line 69)

(source file not found at this revision: src/main/java/org/sonar/server/authentication/AuthenticationError.java)

Removed src/main/java/org/sonar/server/authentication/AuthenticationError.java (line 81)

(source file not found at this revision: src/main/java/org/sonar/server/authentication/AuthenticationError.java)

Removed src/main/java/org/sonar/server/authentication/AuthenticationFilter.java (line 58)

(source file not found at this revision: src/main/java/org/sonar/server/authentication/AuthenticationFilter.java)

Removed src/main/java/org/sonar/server/authentication/IdentityProviderRepository.java (line 54)

(source file not found at this revision: src/main/java/org/sonar/server/authentication/IdentityProviderRepository.java)

Removed src/main/java/org/sonar/server/authentication/InitFilter.java (line 82)

(source file not found at this revision: src/main/java/org/sonar/server/authentication/InitFilter.java)

Removed src/main/java/org/sonar/server/authentication/InitFilter.java (line 88)

(source file not found at this revision: src/main/java/org/sonar/server/authentication/InitFilter.java)

Removed src/main/java/org/sonar/server/authentication/OAuth2CallbackFilter.java (line 77)

(source file not found at this revision: src/main/java/org/sonar/server/authentication/OAuth2CallbackFilter.java)

Removed src/main/java/org/sonar/server/authentication/OAuth2CallbackFilter.java (line 83)

(source file not found at this revision: src/main/java/org/sonar/server/authentication/OAuth2CallbackFilter.java)

Removed src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java (line 81)

(source file not found at this revision: src/main/java/org/sonar/server/authentication/OAuth2ContextFactory.java)

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6230

Please review and merge it into your branch.

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6230

Please review and merge it into your branch.

[nathsou]

I think we should also add an exception for single quotes in String.format, not just backticks?
Otherwise LGTM!

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6231

Please review and merge it into your branch.

[nathsou]

The new quote handling still needs to distinguish a quoted range from an apostrophe in ordinary text.

[romainbrenguier]

Addressed review feedback in 9b3dbb1: refactored hasPlaceholderInBracketsOrQuotes to use hasPlaceholderInMatchingQuotes which looks ahead for a matching closing quote via indexOf before checking for %s between the delimiters. Apostrophes in possessives ("%s's %s") and contractions ("it's %s") are now correctly reported as Noncompliant. Also fixed %%s handling and removed the unreachable double-quote branch.

[nathsou]

The possessive cases are fixed. One quote-pairing case still suppresses a placeholder outside both quoted spans.

[nathsou]

Approved. I left an inline comment on the remaining quote-pairing edge case.

[github-actions]

❌ Ruling needs updating. A fix PR has been created: #6231

Please review and merge it into your branch.

[gitar-bot]

Code Review ✅ Approved 5 closed / 5 findings

🟡 Medium risk · The rule now skips longer or delimited format strings, changing issue detection.

Reduces S9397 scope to short format strings without brackets or backticks by excluding %s markers inside unclosed brackets {}()[] and between backticks, and limiting the rule to strings shorter than 15 characters. Resolved issues with escaped %%s handling, quote detection logic, and span pairing. No issues remain.

✅ 5 closed

✅ Edge Case: Bracket/quote scan treats escaped %%s as a %s placeholder

📄 java-checks/src/main/java/org/sonar/java/checks/StringFormatCheck.java:82-96 📄 java-checks/src/main/java/org/sonar/java/checks/StringFormatCheck.java:108-122
hasPlaceholderInBracketsOrQuotes checks each % character on its own and doesn't skip %% pairs the way countSimplePlaceholders does. For String.format("[%%s] %s", v) (8 chars), the counter finds one real placeholder, outside the brackets. The scanner, though, reads the second % of %% plus the s after it as a %s at depth 1 and returns true. The call gets dropped even though the real placeholder is outside the brackets. The same thing happens inside quotes, e.g. "'%%s' %s". Fix: when the scanner sees %, skip the next character the same way countSimplePlaceholders does, so %% is consumed as one token.

✅ Edge Case: A lone apostrophe excludes every later %s, not just ones between quotes

📄 java-checks/src/main/java/org/sonar/java/checks/StringFormatCheck.java:87-96
The PR says it excludes %s that appears between single quotes. The code flips inQuotes on every ' and never requires a closing quote, so an apostrophe in a possessive or contraction marks everything after it as quoted. For example, String.format("%s's %s", owner, item) (7 chars, two plain placeholders) is no longer reported, and neither is "it's %s". Neither placeholder is actually enclosed in quotes. Fix: only treat a %s as quoted when a matching closing ' follows it, e.g. look ahead for the closing quote before returning true, or collect the quote ranges first.

✅ Quality: Double-quote case in quote detection can never run

📄 java-checks/src/main/java/org/sonar/java/checks/StringFormatCheck.java:62-63 📄 java-checks/src/main/java/org/sonar/java/checks/StringFormatCheck.java:87 📄 java-checks-test-sources/default/src/main/java/checks/StringFormatCheckSample.java:176-178
rawValue is the literal's source text with the outer quotes trimmed. In a Java string literal, a " has to be written as ", and text blocks never reach this code because it only accepts Tree.Kind.STRING_LITERAL. So any format string with a double quote in it already contains a backslash and hits the early return at line 63 before hasPlaceholderInBracketsOrQuotes is called. That makes the new c == '"' branch dead code, so the commit's claim that it extends detection to double quotes adds nothing. That also explains why the commit adds a test for backticks but none for double quotes. Either remove '"' from the condition, or add a sample such as String.format(""%s"", v) and note that the backslash guard is what makes it compliant.

✅ Quality: PR description says single quotes are excluded; code now excludes backticks

📄 java-checks/src/main/java/org/sonar/java/checks/StringFormatCheck.java:89-103 📄 java-checks-test-sources/default/src/main/java/checks/StringFormatCheckSample.java:172-178
The PR description says the rule skips %s placed between single quotes (its example is "'%s'"). This commit changed hasPlaceholderInBracketsOrQuotes to track backticks only. The sample file now marks String.format("'%s'", value) as Noncompliant and "`%s`" as compliant, so the code does the opposite of what the description promises for single quotes. Reviewers and release notes that rely on the description will expect behaviour the rule does not have. Update the PR description and title to say backticks, or bring back single-quote handling if that exclusion is still wanted.

✅ Edge Case: Closing quote is reused as an opening quote, so %s between quoted spans is exempted

📄 java-checks/src/main/java/org/sonar/java/checks/StringFormatCheck.java:112-123
hasPlaceholderInMatchingQuotes looks at every ' or ` in the string, including ones that already closed a pair. Each one is treated as a new opening quote. Take "'a' %s 'b'" (10 chars, so under the length limit). At i=0 the pair 0..2 has no placeholder. The loop then goes to i=2 and pairs the closing quote at 2 with the opening quote at 7. That span contains %s, so the method returns true. The call is treated as compliant even though %s sits between two quoted spans and is inside neither. Backticks do the same: "`a` %s `b`". Fix: after checking a matched pair, move i to close so the closing delimiter can't open another pair. If there is no closing quote, just move on.

Review coverage

🧪 Functional validation 1 of 1 objectives covered

📋 Rules No rules evaluated

🤖 Auto-approval Not enabled · Set up

Implementation Status ✅ 1 of 1 objectives covered

✅ SONARJAVA-7054 - 1 of 1 objectives covered

This PR covers the objective to reduce the scope of S9397 to short format strings without brackets or quotes.

✅ 1 covered here

• ✅ Reduce scope of S9397 to short format strings without brackets or quotes

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
96.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube