luciano/honeysql
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fixes #321 by adding :checking option

Browse source 
Initial linting is only for IN () and IN (NULL)
This commit is contained in:
Sean Corfield 2021-04-22 22:13:32 -07:00
parent f606dc6044
commit 20cba15da2
4 changed files with 63 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -2,6 +2,7 @@
* 2.0.next in progress
* Fix #323 by supporting more than one SQL entity in `:on-conflict`.
* Fix #321 by adding `:checking` mode. Currently only detects potential problems with `IN` clauses.
* 2.0.0-beta2 (for testing; 2021-04-13)
* The documentation continues to be expanded and clarified in response to feedback!

10
doc/getting-started.md
View file

@ -297,7 +297,7 @@ HoneySQL supports quite a few [PostgreSQL extensions](postgresql.md).
## Format Options
In addition to the `:quoted` and `:dialect` options described above,
`format` also accepts `:inline` and `:params`.
`format` also accepts `:checking`, `:inline`, and `:params`.
The `:params` option was mentioned above and is used to specify
the values of named parameters in the DSL.
@ -312,6 +312,14 @@ was wrapped in `[:inline `..`]`:
* keywords and symbols become SQL keywords (uppercase, with `-` replaced by a space),
* everything else is just turned into a string (by calling `str`) and added to the SQL string.
The `:checking` option defaults to `:none`. If `:checking :basic` is
specified, certain obvious errors -- such as `IN` with an empty collection --
is treated as an error and an exception is thrown. If `:checking :strict`
is specified, certain dubious constructs -- such as `IN` with a collection
containing `NULL` values -- is also treated as an error and an exception is
thrown. It is expected that this feature will be expanded over time
to help avoid generating illegal SQL. _[New in version 2.0.next]_
`format` accepts options as either a single hash map argument or
as named arguments (alternating keys and values). If you are using
Clojure 1.11 (or later) you can mix'n'match, providing some options

15
src/honey/sql.cljc
View file

@ -106,6 +106,8 @@
;; in entities; if someone complains about this check, an option
;; can be added to format to turn this on:
(def ^:private ^:dynamic *allow-suspicious-entities* false)
;; "linting" mode (:none, :basic, :strict):
(def ^:private ^:dynamic *checking* :none)
;; clause helpers
@ -929,6 +931,16 @@
(let [[sql-x & params-x] (format-expr x {:nested true})
[sql-y & params-y] (format-expr y {:nested true})
values (unwrap (first params-y) {})]
(when-not (= :none *checking*)
(when (or (and (sequential? y) (empty? y))
(and (sequential? values) (empty? values)))
(throw (ex-info "IN () empty collection is illegal"
{:clause [in x y]})))
(when (and (= :strict *checking*)
(or (and (sequential? y) (some nil? y))
(and (sequential? values) (some nil? values))))
(throw (ex-info "IN (NULL) does not match"
{:clause [in x y]}))))
(if (and (= "?" sql-y) (= 1 (count params-y)) (coll? values))
(let [sql (str "(" (str/join ", " (repeat (count values) "?")) ")")]
(-> [(str sql-x " " (sql-kw in) " " sql)]
@ -1228,6 +1240,9 @@
(let [dialect? (contains? opts :dialect)
dialect (when dialect? (get dialects (check-dialect (:dialect opts))))]
(binding [*dialect* (if dialect? dialect @default-dialect)
*checking* (if (contains? opts :checking)
(:checking opts)
:none)
*clause-order* (if dialect?
(if-let [f (:clause-order-fn dialect)]
(f @base-clause-order)

41
test/honey/sql_test.cljc
View file

@ -6,7 +6,8 @@
#?(:clj [clojure.test :refer [deftest is testing]]
:cljs [cljs.test :refer-macros [deftest is testing]])
[honey.sql :as sut :refer [format]]
[honey.sql.helpers :as h]))
[honey.sql.helpers :as h])
#?(:clj (:import (clojure.lang ExceptionInfo))))
(deftest mysql-tests
(is (= ["SELECT * FROM `table` WHERE `id` = ?" 1]
@ -685,9 +686,9 @@ ORDER BY id = ? DESC
(format))
(is false "; not detected in entity!")
(catch #?(:clj Throwable :cljs :default) e
(is (:disallowed (ex-data e))))))
(is (:disallowed (ex-data e))))))))
;; should not produce: ["SELECT foo, bar FROM mytable ORDER BY foo; select * from users"]
))
(deftest issue-319-test
(testing "that registering a clause is idempotent"
@ -696,3 +697,37 @@ ORDER BY id = ? DESC
(sut/register-clause! :foo (constantly ["FOO"]) nil)
(sut/register-clause! :foo (constantly ["FOO"]) nil)
(format {:foo []}))))))
(deftest issue-321-linting
(testing "empty IN is ignored by default"
(is (= ["WHERE x IN ()"]
(format {:where [:in :x []]})))
(is (= ["WHERE x IN ()"]
(format {:where [:in :x :?y]}
{:params {:y []}}))))
(testing "empty IN is flagged in basic mode"
(is (thrown-with-msg? ExceptionInfo #"empty collection"
(format {:where [:in :x []]}
{:checking :basic})))
(is (thrown-with-msg? ExceptionInfo #"empty collection"
(format {:where [:in :x :?y]}
{:params {:y []} :checking :basic}))))
(testing "IN NULL is ignored by default and basic"
(is (= ["WHERE x IN (NULL)"]
(format {:where [:in :x [nil]]})))
(is (= ["WHERE x IN (NULL)"]
(format {:where [:in :x [nil]]}
{:checking :basic})))
(is (= ["WHERE x IN (?)" nil]
(format {:where [:in :x :?y]}
{:params {:y [nil]}})))
(is (= ["WHERE x IN (?)" nil]
(format {:where [:in :x :?y]}
{:params {:y [nil]} :checking :basic}))))
(testing "IN NULL is flagged in strict mode"
(is (thrown-with-msg? ExceptionInfo #"does not match"
(format {:where [:in :x [nil]]}
{:checking :strict})))
(is (thrown-with-msg? ExceptionInfo #"does not match"
(format {:where [:in :x :?y]}
{:params {:y [nil]} :checking :strict})))))