From e3724dd744eb0bf79bb2880021efd24e6b465ed0 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Wed, 21 Oct 2026 12:42:20 +0400 Subject: [PATCH 1/5] SONARJAVA-7215 Do not report S2695 when part of the query cannot be resolved The placeholder count was a lower bound compared as if it were an upper bound: an unresolvable query fragment was counted as zero placeholders, so the rule asserted "has no parameters" on queries it had not resolved. - propagate unknown through concatenation and "+=" instead of substituting zero for an unresolved fragment - resolve a conditional expression as the maximum of its branches - count over the folded constant via asConstant, which resolves cross-class constants, whole constant chains and text blocks Co-Authored-By: Claude Opus 5 (1M context) --- ...paredStatementAndResultSetCheckSample.java | 101 +++++++++++++++++- .../PreparedStatementAndResultSetCheck.java | 63 ++++++++--- 2 files changed, 147 insertions(+), 17 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java index 23275de5ea0..a89c194b8bc 100644 --- a/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java @@ -155,10 +155,10 @@ public void updateCoffeeSales(HashMap salesForWeek, Connection updateString = "update " + param + ".COFFEES set SALES = ?"; PreparedStatement testParam = con.prepareStatement(updateString); - testParam.setInt(3, 0); // Noncompliant + testParam.setInt(3, 0); // Compliant - "param" is unknown and may itself contain placeholders testParam = con.prepareStatement(param + " update"); - testParam.setInt(3, 0); // Noncompliant + testParam.setInt(3, 0); // Compliant - "param" is unknown and may itself contain placeholders testParam = con.prepareStatement(param + param); testParam.setInt(3, 0); // Compliant @@ -257,4 +257,101 @@ void concatenation3(Connection con, String password, String name, int position) statement.setString(3, password); // compliant statement.setString(4, password); // Noncompliant } + + static class SqlConstants { + static final String SELECT = "SELECT * FROM t WHERE a = ?"; + static final String AND_CLAUSE = " AND b = ?"; + } + + private static final String COUNT_WRAPPER_BEGIN = "select count(*) from ("; + private static final String COUNT_WRAPPER_END = ")"; + + void unknownMethodCallInConcatenation(Connection con) throws SQLException { + String sql = getQuery(); + sql = COUNT_WRAPPER_BEGIN + sql + COUNT_WRAPPER_END; + + PreparedStatement ps = con.prepareStatement(sql); + ps.setInt(1, 1); // Compliant - part of the query is unknown + } + + void unknownMethodCallInPlusAssignment(Connection con) throws SQLException { + String sql = "select * from t where "; + sql += getQuery(); + + PreparedStatement ps = con.prepareStatement(sql); + ps.setInt(1, 1); // Compliant - part of the query is unknown + } + + void ternaryInPlusAssignment(Connection con, boolean isRecall) throws SQLException { + String sql = "SELECT * FROM t WHERE "; + sql += isRecall ? "LENDER_REF_ID = ? AND SENDER_ID = ?" : "BORROWER_REF_ID = ? AND RECIPIENT_ID = ?"; + + PreparedStatement ps = con.prepareStatement(sql); + ps.setString(1, "a"); // Compliant + ps.setLong(2, 1L); // Compliant + ps.setLong(3, 1L); // Noncompliant + } + + void ternaryWithDifferentBranches(Connection con, boolean withImage) throws SQLException { + String sql = "INSERT INTO Product (a, b" + (withImage ? ", c" : "") + ") VALUES (?, ?" + (withImage ? ", ?" : "") + ")"; + + PreparedStatement ps = con.prepareStatement(sql); + ps.setString(1, "a"); // Compliant + ps.setString(2, "b"); // Compliant + ps.setString(3, "c"); // Compliant - the third parameter exists when the condition holds + ps.setString(4, "d"); // Noncompliant + } + + void conditionalClauseGuardedBySameCondition(Connection con, String status) throws SQLException { + String sql = SqlConstants.SELECT + + (status != null ? " AND status = ?" : "") + + " ORDER BY id"; + + PreparedStatement ps = con.prepareStatement(sql); + ps.setString(1, "a"); // Compliant + if (status != null) { + ps.setString(2, status); // Compliant - the clause holding the second placeholder is added under the same condition + } + ps.setString(3, "c"); // Noncompliant + } + + void ternaryWithNonConstantFragment(Connection con, String tenant, int maxTurns) throws SQLException { + String sql = "SELECT * FROM m WHERE m.tenant = ? AND m.session_id = ? " + + (maxTurns > 0 ? "AND t.id IN (SELECT id FROM t WHERE tenant=? AND session=? LIMIT " + maxTurns + ") " : "") + + "ORDER BY m.seq"; + + PreparedStatement ps = con.prepareStatement(sql); + ps.setString(1, tenant); // Compliant + if (maxTurns > 0) { + ps.setString(3, tenant); // Compliant - part of the query is unknown + ps.setString(4, tenant); // Compliant - part of the query is unknown + } + } + + void assignedInBothBranches(Connection con, boolean hasPassword) throws SQLException { + String sql; + if (hasPassword) { + sql = "UPDATE Account SET email = ?, name = ?, password = ? WHERE id = ?"; + } else { + sql = "UPDATE Account SET email = ?, name = ? WHERE id = ?"; + } + + PreparedStatement ps = con.prepareStatement(sql); + ps.setString(1, "a"); // Compliant + if (hasPassword) { + // FALSE POSITIVE: only the closest assignment is resolved, while the branch taken here has 4 parameters. + // Can't be fixed without dataflow analysis. + ps.setString(4, "b"); // Noncompliant + } + } + + void constantsFromAnotherClass(Connection con) throws SQLException { + String sql = SqlConstants.SELECT; + sql += SqlConstants.AND_CLAUSE; + + PreparedStatement ps = con.prepareStatement(sql); + ps.setString(1, "a"); // Compliant + ps.setString(2, "b"); // Compliant + ps.setString(3, "c"); // Noncompliant + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java b/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java index 22c1b6d0a8d..82d5c529b28 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java @@ -29,9 +29,9 @@ import org.sonar.plugins.java.api.tree.Arguments; import org.sonar.plugins.java.api.tree.AssignmentExpressionTree; import org.sonar.plugins.java.api.tree.BinaryExpressionTree; +import org.sonar.plugins.java.api.tree.ConditionalExpressionTree; import org.sonar.plugins.java.api.tree.ExpressionTree; import org.sonar.plugins.java.api.tree.IdentifierTree; -import org.sonar.plugins.java.api.tree.LiteralTree; import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; import org.sonar.plugins.java.api.tree.Tree; @@ -107,29 +107,41 @@ private static Integer getPreparedStatementNumberOfParameters(@Nullable Expressi } /** - * Count the number of query in the given expression. The method will reconstruct the string, if created in multiples steps. - * During the process, we are looking for closest reassignment of a variable. We will look for any reassignment before - * the first token of the argument "startingPointForReassignment". It is used to avoid infinite recursion in case the variable - * is reassigned and read in the same statement. + * Count the number of query parameters in the given expression. The method will reconstruct the string, if created in + * multiples steps. During the process, we are looking for closest reassignment of a variable. We will look for any + * reassignment before the first token of the argument "startingPointForReassignment". It is used to avoid infinite + * recursion in case the variable is reassigned and read in the same statement. + * Returns {@code null} as soon as any part of the query can not be resolved: an unresolved fragment may itself hold + * parameter placeholders, so the total count is unknown rather than zero. */ @CheckForNull private static Integer getNumberQuery(ExpressionTree expression, Tree startingPointForReassignment) { ExpressionTree expr = ExpressionUtils.skipParentheses(expression); + Integer constantCount = countQuery(expr); + if (constantCount != null) { + return constantCount; + } if (expr.is(Tree.Kind.IDENTIFIER)) { return handleVariableUsedAsQuery((IdentifierTree) expr, startingPointForReassignment); - } else if (expr.is(Tree.Kind.PLUS)) { + } + if (expr.is(Tree.Kind.PLUS)) { return handleStringConcatenation((BinaryExpressionTree) expr, startingPointForReassignment); } - return countQuery(expr); + if (expr.is(Tree.Kind.CONDITIONAL_EXPRESSION)) { + return handleConditionalExpression((ConditionalExpressionTree) expr, startingPointForReassignment); + } + return null; } + @CheckForNull private static Integer handleVariableUsedAsQuery(IdentifierTree identifier, Tree startingPointForReassignment) { ExpressionTree lastAssignmentExpr = ReassignmentFinder.getClosestReassignmentOrDeclarationExpression(startingPointForReassignment, identifier.symbol()); if (lastAssignmentExpr != null) { Tree lastAssignment = lastAssignmentExpr.parent(); if (lastAssignment.is(Tree.Kind.PLUS_ASSIGNMENT)) { - return zeroIfNull(getNumberQuery(lastAssignmentExpr, lastAssignment)) - + zeroIfNull(getNumberQuery(((AssignmentExpressionTree) lastAssignment).variable(), lastAssignment)); + return sumOrNull( + getNumberQuery(((AssignmentExpressionTree) lastAssignment).variable(), lastAssignment), + getNumberQuery(lastAssignmentExpr, lastAssignment)); } // If the current assignment contains the identifier, we have to update the starting point for reassignment to avoid infinite recursion. return getNumberQuery(lastAssignmentExpr, lastAssignment); @@ -137,18 +149,39 @@ private static Integer handleVariableUsedAsQuery(IdentifierTree identifier, Tree return null; } + @CheckForNull private static Integer handleStringConcatenation(BinaryExpressionTree expr, Tree startingPointForReassignment) { - Integer left = getNumberQuery(expr.leftOperand(), startingPointForReassignment); - Integer right = getNumberQuery(expr.rightOperand(), startingPointForReassignment); - return (left == null && right == null) ? null : (zeroIfNull(left) + zeroIfNull(right)); + return sumOrNull( + getNumberQuery(expr.leftOperand(), startingPointForReassignment), + getNumberQuery(expr.rightOperand(), startingPointForReassignment)); + } + + /** + * A conditional expression contributes as many placeholders as its most parameterized branch: reporting against the + * smaller branch would flag indices which are valid whenever the other branch is taken. + */ + @CheckForNull + private static Integer handleConditionalExpression(ConditionalExpressionTree expr, Tree startingPointForReassignment) { + Integer trueBranch = getNumberQuery(expr.trueExpression(), startingPointForReassignment); + Integer falseBranch = getNumberQuery(expr.falseExpression(), startingPointForReassignment); + if (trueBranch == null || falseBranch == null) { + return null; + } + return Math.max(trueBranch, falseBranch); } - private static int zeroIfNull(@Nullable Integer intValue) { - return intValue == null ? 0 : intValue; + @CheckForNull + private static Integer sumOrNull(@Nullable Integer left, @Nullable Integer right) { + if (left == null || right == null) { + return null; + } + return left + right; } @CheckForNull private static Integer countQuery(ExpressionTree expression) { - return expression.is(Tree.Kind.STRING_LITERAL) ? StringUtils.countMatches(((LiteralTree) expression).value(), "?") : null; + return expression.asConstant(String.class) + .map(query -> StringUtils.countMatches(query, "?")) + .orElse(null); } } From 8d329d822c9350dc26de0eb8839e693ce0602f30 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Wed, 21 Oct 2026 17:16:43 +0400 Subject: [PATCH 2/5] SONARJAVA-7215 Keep counting S2695 placeholders across a numeric fragment Propagating "unknown" through every unresolved operand went too far: a non-constant number or boolean concatenated into the query made the whole count unknown, dropping correct reports on queries such as "... WHERE id = ? LIMIT " + limit. Such an operand cannot contribute a placeholder whatever its runtime value, so it now counts as zero and the surrounding count stays exact. Characters are excluded, since '?' is a valid one. Co-Authored-By: Claude Opus 5 (1M context) --- ...paredStatementAndResultSetCheckSample.java | 19 +++++++++++++++++++ .../PreparedStatementAndResultSetCheck.java | 15 +++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java index a89c194b8bc..226e0d7fb5f 100644 --- a/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java @@ -345,6 +345,25 @@ void assignedInBothBranches(Connection con, boolean hasPassword) throws SQLExcep } } + void nonConstantNumericFragment(Connection con, int limit, Integer offset) throws SQLException { + String sql = "SELECT * FROM t WHERE id = ? LIMIT " + limit + " OFFSET " + offset; + + PreparedStatement ps = con.prepareStatement(sql); + ps.setInt(1, 1); // Compliant + // A number concatenated into the query can not add a placeholder, so the count stays exact. + ps.setInt(2, 2); // Noncompliant + } + + void nonConstantNumericFragmentFromMethod(Connection con) throws SQLException { + PreparedStatement ps = con.prepareStatement("SELECT * FROM t WHERE id = ? LIMIT " + getIntValue()); + ps.setInt(2, 2); // Noncompliant + } + + void nonConstantCharFragment(Connection con, char separator) throws SQLException { + PreparedStatement ps = con.prepareStatement("SELECT * FROM t WHERE id = ? AND flag = " + separator); + ps.setInt(2, 2); // Compliant - the character may be '?' + } + void constantsFromAnotherClass(Connection con) throws SQLException { String sql = SqlConstants.SELECT; sql += SqlConstants.AND_CLAUSE; diff --git a/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java b/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java index 82d5c529b28..1eae2d979fe 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java @@ -26,6 +26,8 @@ import org.sonar.java.model.ExpressionUtils; import org.sonar.plugins.java.api.semantic.MethodMatchers; import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.semantic.Type; +import org.sonar.plugins.java.api.semantic.Type.Primitives; import org.sonar.plugins.java.api.tree.Arguments; import org.sonar.plugins.java.api.tree.AssignmentExpressionTree; import org.sonar.plugins.java.api.tree.BinaryExpressionTree; @@ -117,6 +119,9 @@ private static Integer getPreparedStatementNumberOfParameters(@Nullable Expressi @CheckForNull private static Integer getNumberQuery(ExpressionTree expression, Tree startingPointForReassignment) { ExpressionTree expr = ExpressionUtils.skipParentheses(expression); + if (cannotHoldPlaceholder(expr)) { + return 0; + } Integer constantCount = countQuery(expr); if (constantCount != null) { return constantCount; @@ -170,6 +175,16 @@ private static Integer handleConditionalExpression(ConditionalExpressionTree exp return Math.max(trueBranch, falseBranch); } + /** + * Numbers and booleans contribute no placeholder whatever their runtime value, so concatenating one keeps the count + * of the surrounding query exact. Characters are excluded: '?' is a valid one. + */ + private static boolean cannotHoldPlaceholder(ExpressionTree expression) { + Type type = expression.symbolType(); + Type primitiveType = type.isPrimitiveWrapper() ? type.primitiveType() : type; + return primitiveType != null && primitiveType.isPrimitive() && !primitiveType.isPrimitive(Primitives.CHAR); + } + @CheckForNull private static Integer sumOrNull(@Nullable Integer left, @Nullable Integer right) { if (left == null || right == null) { From 4162de3f6f8cfce8c85a53b61a541bcb4a4af1d3 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Fri, 23 Oct 2026 11:10:43 +0400 Subject: [PATCH 3/5] SONARJAVA-7215 Bound the S2695 placeholders a character fragment can add A char concatenated into the query was treated as wholly unknown, which made the surrounding count unknown too. But a char is one character, so it adds at most one placeholder: the count is bounded, not unresolvable, and an index past that bound is still reportable. Replace the boolean guard with a bound - zero for a number or boolean, one for a character, unknown for anything else. Co-Authored-By: Claude Opus 5 (1M context) --- ...eparedStatementAndResultSetCheckSample.java | 2 ++ .../PreparedStatementAndResultSetCheck.java | 18 ++++++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java index 226e0d7fb5f..d59a26510af 100644 --- a/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java @@ -362,6 +362,8 @@ void nonConstantNumericFragmentFromMethod(Connection con) throws SQLException { void nonConstantCharFragment(Connection con, char separator) throws SQLException { PreparedStatement ps = con.prepareStatement("SELECT * FROM t WHERE id = ? AND flag = " + separator); ps.setInt(2, 2); // Compliant - the character may be '?' + // A character adds at most one placeholder, so anything past that is still reportable. + ps.setInt(3, 2); // Noncompliant {{This "PreparedStatement" only has 2 parameters.}} } void constantsFromAnotherClass(Connection con) throws SQLException { diff --git a/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java b/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java index 1eae2d979fe..4cbc686c16a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java @@ -119,8 +119,9 @@ private static Integer getPreparedStatementNumberOfParameters(@Nullable Expressi @CheckForNull private static Integer getNumberQuery(ExpressionTree expression, Tree startingPointForReassignment) { ExpressionTree expr = ExpressionUtils.skipParentheses(expression); - if (cannotHoldPlaceholder(expr)) { - return 0; + Integer unresolvedBound = unresolvedPlaceholderBound(expr); + if (unresolvedBound != null) { + return unresolvedBound; } Integer constantCount = countQuery(expr); if (constantCount != null) { @@ -176,13 +177,18 @@ private static Integer handleConditionalExpression(ConditionalExpressionTree exp } /** - * Numbers and booleans contribute no placeholder whatever their runtime value, so concatenating one keeps the count - * of the surrounding query exact. Characters are excluded: '?' is a valid one. + * Upper bound on the placeholders an operand can contribute when its value is unknown: none for a number or a + * boolean, whatever their runtime value, and at most one for a character, since '?' is a valid one. Returns + * {@code null} for anything that can hold an arbitrary number of them. */ - private static boolean cannotHoldPlaceholder(ExpressionTree expression) { + @CheckForNull + private static Integer unresolvedPlaceholderBound(ExpressionTree expression) { Type type = expression.symbolType(); Type primitiveType = type.isPrimitiveWrapper() ? type.primitiveType() : type; - return primitiveType != null && primitiveType.isPrimitive() && !primitiveType.isPrimitive(Primitives.CHAR); + if (primitiveType == null || !primitiveType.isPrimitive()) { + return null; + } + return primitiveType.isPrimitive(Primitives.CHAR) ? 1 : 0; } @CheckForNull From 42f99d4a95bf6b65647f3fa41010c9d146c03e1c Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Fri, 23 Oct 2026 12:33:11 +0400 Subject: [PATCH 4/5] SONARJAVA-7215 Pin the S2695 char-constant false positive, tracked by SONARJAVA-7228 Constant folding renders a char as its numeric value, so a query built as "WHERE id = " + QUESTION_MARK resolves to "WHERE id = 63" and the rule reports a valid index 1 as out of range. The defect is in ExpressionUtils, not in the check: it corrupts every asConstant(String.class) caller that folds a concatenation holding a char constant. Record the false positive here and fix it in SONARJAVA-7228. Also drop conditionalClauseGuardedBySameCondition: its ternary path is already covered by ternaryWithDifferentBranches and its constant path by constantsFromAnotherClass, and the rule does not inspect the enclosing condition its name refers to. Co-Authored-By: Claude Opus 5 (1M context) --- ...paredStatementAndResultSetCheckSample.java | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java index d59a26510af..52788a71ea3 100644 --- a/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java @@ -302,19 +302,6 @@ void ternaryWithDifferentBranches(Connection con, boolean withImage) throws SQLE ps.setString(4, "d"); // Noncompliant } - void conditionalClauseGuardedBySameCondition(Connection con, String status) throws SQLException { - String sql = SqlConstants.SELECT - + (status != null ? " AND status = ?" : "") - + " ORDER BY id"; - - PreparedStatement ps = con.prepareStatement(sql); - ps.setString(1, "a"); // Compliant - if (status != null) { - ps.setString(2, status); // Compliant - the clause holding the second placeholder is added under the same condition - } - ps.setString(3, "c"); // Noncompliant - } - void ternaryWithNonConstantFragment(Connection con, String tenant, int maxTurns) throws SQLException { String sql = "SELECT * FROM m WHERE m.tenant = ? AND m.session_id = ? " + (maxTurns > 0 ? "AND t.id IN (SELECT id FROM t WHERE tenant=? AND session=? LIMIT " + maxTurns + ") " : "") @@ -323,8 +310,8 @@ void ternaryWithNonConstantFragment(Connection con, String tenant, int maxTurns) PreparedStatement ps = con.prepareStatement(sql); ps.setString(1, tenant); // Compliant if (maxTurns > 0) { - ps.setString(3, tenant); // Compliant - part of the query is unknown - ps.setString(4, tenant); // Compliant - part of the query is unknown + ps.setString(3, tenant); // Compliant + ps.setString(4, tenant); // Compliant } } @@ -366,6 +353,16 @@ void nonConstantCharFragment(Connection con, char separator) throws SQLException ps.setInt(3, 2); // Noncompliant {{This "PreparedStatement" only has 2 parameters.}} } + private static final char QUESTION_MARK = '?'; + + void charConstantFragment(Connection con) throws SQLException { + PreparedStatement ps = con.prepareStatement("SELECT * FROM t WHERE id = " + QUESTION_MARK); + // FALSE POSITIVE: index 1 is valid, the character constant is the placeholder. Constant folding renders a char as + // its numeric value, so the query resolves to "... WHERE id = 63" and the placeholder is lost. Fix belongs to the + // frontend, tracked by SONARJAVA-7228; remove this annotation once it lands. + ps.setInt(1, 1); // Noncompliant + } + void constantsFromAnotherClass(Connection con) throws SQLException { String sql = SqlConstants.SELECT; sql += SqlConstants.AND_CLAUSE; From 1f8701ab96e73104633f39a44bc711cc282258b5 Mon Sep 17 00:00:00 2001 From: Gitar Date: Mon, 26 Oct 2026 06:36:50 +0000 Subject: [PATCH 5/5] fix: Count only question mark char literals Co-Authored-By: Claude Opus 5 (1M context) --- ...paredStatementAndResultSetCheckSample.java | 33 +++++++++++++++++++ .../PreparedStatementAndResultSetCheck.java | 14 +++++--- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java index 52788a71ea3..16bcdf7acc9 100644 --- a/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/PreparedStatementAndResultSetCheckSample.java @@ -313,6 +313,13 @@ void ternaryWithNonConstantFragment(Connection con, String tenant, int maxTurns) ps.setString(3, tenant); // Compliant ps.setString(4, tenant); // Compliant } + // The whole query resolves: the int fragment contributes no placeholder and the branch taken contributes two. + ps.setString(5, tenant); // Noncompliant {{This "PreparedStatement" only has 4 parameters.}} + } + + void ternaryWithUnresolvableBranch(Connection con, boolean cond) throws SQLException { + PreparedStatement ps = con.prepareStatement("SELECT * FROM t WHERE a = ? " + (cond ? getQuery() : "")); + ps.setString(5, "x"); // Compliant - one branch of the conditional is unknown, so the count is too } void assignedInBothBranches(Connection con, boolean hasPassword) throws SQLException { @@ -346,6 +353,12 @@ void nonConstantNumericFragmentFromMethod(Connection con) throws SQLException { ps.setInt(2, 2); // Noncompliant } + void boxedCharFragment(Connection con, Character separator) throws SQLException { + PreparedStatement ps = con.prepareStatement("SELECT * FROM t WHERE id = ? AND flag = " + separator); + ps.setInt(2, 2); // Compliant - the character may be '?' + ps.setInt(3, 2); // Noncompliant {{This "PreparedStatement" only has 2 parameters.}} + } + void nonConstantCharFragment(Connection con, char separator) throws SQLException { PreparedStatement ps = con.prepareStatement("SELECT * FROM t WHERE id = ? AND flag = " + separator); ps.setInt(2, 2); // Compliant - the character may be '?' @@ -363,6 +376,26 @@ void charConstantFragment(Connection con) throws SQLException { ps.setInt(1, 1); // Noncompliant } + void charLiteralIsThePlaceholder(Connection con) throws SQLException { + PreparedStatement ps = con.prepareStatement("SELECT * FROM t WHERE id = " + '?'); + ps.setInt(1, 1); // Compliant + ps.setInt(2, 2); // Noncompliant {{This "PreparedStatement" only has 1 parameters.}} + } + + void charLiteralIsNotAPlaceholder(Connection con) throws SQLException { + PreparedStatement ps = con.prepareStatement("SELECT * FROM t WHERE id = ? AND flag = " + 'y'); + ps.setInt(1, 1); // Compliant + ps.setInt(2, 2); // Noncompliant {{This "PreparedStatement" only has 1 parameters.}} + } + + void charLocalVariableFragment(Connection con) throws SQLException { + // the char-typed fragment is bounded at one placeholder + final char questionMark = '?'; + PreparedStatement ps = con.prepareStatement("SELECT * FROM t WHERE id = " + questionMark); + ps.setInt(1, 1); // Compliant + ps.setInt(2, 2); // Noncompliant {{This "PreparedStatement" only has 1 parameters.}} + } + void constantsFromAnotherClass(Connection con) throws SQLException { String sql = SqlConstants.SELECT; sql += SqlConstants.AND_CLAUSE; diff --git a/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java b/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java index 4cbc686c16a..5e6dda996aa 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/PreparedStatementAndResultSetCheck.java @@ -34,6 +34,7 @@ import org.sonar.plugins.java.api.tree.ConditionalExpressionTree; import org.sonar.plugins.java.api.tree.ExpressionTree; import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.LiteralTree; import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; import org.sonar.plugins.java.api.tree.MethodInvocationTree; import org.sonar.plugins.java.api.tree.Tree; @@ -183,12 +184,17 @@ private static Integer handleConditionalExpression(ConditionalExpressionTree exp */ @CheckForNull private static Integer unresolvedPlaceholderBound(ExpressionTree expression) { + if (expression.is(Tree.Kind.CHAR_LITERAL)) { + return "'?'".equals(((LiteralTree) expression).value()) ? 1 : 0; + } Type type = expression.symbolType(); - Type primitiveType = type.isPrimitiveWrapper() ? type.primitiveType() : type; - if (primitiveType == null || !primitiveType.isPrimitive()) { - return null; + if (type.isPrimitive(Primitives.CHAR) || type.is("java.lang.Character")) { + return 1; } - return primitiveType.isPrimitive(Primitives.CHAR) ? 1 : 0; + if (type.isPrimitive() || type.isPrimitiveWrapper()) { + return 0; + } + return null; } @CheckForNull