I did not manage to reproduce a false positive with the code you provided (even when updating it to compile), I’m afraid you removed critical information when providing the reproducer.
I see that ps is declared somewhere else, is it possible that it is re-assigned at some point?
Something like:
PreparedStatement ps;
ps = connection.prepareStatement("SELECT ... FROM ... where job_id = ?");
if (connectReadOnly()) {
ps = connection.prepareStatement("SELECT... FROM ...");
// Do something with ps
} else {
ps.setInt(1, job_id); // Correct
}
It would be great if you could provide all the usages/re-assignment of the impacted variables and a reproducer keeping the control flow of the program.
Thank you for the quick reply. Here’s a full example below. The preparedstatement is initialized as null then set after gettinga connection to the database via connectReadOnly() (method is in parent class).
I have converted the method to use try-with-resources and the false positive warning went away. While it is our long term goal to refactor all of these old classes to try-with-resources it still seems like a false positive?
public AppliedDiscountModel[] getAppliedCoupons(int job_id,int period_id) {
ArrayList<AppliedDiscountModel> recs = new ArrayList<AppliedDiscountModel>();
PreparedStatement ps = null;
ResultSet r = null;
try {
String sql = "select ... from t1 where x = ? and y ?";
log.debug(sql);
if (connectReadOnly())
{
ps = _con.prepareStatement(sql);
ps.setInt(1,job_id);
ps.setInt(2,period_id);
r = executeQueryPS(ps);
while (r.next()) {
try {
AppliedDiscountModel rec = new AppliedDiscountModel();
// set attributes
recs.add(rec);
} catch (Exception e) {
log.error(e,e);
}
}
}
} catch (SQLException e) {
log.error(e,e);
} finally {
try {
disconnect(r,ps); }
catch (Exception e) {log.error(e);}
}
return recs.toArray(new AppliedDiscountModel[0]);
}
It is always a local instance variable but is sometimes built dynamically if the query has a variable number of parameters.
For example, we might loop over an array and build the WHERE clause to include “?” parameters based on the array size. Then there would be a similar loop to set the value of those paramets on the prepared statement. Obviously here we have to use a variable e.g. for (int x=1;x<5 ) {ps.setInt(x++,myParameers[x]); } in this scenario x will always be a positive integer starting at 1 as required by JDBC.
However, the cases I’ve provided already don’t do that dynamic query building. They are pretty straight forward queries with a fixed number of query parameters.
We did a few improvements to this rule since, I invite anybody still having issues with this rule to try with the latest SonarQube version (> 9.2).
If you still see false positives, please open a new thread, with an updated description of the problem, and ideally a minimal reproducer (keeping usages/re-assignment of the impacted variables and the control flow of the program).