Skip to content

Commit

Permalink
SONARJAVA-4842 - Implement S6909: Constant parameters in a PreparedSt…
Browse files Browse the repository at this point in the history
…atement should not be set more than once (#4660)
  • Loading branch information
kaufco committed Feb 21, 2024
1 parent 7463843 commit ecc9b8e
Show file tree
Hide file tree
Showing 10 changed files with 641 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"ruleKey": "S6909",
"hasTruePositives": true,
"falseNegatives": 0,
"falsePositives": 0
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,18 @@ public static Set<MethodTree> findReachableMethodsInSameFile(Tree tree) {
return finder.getReachableMethods();
}

public static Tree findClosestParentOfKind(Tree tree, Set<Tree.Kind> nodeKinds) {
while (tree != null) {
if (nodeKinds.contains(tree.kind())) {
return tree;
}
tree = tree.parent();
}
return null;
}

private static class ReachableMethodsFinder extends BaseTreeVisitor {
private Map<MethodTree, Void> reachableMethods = new IdentityHashMap<>();
private final Map<MethodTree, Void> reachableMethods = new IdentityHashMap<>();

@Override
public void visitMethodInvocation(MethodInvocationTree tree) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* SonarQube Java
* Copyright (C) 2012-2024 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* 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 GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.java.checks.helpers;

import java.util.Set;
import javax.annotation.Nullable;
import org.junit.jupiter.api.Test;
import org.sonar.java.model.statement.BlockTreeImpl;
import org.sonar.plugins.java.api.tree.BlockTree;
import org.sonar.plugins.java.api.tree.ExpressionStatementTree;
import org.sonar.plugins.java.api.tree.ForEachStatement;
import org.sonar.plugins.java.api.tree.IfStatementTree;
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
import org.sonar.plugins.java.api.tree.MethodTree;
import org.sonar.plugins.java.api.tree.Tree;
import org.sonar.plugins.java.api.tree.WhileStatementTree;

import static org.assertj.core.api.Assertions.assertThat;

class TreeHelperTest extends JParserTestUtils {

@Test
void testFindClosestParentOfKind() {
String code = newCode("int bar() {",
" boolean a = foo();",
" while (true) {",
" if (condition) {",
" bar();",
" }",
" for (var a: x) {",
" for (var b: x) {",
" baz();",
" }",
" }",
" }",
"}");

MethodTree method = methodTree(code);
var whileBody = ((BlockTree) ((WhileStatementTree) method.block().body().get(1)).statement()).body();
var barCall = (MethodInvocationTree) (
(ExpressionStatementTree) (
(BlockTreeImpl) (
(IfStatementTree) whileBody.get(0)
).thenStatement()
).body().get(0)
).expression();

var bazCall = ((ExpressionStatementTree) (
(BlockTreeImpl) (
(ForEachStatement) (
(BlockTreeImpl) (
((ForEachStatement) whileBody.get(1)).statement()
)
).body().get(0)
).statement()
).body().get(0)).expression();

assertFoundNode(barCall, Set.of(Tree.Kind.IF_STATEMENT), Tree.Kind.IF_STATEMENT);
assertFoundNode(barCall, Set.of(Tree.Kind.WHILE_STATEMENT), Tree.Kind.WHILE_STATEMENT);
assertFoundNode(barCall, Set.of(Tree.Kind.IF_STATEMENT, Tree.Kind.WHILE_STATEMENT), Tree.Kind.IF_STATEMENT);
assertFoundNode(barCall, Set.of(Tree.Kind.METHOD), Tree.Kind.METHOD);
assertFoundNode(barCall, Set.of(Tree.Kind.CLASS), Tree.Kind.CLASS);
assertFoundNode(barCall, Set.of(Tree.Kind.DO_STATEMENT), null);
assertFoundNode(barCall, Set.of(Tree.Kind.DO_STATEMENT, Tree.Kind.WHILE_STATEMENT), Tree.Kind.WHILE_STATEMENT);
assertFoundNode(barCall, Set.of(Tree.Kind.FOR_EACH_STATEMENT, Tree.Kind.DO_STATEMENT, Tree.Kind.WHILE_STATEMENT),
Tree.Kind.WHILE_STATEMENT);

var loopOverB = (ForEachStatement) assertFoundNode(bazCall,
Set.of(Tree.Kind.FOR_EACH_STATEMENT, Tree.Kind.DO_STATEMENT, Tree.Kind.WHILE_STATEMENT), Tree.Kind.FOR_EACH_STATEMENT);
assertThat(loopOverB.variable().simpleName().name()).isEqualTo("b");

}

private static Tree assertFoundNode(Tree tree, Set<Tree.Kind> kinds, @Nullable Tree.Kind expected) {
var actual = TreeHelper.findClosestParentOfKind(tree, kinds);
if (expected != null) {
assertThat(actual).isNotNull();
assertThat(actual.kind()).isEqualTo(expected);
} else {
assertThat(actual).isNull();
}
return actual;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,210 @@
package checks;

import java.sql.Blob;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.time.LocalDate;
import java.sql.Date;
import java.util.List;

public class PreparedStatementLoopInvariantCheckSample {

public void basicCase1(PreparedStatement preparedStatement, List<Order> orders) throws SQLException {
Date today = java.sql.Date.valueOf(LocalDate.now());
for(Order order: orders) {
preparedStatement.setDate(0, today); // Noncompliant [[sc=7;ec=42;secondary=-1]] {{Move this loop-invariant setter invocation out of this loop.}}
preparedStatement.executeUpdate();
}
}

public void basicCase2(PreparedStatement preparedStatement, List<Order> orders) throws SQLException {
Date today = java.sql.Date.valueOf(LocalDate.now());
for(Order order: orders) {
preparedStatement.setDate(0, today); // Compliant
preparedStatement.executeUpdate();
today = java.sql.Date.valueOf(LocalDate.now());
}
}

public void basicCase3(PreparedStatement preparedStatement, List<Order> orders) throws SQLException {
for(Order order: orders) {
Date today = java.sql.Date.valueOf(LocalDate.now());
preparedStatement.setDate(0, today); // Compliant
preparedStatement.executeUpdate();
}
}

public void basicCase4(NoPreparedStatement noPreparedStatement, List<Order> orders) throws SQLException {
Date today = java.sql.Date.valueOf(LocalDate.now());
for(Order order: orders) {
noPreparedStatement.setDate(0, today); // Compliant
noPreparedStatement.executeUpdate();
}
}

public void someOtherFunctions(PreparedStatement preparedStatement, List<Order> orders, Blob blob) throws SQLException {
preparedStatement.setBlob(1, blob); // Compliant
preparedStatement.setInt(2, 23); // Compliant
preparedStatement.setString(3, "Text"); // Compliant
preparedStatement.setCursorName("Name"); // Compliant
preparedStatement.setInt(4, constant); // Compliant
preparedStatement.setInt(5, noConstant); // Compliant
for(Order order: orders) {
preparedStatement.setBlob(1, blob); // Noncompliant
preparedStatement.setInt(2, 23); // Noncompliant
preparedStatement.setString(3, "Text"); // Noncompliant
preparedStatement.setCursorName("Name"); // Noncompliant
preparedStatement.setInt(4, constant); // Noncompliant
preparedStatement.setInt(5, noConstant); // Noncompliant
}
}

public void checkNonVarOrFieldAssignment(
PreparedStatement preparedStatement,
List<Order> orders,
String[] strings,
String[] moreStrings
) throws SQLException {
preparedStatement.setString(0, strings[1]); // Compliant
preparedStatement.setString(1, moreStrings[2]); // Compliant
for(Order order: orders) {
preparedStatement.setString(0, strings[1]); // Compliant
preparedStatement.setString(1, moreStrings[2]); // Compliant
strings[1] = "foo";
strings[2] = "bar";
}
}

private static final int constant = 23;

private int noConstant = getLength();

private int getLength() {
return 42;
}

public void nestedSample(
PreparedStatement preparedStatement,
List<Order> orders,
boolean condition1,
boolean condition2,
Date yesterday,
Date today,
Date tomorrow,
Date dayAfterTomorrow
) throws SQLException {
while(true) {
preparedStatement.setDate(0, yesterday); // Noncompliant [[secondary=-1]]
preparedStatement.setDate(0, today); // Compliant
preparedStatement.setDate(0, tomorrow); // Compliant
preparedStatement.setDate(0, dayAfterTomorrow); // Compliant
today = java.sql.Date.valueOf(LocalDate.now());
if (condition1) do {
preparedStatement.setDate(0, yesterday); // Noncompliant [[secondary=-1]]
preparedStatement.setDate(0, today); // Noncompliant [[secondary=-2]]
preparedStatement.setDate(0, tomorrow); // Compliant
preparedStatement.setDate(0, dayAfterTomorrow); // Compliant
tomorrow = java.sql.Date.valueOf(LocalDate.now());
if (condition2) {
for(Order order: orders) {
preparedStatement.setDate(0, yesterday); // Noncompliant [[secondary=-1]]
preparedStatement.setDate(0, today); // Noncompliant [[secondary=-2]]
preparedStatement.setDate(0, tomorrow); // Noncompliant [[secondary=-3]]
preparedStatement.setDate(0, dayAfterTomorrow); // Compliant
dayAfterTomorrow = java.sql.Date.valueOf(LocalDate.now());
}
}
for(int i = 0; i < 10; i++) {
preparedStatement.setDate(0, yesterday); // Noncompliant [[secondary=-1]]
preparedStatement.setDate(0, today); // Noncompliant [[secondary=-2]]
preparedStatement.setDate(0, tomorrow); // Noncompliant [[secondary=-3]]
preparedStatement.setDate(0, dayAfterTomorrow); // Compliant
if (condition2) {
dayAfterTomorrow = java.sql.Date.valueOf(LocalDate.now());
}
}
} while (true);
}
}

private void postIncDec(PreparedStatement preparedStatement, List<String> uids) throws SQLException {
int index = 0;
for (String uid : uids) {
preparedStatement.setString(index, ""); // Compliant
index++;
}

for (String uid : uids) {
preparedStatement.setString(index, ""); // Compliant
index--;
}

for (String uid : uids) {
preparedStatement.setString(index, ""); // Noncompliant
}
}

private void preIncDec(PreparedStatement preparedStatement, List<String> uids) throws SQLException {
int index = -1;
for (String uid : uids) {
preparedStatement.setString(index, ""); // Compliant
++index;
}

for (String uid : uids) {
preparedStatement.setString(index, ""); // Compliant
--index;
}

for (String uid : uids) {
preparedStatement.setString(index, ""); // Compliant
foo(++index);
}

for (String uid : uids) {
preparedStatement.setString(index, ""); // Noncompliant
foo(~index);
}

for (String uid : uids) {
preparedStatement.setString(index, ""); // Noncompliant
}
}

private void forCoverage(PreparedStatement preparedStatement, List<String> uids, int[] buf) throws SQLException {
int index = -1;
for (String uid : uids) {
preparedStatement.setString(index, ""); // Noncompliant
buf[index]++;
}
}

private static void foo(int arg) {}

private void forLoop(PreparedStatement preparedStatement) throws SQLException {
for (int i = 0; i < 10; i++ ) {
preparedStatement.setString(i, ""); // Compliant
}

int index = 0;
for (int i = 0; i < 10; i++ ) {
preparedStatement.setString(index, ""); // Noncompliant
}
}

private void loopVarIsNoInvariant(PreparedStatement preparedStatement, List<String> uids) throws SQLException {
for (String uid: uids) {
preparedStatement.setString(0, uid); // Compliant
}
}

public record Order(String id, String price) {}

private static class NoPreparedStatement {
public void setDate(int parameterIndex, java.sql.Date x) throws SQLException {
}

public void executeUpdate() {
}
}
}
Loading

0 comments on commit ecc9b8e

Please sign in to comment.