Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package checks;

import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.util.List;
import java.util.regex.Pattern;

class CompilationOrPreparationInLoopCheckSample {

private static final String CONSTANT_PATTERN = "[a-z]+";

void patternCompileNoncompliant(List<String> inputs) {
for (String input : inputs) {
Pattern.compile("[a-z]+").matcher(input).find(); // Noncompliant {{Move this "compile" call outside the loop.}}
Comment thread
gitar-bot[bot] marked this conversation as resolved.
//^^^^^^^^^^^^^^^^^^^^^^^^^
}

int i = 0;
while (i++ < inputs.size()) {
Pattern.compile("[a-z]+"); // Noncompliant
}

for (String input : inputs) {
Pattern.compile(CONSTANT_PATTERN).matcher(input).find(); // Noncompliant
}

String invariantPattern = "[a-z]+";
for (String input : inputs) {
Pattern.compile(invariantPattern).matcher(input).find(); // Noncompliant
}
}

void stringMethodsNoncompliant(List<String> inputs) {
for (String input : inputs) {
input.matches("[a-z]+"); // Noncompliant
input.replaceAll("[a-z]+", "X"); // Noncompliant
input.replaceFirst("[a-z]+", "X"); // Noncompliant
input.split("[,;]"); // Noncompliant
}
}

void prepareStatementNoncompliant(Connection conn, List<Integer> ids) throws SQLException {
for (int id : ids) {
PreparedStatement ps = conn.prepareStatement("SELECT * FROM t WHERE id = ?"); // Noncompliant
ps.setInt(1, id);
ps.execute();
ps.close();
}
}

void compliant(List<String> inputs, Connection conn, List<Integer> ids) throws SQLException {
Pattern p = Pattern.compile("[a-z]+");
for (String input : inputs) {
p.matcher(input).find();
}

for (String input : inputs) {
input.toLowerCase(); // not a regex method
}

PreparedStatement ps = conn.prepareStatement("SELECT * FROM t WHERE id = ?");
for (int id : ids) {
ps.setInt(1, id);
ps.execute();
}
}

void patternVariesPerIteration(List<String> patterns, List<String> inputs) {
for (int i = 0; i < inputs.size(); i++) {
String pattern = patterns.get(i);
Pattern.compile(pattern).matcher(inputs.get(i)).find(); // Compliant - pattern changes per iteration
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
/*
* SonarQube Java
* Copyright (C) SonarSource Sàrl
* mailto:info AT sonarsource DOT com
*
* You can redistribute and/or modify this program under the terms of
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
* See the Sonar Source-Available License for more details.
*
* You should have received a copy of the Sonar Source-Available License
* along with this program; if not, see https://sonarsource.com/license/ssal/
*/
package org.sonar.java.checks;

import java.util.Collections;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.sonar.check.Rule;
import org.sonar.java.checks.helpers.TreeHelper;
import org.sonar.java.model.ExpressionUtils;
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
import org.sonar.plugins.java.api.semantic.MethodMatchers;
import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
import org.sonar.plugins.java.api.tree.ExpressionTree;
import org.sonar.plugins.java.api.tree.ForEachStatement;
import org.sonar.plugins.java.api.tree.IdentifierTree;
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
import org.sonar.plugins.java.api.tree.Tree;
import org.sonar.plugins.java.api.tree.UnaryExpressionTree;
import org.sonar.plugins.java.api.tree.VariableTree;

@Rule(key = "S9142")
public class CompilationOrPreparationInLoopCheck extends IssuableSubscriptionVisitor {

private static final Set<Tree.Kind> LOOP_KINDS = EnumSet.of(
Tree.Kind.FOR_STATEMENT, Tree.Kind.FOR_EACH_STATEMENT,
Tree.Kind.WHILE_STATEMENT, Tree.Kind.DO_STATEMENT
);

private static final MethodMatchers MATCHERS = MethodMatchers.or(
MethodMatchers.create()
.ofTypes("java.util.regex.Pattern")
.names("compile")
.withAnyParameters()
.build(),
MethodMatchers.create()
.ofTypes("java.lang.String")
.names("matches", "replaceAll", "replaceFirst", "split")
.withAnyParameters()
.build(),
MethodMatchers.create()
.ofSubTypes("java.sql.Connection")
.names("prepareStatement")
.withAnyParameters()
.build()
);

@Override
public List<Tree.Kind> nodesToVisit() {
return Collections.singletonList(Tree.Kind.METHOD_INVOCATION);
}

@Override
public void visitNode(Tree tree) {
MethodInvocationTree mit = (MethodInvocationTree) tree;
if (!MATCHERS.matches(mit) || mit.arguments().isEmpty()) {
return;
}
Tree loop = TreeHelper.findClosestParentOfKind(mit, LOOP_KINDS);
if (loop == null) {
return;
}
ExpressionTree patternArg = mit.arguments().get(0);
if (isLoopInvariant(patternArg, loop)) {
reportIssue(mit, String.format(
"Move this \"%s\" call outside the loop.", ExpressionUtils.methodName(mit).name()));
}
Comment thread
gitar-bot[bot] marked this conversation as resolved.
Outdated
}

private static boolean isLoopInvariant(ExpressionTree arg, Tree loop) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

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).

if (arg.is(Tree.Kind.IDENTIFIER)) {
var collector = new DeclaredOrAssignedLocalsCollector();
loop.accept(collector);
return !collector.names.contains(((IdentifierTree) arg).name());
}
return ExpressionUtils.resolveAsConstant(arg) != null;
}

private static class DeclaredOrAssignedLocalsCollector extends BaseTreeVisitor {

final Set<String> names = new HashSet<>();

@Override
public void visitVariable(VariableTree tree) {
Comment thread
gitar-bot[bot] marked this conversation as resolved.
super.visitVariable(tree);
names.add(tree.simpleName().name());
}

@Override
public void visitAssignmentExpression(AssignmentExpressionTree tree) {
super.visitAssignmentExpression(tree);
if (tree.variable().is(Tree.Kind.IDENTIFIER)) {
names.add(((IdentifierTree) tree.variable()).name());
}
}

@Override
public void visitUnaryExpression(UnaryExpressionTree tree) {
super.visitUnaryExpression(tree);
switch (tree.kind()) {
case POSTFIX_INCREMENT, POSTFIX_DECREMENT, PREFIX_INCREMENT, PREFIX_DECREMENT -> {
if (tree.expression().is(Tree.Kind.IDENTIFIER)) {
names.add(((IdentifierTree) tree.expression()).name());
}
}
default -> {
// not a mutation
}
}
}

@Override
public void visitForEachStatement(ForEachStatement tree) {
super.visitForEachStatement(tree);
names.add(tree.variable().simpleName().name());
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* SonarQube Java
* Copyright (C) SonarSource Sàrl
* mailto:info AT sonarsource DOT com
*
* You can redistribute and/or modify this program under the terms of
* the Sonar Source-Available License Version 1, as published by SonarSource Sàrl.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
* See the Sonar Source-Available License for more details.
*
* You should have received a copy of the Sonar Source-Available License
* along with this program; if not, see https://sonarsource.com/license/ssal/
*/
package org.sonar.java.checks;

import org.junit.jupiter.api.Test;
import org.sonar.java.checks.verifier.CheckVerifier;

import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;

class CompilationOrPreparationInLoopCheckTest {

@Test
void test() {
CheckVerifier.newVerifier()
.onFile(mainCodeSourcesPath("checks/CompilationOrPreparationInLoopCheckSample.java"))
.withCheck(new CompilationOrPreparationInLoopCheck())
.verifyIssues();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<p>This is an issue when compilation or preparation methods are called inside loop bodies with constant or loop-invariant arguments. This includes
pattern compilation methods for regular expressions, string methods that accept regular expression patterns (such as match, replace, and split
operations), and database statement preparation methods.</p>
<p>In Java, this specifically refers to <code>Pattern.compile()</code>, String regex methods (<code>matches()</code>, <code>replaceAll()</code>,
<code>replaceFirst()</code>, <code>split()</code>), and <code>Connection.prepareStatement()</code>.</p>
<h2>Why is this an issue?</h2>
<p>Compilation and preparation operations are expensive because they involve parsing, validation, and internal representation building. When these
operations are performed inside loops with constant or loop-invariant arguments, the same work is repeated unnecessarily on every iteration.</p>
<h2>Regular expression compilation</h2>
<p>When you call functions that compile regular expressions from strings or use string methods that accept regex patterns, the language runtime
must:</p>
<ul>
<li>Parse the regular expression syntax</li>
<li>Validate the pattern</li>
<li>Build an internal finite automaton (state machine)</li>
<li>Allocate memory for the compiled pattern</li>
</ul>
<p>These steps happen every time, even when the pattern string is identical. For example, calling a string matching method with a pattern like
<code>"\d+"</code> inside a loop that processes 1,000 items means compiling the same pattern 1,000 times.</p>
<h2>Database prepared statement preparation</h2>
<p>When you call methods that create prepared statements from SQL strings, the database driver must:</p>
<ul>
<li>Send the SQL string to the database server</li>
<li>Parse and validate the SQL syntax</li>
<li>Create an execution plan</li>
<li>Allocate server-side resources (cursors, statement handles)</li>
<li>Return a client-side prepared statement object</li>
</ul>
<p>Prepared statements exist specifically to avoid this overhead by allowing you to compile once and execute many times with different parameters.
Calling statement preparation methods inside a loop with the same SQL string defeats this purpose entirely.</p>
<h2>The performance cost</h2>
<p>The repeated compilation/preparation causes:</p>
<ul>
<li><strong>CPU waste</strong>: Parsing and compilation happen repeatedly instead of once</li>
<li><strong>Memory churn</strong>: Temporary objects are created and discarded on each iteration</li>
<li><strong>Network overhead</strong>: For database operations, each preparation call may involve network round-trips</li>
<li><strong>Slower execution</strong>: A loop that should take milliseconds might take seconds when processing large datasets</li>
</ul>
<h3>What is the potential impact?</h3>
<p>The application may experience:</p>
<ul>
<li><strong>Degraded performance</strong>: Operations that should be fast become noticeably slow, especially when processing large collections or
datasets</li>
<li><strong>Resource exhaustion</strong>: For database operations, repeatedly creating parameterized query objects can exhaust server-side cursor or
statement handle limits, causing connection failures</li>
<li><strong>Poor scalability</strong>: The performance penalty multiplies as data volume increases, making the application unable to handle
production workloads efficiently</li>
<li><strong>Increased costs</strong>: Higher CPU usage and longer execution times can lead to increased infrastructure costs in cloud
environments</li>
</ul>
<h2>How to fix it</h2>
<p>For regular expression operations, compile the <code>Pattern</code> once before the loop and reuse it inside the loop. Use the
<code>Pattern.matcher()</code> method to apply the pattern to different input strings.</p>
<h3>Code examples</h3>
<h4>Noncompliant code example</h4>
<pre data-diff-id="1" data-diff-type="noncompliant">
// Direct Pattern.compile in loop
for (String input : inputs) {
Pattern p = Pattern.compile("[a-z]+"); // Noncompliant
Matcher m = p.matcher(input);
if (m.find()) {
handle(m.group());
}
}
</pre>
<h4>Compliant solution</h4>
<pre data-diff-id="1" data-diff-type="compliant">
// Compile once, reuse Pattern
Pattern LOWER = Pattern.compile("[a-z]+");
for (String input : inputs) {
Matcher m = LOWER.matcher(input);
if (m.find()) {
handle(m.group());
}
}
</pre>
<h2>Resources</h2>
<h3>Documentation</h3>
<ul>
<li>Oracle Java Documentation - <a href="https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/regex/Pattern.html">Pattern (Java SE
17 &amp; JDK 17)</a></li>
<li>Oracle Java Documentation - <a
href="https://docs.oracle.com/en/java/javase/17/docs/api/java.sql/java/sql/PreparedStatement.html">PreparedStatement (Java SE 17 &amp; JDK
17)</a></li>
<li>Baeldung - <a href="https://www.baeldung.com/regular-expressions-java">Guide to Java Regular Expressions API</a></li>
<li>SpotBugs Documentation - <a href="https://spotbugs.readthedocs.io/en/latest/bugDescriptions.html#IIL_PATTERN_COMPILE_IN_LOOP">Bug Descriptions -
Pattern compile in loop</a></li>
</ul>

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"title": "Expensive compilation or preparation operations should not be performed inside loops",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5 min"
},
"tags": [
"performance",
"regex",
"sql"
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-9142",
"sqKey": "S9142",
"scope": "All",
"quickfix": "unknown",
"code": {
"impacts": {
"MAINTAINABILITY": "MEDIUM"
},
"attribute": "EFFICIENT"
}
}
Empty file.
Loading