Skip to content

Update rewrite-clj #342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ executors:
<<: *defaults
openjdk11:
docker:
- image: circleci/clojure:openjdk-11-lein-2.9.1-node
- image: circleci/clojure:openjdk-11-lein-2.9.3-buster-node
environment:
LEIN_ROOT: "true" # we intended to run lein as root
JVM_OPTS: -Xmx3200m --illegal-access=deny # forbid reflective access (this flag doesn't exist for JDK8 or JDK17+)
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
* Upgrade Orchard
* Worth emphasizing: now the following options are available https://github.com/clojure-emacs/orchard/tree/v0.7.0#configuration-options
* They can make the refactor-nrepl experience simpler / more robust.
* Upgrade rewrite-clj
* This fixes some features such as `rename-file-or-dir`.
* Reliability improvement: try using `require` prior to `find-ns`
* This increases the chances that a namespace will be found, which in turns makes refactor-nrepl more complete/accurate.
* Replace Cheshire with `clojure.data.json`
Expand Down
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ test: .inline-deps
cljfmt:
lein with-profile -user,+$(VERSION),+cljfmt cljfmt check

cljfmt-fix:
lein with-profile -user,+$(VERSION),+cljfmt cljfmt fix

eastwood:
lein with-profile -user,+$(VERSION),+eastwood eastwood

Expand Down
18 changes: 9 additions & 9 deletions project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
:url "http://github.com/clojure-emacs/refactor-nrepl"
:license {:name "Eclipse Public License"
:url "http://www.eclipse.org/legal/epl-v10.html"}
:dependencies [[nrepl "0.8.3"]
:dependencies [[nrepl "0.9.0-beta3"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to use 0.9 here?

Copy link
Member Author

@vemv vemv Oct 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No super strong rationale other than "keep the important deps @ latest".

It's what cider.el is using as well, which we intend to share with the world as 'stable' soon. So I'd guess 0.9.x would be the version that most realistically will reflect intended usage?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CIDER uses the latest version only on its snapshot as some features don't work without it. CIDER 1.2 will happen after nREPL 0.9 is finalized. In general I avoid adding deps to betas/alphas unless they are truly needed, although I agree it's not a big deal here.

^:inline-dep [http-kit "2.5.3"]
^:inline-dep [org.clojure/data.json "2.3.1"]
^:inline-dep [org.clojure/tools.analyzer.jvm "1.1.0"]
^:inline-dep [org.clojure/tools.namespace "1.1.0" :exclusions [org.clojure/tools.reader]]
^:inline-dep [org.clojure/tools.reader "1.3.5"]
^:inline-dep [cider/orchard "0.7.1"]
^:inline-dep [cljfmt "0.7.0"]
^:inline-dep [org.clojure/tools.reader "1.3.6"]
^:inline-dep [cider/orchard "0.7.3"]
^:inline-dep [cljfmt "0.8.0" :exclusions [rewrite-clj rewrite-cljs]]
^:inline-dep [clj-commons/fs "1.6.307"]
^:inline-dep [rewrite-clj "0.6.1"]
^:inline-dep [rewrite-clj "1.0.699-alpha"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need an alpha version here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All versions in the 1.x series (i.e @lread's work) are suffixed -alpha. See https://github.com/clj-commons/rewrite-clj#used-in

In practice this means that various modern constructs are better supported (or supported at all)

^:inline-dep [version-clj "1.0.0"]]
:exclusions [org.clojure/clojure] ; see versions matrix below

Expand All @@ -30,7 +30,7 @@
:unresolved-tree false}
:filespecs [{:type :bytes :path "refactor-nrepl/refactor-nrepl/project.clj" :bytes ~(slurp "project.clj")}]
:profiles {;; Clojure versions matrix
:provided {:dependencies [[cider/cider-nrepl "0.25.9"]
:provided {:dependencies [[cider/cider-nrepl "0.27.2"]
[org.clojure/clojure "1.9.0"]
;; For satisfying `:pedantic?`:
[com.google.code.findbugs/jsr305 "3.0.2"]
Expand All @@ -45,7 +45,7 @@
[org.clojure/clojure "1.11.0-master-SNAPSHOT" :classifier "sources"]]}

:test {:dependencies [[print-foo "1.0.2"]]}
:dev {:dependencies [[org.clojure/clojurescript "1.10.520"]
:dev {:dependencies [[org.clojure/clojurescript "1.10.879"]
[org.clojure/core.async "1.3.618" :exclusions [org.clojure/clojure org.clojure/tools.reader]]
[cider/piggieback "0.5.2"]
[commons-io/commons-io "2.8.0"]]
Expand All @@ -56,7 +56,7 @@
"testproject/src"]
:repositories [["snapshots" "https://oss.sonatype.org/content/repositories/snapshots"]]}
:cljfmt [:test
{:plugins [[lein-cljfmt "0.7.0" :exclusions [org.clojure/clojure
{:plugins [[lein-cljfmt "0.8.0" :exclusions [org.clojure/clojure
org.clojure/clojurescript]]]
:cljfmt {:indents {as-> [[:inner 0]]
as->* [[:inner 0]]
Expand All @@ -73,7 +73,7 @@
:add-linters [:performance :boxed-math]
:config-files ["eastwood.clj"]}}
:clj-kondo [:test
{:dependencies [[clj-kondo "2021.09.15"]]}]}
{:dependencies [[clj-kondo "2021.10.19"]]}]}

:jvm-opts ~(cond-> []
(System/getenv "CI")
Expand Down
3 changes: 2 additions & 1 deletion src/refactor_nrepl/ns/resolve_missing.clj
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@

;; This happends when class `candidate` depends on a class that is
;; not available on the classpath.
(catch NoClassDefFoundError _e
(catch NoClassDefFoundError e
(refactor-nrepl.util/maybe-log-exception e)
{:name candidate :type :class})))
candidates))

Expand Down
12 changes: 8 additions & 4 deletions src/refactor_nrepl/s_expressions.clj
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
(ns refactor-nrepl.s-expressions
(:require [rewrite-clj.parser :as zip-parser]
[rewrite-clj.reader :as zip-reader]
[rewrite-clj.zip :as zip]))
(:require
[rewrite-clj.parser :as zip-parser]
[rewrite-clj.reader :as zip-reader]
[rewrite-clj.zip :as zip])
(:import
(clojure.tools.reader.reader_types IndexingPushbackReader)))

(defn all-zlocs
"Generate a seq of all zlocs in a depth-first manner"
Expand All @@ -16,7 +19,8 @@

(defn get-first-sexp
^String [file-content]
(let [reader (zip-reader/string-reader file-content)]
(let [^IndexingPushbackReader
reader (zip-reader/string-reader file-content)]
(loop [sexp (zip-parser/parse reader)]
(let [zloc (zip/edn sexp)]
(if (and zloc (not (comment-or-string-or-uneval-or-nil? zloc)))
Expand Down
3 changes: 2 additions & 1 deletion test/refactor_nrepl/ns/class_search_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#{"com/github/luben/zstd/ZstdInputStream"
"org/brotli/dec/BrotliInputStream"
"org/apache/tools/ant/Task"
"org/apache/tools/ant/launch/AntMain"
"com/sun/jdi/request/EventRequest"})

(def non-initializable-classes
Expand All @@ -18,7 +19,7 @@
false
(-> (Thread/currentThread) .getContextClassLoader))
(catch NoClassDefFoundError e
;; there are only 4 in ~7922 classes that cause NoClassDefFoundError,
;; there are only ~5 in ~7922 classes that cause NoClassDefFoundError,
;; see `#'acceptable-error-messages`.
;; They don't have to do with classpath parsing so there's nothing to be fixed.
(is (contains? acceptable-error-messages (.getMessage e))
Expand Down
55 changes: 34 additions & 21 deletions test/refactor_nrepl/ns/resolve_missing_test.clj
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
(ns refactor-nrepl.ns.resolve-missing-test
(:require [cider.piggieback :as piggieback]
[clojure
[edn :as edn]
[test :as t]]
[nrepl.core :as nrepl]
[nrepl.server :as server]
[refactor-nrepl.middleware :as middleware]))
(:require
[clojure.java.shell :as shell]
[cider.piggieback :as piggieback]
[clojure.edn :as edn]
[clojure.test :as t :refer [testing]]
[nrepl.core :as nrepl]
[nrepl.server :as server]
[refactor-nrepl.middleware :as middleware]))

(def ^:dynamic *handler* (server/default-handler #'middleware/wrap-refactor))
(def ^:dynamic *session* nil)
Expand Down Expand Up @@ -47,41 +48,53 @@
(t/use-fixtures :each piggieback-fixture)

(t/deftest sanity
(let [{:keys [exit]
:as v} (shell/sh "node" "--version")]
(assert (-> exit long zero?)
(pr-str v)))

(t/testing "cljs repl is active"
(let [response (message {:op :eval
:code (nrepl/code js/console)})]
(t/is (= "cljs.user" (:ns response)))
(t/is (= #{"done"} (:status response)))))
(testing (pr-str response)
(t/is (= "cljs.user" (:ns response)))
(t/is (= #{"done"} (:status response))))))

(t/testing "eval works"
(let [response (message {:op :eval
:code (nrepl/code (map even? (range 6)))})]
(t/is (= "cljs.user" (:ns response)))
(t/is (= ["(true false true false true false)"] (:value response)))
(t/is (= #{"done"} (:status response)))))
(testing (pr-str response)
(t/is (= "cljs.user" (:ns response)))
(t/is (= ["(true false true false true false)"] (:value response)))
(t/is (= #{"done"} (:status response))))))

(t/testing "errors handled properly"
(let [response (message {:op :eval
:code (nrepl/code (ffirst 1))})]
(t/is (= "class clojure.lang.ExceptionInfo"
(:ex response)
(:root-ex response)))
(t/is (string? (:err response)))
(t/is (= #{"eval-error" "done"} (:status response))))))
(testing (pr-str response)
(t/is (= "class clojure.lang.ExceptionInfo"
(:ex response)
(:root-ex response)))
(t/is (string? (:err response)))
(t/is (= #{"eval-error" "done"} (:status response)))))))

(t/deftest resolve-missing-test
(t/testing "Finds functions is regular namespaces"
(let [{:keys [^String error] :as response} (message {:op :resolve-missing :symbol 'print-doc})
_ (assert (string? (:candidates response))
(pr-str response))
{:keys [name type]} (first (edn/read-string (:candidates response)))]
(when error
(println error)
(throw (RuntimeException. error)))
(t/is (= 'cljs.repl name))
(t/is (= :ns type)))
(testing (pr-str response)
(t/is (= 'cljs.repl name))
(t/is (= :ns type))))
(t/testing "Finds macros"
(let [{:keys [^String error] :as response} (message {:op :resolve-missing :symbol 'dir})
{:keys [name type]} (first (edn/read-string (:candidates response)))]
(when error
(throw (RuntimeException. error)))
(t/is (= 'cljs.repl name))
(t/is (= :macro type))))))
(testing (pr-str response)
(t/is (= 'cljs.repl name))
(t/is (= :macro type)))))))
2 changes: 0 additions & 2 deletions test/refactor_nrepl/rename_file_or_dir_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,8 @@
(is (some #(.endsWith ^String % "non_clj_file") @files))
(is (= 4 (count (filter #(.endsWith ^String % ".cljs") @files)))))))


;;; cljs


(def from-file-path-cljs (.getAbsolutePath (File. "testproject/src/com/move/ns_to_be_moved_cljs.cljs")))
(def to-file-path-cljs (.getAbsolutePath (File. "testproject/src/com/move/moved_ns_cljs.cljs")))

Expand Down