Fixes #821 by adding analyzers for common JDBC wrappers

corpus/jdbc/cjj_test.clj

@@ -0,0 +1,32 @@
+(ns jdbc.cjj-test
+  (:require [clojure.java.jdbc :as jdbc]))
+
+(defn tx-check
+  []
+  (let [db {:dbtype "some-db" :dbname "kondo"}]
+    ;; should not flag tx
+    (jdbc/with-db-transaction [tx (jdbc/get-connection db)]
+      (jdbc/execute! tx ["select * from table where foo = ?" 123]))
+    ;; should not flag tx or binding arity
+    (jdbc/with-db-transaction [tx (jdbc/get-connection db) {}]
+      (jdbc/execute! tx ["select * from table where foo = ?" 123]))))
+
+(defn con-check
+  []
+  (let [db {:dbtype "some-db" :dbname "kondo"}]
+    ;; should not flag con
+    (jdbc/with-db-connection [con db]
+      (jdbc/query con ["select * from table where foo = ?" 123]))
+    ;; should not flag con or binding arity
+    (jdbc/with-db-connection [con db {}]
+      (jdbc/query con ["select * from table where foo = ?" 123]))))
+
+(defn meta-check
+  []
+  (let [db {:dbtype "some-db" :dbname "kondo"}]
+    ;; should not flag m-con
+    (jdbc/with-db-metadata [m-con db]
+      (jdbc/metadata-query (.getTables m-con nil nil nil (into-array String [\"TABLE\"]))))
+    ;; should not flag m-con or binding arity
+    (jdbc/with-db-metadata [m-con db {}]
+      (jdbc/metadata-query (.getTables m-con nil nil nil (into-array String [\"TABLE\"]))))))

corpus/jdbc/next.clj

@@ -0,0 +1,19 @@
+(ns jdbc.next-test
+  "Syntax check examples for next.jdbc/with-transaction.
+
+  The clojure.java.jdbc/with-db-* functions would all be treated the same way."
+  (:require [next.jdbc :as jdbc]))
+
+(def db {:dbtype "some-db" :dbname "kondo"})
+
+(jdbc/with-transaction [tx (jdbc/get-datasource db) {} {}] ;; 2 or 3 forms
+  (jdbc/execute! tx ["select * from table where foo = ?" 123]))
+
+(jdbc/with-transaction [tx] ;; 2 or 3 forms
+  (jdbc/execute! tx ["select * from table where foo = ?" 123]))
+
+(jdbc/with-transaction ;; requires vector for binding
+  (jdbc/execute! tx ["select * from table where foo = ?" 123]))
+
+(jdbc/with-transaction [[tx] (jdbc/get-datasource db)] ;; requires a symbol
+  (jdbc/execute! tx ["select * from table where foo = ?" 123]))

corpus/jdbc/next_test.clj

@@ -0,0 +1,12 @@
+(ns jdbc.next-test
+  (:require [next.jdbc :as jdbc]))
+
+(defn tx-check
+  []
+  (let [db {:dbtype "some-db" :dbname "kondo"}]
+    ;; should not flag tx
+    (jdbc/with-transaction [tx (jdbc/get-datasource db)]
+      (jdbc/execute! tx ["select * from table where foo = ?" 123]))
+    ;; should not flag tx or binding arity
+    (jdbc/with-transaction [tx (jdbc/get-datasource db) {}]
+      (jdbc/execute! tx ["select * from table where foo = ?" 123]))))

src/clj_kondo/impl/analyzer.clj

@@ -6,6 +6,7 @@
    [clj-kondo.impl.analyzer.compojure :as compojure]
    [clj-kondo.impl.analyzer.core-async :as core-async]
    [clj-kondo.impl.analyzer.datalog :as datalog]
+   [clj-kondo.impl.analyzer.jdbc :as jdbc]
    [clj-kondo.impl.analyzer.namespace :as namespace-analyzer
     :refer [analyze-ns-decl]]
    [clj-kondo.impl.analyzer.potemkin :as potemkin]
@@ -1296,6 +1297,11 @@
                      [compojure.core context]
                      [compojure.core rfn])
                     (compojure/analyze-compojure-macro ctx expr resolved-as-name)
+                    ([clojure.java.jdbc with-db-transaction]
+                     [clojure.java.jdbc with-db-connection]
+                     [clojure.java.jdbc with-db-metadata]
+                     [next.jdbc with-transaction])
+                    (jdbc/analyze-like-jdbc-with ctx expr)
                     ;; catch-all
                     (let [next-ctx (cond-> ctx
                                      (= '[clojure.core.async thread]
@@ -1537,6 +1543,7 @@
 ;; with GraalVM
 (vreset! common {'analyze-expression** analyze-expression**
                  'analyze-children analyze-children
+                 'analyze-like-let analyze-like-let
                  'ctx-with-bindings ctx-with-bindings
                  'extract-bindings extract-bindings
                  'analyze-defn analyze-defn

src/clj_kondo/impl/analyzer/common.clj

@@ -3,6 +3,9 @@
 
 (defonce common (volatile! {}))
 
+(defn analyze-like-let [ctx expr]
+  ((get @common 'analyze-like-let) ctx expr))
+
 (defn analyze-expression** [ctx expr]
   ((get @common 'analyze-expression**) ctx expr))
 

src/clj_kondo/impl/analyzer/jdbc.clj

@@ -0,0 +1,41 @@
+(ns clj-kondo.impl.analyzer.jdbc
+  {:no-doc true}
+  (:require
+   [clj-kondo.impl.analyzer.common :refer [analyze-expression**
+                                           analyze-like-let]]
+   [clj-kondo.impl.findings :as findings]
+   [clj-kondo.impl.utils :as utils :refer [node->line]]))
+
+(defn analyze-like-jdbc-with
+  "clojure.java.jdbc/with-db-* and next.jdbc/with-transaction are almost
+  like a let binding: they accept a binding vector which has a single symbol,
+  an expression, and an optional third expression (the options to apply).
+
+  We check there are 2 or 3 forms in the first argument (the binding vector).
+  We analyze the 3rd form if it is present (the options).
+  We analyze the whole expression as a let, after modifying the binding
+  expression to only have two children."
+  [{:keys [filename callstack] :as ctx} expr]
+  (let [call (-> callstack first second)
+        [f bindings & body] (:children expr)
+        binding-forms (:children bindings)
+        [sym db-expr opts] binding-forms]
+    (when-not (<= 2 (count binding-forms) 3)
+      (findings/reg-finding!
+       ctx
+       (node->line filename bindings :error :syntax
+                   (format "%s binding form requires exactly 2 or 3 forms" call))))
+    (when-not (utils/symbol-token? sym)
+      (findings/reg-finding!
+       ctx
+       (node->line filename sym :error :syntax
+                   (format "%s binding form requires a symbol" call))))
+    (let [opts-analyzed (analyze-expression** ctx opts)
+          ;; a normal binding vector with just these two forms:
+          simple-binding (assoc bindings :children (list sym db-expr))
+          ;; reconstruct the expression with the normal binding instead:
+          expr-with-simple-binding
+          (assoc expr :children (concat [f simple-binding] body))
+          ;; now we can treat it like a regular let expression:
+          analyzed (analyze-like-let ctx expr-with-simple-binding)]
+      (concat opts-analyzed (doall analyzed)))))

test/clj_kondo/jdbc_test.clj

@@ -0,0 +1,25 @@
+(ns clj-kondo.jdbc-test
+  (:require
+   [clj-kondo.test-utils :refer [assert-submaps lint!]]
+   [clojure.java.io :as io]
+   [clojure.test :refer [deftest is]]
+   [missing.test.assertions]))
+
+(deftest next-jdbc-test
+  (is (empty? (lint! (io/file "corpus" "jdbc" "next_test.clj")
+                     {:linters {:unresolved-symbol {:level :error}}}))))
+
+(deftest clojure-java-jdbc-test
+  (is (empty? (lint! (io/file "corpus" "jdbc" "cjj_test.clj")
+                     {:linters {:unresolved-symbol {:level :error}}}))))
+
+(deftest detected-issues-test
+  (assert-submaps '({:file "corpus/jdbc/next.clj", :row 9, :col 24, :level :error,
+                     :message "with-transaction binding form requires exactly 2 or 3 forms"}
+                    {:file "corpus/jdbc/next.clj", :row 12, :col 24, :level :error,
+                     :message "with-transaction binding form requires exactly 2 or 3 forms"}
+                    {:file "corpus/jdbc/next.clj", :row 16, :col 3, :level :error,
+                     :message "with-transaction requires a vector for its binding"}
+                    {:file "corpus/jdbc/next.clj", :row 18, :col 25, :level :error,
+                     :message "with-transaction binding form requires a symbol"})
+                  (lint! (io/file "corpus" "jdbc" "next.clj"))))