RC-287 - Restrict S9142 to prepared statements

[NoemieBenard]

---

Summary by Gitar

• Rule Updates:
  • Renamed CompilationOrPreparationInLoopCheck to PreparedStatementInsideLoopCheck for rule S9142
  • Restricted rule scope from regex compilation and string methods to PreparedStatement creations only

_{This will update automatically on new commits.}

[hashicorp-vault-sonar-prod]

RC-287

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
1 Fixed issue
0 Accepted issues

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

See analysis details on SonarQube

[gitar-bot]

⚠️ Quality: Deleted samples leave retained loop-detection paths untested

The old sample exercised two scenarios of isExecutedOncePerLoop that the new sample no longer covers: a call in a for-each expression (for (String s : Pattern.compile(";").split(text)) → compliant, hitting the current == anchor true branch for ForEachStatement) and the same construct nested inside an outer loop (forEachIterableInsideOuterLoop → the path at lines 70-78 where the outer-loop lookup succeeds and an issue is reported). Both branches are still reachable with prepareStatement and are now completely untested, so a regression in the once-per-iteration/nested-loop handling would pass CI, and the renamed file will show uncovered branches on new code.

Re-add the two dropped scenarios to PreparedStatementInsideLoopCheckSample, expressed with prepareStatement instead of Pattern.compile.:

void forEachExpression(Connection conn) throws SQLException {
  for (PreparedStatement p : List.of(conn.prepareStatement("SELECT 1"))) { // Compliant: expression evaluated once
    p.close();
  }
}

void forEachExpressionInsideOuterLoop(Connection conn, List<Integer> ids) throws SQLException {
  for (int id : ids) {
    for (PreparedStatement p : List.of(conn.prepareStatement("SELECT 1"))) { // Noncompliant
      p.close();
    }
  }
}

• Apply fix

_{Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

💡 Quality: S9142 description scope and leftover generic wording are inaccurate

The new intro says the issue occurs when the statement is created "inside a loop body", but the check reports any prepareStatement call under a loop except the for initializer / for-each expression — the sample itself marks a call in a for condition as Noncompliant (for (; conn.prepareStatement("SELECT 1") != null; )). The impact bullets also still carry the pre-narrowing generic phrasing ("For database operations, ...", "especially when processing large collections or datasets") which implies non-database operations are in scope. Reword the intro to "inside a loop" and drop the "For database operations" qualifier now that the rule only covers prepared statements.

Fix 1: Match the documented scope to the implemented one (any position inside the loop, not just the body).

<p>This is an issue when a database <code>PreparedStatement</code> object is created inside a loop with a constant or loop-invariant SQL string
argument.</p>

• Apply fix

Fix 2: Remove the leftover "For database operations" qualifier from the impact list.

<li><strong>Resource exhaustion</strong>: Repeatedly creating parameterized query objects can exhaust server-side cursor or
statement handle limits, causing connection failures</li>

• Apply fix

_{Check a box to apply a fix or reply for a change | Was this helpful? React with 👍 / 👎}

[gitar-bot]

Code Review ⚠️ Changes requested 0 resolved / 2 findings

Refactors rule S9142 to focus exclusively on PreparedStatement creations inside loops, renaming the check accordingly. However, test coverage for loop-detection logic has regressed: deleted samples no longer exercise the for-each expression and nested-loop branches that remain reachable in the code, risking undetected regressions. Additionally, the rule description inaccurately states the issue occurs "inside a loop body" when the check actually reports calls in loop conditions, and retains generic pre-narrowing language ("For database operations...") that no longer applies now that the scope is restricted to prepared statements.

⚠️ Quality: Deleted samples leave retained loop-detection paths untested

📄 java-checks/src/main/java/org/sonar/java/checks/PreparedStatementInsideLoopCheck.java:70-75 📄 java-checks/src/main/java/org/sonar/java/checks/PreparedStatementInsideLoopCheck.java:98-101 📄 java-checks-test-sources/default/src/main/java/checks/PreparedStatementInsideLoopCheckSample.java:36-43

The old sample exercised two scenarios of isExecutedOncePerLoop that the new sample no longer covers: a call in a for-each expression (for (String s : Pattern.compile(";").split(text)) → compliant, hitting the current == anchor true branch for ForEachStatement) and the same construct nested inside an outer loop (forEachIterableInsideOuterLoop → the path at lines 70-78 where the outer-loop lookup succeeds and an issue is reported). Both branches are still reachable with prepareStatement and are now completely untested, so a regression in the once-per-iteration/nested-loop handling would pass CI, and the renamed file will show uncovered branches on new code.

Re-add the two dropped scenarios to PreparedStatementInsideLoopCheckSample, expressed with prepareStatement instead of Pattern.compile.

void forEachExpression(Connection conn) throws SQLException {
  for (PreparedStatement p : List.of(conn.prepareStatement("SELECT 1"))) { // Compliant: expression evaluated once
    p.close();
  }
}

void forEachExpressionInsideOuterLoop(Connection conn, List<Integer> ids) throws SQLException {
  for (int id : ids) {
    for (PreparedStatement p : List.of(conn.prepareStatement("SELECT 1"))) { // Noncompliant
      p.close();
    }
  }
}

💡 Quality: S9142 description scope and leftover generic wording are inaccurate

📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9142.html:1-2 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9142.html:9-12 📄 java-checks-test-sources/default/src/main/java/checks/PreparedStatementInsideLoopCheckSample.java:40

The new intro says the issue occurs when the statement is created "inside a loop body", but the check reports any prepareStatement call under a loop except the for initializer / for-each expression — the sample itself marks a call in a for condition as Noncompliant (for (; conn.prepareStatement("SELECT 1") != null; )). The impact bullets also still carry the pre-narrowing generic phrasing ("For database operations, ...", "especially when processing large collections or datasets") which implies non-database operations are in scope. Reword the intro to "inside a loop" and drop the "For database operations" qualifier now that the rule only covers prepared statements.

Match the documented scope to the implemented one (any position inside the loop, not just the body).

<p>This is an issue when a database <code>PreparedStatement</code> object is created inside a loop with a constant or loop-invariant SQL string
argument.</p>

Remove the leftover "For database operations" qualifier from the impact list.

<li><strong>Resource exhaustion</strong>: Repeatedly creating parameterized query objects can exhaust server-side cursor or
statement handle limits, causing connection failures</li>

🤖 Prompt for agents

Code Review: Refactors rule S9142 to focus exclusively on `PreparedStatement` creations inside loops, renaming the check accordingly. However, test coverage for loop-detection logic has regressed: deleted samples no longer exercise the for-each expression and nested-loop branches that remain reachable in the code, risking undetected regressions. Additionally, the rule description inaccurately states the issue occurs "inside a loop body" when the check actually reports calls in loop conditions, and retains generic pre-narrowing language ("For database operations...") that no longer applies now that the scope is restricted to prepared statements.

1. ⚠️ Quality: Deleted samples leave retained loop-detection paths untested
   Files: java-checks/src/main/java/org/sonar/java/checks/PreparedStatementInsideLoopCheck.java:70-75, java-checks/src/main/java/org/sonar/java/checks/PreparedStatementInsideLoopCheck.java:98-101, java-checks-test-sources/default/src/main/java/checks/PreparedStatementInsideLoopCheckSample.java:36-43

   The old sample exercised two scenarios of `isExecutedOncePerLoop` that the new sample no longer covers: a call in a for-each *expression* (`for (String s : Pattern.compile(";").split(text))` → compliant, hitting the `current == anchor` true branch for `ForEachStatement`) and the same construct nested inside an outer loop (`forEachIterableInsideOuterLoop` → the path at lines 70-78 where the outer-loop lookup succeeds and an issue is reported). Both branches are still reachable with `prepareStatement` and are now completely untested, so a regression in the once-per-iteration/nested-loop handling would pass CI, and the renamed file will show uncovered branches on new code.

   Fix (Re-add the two dropped scenarios to PreparedStatementInsideLoopCheckSample, expressed with prepareStatement instead of Pattern.compile.):
   void forEachExpression(Connection conn) throws SQLException {
     for (PreparedStatement p : List.of(conn.prepareStatement("SELECT 1"))) { // Compliant: expression evaluated once
       p.close();
     }
   }
   
   void forEachExpressionInsideOuterLoop(Connection conn, List<Integer> ids) throws SQLException {
     for (int id : ids) {
       for (PreparedStatement p : List.of(conn.prepareStatement("SELECT 1"))) { // Noncompliant
         p.close();
       }
     }
   }

2. 💡 Quality: S9142 description scope and leftover generic wording are inaccurate
   Files: sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9142.html:1-2, sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9142.html:9-12, java-checks-test-sources/default/src/main/java/checks/PreparedStatementInsideLoopCheckSample.java:40

   The new intro says the issue occurs when the statement is created "inside a loop body", but the check reports any `prepareStatement` call under a loop except the `for` initializer / for-each expression — the sample itself marks a call in a `for` condition as Noncompliant (`for (; conn.prepareStatement("SELECT 1") != null; )`). The impact bullets also still carry the pre-narrowing generic phrasing ("For database operations, ...", "especially when processing large collections or datasets") which implies non-database operations are in scope. Reword the intro to "inside a loop" and drop the "For database operations" qualifier now that the rule only covers prepared statements.

   Fix (Match the documented scope to the implemented one (any position inside the loop, not just the body).):
   <p>This is an issue when a database <code>PreparedStatement</code> object is created inside a loop with a constant or loop-invariant SQL string
   argument.</p>

   Fix (Remove the leftover "For database operations" qualifier from the impact list.):
   <li><strong>Resource exhaustion</strong>: Repeatedly creating parameterized query objects can exhaust server-side cursor or
   statement handle limits, causing connection failures</li>

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.
Unblock → Override a blocking verdict and allow merging.

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

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

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