Fix various false positives for clojure.core.async/go

Closes #411

.circleci/crucible.sh

@@ -21,7 +21,7 @@ if grep --silent "Reflection warning" output; then
   exit 1
 fi
 
-grep --silent "== Warnings: 34. Exceptions thrown: 0" output || exit 1
+grep --silent "== Warnings: 29. Exceptions thrown: 0" output || exit 1
 
 # Exercise tools.reader, see https://github.com/jonase/eastwood/issues/413
 cd ../tools.reader || exit 1

changes.md

@@ -1,3 +1,13 @@
+## Changes from 0.9.4 to 0.9.5
+
+#### Bugfixes
+
+* Fix various false positives which could arise when using the `clojure.core.async/go` macro.
+  * Fixes https://github.com/jonase/eastwood/issues/411
+* Upgrade `tools.reader`
+  * Closes https://github.com/jonase/eastwood/issues/297
+  * Fixes https://github.com/jonase/eastwood/issues/413 
+
 ## Changes from 0.9.3 to 0.9.4
 
 #### Bugfixes

resource/eastwood/config/clojure.clj

@@ -129,3 +129,31 @@ their intention being to write to stdout and therefore have the `with-out-str` r
   :qualifier true
   :within-depth 9
   :reason "Support a specific pattern that tends to fail better."})
+
+(do
+  ;; All these disablings within `go` are here because it's a peculiar macro which seemingly doesn't integrate well with the :if-inside-macroexpansion-of implementation.
+  ;; These disablings can duplicate logic placed elsewhere.
+  ;; This section might have to groe in a future. Maybe there's a way to de-duplicate the disablings or impl logic.
+
+  (disable-warning
+   {:linter :suspicious-expression
+    :for-macro 'clojure.core/or
+    :if-inside-macroexpansion-of #{'clojure.core.async/go}
+    :reason "https://github.com/jonase/eastwood/issues/411. Note that `alt!` is not caught as a macro, possibly because `go` is a peculiar macro."})
+
+  (disable-warning
+   {:linter :suspicious-expression
+    :for-macro 'clojure.core/and
+    :if-inside-macroexpansion-of #{'clojure.core.async/go}})
+
+  (doseq [v [true false]]
+    (disable-warning
+     {:linter :constant-test
+      :qualifier v
+      :for-macro 'clojure.core/assert
+      :if-inside-macroexpansion-of #{'clojure.core.async/go}}))
+
+  (disable-warning
+   {:linter :constant-test
+    :for-macro 'clojure.test/is
+    :if-inside-macroexpansion-of #{'clojure.core.async/go}}))

test-third-party-deps/eastwood/third_party_deps_test.clj

@@ -36,15 +36,34 @@
            (eastwood.lint/eastwood (assoc eastwood.lint/default-opts :namespaces #{'testcases.spec-tools-example}))))))
 
 (deftest core-async-example
-  (let [opts (assoc eastwood.lint/default-opts :namespaces #{'testcases.core-async-example})]
+  (testing "Handling of hard-to-analyze `go` forms"
+    (let [opts (assoc eastwood.lint/default-opts :namespaces #{'testcases.core-async-example})]
 
-    (testing "When encountering `go` usages that throw exceptions, it omits the exception"
-      (is (= {:some-warnings false :some-errors false}
-             (eastwood.lint/eastwood opts))))
+      (testing "When encountering `go` usages that throw exceptions, it omits the exception"
+        (is (= {:some-warnings false :some-errors false}
+               (eastwood.lint/eastwood opts))))
 
-    ;; This counterexample is important not only for covering the option itself,
-    ;; but also for making sure the previous test is logically valid:
-    (testing "When encountering `go` usages that throw exceptions, and passing an opt-out flag,
+      ;; This counterexample is important not only for covering the option itself,
+      ;; but also for making sure the previous test is logically valid:
+      (testing "When encountering `go` usages that throw exceptions, and passing an opt-out flag,
 it doesn't omit the exception"
-      (is (= {:some-warnings true :some-errors true}
-             (eastwood.lint/eastwood (assoc opts :abort-on-core-async-exceptions? true)))))))
+        (is (= {:some-warnings true :some-errors true}
+               (eastwood.lint/eastwood (assoc opts :abort-on-core-async-exceptions? true)))))))
+
+
+  (let [base-opts (assoc eastwood.lint/default-opts :abort-on-core-async-exceptions? true)] ;; Make test more logically robust
+
+    (testing "`alt!` can be used without false positives, see https://github.com/jonase/eastwood/issues/411"
+      (is (= {:some-warnings false :some-errors false}
+             (eastwood.lint/eastwood (assoc base-opts
+                                            :namespaces #{'testcases.core-async-example.alt})))))
+
+    (testing "`assert` can be used without false positives"
+      (is (= {:some-warnings false :some-errors false}
+             (eastwood.lint/eastwood (assoc base-opts
+                                            :namespaces #{'testcases.core-async-example.assert})))))
+
+    (testing "All other (relevant) silencings present in resource/clojure.clj"
+      (is (= {:some-warnings false :some-errors false}
+             (eastwood.lint/eastwood (assoc base-opts
+                                            :namespaces #{'testcases.core-async-example.misc})))))))

test-third-party-deps/testcases/core_async_example/alt.clj

@@ -0,0 +1,14 @@
+(ns testcases.core-async-example.alt
+  (:require
+   [clojure.core.async :refer [alt! go timeout]]))
+
+(defn sample
+  "https://github.com/jonase/eastwood/issues/411"
+  [trade-ch]
+  (go
+    (let [timeout-ch (timeout 1000)
+          trade 100]
+      (-> (alt!
+            [[trade-ch trade]] :sent
+            timeout-ch :timed-out)
+          print))))

test-third-party-deps/testcases/core_async_example/assert.clj

@@ -0,0 +1,8 @@
+(ns testcases.core-async-example.assert
+  (:require
+   [clojure.core.async :refer [go]]))
+
+(defn sample []
+  (go
+    (assert false)
+    (assert true)))

test-third-party-deps/testcases/core_async_example/misc.clj

@@ -0,0 +1,66 @@
+(ns testcases.core-async-example.misc
+  (:require
+   [clojure.core.async :refer [go]]
+   [clojure.spec.alpha :as spec]
+   [clojure.test :refer [are assert-expr deftest do-report is join-fixtures testing use-fixtures]])
+  (:import
+   (java.io PrintWriter)))
+
+(defn sample1 []
+  (go
+    (spec/and ::foo ::bar)))
+
+(defn sample2 []
+  (go
+    (spec/keys :req [::foo])))
+
+(defn sample3 []
+  (go
+    (spec/coll-of int?)))
+
+(defmethod assert-expr 'truthy?
+  [msg [pred v]]
+  `(do-report (if ~v
+                {:type :pass, :message ~msg}
+                {:type :fail, :message ~msg})))
+
+(defn func [f] "")
+
+(defn throws-exception []
+  (throw (ex-info "" {})))
+
+;; https://github.com/jonase/eastwood/issues/116
+(deftest exercises-thrown
+  (go
+    (is (thrown? Exception (throws-exception)))))
+
+;; https://github.com/jonase/eastwood/issues/313
+(deftest exercises-truthy
+  (go
+    (is (truthy? 42))))
+
+;; https://github.com/jonase/eastwood/issues/207
+(deftest b-test
+  (go
+    (is (instance? String (func +)))))
+
+(defn sample4 []
+  (go
+    (is false)))
+
+(defn sample5 []
+  (go
+    (cond-> 1
+      true inc)))
+
+(defn sample6 [stream ^PrintWriter pw]
+  (go
+    (with-out-str
+      (-> pw (.print "foo"))
+      (spit stream 42)
+      1)))
+
+(defn sample7 [stream]
+  (go
+    (while true
+      (throw (ex-info "." {})))))