diff --git a/.circleci/crucible.sh b/.circleci/crucible.sh index ff0163cd..00473cc2 100755 --- a/.circleci/crucible.sh +++ b/.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 diff --git a/changes.md b/changes.md index 98d02f97..68bd4196 100644 --- a/changes.md +++ b/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 diff --git a/resource/eastwood/config/clojure.clj b/resource/eastwood/config/clojure.clj index 35adf869..d2b05caf 100644 --- a/resource/eastwood/config/clojure.clj +++ b/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}})) diff --git a/test-third-party-deps/eastwood/third_party_deps_test.clj b/test-third-party-deps/eastwood/third_party_deps_test.clj index a32777ca..14262b68 100644 --- a/test-third-party-deps/eastwood/third_party_deps_test.clj +++ b/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}))))))) diff --git a/test-third-party-deps/testcases/core_async_example/alt.clj b/test-third-party-deps/testcases/core_async_example/alt.clj new file mode 100644 index 00000000..e2bc42ac --- /dev/null +++ b/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)))) diff --git a/test-third-party-deps/testcases/core_async_example/assert.clj b/test-third-party-deps/testcases/core_async_example/assert.clj new file mode 100644 index 00000000..ae385f05 --- /dev/null +++ b/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))) diff --git a/test-third-party-deps/testcases/core_async_example/misc.clj b/test-third-party-deps/testcases/core_async_example/misc.clj new file mode 100644 index 00000000..45a64610 --- /dev/null +++ b/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 "." {})))))