Skip to content

Commit

Permalink
Omit errors if they come from forms containing async/go
Browse files Browse the repository at this point in the history
Fixes #395
  • Loading branch information
vemv committed Jun 24, 2021
1 parent 6e41d0e commit aa569a5
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 13 deletions.
1 change: 1 addition & 0 deletions .circleci/core.async
Submodule core.async added at 7282b0
12 changes: 12 additions & 0 deletions .circleci/integration_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,16 @@ fi

grep --silent "== Warnings: 0 (not including reflection warnings) Exceptions thrown: 0" output || exit 1

git submodule update --init --recursive

cd ../core.async || exit 1

if lein with-profile +test update-in :plugins conj "[jonase/eastwood \"RELEASE\"]" -- eastwood > output; then
echo "Should have failed! Emitted output:"
cat output
exit 1
fi

grep --silent "== Warnings: 39 (not including reflection warnings) Exceptions thrown: 0" output || exit 1

exit 0
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[submodule ".circleci/core.async"]
path = .circleci/core.async
url = git@github.com:clojure/core.async.git
7 changes: 7 additions & 0 deletions changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

## Changes from 0.5.2 to

#### New

* Omit exceptions coming from individual top-level forms, if they made use of the `clojure.core.async/go` macro.
* Closes https://github.com/jonase/eastwood/issues/395
* This makes Eastwood more capable of analyzing core.async -based projects. It's a temporary measure though, as it ideally this would be simply fixed in the tools.analyzer project.
* You can revert to the old behavior (which most likely will result in errors by passing `:abort-on-core-async-exceptions? true`) as a top-level Eastwood option.

#### Bugfixes

* Fix the `eastwood.lint` `-main` program when invoked with no arguments.
Expand Down
3 changes: 2 additions & 1 deletion project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@
:dependencies [[com.nedap.staffing-solutions/speced.def "2.0.0"]
[com.taoensso/timbre "5.1.2"]
[com.taoensso/tufte "2.2.0"]
[manifold "0.1.9-alpha4"]]}
[manifold "0.1.9-alpha4"]
[org.clojure/core.async "1.3.618"]]}
:clj-kondo {:dependencies [[clj-kondo "2021.06.18"]]}
:antq {:plugins [[com.github.liquidz/antq "0.15.3"]]}
:1.7 {:dependencies [[org.clojure/clojure "1.7.0"]]}
Expand Down
87 changes: 75 additions & 12 deletions src/eastwood/analyze_ns.clj
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
[clojure.java.io :as io]
[clojure.pprint :as pp]
[clojure.string :as string]
[clojure.walk :as walk]
[eastwood.copieddeps.dep1.clojure.tools.analyzer.ast :as ast]
[eastwood.copieddeps.dep1.clojure.tools.analyzer.env :as env]
[eastwood.copieddeps.dep1.clojure.tools.analyzer.passes :refer [schedule]]
Expand All @@ -17,6 +18,7 @@
[eastwood.passes :as pass]
[eastwood.util :as util])
(:import
(clojure.lang Namespace)
(java.net URL)))

;; uri-for-ns, pb-reader-for-ns were copied from library
Expand Down Expand Up @@ -268,6 +270,14 @@
should-change? (update 1 vary-meta dissoc :const)
should-change? seq)))

(defn var->symbol [var]
(if (util/clojure-1-10-or-later)
;; use the most accurate method (as it can't be deceived by external metadata mutation):
(-> var symbol)
(let [^Namespace ns (-> var meta :ns)]
(symbol (-> ns .-name name)
(-> var meta :name name)))))

(defn analyze-file
"Takes a file path and optionally a pushback reader. Returns a map
with at least the following keys:
Expand Down Expand Up @@ -299,6 +309,9 @@
Options:
- :reader a pushback reader to use to read the namespace forms
- :opt a map of analyzer options
- `:abort-on-core-async-exceptions?` (default: falsey)
- if true, analyze+eval exceptions stemming from `go` usage will abort execution.`
- currently, execution is not aborted, rationale being https://github.com/jonase/eastwood/issues/395
- :debug A set of keywords.
- :all Enable all of the following debug messages.
- :progress Print simple progress messages as analysis proceeds.
Expand Down Expand Up @@ -336,31 +349,81 @@
(let [{:keys [val out err]}
(util/with-out-str2
(binding [jvm/run-passes run-passes]
(jvm/analyze+eval
form (jvm/empty-env)
{:passes-opts eastwood-passes-opts})))]
(let [{:keys [result] :as v} (jvm/analyze+eval form
(jvm/empty-env)
{:passes-opts eastwood-passes-opts})]
(when (and (var? result)
(-> result meta :arglists vector?)
(some-> result meta :arglists first seq?)
(some-> result meta :arglists first first #{'quote}))
;; Cleanup odd arglists such as git.io/Jnidv
;; (as they have unexpected quotes, and a swapped choice of vectors/lists):
(alter-meta! result (fn [{:keys [arglists]
:as m}]
(assoc m
:arglists
(->> arglists
(mapv (fn [x]
(cond-> x
(and (seq? x)
(-> x count #{2})
(-> x first #{'quote}))
last)))
(list))))))
v)))]
(do-eval-output-callbacks out err (:cwd opt))
[nil val])
(catch Exception e
[e nil]))]
(let [had-go-call? (atom false)]
(when-not (:abort-on-core-async-exceptions? opt)
(->> form
(walk/postwalk (fn [x]
(when (and (not @had-go-call?) ;; performance optimization
(seq? x)
(-> x first symbol?)
(= 'clojure.core.async/go
(try
(some->> x
first
(ns-resolve *ns*)
var->symbol)
;; ns-resolve can fail pre Clojure 1.10:
(catch Exception _
nil))))
(reset! had-go-call? true))
x))))
(if @had-go-call?
(try
;; eval the form (without tools.analyzer),
;; increasing the chances that its result can be useful, queried, etc
(eval form)
[nil ::omit]
(catch Exception _
;; if the `eval` failed, return the tools.analyzer exception - not the `eval` one:
[e nil]))
[e nil]))))]
(if exc
{:forms nil
:asts asts, :exception exc, :exception-phase :analyze+eval,
:asts asts,
:exception exc,
:exception-phase :analyze+eval,
:exception-form form}

(if-let [first-exc-ast (first (asts-with-eval-exception ast))]
{:forms (remaining-forms reader (conj forms form)),
:asts asts,
:exception (wrapped-exception? (:result first-exc-ast)),
:exception (-> first-exc-ast :result wrapped-exception?),
:exception-phase :eval,
:exception-form (if-let [f (first (:raw-forms first-exc-ast))]
:exception-form (if-let [f (-> first-exc-ast :raw-forms first)]
f
(:form first-exc-ast))}
(do
(let [conj? (not= ast ::omit)]
(post-analyze-debug source-path asts form ast *ns* opt)
(recur (conj forms form)
(conj asts
(eastwood-ast-additions
ast (count asts)))))))))))))))
(recur (cond-> forms
conj? (conj form))
(cond-> asts
conj? (conj (eastwood-ast-additions
ast (count asts))))))))))))))))

(defn analyze-ns
"Takes an IndexingReader and a namespace symbol.
Expand Down
14 changes: 14 additions & 0 deletions test-third-party-deps/eastwood/third_party_deps_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,17 @@
(testing "Is able to process usages of the `manifold` library without false positives"
(is (= {:some-warnings false :some-errors false}
(eastwood.lint/eastwood (assoc eastwood.lint/default-opts :namespaces #{'testcases.manifold-example}))))))

(deftest core-async-example
(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))))

;; 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)))))))
16 changes: 16 additions & 0 deletions test-third-party-deps/testcases/core_async_example.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
(ns testcases.core-async-example
(:require
[clojure.core.async :refer [go]]))

(defprotocol P
(x [p]))

;; taken from the core.async test suite
(defrecord R [z]
P
(x [this]
(go
(loop []
(if (zero? (rand-int 3))
[z (.z this)]
(recur))))))

0 comments on commit aa569a5

Please sign in to comment.