SONARJAVA-6722 Implement S9142: Expensive compilation or preparation operations should not be performed inside loops

[NoemieBenard]

---

Summary by Gitar

• New checks:
  • Implemented CompilationOrPreparationInLoopCheck (S9142) to detect regex compilation and database preparation inside loops
  • Added sample test sources and unit tests for S9142 rule verification
• Rule metadata:
  • Added rule definition and localization documentation for S9142

_{This will update automatically on new commits.}

[hashicorp-vault-sonar-prod]

SONARJAVA-6722

[github-actions]

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

Please review and merge it into your branch.

[github-actions]

Ruling Diff Summary

Detected changes in 2 rule files: 0 issues removed, 11 issues added.

S9142 (java) on eclipse-jetty - 0 issues removed, 8 issues added - new ruling file

Added jetty-server/src/test/java/org/eclipse/jetty/server/MultiPartCaptureTest.java (line 220)

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

Added jetty-server/src/test/java/org/eclipse/jetty/server/handler/InetAccessHandlerTest.java (line 91)

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

Added jetty-server/src/test/java/org/eclipse/jetty/server/handler/InetAccessHandlerTest.java (line 98)

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

Added jetty-server/src/test/java/org/eclipse/jetty/server/handler/InetAccessHandlerTest.java (line 105)

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

Added jetty-server/src/test/java/org/eclipse/jetty/server/handler/NcsaRequestLogTest.java (line 806)

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

Added jetty-server/src/test/java/org/eclipse/jetty/server/handler/NcsaRequestLogTest.java (line 808)

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

S9142 (java) on sonar-server - 0 issues removed, 3 issues added - new ruling file

Added src/main/java/org/sonar/server/source/DecorationDataHolder.java (line 48)

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

Added src/main/java/org/sonar/server/source/DecorationDataHolder.java (line 60)

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

Added src/main/java/org/sonar/server/source/DecorationDataHolder.java (line 73)

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

[sonarqube-next]

Quality Gate failed

Failed conditions
86.1% Coverage on New Code (required ≥ 90%)

See analysis details on SonarQube

[github-actions]

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

Please review and merge it into your branch.

[nathsou]

Checking for loop invariants sounds like something that would be nice to extract as a common helper, I checked and rule S6909 defines a very similar method:

sonar-java/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementLoopInvariantCheck.java

Line 93 in c9f819a

private static boolean isLoopInvariant(Set<String> declaredOrAssignedLocals, Candidate candidate) {

but your new implementation is much more robust so it might be worth refactoring S6909 and share the method impl. (could be done in another PR as a refactoring or here if you prefer).

[NoemieBenard]

Let's do it in a follow-up PR since it requires some refactoring on S6909

[nathsou]

FP found by an LLM:

The iterable expression in an enhanced for-loop (e.g. for (String s : text.split(";")) or for (String s : Pattern.compile(";").split(text))) is evaluated only once before the loop starts (JLS §14.14.2).

Because isInForInitializer only checks ForStatementTree.initializer(), calls inside ForEachStatement.expression() will be flagged as false positives.

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

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

See analysis details on SonarQube

[gitar-bot]

Code Review ✅ Approved 4 resolved / 4 findings

Implements the S9142 rule to detect expensive compilation or preparation operations inside loops, resolving several edge cases including false positives on loop-once calls and field mutations. No issues found.

✅ 4 resolved

✅ Quality: Misleading message for String instance regex methods

📄 java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java:82-84
For String.matches/replaceAll/replaceFirst/split the message is "Move this "matches" call outside the loop.", but these are instance calls on the per-iteration loop variable and cannot simply be moved out — the correct remediation (per the rule HTML) is to precompile a Pattern once and reuse matcher() inside the loop. Consider a method-specific message advising precompilation for the String-regex matchers versus the Pattern.compile/prepareStatement cases.

✅ Edge Case: Missing test coverage for key edge cases

📄 java-checks-test-sources/default/src/main/java/checks/CompilationOrPreparationInLoopCheckSample.java:1-15
The sample omits several important scenarios: nested loops (invariant relative to inner loop), an argument that is a mutated field (this.field = ...), lambda/anonymous-class bodies inside the loop, and compound assignments (pattern += ...). Adding these would pin down the intended behavior and guard against the false positives described above.

✅ Edge Case: Invariance check misses field mutations, causing false positives

📄 java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java:87-101
isLoopInvariant decides invariance by matching variable names and only tracks assignments/increments whose target is(Tree.Kind.IDENTIFIER). A field reassigned inside the loop via a member select (this.field = ..., this.field++) or mutated through a method call is never added to names, so Pattern.compile(field) in the loop is wrongly flagged as loop-invariant. Prefer resolving the argument's Symbol (semantic model) and confirming it is a local variable/parameter that is not reassigned in the loop; at minimum handle MEMBER_SELECT assignment targets and treat fields conservatively (do not report).

✅ Edge Case: Loop-once check ignores enclosing outer loops

📄 java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java:112-126 📄 java-checks/src/main/java/org/sonar/java/checks/CompilationOrPreparationInLoopCheck.java:92-95
isExecutedOncePerLoop returns true whenever the invocation sits in the closest loop's for-each expression (or for-initializer), but that expression still re-executes on every iteration of any enclosing loop. E.g. while (cond) { for (String s : Pattern.compile(";").split(text)) {...} } compiles the pattern once per outer iteration yet is not flagged, a false negative. Consider re-checking against the next enclosing loop when the invocation is executed once per the closest loop, rather than returning early.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

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}