Kibit 0.0.8 fails with Stack Overflow error when reading #{} set literals

[jasonab]

Exception in thread "main" java.lang.StackOverflowError
at clojure.core.logic$tree_term_QMARK_.invoke(logic.clj:972)
at clojure.core.logic$walk_STAR_$fn__1462.invoke(logic.clj:419)
at clojure.core.logic$eval1645$fn__1646.invoke(logic.clj:1068)
at clojure.core.logic$eval375$fn__376$G__366__383.invoke(logic.clj:85)
at clojure.core.logic$walk_STAR_.invoke(logic.clj:416)
at clojure.core.logic$walk_STAR_$fn__1462.invoke(logic.clj:420)
at clojure.core.logic$eval1645$fn__1646.invoke(logic.clj:1068)
at clojure.core.logic$eval375$fn__376$G__366__383.invoke(logic.clj:85)
at clojure.core.logic$walk_STAR_.invoke(logic.clj:416)
at clojure.core.logic$walk_STAR_$fn__1462.invoke(logic.clj:420)
at clojure.core.logic$eval1645$fn__1646.invoke(logic.clj:1068)
at clojure.core.logic$eval375$fn__376$G__366__383.invoke(logic.clj:85)

[jonase]

Did you run kibit on a codebase that is publicly available?

[jasonab]

Sorry, no, it ran on my company's codebase. I can completely believe that there's something weird inside it, but unfortunately I don't have any more information to go on.

Maybe an option to report what file kibit is processing?

[nbeloglazov]

Try run it on incanter's core.clj: https://github.com/liebke/incanter/blob/master/modules/incanter-core/src/incanter/core.clj
It fails with stackoverflow on my computer.

[nbeloglazov]

It fails on following function:

(defn- except-for-cols
"
"
  ([data except-cols]
     (let [colnames (:column-names data)
           _except-cols (if (coll? except-cols)
                          (map #(get-column-id data %) except-cols)
                          [(get-column-id data except-cols)])
           except-names  (if (some number? _except-cols)
                           (map #(nth colnames %) _except-cols)
                           _except-cols)]
       (for [name colnames :when (not (some #{name} except-names))]
         name))))

Actually on this part:

(not (some #{name} except-names))

[clyce]

on my project, stack over flows too

[aspasia]

same here

[willgorman]

I can trigger a stack overflow running kibit on a file containing the form (->> [[1 2]] (into #{})) It looks like it may have something to do with the combination of the ->> macro and the set reader macro. The form (->> [[1 2]] (into {})) does not cause a stack overflow. I'm using kibit 0.0.8, leiningen 2.3.4, clojure 1.5.1, and java 1.7.0_45.

[dustinconrad]

This is happening for me. Codebase is publicly available.

[willgorman]

I think the problem is that core.logic doesn't support sets (Also: LOGIC-154)

[dustinconrad]

Will, I'm not sure that is it, as I replaced instances of #{...} with (hash-set ...) and now the stackoverflow isn't happening.

[danielcompton]

This is still happening on core.logic 0.8.8. It looks possible (though maybe not advisable) to extend core.logic to be able to handle sets in kibit. - http://stackoverflow.com/questions/18017924/core-logic-stackoverflow-when-using-sets. Another way would be to modify core.logic.core/tree-term? from inside kibit, but that would be an even dirtier hack. Modifying tree-term? will let us read Clojure code, without throwing exceptions when we run across a set.

(defn tree-term? [x]
  (and (or (coll? x)
           (instance? ITreeTerm x))
       (not (instance? IPersistentSet x))))

[eigenhombre]

This failed for us recently too and cost us an hour or so. Here's what it barfs on:

(defn killit [coll]
  (not (some #{"string1" "string2"} (map ffirst coll))))

[replomancer]

Hey, same problem here (Kibit 0.0.8, Leiningen 2.5.1, Clojure both 1.7.0-alpha6 and 1.6.0). In my case it's this:
(defn f [x] (when (some #{x} [1]) (do (println "hello") 0)))
Kibit runs OK when I replace (some #{x} [1]) with true or if I remove the redundant do (either of these simplifications prevents StackOverflowError).

[schmir]

Can we catch the error and go on with checking expressions until a proper fix is in place?

[a613]

Still happening

[torbjornvatn]

Any hope for this beeing fixed? It's really annoying to get that looooong stacktrace in the output

[arrdem]

The "quick fix" which for the record I fully support would be to modify tree-term? as described by @danielcompton above. It's a dirty hack, but I think that influencing core.logic in this matter is either unlikely or at least likely to be slow. See how this issue has sat around for three years, and how the comments above about core.logic support for sets are in the same vintage.

Unless someone here wants to make the effort to enable core.logic to support sets properly.

[Rovanion]

Another example of crash causing code:

(-> 1 (into #{}))

[arrdem]

After talking with @danielcompton about this, the consensus seems to be that kibit will take the low road here. I'll drop a patch tonight which will run the kibit linter with a monkeypatch of the relevant part(s) of core.logic.

[arrdem]

Patch:

commit 07292e392c6b8adb44f4a37ba3dd952d78beffa3
Author: Reid 'arrdem' McKenzie <me@arrdem.com>
Date:   Thu Nov 10 00:10:56 2016 -0800

    WIP

diff --git a/src/kibit/driver.clj b/src/kibit/driver.clj
index caab14e..02bc091 100644
--- a/src/kibit/driver.clj
+++ b/src/kibit/driver.clj
@@ -3,9 +3,17 @@
             [kibit.rules :refer [all-rules]]
             [kibit.check :refer [check-file]]
             [kibit.reporters :refer :all]
-            [clojure.tools.cli :refer [cli]])
+            [clojure.tools.cli :refer [cli]]
+            [clojure.core.logic :refer [tree-term?]])
   (:import [java.io File]))

+(.setDynamic ^clojure.lang.Var #'tree-term?)
+
+(defn our-tree-term? [x]
+  (and (or (coll? x)
+           (instance? clojure.core.logic.protocols.ITreeTerm x))
+       (not (instance? clojure.lang.IPersistentSet x))))
+
 (def cli-specs [["-r" "--reporter"
                  "The reporter used when rendering suggestions"
                  :default "text"]])
@@ -33,16 +41,19 @@

 (defn run [source-paths rules & args]
   (let [[options file-args usage-text] (apply (partial cli args) cli-specs)
-        source-files (mapcat #(-> % io/file find-clojure-sources-in-dir)
-                             (if (empty? file-args) source-paths file-args))]
-    (mapcat (fn [file] (try (check-file file
-                                        :reporter (name-to-reporter (:reporter options)
-                                                                    cli-reporter)
-                                        :rules (or rules all-rules))
-                            (catch Exception e
-                              (binding [*out* *err*]
-                                (println "Check failed -- skipping rest of file")
-                                (println (.getMessage e))))))
+        source-files                   (mapcat #(-> % io/file find-clojure-sources-in-dir)
+                                               (if (empty? file-args) source-paths file-args))]
+    (mapcat (fn [file]
+              (try
+                (with-bindings {}
+                  (check-file file
+                              :reporter (name-to-reporter (:reporter options)
+                                                          cli-reporter)
+                              :rules (or rules all-rules)))
+                (catch Exception e
+                  (binding [*out* *err*]
+                    (println "Check failed -- skipping rest of file")
+                    (println (.getMessage e))))))
             source-files)))

 (defn external-run
diff --git a/test/kibit/test/collections.clj b/test/kibit/test/collections.clj
index e219f86..a81a1ea 100644
--- a/test/kibit/test/collections.clj
+++ b/test/kibit/test/collections.clj
@@ -4,24 +4,30 @@

 (deftest collections-are
   (are [expected-alt-form test-form]
-       (= expected-alt-form (:alt (kibit/check-expr test-form)))
-    '(seq a) '(not (empty? a))
+      (= expected-alt-form (:alt (kibit/check-expr test-form)))
+    '(seq a)          '(not (empty? a))
     '(when (seq a) b) '(when-not (empty? a) b)
     '(when (seq a) b) '(when (not (empty? a)) b)
-    '(vector a) '(conj [] a)
+
+    '(vector a)   '(conj [] a)
     '(vector a b) '(conj [] a b)
-    '(vec coll) '(into [] coll)
+    '(vec coll)   '(into [] coll)
+
     '(set coll) '(into #{} coll)
+
     '(update-in coll [k] f) '(assoc coll k (f (k coll)))
     '(update-in coll [k] f) '(assoc coll k (f (coll k)))
     '(update-in coll [k] f) '(assoc coll k (f (get coll k)))
+
     '(assoc-in coll [k0 k1] a) '(assoc coll k0 (assoc (k0 coll) k1 a))
     '(assoc-in coll [k0 k1] a) '(assoc coll k0 (assoc (coll k0) k1 a))
     '(assoc-in coll [k0 k1] a) '(assoc coll k0 (assoc (get coll k0) k1 a))
+    '(assoc-in coll [k1 k2] v) '(update-in coll [k1 k2] assoc v)
+
     '(update-in coll [k] f a b c) '(assoc coll k (f (k coll) a b c))
     '(update-in coll [k] f a b c) '(assoc coll k (f (coll k) a b c))
     '(update-in coll [k] f a b c) '(assoc coll k (f (get coll k) a b c))
-    '(assoc-in coll [k1 k2] v) '(update-in coll [k1 k2] assoc v)
+
     '(repeatedly 10 (constantly :foo)) '(take 10 (repeatedly (constantly :foo)))

     ;; some wrong simplifications happened in the past:
diff --git a/test/kibit/test/driver.clj b/test/kibit/test/driver.clj
index 2cd3534..ed93f4d 100644
--- a/test/kibit/test/driver.clj
+++ b/test/kibit/test/driver.clj
@@ -13,5 +13,9 @@
 (deftest find-clojure-sources-are
   (is (= [(io/file "test/resources/first.clj")
           (io/file "test/resources/second.cljx")
+          (io/file "test/resources/sets.clj")
           (io/file "test/resources/third.cljs")]
          (driver/find-clojure-sources-in-dir (io/file "test/resources")))))
+
+(deftest test-set-file
+  (is (driver/run ["test/resources/sets.clj"] nil)))
diff --git a/test/resources/sets.clj b/test/resources/sets.clj
new file mode 100644
index 0000000..f9025cd
--- /dev/null
+++ b/test/resources/sets.clj
@@ -0,0 +1,5 @@
+(ns resources.sets)
+
+(defn killit [coll]
+  (not (some #{"string1" "string2"}
+             (map ffirst coll))))

Test output:

lein test kibit.test.arithmetic

lein test kibit.test.check

lein test kibit.test.collections

lein test kibit.test.control-structures

lein test kibit.test.core

lein test kibit.test.driver

lein test :only kibit.test.driver/test-set-file

ERROR in (test-set-file) (logic.clj:359)
expected: (driver/run ["test/resources/sets.clj"] nil)
  actual: java.lang.StackOverflowError: null
 at clojure.core.logic.Substitutions.walk (logic.clj:359)
    clojure.core.logic$walk_STAR_$fn__1798.invoke (logic.clj:231)
    clojure.core.logic$eval1980$fn__1981.invoke (logic.clj:984)
    clojure.core.logic.protocols$eval456$fn__457$G__447__464.invoke (protocols.clj:55)
    clojure.core.logic$walk_STAR_.invokeStatic (logic.clj:229)
    clojure.core.logic$walk_STAR_.invoke (logic.clj:227)
    clojure.core.logic$walk_STAR_$fn__1798.invoke (logic.clj:233)
    clojure.core.logic$eval1980$fn__1981.invoke (logic.clj:984)
    [[ repeating frames elided ]]

lein test kibit.test.equality

lein test kibit.test.misc

lein test kibit.test.reporters

lein test resources.sets

Ran 15 tests containing 121 assertions.
0 failures, 1 errors.

[michael-cenx]

I seemingly reproduced this stack overflow bug with nested #{}

(def data {"a" #{"b" "c"}, "d" #{"f" "e"}}) => #'user/data (->> (for [[k v] data] (assoc {} :entity-id k :contents (into #{} v))) (into #{}))

Workaround: Replace all #{} with (hash-set). We may want to re-open this bug

[danielcompton]

Can you try again with 0.1.4? I think that should work.

[michael-cenx]

Sorry I just tried it and it failed.

lein kibit Exception in thread "main" java.lang.StackOverflowError, compiling:(/private/var/folders/ch/lwy55whj30xc297m70qqhpfw0000gp/T/form-init8711051728291748004.clj:1:124) at clojure.lang.Compiler.load(Compiler.java:7142) at clojure.lang.Compiler.loadFile(Compiler.java:7086) at clojure.main$load_script.invoke(main.clj:274) at clojure.main$init_opt.invoke(main.clj:279) at clojure.main$initialize.invoke(main.clj:307) at clojure.main$null_opt.invoke(main.clj:342) at clojure.main$main.doInvoke(main.clj:420) at clojure.lang.RestFn.invoke(RestFn.java:421) at clojure.lang.Var.invoke(Var.java:383) at clojure.lang.AFn.applyToHelper(AFn.java:156) at clojure.lang.Var.applyTo(Var.java:700) at clojure.main.main(main.java:37) Caused by: java.lang.StackOverflowError at clojure.core.logic$eval1821$fn__1822.invoke(logic.clj:979) at clojure.core.logic.protocols$eval277$fn__278$G__268__285.invoke(protocols.clj:55) at clojure.core.logic$walk_STAR_.invoke(logic.clj:229) at clojure.core.logic$walk_STAR_$fn__1620.invoke(logic.clj:233) at clojure.core.logic$eval1821$fn__1822.invoke(logic.clj:984) at clojure.core.logic.protocols$eval277$fn__278$G__268__285.invoke(protocols.clj:55) at clojure.core.logic$walk_STAR_.invoke(logic.clj:229) at clojure.core.logic$walk_STAR_$fn__1620.invoke(logic.clj:233) at clojure.core.logic$eval1821$fn__1822.invoke(logic.clj:984) at clojure.core.logic.protocols$eval277$fn__278$G__268__285.invoke(protocols.clj:55) ...

[danielcompton]

@arrdem thoughts?

[arrdem]

Investigating.