From 4ca74f2b0d0f87827ce34d9baf8dcc8d086ce18e Mon Sep 17 00:00:00 2001
From: Sean Corfield <sean@corfield.org>
Date: Sat, 7 Sep 2019 14:02:04 -0700
Subject: [PATCH] Fixes #139 by checking columns arguments

---
 CHANGES.md                |  1 +
 src/honeysql/helpers.cljc | 33 +++++++++++++++++++++++++++------
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 48bea9a..e3bf2a2 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,6 +1,7 @@
 ## Changes coming in 0.9.7
 
 * Fix #248 by treating alias as "not a subquery" when generating SQL for it. (@seancorfield)
+* Fix #139 by checking arguments to `columns`/`merge-columns` and throwing an exception if a single collection is supplied (instead of varargs).
 * Fix #128 by adding `truncate` support.
 * Fix #99 by adding a note to the first use of `select` in the README that column names can be keywords or symbols but not strings.
 
diff --git a/src/honeysql/helpers.cljc b/src/honeysql/helpers.cljc
index d177e68..1070777 100644
--- a/src/honeysql/helpers.cljc
+++ b/src/honeysql/helpers.cljc
@@ -221,13 +221,34 @@
   ([table] (insert-into nil table))
   ([m table] (build-clause :insert-into m table)))
 
-(macros/usetime
- (defhelper columns [m fields]
-   (assoc m :columns (collify fields))))
+(defn- check-varargs
+  "Called for helpers that require unrolled arguments to catch the mistake
+  of passing a collection as a single argument."
+  [helper args]
+  (when (and (coll? args) (= 1 (count args)) (coll? (first args)))
+    (let [msg (str (name helper) " takes varargs, not a single collection")]
+      (throw #?(:clj (IllegalArgumentException. msg)
+                :cljs (js/Error. msg))))))
 
-(macros/usetime
- (defhelper merge-columns [m fields]
-   (update-in m [:columns] concat (collify fields))))
+(defmethod build-clause :columns [_ m fields]
+  (assoc m :columns (collify fields)))
+
+(defn columns [& args]
+  (let [[m fields] (if (map? (first args))
+                     [(first args) (rest args)]
+                     [{} args])]
+    (check-varargs :columns fields)
+    (build-clause :columns m fields)))
+
+(defmethod build-clause :merge-columns [_ m fields]
+  (update-in m [:columns] concat (collify fields)))
+
+(defn merge-columns [& args]
+  (let [[m fields] (if (map? (first args))
+                     [(first args) (rest args)]
+                     [{} args])]
+    (check-varargs :merge-columns fields)
+    (build-clause :merge-columns m fields)))
 
 (defmethod build-clause :values [_ m vs]
   (assoc m :values vs))