From 43fcb942927dea865fcf3cdfcde522912672ccc0 Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 20 Jun 2023 21:18:00 +0200 Subject: [PATCH 01/15] Add test cases --- src/refactor_nrepl/ns/libspecs.clj | 18 +- src/refactor_nrepl/ns/ns_parser.clj | 56 +++- src/refactor_nrepl/ns/suggest_libspecs.clj | 314 ++++++++++++++++-- test-resources/test_clj_ns.clj | 1 + test-resources/test_cljc_ns.cljc | 1 + test-resources/test_cljc_ns_2.cljc | 1 + test-resources/test_cljs_ns.cljs | 1 + test-resources/test_cljs_ns_2.cljs | 1 + .../ns/suggest_libspecs_test.clj | 109 ++++++ test/refactor_nrepl/s_expressions_test.clj | 39 ++- test/test_aliases_sample.cljc | 7 + 11 files changed, 484 insertions(+), 64 deletions(-) create mode 100644 test-resources/test_clj_ns.clj create mode 100644 test-resources/test_cljc_ns.cljc create mode 100644 test-resources/test_cljc_ns_2.cljc create mode 100644 test-resources/test_cljs_ns.cljs create mode 100644 test-resources/test_cljs_ns_2.cljs create mode 100644 test/refactor_nrepl/ns/suggest_libspecs_test.clj create mode 100644 test/test_aliases_sample.cljc diff --git a/src/refactor_nrepl/ns/libspecs.clj b/src/refactor_nrepl/ns/libspecs.clj index 3b1ebf72..b248177d 100644 --- a/src/refactor_nrepl/ns/libspecs.clj +++ b/src/refactor_nrepl/ns/libspecs.clj @@ -15,7 +15,7 @@ (->> libspecs (map (juxt :as :ns)) (remove #(nil? (first %))) - distinct)) + distinct)) ;; XXX distinct (defn- aliases-by-frequencies [libspecs] (let [grouped (->> libspecs @@ -37,13 +37,27 @@ (when (= ts (.lastModified f)) v))) +;; XXX I use `distinct` in misc places for libspecs. Meta should be merged, instead of dropping one branch +(defn add-used-from-meta [libspecs ^File f] + (let [extension (case (re-find #"\.clj[cs]?$" (-> f .getAbsolutePath)) + ".clj" :clj + ".cljs" :cljs + ".cljc" :cljc + nil)] + (if-not extension + libspecs + (into [] + (map (fn [libspec] + (update libspec :ns vary-meta assoc :used-from extension))) + libspecs)))) + (defn- put-cached-ns-info! [^File f lang] (binding [;; briefly memoize this function to avoid repeating its IO cost while `f` is being cached: ns-parser/*read-ns-form-with-meta* (memoize core/read-ns-form-with-meta)] (let [libspecs (ns-parser/get-libspecs-from-file lang f) [_ namespace-name] (ns-parser/*read-ns-form-with-meta* lang f) suggested-aliases (suggest-aliases/suggested-aliases namespace-name) - v {:libspecs libspecs + v {:libspecs (add-used-from-meta libspecs f) :namespace-name namespace-name :suggested-aliases suggested-aliases :test-like-ns-name? (suggest-aliases/test-like-ns-name? namespace-name)}] diff --git a/src/refactor_nrepl/ns/ns_parser.clj b/src/refactor_nrepl/ns/ns_parser.clj index 32a845a9..b1647a6d 100644 --- a/src/refactor_nrepl/ns/ns_parser.clj +++ b/src/refactor_nrepl/ns/ns_parser.clj @@ -14,6 +14,7 @@ (:require [clojure.java.io :as io] [clojure.set :as set] + [clojure.string :as string] [refactor-nrepl.core :as core]) (:import (java.io File))) @@ -25,6 +26,39 @@ (into {:ns ns} (->> specs (partition 2) (map vec)))) {:ns (symbol libspec)})) +(defn ns-decl->resource-path [ns-decl extension] + (-> ns-decl + second + str + munge + (string/replace "." "/") + (str extension))) + +(defn resource-path->filenames [resource-path] + (->> (-> (Thread/currentThread) + (.getContextClassLoader) + (.getResources resource-path)) + (enumeration-seq) + (distinct) + (mapv str))) + +(defn ns-sym->ns-filenames [ns-sym] + (let [base (-> ns-sym + str + (string/replace "-" "_") + (string/replace "." "/"))] + (not-empty (into [] + (comp (keep (fn [extension] + (-> base (str extension) resource-path->filenames not-empty))) + cat) + [".clj" ".cljs" ".cljc"])))) + +(defn add-file-meta [{ns-sym :ns :as m}] + {:pre [ns-sym]} + (let [files (ns-sym->ns-filenames ns-sym)] + (cond-> m + files (update :ns vary-meta assoc :files files)))) + (defn- expand-prefix-specs "Eliminate prefix lists." [libspecs] @@ -57,17 +91,20 @@ ~@body)) (defn- extract-libspecs [ns-form] - (mapcat identity + (reduce into + [] [(with-libspecs-from ns-form :require - (->> libspecs - expand-prefix-specs - (map libspec-vector->map))) + (into [] + (comp (map libspec-vector->map) + (map add-file-meta)) + (expand-prefix-specs libspecs))) (with-libspecs-from ns-form :use - (->> libspecs - expand-prefix-specs - (map use-to-refer-all) - (map libspec-vector->map)))])) + (into [] + (comp (map use-to-refer-all) + (map libspec-vector->map) + (map add-file-meta)) + (expand-prefix-specs libspecs)))])) (defn get-libspecs [ns-form] (some->> ns-form @@ -139,12 +176,13 @@ Dialect is either :clj or :cljs, the default is :clj." ([^File f] (get-libspecs-from-file :clj f)) + ([dialect ^File f] (some->> f .getAbsolutePath (*read-ns-form-with-meta* dialect) ((juxt get-libspecs get-required-macros)) - (mapcat identity)))) + (reduce into [])))) (defn aliases "Return a map of namespace aliases given a seq of libspecs. diff --git a/src/refactor_nrepl/ns/suggest_libspecs.clj b/src/refactor_nrepl/ns/suggest_libspecs.clj index e5ca161f..9910f713 100644 --- a/src/refactor_nrepl/ns/suggest_libspecs.clj +++ b/src/refactor_nrepl/ns/suggest_libspecs.clj @@ -1,54 +1,298 @@ (ns refactor-nrepl.ns.suggest-libspecs "Beta middleware, meant only for internal development. Subject to change." + (:refer-clojure :exclude [alias]) (:require [refactor-nrepl.core :as core] - [refactor-nrepl.ns.libspecs :as libspecs])) + [refactor-nrepl.ns.libspecs :as libspecs] + [refactor-nrepl.ns.ns-parser :as ns-parser])) -(defn suggest-libspecs-response - "Very basic POC for https://github.com/clojure-emacs/refactor-nrepl/issues/384. +(defn vconj [coll x] + (if coll + (conj coll x) + [x])) + +(def parse-preferred-aliases + (memoize (fn parse-preferred-aliases* [preferred-aliases] + (let [m (volatile! {})] + (doseq [[prefix ns-name _only-keyword only] (mapv (partial mapv symbol) + preferred-aliases) + :let [files (ns-parser/ns-sym->ns-filenames ns-name) + ns-name (cond-> ns-name + files (vary-meta assoc :files files)) + only (or only [:clj :cljs]) + only (if (coll? only) + (vec only) + (vector only)) + clj? (some #{"clj" :clj 'clj} only) + cljs? (some #{"cljs" :cljs 'cljs} only)]] + (when clj? + (vswap! m update-in [:clj prefix] vconj ns-name)) + (when cljs? + (vswap! m update-in [:cljs prefix] vconj ns-name))) + @m)))) + +(comment + (parse-preferred-aliases [["set" "clojure.set"] + ["cljs" "cljs" :only "cljs"] + ["clj" "clj" :only "clj"]])) + +(defn build-reader-conditional [left-branch left-libspec _other-branch right-libspec as-alias] + (let [l-clj? (= left-branch :clj) + clj-clause (if l-clj? + left-libspec + right-libspec) + cljs-clause (if l-clj? + right-libspec + left-libspec)] + (format "#?(:clj [%s :as %s]\n :cljs [%s :as %s])" + clj-clause + as-alias + cljs-clause + as-alias))) + +;; XXX cache +(defn filenames->extensions [filenames] + (into [] + (comp (map (fn [s] + (re-find #".clj[cs]?$" s))) + (distinct)) + filenames)) + +(defn files->platform [filenames] + (let [extensions (filenames->extensions filenames) + cljc? (some #{".cljc"} extensions) + clj? (some #{".clj"} extensions) + cljs? (some #{".cljs"} extensions)] + (cond + (and cljc? + (not cljs?)) ;; a ns backed by .cljc and .cljs files most likely is only a cljs-oriented ns in practice, since the .cljc file only intends to define macros (this is the case for clojurescript's clojure.test) + :cljc + + (and clj? cljs?) + :cljc, + + clj? + :clj, + + cljs? + :cljs + + (and cljc? cljs?) ;; See comment above + :cljs))) - Only focuses on its API and giving some basic (but useful) results. +(defn valid-cljc-files? + "Does the set of `filenames` denote a namespace that can be required from both Clojure and ClojureScript?" + [filenames] + {:post [(boolean? %)]} + (= :cljc (files->platform filenames))) - The results are returned in no particular order." +(defn build-reader-conditionals-from [ns-syms as-alias] + (let [{:keys [clj cljs cljc]} (->> ns-syms + (group-by (fn [ns-sym] + (some-> ns-sym meta :files files->platform))))] + (into [] + (comp cat + (distinct)) + [(for [clj clj + cljs cljs] + (build-reader-conditional :clj clj + :cljs cljs + as-alias)) + (for [clj clj + cljs cljc] + (build-reader-conditional :clj clj + :cljs cljs + as-alias)) + (for [clj cljc + cljs cljs] + (build-reader-conditional :clj clj + :cljs cljs + as-alias)) + + (for [i cljc + j cljc + :when (not= i j) + :let [[left right] (if (-> i meta :used-from #{:clj}) + [i j] + [j i])]] + (build-reader-conditional :clj left + :cljs right + as-alias))]))) + +(defn build-partial-reader-conditional [ns-sym as-alias i-cljc?] + (let [files (-> ns-sym meta :files (doto (assert "No :files found"))) + platform (if (some #{".clj"} (filenames->extensions files)) + :clj + :cljs) + other-platform (if (= :clj platform) + :cljs + :clj)] + (if i-cljc? + (build-reader-conditional platform ns-sym + other-platform "" + as-alias) + (format "#?(%s [%s :as %s])" + platform + ns-sym + as-alias)))) + +(defn vec-distinct-into [x y] + (vec (distinct (into x y)))) + +(defn add-cljc-key [{:keys [clj cljs] :as m} as-alias] + (let [left (get clj as-alias) + right (get cljs as-alias) + v (not-empty (into [] + (comp cat + (filter (fn [candidate] + (some-> candidate meta :files valid-cljc-files?))) + (distinct)) + [left right]))] + (cond-> m + v (assoc-in [:cljc as-alias] v)))) + +(defn suggest-libspecs-response + "Implements https://github.com/clojure-emacs/refactor-nrepl/issues/384." [{:keys [lib-prefix ;; "set", representing that the user typed `set/` - ^String language-context ;; "clj" "cljs" or "cljc" representing the filename extension (or a user choice for edge cases e.g. it's a buffer/repl without a filename) + ^String buffer-language-context ;; Represents the file extension (or more rarely, major-mode, repl type, etc). + ;; * One of: cljs, clj, cljc + ^String input-language-context ;; Represents the context of what the user is typing. + ;; * Outside a reader conditional, its value should be identical to that of buffer-language-context + ;; * Inside a reader conditional, its value should be typically one of clj, cljs (not cljc) + ;; * For the edge case of typing #?(io/), you can default to buffer-language-context preferred-aliases ;; the entire value of cljr-magic-require-namespaces. See also https://github.com/clojure-emacs/clj-refactor.el/issues/530 for an intended future feature. suggest ;; the value of cljr-suggest-namespace-aliases - ignore-errors] + ignore-errors + namespace-aliases-fn] :or {ignore-errors true suggest true}}] {:pre [lib-prefix - language-context + buffer-language-context + input-language-context preferred-aliases - (boolean? suggest) - (boolean? ignore-errors)]} - (let [alias (symbol lib-prefix) - aliases (libspecs/namespace-aliases ignore-errors - (core/source-dirs-on-classpath) - suggest) - ks (cond-> #{} - (.equals language-context "clj") (conj :clj) - (.equals language-context "cljs") (conj :cljs) - (.equals language-context "cljc") (conj :clj :cljs) - true vec) - maps (map (fn [k] - (get aliases k)) - ks) - merged (apply merge-with (fn [x y] - (vec (distinct (into x y)))) - maps) - candidates (get merged alias)] - (->> candidates - (mapv (fn [candidate] - (format "[%s :as %s]" - candidate - alias)))))) + (not (string? suggest)) ;; should be boolean or nil + (not (string? ignore-errors))]} ;; should be boolean or nil + (let [namespace-aliases-fn (or namespace-aliases-fn + libspecs/namespace-aliases) + ^String input-language-context (or (not-empty input-language-context) + buffer-language-context) + as-alias (symbol lib-prefix) + b-cljc? (.equals buffer-language-context "cljc") + i-clj? (.equals input-language-context "clj") + i-cljs? (.equals input-language-context "cljs") + i-cljc? (.equals input-language-context "cljc") + aliases (namespace-aliases-fn ignore-errors + (core/source-dirs-on-classpath) + suggest) + aliases (cond-> aliases + b-cljc? (add-cljc-key as-alias)) ;; use get-in [:clj as-alias], filter cljc. same for cljs, use vec-distinct-into + ks (into [] + (comp (filter identity) + (distinct)) + [(when i-clj? + :clj) + (when i-cljs? + :cljs) + (when i-cljc? + :clj) + (when i-cljc? + :cljs) + (when b-cljc? + :cljc)]) + maps (keep (fn [k] + (get aliases k)) + ks) + parsed-preferred-aliases (parse-preferred-aliases preferred-aliases) + map-from-preferred (->> ks + (map (fn [k] + (get parsed-preferred-aliases k))) + (apply merge-with vec-distinct-into)) + merged (apply merge-with vec-distinct-into maps) + final (merge-with (fn [existing from-preferred] + ;; IF b-cljc? AND `existing` only exists in one branch (:clj, :cljs), AND from-preferred applies to the other branch, + ;; include both. + (let [reader-conditionals + (when b-cljc? + (let [branches-left (->> [:clj :cljs] + (filter (fn [k] + (some (set (get-in aliases [k as-alias])) + existing)))) + branches-right (->> [:clj :cljs] + (filter (fn [k] + (some (set (get-in parsed-preferred-aliases [k as-alias])) + from-preferred)))) + other-branch? (and (-> branches-left count #{1}) + (seq branches-right) + (some (complement (set branches-left)) + branches-right)) + left-branch (first branches-left) + other-branch (when other-branch? + (case left-branch + :clj :cljs + :cljs :clj))] + (when other-branch + (into [] + (comp (filter (fn [libspec] + ((set (get-in parsed-preferred-aliases [other-branch as-alias])) + libspec))) + (map (fn [right-libspec] + (keep (fn [left-libspec] + (when-not (= left-libspec right-libspec) + (build-reader-conditional left-branch left-libspec + other-branch right-libspec + as-alias))) + existing))) + cat) + from-preferred))))] + (cond-> existing ;; The baseline approach is to disregard `from-preferred` (i.e. any data from `map-from-preferred`) on conflict. + reader-conditionals (into reader-conditionals)))) + merged + map-from-preferred) + candidates (get final as-alias) + candidates (into candidates + (when b-cljc? ;; XXX maybe I need other conditions, refining b-cljc? + (build-reader-conditionals-from candidates as-alias))) + candidates (if-not b-cljc? ;; XXX maybe I need other conditions, refining b-cljc? + candidates + (or (not-empty (into [] + (remove (fn invalid? [x] + (and (not (string? x)) ;; reader conditionals are ok + ;; single platform suggestions are not valid in a reader conditional context + ;; XXX but they are on i-clj, i-cljs + (contains? #{:clj :cljs} + (some-> x meta :files files->platform))))) + candidates)) + candidates)) + _candidates-count (count candidates)] + (into [] + (keep (fn [candidate] + (cond + (string? candidate) + candidate ;; was already processed as a reader conditional string, leave as-is + + (and b-cljc? + (false? (some-> candidate meta :files valid-cljc-files?))) + #_(when (= 1 _candidates-count)) ;; only suggest partial reader conditionals if there's nothing better to do (NOTE: discarded condition. No tests fail without it) + (build-partial-reader-conditional candidate as-alias i-cljc?) + + :else ;; it's data, format it: + (format "[%s :as %s]" + candidate + as-alias)))) + candidates))) (comment - (suggest-libspecs-response {:lib-prefix "add-on" - :language-context "clj" + (suggest-libspecs-response {:lib-prefix "set" + :buffer-language-context "clj" + :input-language-context "clj" + :preferred-aliases []}) + + (suggest-libspecs-response {:lib-prefix "test" + :buffer-language-context "cljc" + :input-language-context "clj" :preferred-aliases []}) - (suggest-libspecs-response {:lib-prefix "s" - :language-context "cljc" + (suggest-libspecs-response {:lib-prefix "test" + :buffer-language-context "cljc" + :input-language-context "cljs" :preferred-aliases []})) diff --git a/test-resources/test_clj_ns.clj b/test-resources/test_clj_ns.clj new file mode 100644 index 00000000..3da7ac4c --- /dev/null +++ b/test-resources/test_clj_ns.clj @@ -0,0 +1 @@ +(ns test-clj-ns) diff --git a/test-resources/test_cljc_ns.cljc b/test-resources/test_cljc_ns.cljc new file mode 100644 index 00000000..36d709c0 --- /dev/null +++ b/test-resources/test_cljc_ns.cljc @@ -0,0 +1 @@ +(ns test-cljc-ns) diff --git a/test-resources/test_cljc_ns_2.cljc b/test-resources/test_cljc_ns_2.cljc new file mode 100644 index 00000000..004c1acc --- /dev/null +++ b/test-resources/test_cljc_ns_2.cljc @@ -0,0 +1 @@ +(ns test-cljc-ns-2) diff --git a/test-resources/test_cljs_ns.cljs b/test-resources/test_cljs_ns.cljs new file mode 100644 index 00000000..7fe374c5 --- /dev/null +++ b/test-resources/test_cljs_ns.cljs @@ -0,0 +1 @@ +(ns test-cljs-ns) diff --git a/test-resources/test_cljs_ns_2.cljs b/test-resources/test_cljs_ns_2.cljs new file mode 100644 index 00000000..1f4560d4 --- /dev/null +++ b/test-resources/test_cljs_ns_2.cljs @@ -0,0 +1 @@ +(ns test-cljs-ns-2) diff --git a/test/refactor_nrepl/ns/suggest_libspecs_test.clj b/test/refactor_nrepl/ns/suggest_libspecs_test.clj new file mode 100644 index 00000000..6183c6a7 --- /dev/null +++ b/test/refactor_nrepl/ns/suggest_libspecs_test.clj @@ -0,0 +1,109 @@ +(ns refactor-nrepl.ns.suggest-libspecs-test + (:require + [clojure.test :refer [are deftest is testing]] + [clojure.walk :as walk] + [refactor-nrepl.ns.ns-parser :as ns-parser] + [refactor-nrepl.ns.suggest-libspecs :as sut])) + +(defn add-file-meta [libspecs] + (let [{:keys [clj cljs] :as libspecs} (->> libspecs + (walk/postwalk (fn [x] + (cond->> x + (vector? x) (mapv (fn [s] + (let [files (ns-parser/ns-sym->ns-filenames s)] + (cond-> s + files (vary-meta assoc :files files))))))))) + c-paths (keys (:clj libspecs)) + s-paths (keys (:cljs libspecs)) + libspecs (reduce (fn [libspecs k] + (update-in libspecs [:clj k] (fn [candidates] + (mapv (fn [candidate] + (vary-meta candidate assoc :used-from :clj)) + candidates)))) + libspecs + c-paths)] + (reduce (fn [libspecs k] + (update-in libspecs [:cljs k] (fn [candidates] + (mapv (fn [candidate] + (vary-meta candidate assoc :used-from :cljs)) + candidates)))) + libspecs + s-paths))) + +(deftest suggest-libspecs-response + (reset! @#'refactor-nrepl.ns.libspecs/cache {}) + (are [lib-prefix buffer-lc input-lc preferred-aliases project-libspecs expected] + (testing [lib-prefix buffer-lc input-lc preferred-aliases project-libspecs] + (is (= expected + (sut/suggest-libspecs-response {:lib-prefix lib-prefix + :buffer-language-context buffer-lc + :input-language-context input-lc + :preferred-aliases preferred-aliases + :namespace-aliases-fn (when (seq project-libspecs) + (constantly (add-file-meta project-libspecs)))}))) + true) + #_lib-prefix #_buffer-lc #_input-lc #_preferred-aliases #_project-libspecs #_expected + "test" "clj" "clj" [] {} ["[clojure.test :as test]"] + "test" "cljs" "cljs" [] {} ["[cljs.test :as test]"] + "test" "cljc" "clj" [] {} ["#?(:clj [clojure.test :as test])"] + "test" "cljc" "cljs" [] {} ["#?(:cljs [cljs.test :as test])"] + "test" "cljc" "cljc" [] {} ["#?(:clj [clojure.test :as test]\n :cljs [cljs.test :as test])"] + ;; Example story 1: + "set" "cljc" "cljc" [["set" "clojure.set"]] {} ["[clojure.set :as set]"] + ;; Story 2 - preferred-aliases are disregarded if the libspecs found in the project differ: + "set" "cljc" "cljc" [["set" "clojure.set"]] '{:clj {set [something-else]} + :cljs {set [something-else]}} ["[something-else :as set]"] + ;; `preferred-aliases` are taken into account when they don't conflict with the `project-libspecs`: + "set" "cljc" "cljc" [["set" "clojure.set"]] '{:clj {x [y]} + :cljs {x [y]}} ["[clojure.set :as set]"] + + ;; Story 3: + "set" "cljc" "cljc" [["set" "clojure.set"]] '{:clj {set [something-else + clojure.set]} + :cljs {set [something-else + clojure.set]}} ["[something-else :as set]" "[clojure.set :as set]"] + + ;; Story 4: + "io" "clj" "clj" [["io" "clojure.java.io" :only :clj]] '{:cljs {io [something-else]}} ["[clojure.java.io :as io]"] + ;; Story 5: + "io" "clj" "clj" [["io" "clojure.java.io" :only :clj]] '{:clj {io [something-else]}} ["[something-else :as io]"] + "io" "clj" "clj" [["io" "something-else" :only :cljs]] {} ["[clojure.java.io :as io]"] + #_lib-prefix #_buffer-lc #_input-lc #_preferred-aliases #_project-libspecs #_expected + ;; Story 6: + "io" "cljc" "cljc" [["io" "clojure.java.io" :only :clj]] '{:cljs {io [test-cljc-ns]}} ["[test-cljc-ns :as io]" + "#?(:clj [clojure.java.io :as io]\n :cljs [test-cljc-ns :as io])"] + "io" "cljc" "clj" [["io" "clojure.java.io" :only :clj]] '{:cljs {io [test-cljc-ns]}} ["[test-cljc-ns :as io]" + "#?(:clj [clojure.java.io :as io]\n :cljs [test-cljc-ns :as io])"] + "io" "cljc" "cljs" [["io" "clojure.java.io" :only :clj]] '{:cljs {io [test-cljc-ns]}} ["[test-cljc-ns :as io]"] + "io" "cljc" "clj" [["io" "clojure.java.io" :only :clj]] '{:clj {io [test-clj-ns]}} ["#?(:clj [test-clj-ns :as io])"] + ;; The difference here is that test-cljs-ns is backed by a .cljs extension and therefore is not a valid .cljc or :clj suggestion + "io" "cljc" "cljc" [["io" "clojure.java.io" :only :clj]] '{:cljs {io [test-cljs-ns]}} ["#?(:clj [clojure.java.io :as io]\n :cljs [test-cljs-ns :as io])"] + ;; Returns an empty form for the :clj branch when there's no valid suggestion for it: + "io" "cljc" "cljc" [] '{:cljs {io [test-cljs-ns]}} ["#?(:clj [ :as io]\n :cljs [test-cljs-ns :as io])"] + "io" "cljc" "cljc" [] '{:cljs {io [test-cljc-ns]}} ["[test-cljc-ns :as io]"] + ;; https://github.com/clojure-emacs/refactor-nrepl/issues/384#issuecomment-1221622306 extra cases + ;; discards user preference, and offers both cljc choices individually and as a reader conditional (only one reader conditional! switching its branches makes less sense) + "io" "cljc" "cljc" [["io" "clojure.java.io" :only :clj]] '{:clj {io [test-cljc-ns]} + :cljs {io [test-cljc-ns-2]}} ["[test-cljc-ns :as io]" + "[test-cljc-ns-2 :as io]" + "#?(:clj [test-cljc-ns :as io]\n :cljs [test-cljc-ns-2 :as io])"] + "io" "cljc" "cljc" [] '{:clj {io [test-clj-ns]} + :cljs {io [test-cljs-ns]}} ["#?(:clj [test-clj-ns :as io]\n :cljs [test-cljs-ns :as io])"] + "io" "cljc" "cljc" [] '{:cljs {io [test-cljs-ns + test-cljs-ns-2]}} ["#?(:clj [ :as io]\n :cljs [test-cljs-ns :as io])" + "#?(:clj [ :as io]\n :cljs [test-cljs-ns-2 :as io])"] + "io" "cljc" "cljc" [] '{:clj {io [test-cljc-ns]} + :cljs {io [test-cljs-ns]}} ["[test-cljc-ns :as io]" + "#?(:clj [test-cljc-ns :as io]\n :cljs [test-cljs-ns :as io])"] + "io" "cljc" "cljc" [] '{:clj {io [test-cljc-ns]} + :cljs {io [test-cljc-ns-2]}} ["[test-cljc-ns :as io]" + "[test-cljc-ns-2 :as io]" + "#?(:clj [test-cljc-ns :as io]\n :cljs [test-cljc-ns-2 :as io])"] + "io" "cljc" "clj" [] '{:clj {io [test-cljc-ns]} + :cljs {io [test-cljc-ns-2]}} ["[test-cljc-ns :as io]" + "[test-cljc-ns-2 :as io]" ;; questionable but OK. It could be removed but it's not impossible thet the user wants it + "#?(:clj [test-cljc-ns :as io]\n :cljs [test-cljc-ns-2 :as io])"] + "io" "cljc" "cljs" [] '{:clj {io [test-cljc-ns]} + :cljs {io [test-cljc-ns-2]}} ["[test-cljc-ns-2 :as io]" + "[test-cljc-ns :as io]" ;; questionable but OK (Note that the questionable choice at least does not appear first) + "#?(:clj [test-cljc-ns :as io]\n :cljs [test-cljc-ns-2 :as io])"])) diff --git a/test/refactor_nrepl/s_expressions_test.clj b/test/refactor_nrepl/s_expressions_test.clj index 0af4a3c6..cea1c5a0 100644 --- a/test/refactor_nrepl/s_expressions_test.clj +++ b/test/refactor_nrepl/s_expressions_test.clj @@ -1,16 +1,19 @@ (ns refactor-nrepl.s-expressions-test (:require - [clojure.test :as t] + [clojure.test :as test] [refactor-nrepl.s-expressions :as sut])) (def file-content (slurp "testproject/src/com/example/sexp_test.clj")) + (def weird-file-content ";; some weird file ;; not even clojure ;; perhaps? no parens!") + (def file-content-with-set ";; with set #{foo bar baz} ;; some other stuff (foobar baz)") + (def file-content-with-uneval "#_ foo (foobar baz)") @@ -21,25 +24,25 @@ (def println-location [5 8]) (def when-not-location [10 9]) -(t/deftest get-enclosing-sexp-test - (t/is (= "[some :bindings +(test/deftest get-enclosing-sexp-test + (test/is (= "[some :bindings more :bindings]" - (apply sut/get-enclosing-sexp file-content binding-location))) - (t/is (= "(println #{some} + (apply sut/get-enclosing-sexp file-content binding-location))) + (test/is (= "(println #{some} ;; unhelpful comment ) (prn {\"foo\" {:qux [#{more}]}}))" - (apply sut/get-enclosing-sexp file-content println-location))) - (t/is (= "#{more}" (apply sut/get-enclosing-sexp file-content set-location))) - (t/is (= "{:qux [#{more}]}" (apply sut/get-enclosing-sexp file-content map-location))) - (t/is (= nil (apply sut/get-enclosing-sexp weird-file-content weird-location))) - (t/is (= "(when-not (= true true) + (apply sut/get-enclosing-sexp file-content println-location))) + (test/is (= "#{more}" (apply sut/get-enclosing-sexp file-content set-location))) + (test/is (= "{:qux [#{more}]}" (apply sut/get-enclosing-sexp file-content map-location))) + (test/is (= nil (apply sut/get-enclosing-sexp weird-file-content weird-location))) + (test/is (= "(when-not (= true true) (= 5 (* 2 2)))" - (apply sut/get-enclosing-sexp file-content when-not-location))) - (t/is (= nil (sut/get-first-sexp weird-file-content))) - (t/is (= "#{foo bar baz}" (sut/get-first-sexp file-content-with-set)))) + (apply sut/get-enclosing-sexp file-content when-not-location))) + (test/is (= nil (sut/get-first-sexp weird-file-content))) + (test/is (= "#{foo bar baz}" (sut/get-first-sexp file-content-with-set)))) -(t/deftest get-first-sexp - (t/is (= "(ns com.example.sexp-test)" - (sut/get-first-sexp file-content))) - (t/is (= "(foobar baz)" - (sut/get-first-sexp file-content-with-uneval)))) +(test/deftest get-first-sexp + (test/is (= "(ns com.example.sexp-test)" + (sut/get-first-sexp file-content))) + (test/is (= "(foobar baz)" + (sut/get-first-sexp file-content-with-uneval)))) diff --git a/test/test_aliases_sample.cljc b/test/test_aliases_sample.cljc new file mode 100644 index 00000000..2154dc48 --- /dev/null +++ b/test/test_aliases_sample.cljc @@ -0,0 +1,7 @@ +(ns test-aliases-sample + "Supports `refactor-nrepl.ns.suggest-libspecs-test`." + (:require + #?(:clj [clojure.test :as test] :cljs [cljs.test :as test]))) + +(test/deftest foo + (test/is (pos? (inc (rand-int 1))))) From 265f5ee5c685af8a2970301e738c891938a27d74 Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 20 Jun 2023 22:35:52 +0200 Subject: [PATCH 02/15] Guard against string requires --- src/refactor_nrepl/ns/libspecs.clj | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/refactor_nrepl/ns/libspecs.clj b/src/refactor_nrepl/ns/libspecs.clj index b248177d..753b4831 100644 --- a/src/refactor_nrepl/ns/libspecs.clj +++ b/src/refactor_nrepl/ns/libspecs.clj @@ -48,7 +48,9 @@ libspecs (into [] (map (fn [libspec] - (update libspec :ns vary-meta assoc :used-from extension))) + (cond-> libspec + (not (-> libspec :ns string?)) + (update :ns vary-meta assoc :used-from extension)))) libspecs)))) (defn- put-cached-ns-info! [^File f lang] From aa97d989ffb60ea8287cd4943693fea6e988c1f5 Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 20 Jun 2023 22:40:59 +0200 Subject: [PATCH 03/15] kondo --- test/refactor_nrepl/ns/suggest_libspecs_test.clj | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/refactor_nrepl/ns/suggest_libspecs_test.clj b/test/refactor_nrepl/ns/suggest_libspecs_test.clj index 6183c6a7..1f0f018c 100644 --- a/test/refactor_nrepl/ns/suggest_libspecs_test.clj +++ b/test/refactor_nrepl/ns/suggest_libspecs_test.clj @@ -2,17 +2,18 @@ (:require [clojure.test :refer [are deftest is testing]] [clojure.walk :as walk] + [refactor-nrepl.ns.libspecs] [refactor-nrepl.ns.ns-parser :as ns-parser] [refactor-nrepl.ns.suggest-libspecs :as sut])) (defn add-file-meta [libspecs] - (let [{:keys [clj cljs] :as libspecs} (->> libspecs - (walk/postwalk (fn [x] - (cond->> x - (vector? x) (mapv (fn [s] - (let [files (ns-parser/ns-sym->ns-filenames s)] - (cond-> s - files (vary-meta assoc :files files))))))))) + (let [libspecs (->> libspecs + (walk/postwalk (fn [x] + (cond->> x + (vector? x) (mapv (fn [s] + (let [files (ns-parser/ns-sym->ns-filenames s)] + (cond-> s + files (vary-meta assoc :files files))))))))) c-paths (keys (:clj libspecs)) s-paths (keys (:cljs libspecs)) libspecs (reduce (fn [libspecs k] From bb73d263fee420dcc5e53374f187e6890fa9562e Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 20 Jun 2023 22:43:34 +0200 Subject: [PATCH 04/15] Clojure <= 1.9 compat --- src/refactor_nrepl/ns/suggest_libspecs.clj | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/refactor_nrepl/ns/suggest_libspecs.clj b/src/refactor_nrepl/ns/suggest_libspecs.clj index 9910f713..ce4f7ddb 100644 --- a/src/refactor_nrepl/ns/suggest_libspecs.clj +++ b/src/refactor_nrepl/ns/suggest_libspecs.clj @@ -14,7 +14,8 @@ (def parse-preferred-aliases (memoize (fn parse-preferred-aliases* [preferred-aliases] (let [m (volatile! {})] - (doseq [[prefix ns-name _only-keyword only] (mapv (partial mapv symbol) + (doseq [[prefix ns-name _only-keyword only] (mapv (partial mapv (comp symbol + name)) ;; `name` for Clojure <= 1.9 compat preferred-aliases) :let [files (ns-parser/ns-sym->ns-filenames ns-name) ns-name (cond-> ns-name From 5c96b8e74ec2a639352281b6bac9509fca1c1508 Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 20 Jun 2023 22:45:05 +0200 Subject: [PATCH 05/15] Eastwood --- test/test_aliases_sample.cljc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_aliases_sample.cljc b/test/test_aliases_sample.cljc index 2154dc48..9354afea 100644 --- a/test/test_aliases_sample.cljc +++ b/test/test_aliases_sample.cljc @@ -4,4 +4,4 @@ #?(:clj [clojure.test :as test] :cljs [cljs.test :as test]))) (test/deftest foo - (test/is (pos? (inc (rand-int 1))))) + (test/is (pos? (inc (int (rand-int 1)))))) From 9f9ec93c0d1be59c23c49af884879dc81e3bc118 Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 20 Jun 2023 22:58:31 +0200 Subject: [PATCH 06/15] `boolean?` compat --- src/refactor_nrepl/ns/suggest_libspecs.clj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/refactor_nrepl/ns/suggest_libspecs.clj b/src/refactor_nrepl/ns/suggest_libspecs.clj index ce4f7ddb..38cc6d57 100644 --- a/src/refactor_nrepl/ns/suggest_libspecs.clj +++ b/src/refactor_nrepl/ns/suggest_libspecs.clj @@ -84,7 +84,7 @@ (defn valid-cljc-files? "Does the set of `filenames` denote a namespace that can be required from both Clojure and ClojureScript?" [filenames] - {:post [(boolean? %)]} + {:post [(instance? Boolean %)]} (= :cljc (files->platform filenames))) (defn build-reader-conditionals-from [ns-syms as-alias] From 8a8532336b7af63f894f92b61faff42c740617c6 Mon Sep 17 00:00:00 2001 From: vemv Date: Wed, 21 Jun 2023 20:27:44 +0200 Subject: [PATCH 07/15] Use a metadata-aware `distinct` --- src/refactor_nrepl/ns/libspecs.clj | 41 +++++++++++++----- src/refactor_nrepl/ns/suggest_libspecs.clj | 42 ++++++++++++------- src/refactor_nrepl/util/meta.clj | 22 ++++++++++ .../ns/suggest_libspecs_test.clj | 4 +- test/refactor_nrepl/util/meta_test.clj | 36 ++++++++++++++++ 5 files changed, 116 insertions(+), 29 deletions(-) create mode 100644 src/refactor_nrepl/util/meta.clj create mode 100644 test/refactor_nrepl/util/meta_test.clj diff --git a/src/refactor_nrepl/ns/libspecs.clj b/src/refactor_nrepl/ns/libspecs.clj index 753b4831..f6eca5a3 100644 --- a/src/refactor_nrepl/ns/libspecs.clj +++ b/src/refactor_nrepl/ns/libspecs.clj @@ -3,7 +3,8 @@ [refactor-nrepl.core :as core] [refactor-nrepl.ns.ns-parser :as ns-parser] [refactor-nrepl.ns.suggest-aliases :as suggest-aliases] - [refactor-nrepl.util :as util]) + [refactor-nrepl.util :as util] + [refactor-nrepl.util.meta :as meta]) (:import (java.io File))) @@ -11,15 +12,34 @@ ;; where lang is either :clj or :cljs (def ^:private cache (atom {})) +(defn vec-distinct-into [x y] + (into [] + (comp cat + (distinct)) + [x y])) + +(defn merge-libspecs-meta [a b] + (let [{:keys [used-from files]} (meta b)] + (cond-> a + (seq used-from) (vary-meta update :used-from vec-distinct-into used-from) + (seq files) (vary-meta update :files vec-distinct-into files)))) + +(comment + (binding [*print-meta* true] + (clojure.pprint/pprint (merge-libspecs-meta ^{:files [1] :used-from [1]} {} + ^{:files [2] :used-from [2]} {})))) + (defn- aliases [libspecs] - (->> libspecs - (map (juxt :as :ns)) - (remove #(nil? (first %))) - distinct)) ;; XXX distinct + (meta/distinct merge-libspecs-meta + (into [] + (comp (map (juxt :as :ns)) + (filter first)) + libspecs))) (defn- aliases-by-frequencies [libspecs] - (let [grouped (->> libspecs - (mapcat aliases) ; => [[str clojure.string] ...] + (let [grouped (->> (into [] + (mapcat aliases) ; => [[str clojure.string] ...] + libspecs) (sort-by (comp str second)) (group-by first) ; => {str [[str clojure.string] [str clojure.string]] ...} )] @@ -37,12 +57,11 @@ (when (= ts (.lastModified f)) v))) -;; XXX I use `distinct` in misc places for libspecs. Meta should be merged, instead of dropping one branch (defn add-used-from-meta [libspecs ^File f] (let [extension (case (re-find #"\.clj[cs]?$" (-> f .getAbsolutePath)) - ".clj" :clj - ".cljs" :cljs - ".cljc" :cljc + ".clj" [:clj] ;; these are expressed as vectors, so that `#'merge-libspecs-meta` can operate upon them + ".cljs" [:cljs] + ".cljc" [:cljc] nil)] (if-not extension libspecs diff --git a/src/refactor_nrepl/ns/suggest_libspecs.clj b/src/refactor_nrepl/ns/suggest_libspecs.clj index 38cc6d57..b1797531 100644 --- a/src/refactor_nrepl/ns/suggest_libspecs.clj +++ b/src/refactor_nrepl/ns/suggest_libspecs.clj @@ -4,7 +4,8 @@ (:require [refactor-nrepl.core :as core] [refactor-nrepl.ns.libspecs :as libspecs] - [refactor-nrepl.ns.ns-parser :as ns-parser])) + [refactor-nrepl.ns.ns-parser :as ns-parser] + [refactor-nrepl.util.meta :as meta])) (defn vconj [coll x] (if coll @@ -18,14 +19,19 @@ name)) ;; `name` for Clojure <= 1.9 compat preferred-aliases) :let [files (ns-parser/ns-sym->ns-filenames ns-name) - ns-name (cond-> ns-name - files (vary-meta assoc :files files)) only (or only [:clj :cljs]) only (if (coll? only) (vec only) (vector only)) - clj? (some #{"clj" :clj 'clj} only) - cljs? (some #{"cljs" :cljs 'cljs} only)]] + clj? (some #{"clj" :clj 'clj} only) + cljs? (some #{"cljs" :cljs 'cljs} only) + used-from (cond + (and clj? cljs?) [:clj :cljs] + clj? [:clj] + cljs? [:cljs]) + ns-name (cond-> ns-name + files (vary-meta assoc :files files) + used-from (vary-meta assoc :used-from used-from))]] (when clj? (vswap! m update-in [:clj prefix] vconj ns-name)) (when cljs? @@ -113,7 +119,7 @@ (for [i cljc j cljc :when (not= i j) - :let [[left right] (if (-> i meta :used-from #{:clj}) + :let [[left right] (if (->> i meta :used-from (some #{:clj})) [i j] [j i])]] (build-reader-conditional :clj left @@ -137,21 +143,25 @@ ns-sym as-alias)))) -(defn vec-distinct-into [x y] - (vec (distinct (into x y)))) - (defn add-cljc-key [{:keys [clj cljs] :as m} as-alias] (let [left (get clj as-alias) right (get cljs as-alias) - v (not-empty (into [] - (comp cat - (filter (fn [candidate] - (some-> candidate meta :files valid-cljc-files?))) - (distinct)) - [left right]))] + v (some->> [left right] + (into [] + (comp cat + (filter (fn [candidate] + (some-> candidate meta :files valid-cljc-files?))))) + (not-empty) + (meta/distinct libspecs/merge-libspecs-meta))] (cond-> m v (assoc-in [:cljc as-alias] v)))) +(defn vec-distinct-into [x y] + (->> [x y] + (reduce into []) + (meta/distinct libspecs/merge-libspecs-meta))) + +;; XXX split into nicer fns (defn suggest-libspecs-response "Implements https://github.com/clojure-emacs/refactor-nrepl/issues/384." [{:keys [lib-prefix ;; "set", representing that the user typed `set/` @@ -186,7 +196,7 @@ (core/source-dirs-on-classpath) suggest) aliases (cond-> aliases - b-cljc? (add-cljc-key as-alias)) ;; use get-in [:clj as-alias], filter cljc. same for cljs, use vec-distinct-into + b-cljc? (add-cljc-key as-alias)) ks (into [] (comp (filter identity) (distinct)) diff --git a/src/refactor_nrepl/util/meta.clj b/src/refactor_nrepl/util/meta.clj new file mode 100644 index 00000000..dd322cf4 --- /dev/null +++ b/src/refactor_nrepl/util/meta.clj @@ -0,0 +1,22 @@ +(ns refactor-nrepl.util.meta + "Metadata-oriented helpers." + (:refer-clojure :exclude [distinct])) + +(defn distinct + "Like `#'clojure.core/distinct`, but takes a 2-arg `f` that will choose/build the winning value whenever to equal ones are found. + + This helps merging metadata according to custom rules." + [f coll] + (let [index (volatile! {})] + (reduce (fn [acc x] + (let [entry (find @index x) + i (some-> entry val) + acc (cond-> acc + entry (update i (fn [existing-value] + (f existing-value x))) + (not entry) (conj x))] + (when-not entry + (vswap! index assoc x (dec (count acc)))) + acc)) + [] + coll))) diff --git a/test/refactor_nrepl/ns/suggest_libspecs_test.clj b/test/refactor_nrepl/ns/suggest_libspecs_test.clj index 1f0f018c..b1823a9b 100644 --- a/test/refactor_nrepl/ns/suggest_libspecs_test.clj +++ b/test/refactor_nrepl/ns/suggest_libspecs_test.clj @@ -19,14 +19,14 @@ libspecs (reduce (fn [libspecs k] (update-in libspecs [:clj k] (fn [candidates] (mapv (fn [candidate] - (vary-meta candidate assoc :used-from :clj)) + (vary-meta candidate assoc :used-from [:clj])) candidates)))) libspecs c-paths)] (reduce (fn [libspecs k] (update-in libspecs [:cljs k] (fn [candidates] (mapv (fn [candidate] - (vary-meta candidate assoc :used-from :cljs)) + (vary-meta candidate assoc :used-from [:cljs])) candidates)))) libspecs s-paths))) diff --git a/test/refactor_nrepl/util/meta_test.clj b/test/refactor_nrepl/util/meta_test.clj new file mode 100644 index 00000000..d268876f --- /dev/null +++ b/test/refactor_nrepl/util/meta_test.clj @@ -0,0 +1,36 @@ +(ns refactor-nrepl.util.meta-test + (:require + [clojure.test :refer [deftest is]] + [refactor-nrepl.util.meta :as sut])) + +(deftest distinct-test + (let [f (fn [x _y] + x)] + (is (= [] + (sut/distinct f []))) + (is (= [3 1 2] + (sut/distinct f [3 1 2 3 1 2])))) + + (let [f (fn [x y] + (vary-meta x merge (meta y))) + [x :as all] (sut/distinct f + [^:foo {} + ^:bar {} + ^:baz {}])] + (is (= [{}] all)) + (is (= {:foo true, :bar true, :baz true} + (meta x)))) + + (let [f (fn [x y] + (vary-meta x merge (meta y))) + [x y :as all] (sut/distinct f + [^:foo {} + ^:quux {1 1} + ^:bar {} + ^:baz {} + ^:quuz {1 1}])] + (is (= [{} {1 1}] all)) + (is (= {:foo true, :bar true, :baz true} + (meta x))) + (is (= {:quux true, :quuz true} + (meta y))))) From b3e76b19ddbfaf7e3e3aef188d5f56587b83b72f Mon Sep 17 00:00:00 2001 From: vemv Date: Wed, 21 Jun 2023 21:11:59 +0200 Subject: [PATCH 08/15] Extract `maybe-add-reader-conditionals-from-preferences` --- src/refactor_nrepl/ns/libspecs.clj | 5 -- src/refactor_nrepl/ns/suggest_libspecs.clj | 83 +++++++++++----------- test/refactor_nrepl/util/meta_test.clj | 6 +- 3 files changed, 45 insertions(+), 49 deletions(-) diff --git a/src/refactor_nrepl/ns/libspecs.clj b/src/refactor_nrepl/ns/libspecs.clj index f6eca5a3..aa364f6a 100644 --- a/src/refactor_nrepl/ns/libspecs.clj +++ b/src/refactor_nrepl/ns/libspecs.clj @@ -24,11 +24,6 @@ (seq used-from) (vary-meta update :used-from vec-distinct-into used-from) (seq files) (vary-meta update :files vec-distinct-into files)))) -(comment - (binding [*print-meta* true] - (clojure.pprint/pprint (merge-libspecs-meta ^{:files [1] :used-from [1]} {} - ^{:files [2] :used-from [2]} {})))) - (defn- aliases [libspecs] (meta/distinct merge-libspecs-meta (into [] diff --git a/src/refactor_nrepl/ns/suggest_libspecs.clj b/src/refactor_nrepl/ns/suggest_libspecs.clj index b1797531..b3bd77b0 100644 --- a/src/refactor_nrepl/ns/suggest_libspecs.clj +++ b/src/refactor_nrepl/ns/suggest_libspecs.clj @@ -161,7 +161,46 @@ (reduce into []) (meta/distinct libspecs/merge-libspecs-meta))) -;; XXX split into nicer fns +(defn maybe-add-reader-conditionals-from-preferences + [b-cljc? aliases as-alias parsed-preferred-aliases existing from-preferred] + ;; IF b-cljc? AND `existing` only exists in one branch (:clj, :cljs), AND from-preferred applies to the other branch, + ;; include both. + (let [reader-conditionals + (when b-cljc? + (let [branches-left (->> [:clj :cljs] + (filter (fn [k] + (some (set (get-in aliases [k as-alias])) + existing)))) + branches-right (->> [:clj :cljs] + (filter (fn [k] + (some (set (get-in parsed-preferred-aliases [k as-alias])) + from-preferred)))) + other-branch? (and (-> branches-left count #{1}) + (seq branches-right) + (some (complement (set branches-left)) + branches-right)) + left-branch (first branches-left) + other-branch (when other-branch? + (case left-branch + :clj :cljs + :cljs :clj))] + (when other-branch + (into [] + (comp (filter (fn [libspec] + ((set (get-in parsed-preferred-aliases [other-branch as-alias])) + libspec))) + (map (fn [right-libspec] + (keep (fn [left-libspec] + (when-not (= left-libspec right-libspec) + (build-reader-conditional left-branch left-libspec + other-branch right-libspec + as-alias))) + existing))) + cat) + from-preferred))))] + (cond-> existing ;; The baseline approach is to disregard `from-preferred` (i.e. any data from `map-from-preferred`) on conflict. + reader-conditionals (into reader-conditionals)))) + (defn suggest-libspecs-response "Implements https://github.com/clojure-emacs/refactor-nrepl/issues/384." [{:keys [lib-prefix ;; "set", representing that the user typed `set/` @@ -220,43 +259,7 @@ (apply merge-with vec-distinct-into)) merged (apply merge-with vec-distinct-into maps) final (merge-with (fn [existing from-preferred] - ;; IF b-cljc? AND `existing` only exists in one branch (:clj, :cljs), AND from-preferred applies to the other branch, - ;; include both. - (let [reader-conditionals - (when b-cljc? - (let [branches-left (->> [:clj :cljs] - (filter (fn [k] - (some (set (get-in aliases [k as-alias])) - existing)))) - branches-right (->> [:clj :cljs] - (filter (fn [k] - (some (set (get-in parsed-preferred-aliases [k as-alias])) - from-preferred)))) - other-branch? (and (-> branches-left count #{1}) - (seq branches-right) - (some (complement (set branches-left)) - branches-right)) - left-branch (first branches-left) - other-branch (when other-branch? - (case left-branch - :clj :cljs - :cljs :clj))] - (when other-branch - (into [] - (comp (filter (fn [libspec] - ((set (get-in parsed-preferred-aliases [other-branch as-alias])) - libspec))) - (map (fn [right-libspec] - (keep (fn [left-libspec] - (when-not (= left-libspec right-libspec) - (build-reader-conditional left-branch left-libspec - other-branch right-libspec - as-alias))) - existing))) - cat) - from-preferred))))] - (cond-> existing ;; The baseline approach is to disregard `from-preferred` (i.e. any data from `map-from-preferred`) on conflict. - reader-conditionals (into reader-conditionals)))) + (maybe-add-reader-conditionals-from-preferences b-cljc? aliases as-alias parsed-preferred-aliases existing from-preferred)) merged map-from-preferred) candidates (get final as-alias) @@ -273,8 +276,7 @@ (contains? #{:clj :cljs} (some-> x meta :files files->platform))))) candidates)) - candidates)) - _candidates-count (count candidates)] + candidates))] (into [] (keep (fn [candidate] (cond @@ -283,7 +285,6 @@ (and b-cljc? (false? (some-> candidate meta :files valid-cljc-files?))) - #_(when (= 1 _candidates-count)) ;; only suggest partial reader conditionals if there's nothing better to do (NOTE: discarded condition. No tests fail without it) (build-partial-reader-conditional candidate as-alias i-cljc?) :else ;; it's data, format it: diff --git a/test/refactor_nrepl/util/meta_test.clj b/test/refactor_nrepl/util/meta_test.clj index d268876f..5514e295 100644 --- a/test/refactor_nrepl/util/meta_test.clj +++ b/test/refactor_nrepl/util/meta_test.clj @@ -19,7 +19,7 @@ ^:baz {}])] (is (= [{}] all)) (is (= {:foo true, :bar true, :baz true} - (meta x)))) + (-> x meta (select-keys [:foo :bar :baz]))))) (let [f (fn [x y] (vary-meta x merge (meta y))) @@ -31,6 +31,6 @@ ^:quuz {1 1}])] (is (= [{} {1 1}] all)) (is (= {:foo true, :bar true, :baz true} - (meta x))) + (-> x meta (select-keys [:foo :bar :baz])))) (is (= {:quux true, :quuz true} - (meta y))))) + (-> y meta (select-keys [:quux :quuz])))))) From bafe2f567dea08b459faffb96ec64b7252b61dfb Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 4 Jul 2023 14:14:06 +0200 Subject: [PATCH 09/15] Add a test case --- test/refactor_nrepl/ns/suggest_libspecs_test.clj | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/refactor_nrepl/ns/suggest_libspecs_test.clj b/test/refactor_nrepl/ns/suggest_libspecs_test.clj index b1823a9b..000b2c17 100644 --- a/test/refactor_nrepl/ns/suggest_libspecs_test.clj +++ b/test/refactor_nrepl/ns/suggest_libspecs_test.clj @@ -96,6 +96,9 @@ "io" "cljc" "cljc" [] '{:clj {io [test-cljc-ns]} :cljs {io [test-cljs-ns]}} ["[test-cljc-ns :as io]" "#?(:clj [test-cljc-ns :as io]\n :cljs [test-cljs-ns :as io])"] + + "io" "cljc" "cljs" [] '{:cljs {io [test-cljs-ns]}} ["#?(:cljs [test-cljs-ns :as io])"] + "io" "cljc" "cljc" [] '{:clj {io [test-cljc-ns]} :cljs {io [test-cljc-ns-2]}} ["[test-cljc-ns :as io]" "[test-cljc-ns-2 :as io]" From d9147def7144a295006fde5650c538704bd51a63 Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 4 Jul 2023 14:14:19 +0200 Subject: [PATCH 10/15] Add a comment --- test/refactor_nrepl/ns/suggest_libspecs_test.clj | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/refactor_nrepl/ns/suggest_libspecs_test.clj b/test/refactor_nrepl/ns/suggest_libspecs_test.clj index 000b2c17..dd3a5cd5 100644 --- a/test/refactor_nrepl/ns/suggest_libspecs_test.clj +++ b/test/refactor_nrepl/ns/suggest_libspecs_test.clj @@ -41,6 +41,11 @@ :input-language-context input-lc :preferred-aliases preferred-aliases :namespace-aliases-fn (when (seq project-libspecs) + ;; if provided, replace `refactor-nrepl.ns.libspecs/namespace-aliases` + ;; with a mocked value, for test simplicity. + ;; NOTE: an alternative approach would be to always use the original fn, + ;; but perform a 'nested select keys' on it. + ;; That way, we make sure the values are realistic. (constantly (add-file-meta project-libspecs)))}))) true) #_lib-prefix #_buffer-lc #_input-lc #_preferred-aliases #_project-libspecs #_expected From 4d60a8d82672f89c42e862ce7d60224f1610db0b Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 4 Jul 2023 15:33:52 +0200 Subject: [PATCH 11/15] Never suggest cljs.test --- src/refactor_nrepl/ns/suggest_libspecs.clj | 46 +++++++++---------- test/donkey/jvm.clj | 2 + test/donkeyscript.cljs | 2 + .../ns/suggest_libspecs_test.clj | 20 ++++++-- test/test_aliases_sample.cljc | 1 + 5 files changed, 44 insertions(+), 27 deletions(-) create mode 100644 test/donkey/jvm.clj create mode 100644 test/donkeyscript.cljs diff --git a/src/refactor_nrepl/ns/suggest_libspecs.clj b/src/refactor_nrepl/ns/suggest_libspecs.clj index b3bd77b0..9f145ec7 100644 --- a/src/refactor_nrepl/ns/suggest_libspecs.clj +++ b/src/refactor_nrepl/ns/suggest_libspecs.clj @@ -2,6 +2,8 @@ "Beta middleware, meant only for internal development. Subject to change." (:refer-clojure :exclude [alias]) (:require + [clojure.string :as string] + [clojure.walk :as walk] [refactor-nrepl.core :as core] [refactor-nrepl.ns.libspecs :as libspecs] [refactor-nrepl.ns.ns-parser :as ns-parser] @@ -38,11 +40,6 @@ (vswap! m update-in [:cljs prefix] vconj ns-name))) @m)))) -(comment - (parse-preferred-aliases [["set" "clojure.set"] - ["cljs" "cljs" :only "cljs"] - ["clj" "clj" :only "clj"]])) - (defn build-reader-conditional [left-branch left-libspec _other-branch right-libspec as-alias] (let [l-clj? (= left-branch :clj) clj-clause (if l-clj? @@ -69,8 +66,14 @@ (let [extensions (filenames->extensions filenames) cljc? (some #{".cljc"} extensions) clj? (some #{".clj"} extensions) - cljs? (some #{".cljs"} extensions)] + cljs? (some #{".cljs"} extensions) + jvm-clojure-test? (some (fn [s] + (string/ends-with? s "clojure/test.clj")) + filenames)] (cond + jvm-clojure-test? ;; 'clojure.test can always be used in any platform + :cljc + (and cljc? (not cljs?)) ;; a ns backed by .cljc and .cljs files most likely is only a cljs-oriented ns in practice, since the .cljc file only intends to define macros (this is the case for clojurescript's clojure.test) :cljc @@ -201,6 +204,18 @@ (cond-> existing ;; The baseline approach is to disregard `from-preferred` (i.e. any data from `map-from-preferred`) on conflict. reader-conditionals (into reader-conditionals)))) +(defn alias-clojure-test [aliases] + (letfn [(sugar-cljs-test [coll] + (->> coll + (walk/postwalk (fn [x] + (if (= x 'cljs.test) + (with-meta 'clojure.test + (meta x)) + x)))))] + (cond-> aliases + (contains? aliases :cljc) (update :cljc sugar-cljs-test) + true (update :cljs sugar-cljs-test)))) + (defn suggest-libspecs-response "Implements https://github.com/clojure-emacs/refactor-nrepl/issues/384." [{:keys [lib-prefix ;; "set", representing that the user typed `set/` @@ -235,7 +250,8 @@ (core/source-dirs-on-classpath) suggest) aliases (cond-> aliases - b-cljc? (add-cljc-key as-alias)) + b-cljc? (add-cljc-key as-alias) + true alias-clojure-test) ks (into [] (comp (filter identity) (distinct)) @@ -292,19 +308,3 @@ candidate as-alias)))) candidates))) - -(comment - (suggest-libspecs-response {:lib-prefix "set" - :buffer-language-context "clj" - :input-language-context "clj" - :preferred-aliases []}) - - (suggest-libspecs-response {:lib-prefix "test" - :buffer-language-context "cljc" - :input-language-context "clj" - :preferred-aliases []}) - - (suggest-libspecs-response {:lib-prefix "test" - :buffer-language-context "cljc" - :input-language-context "cljs" - :preferred-aliases []})) diff --git a/test/donkey/jvm.clj b/test/donkey/jvm.clj new file mode 100644 index 00000000..93a25b69 --- /dev/null +++ b/test/donkey/jvm.clj @@ -0,0 +1,2 @@ +(ns donkey.jvm + "Supports `refactor-nrepl.ns.suggest-libspecs-test`.") diff --git a/test/donkeyscript.cljs b/test/donkeyscript.cljs new file mode 100644 index 00000000..84571fcc --- /dev/null +++ b/test/donkeyscript.cljs @@ -0,0 +1,2 @@ +(ns donkeyscript + "Supports `refactor-nrepl.ns.suggest-libspecs-test`.") diff --git a/test/refactor_nrepl/ns/suggest_libspecs_test.clj b/test/refactor_nrepl/ns/suggest_libspecs_test.clj index dd3a5cd5..4a9ea757 100644 --- a/test/refactor_nrepl/ns/suggest_libspecs_test.clj +++ b/test/refactor_nrepl/ns/suggest_libspecs_test.clj @@ -49,11 +49,23 @@ (constantly (add-file-meta project-libspecs)))}))) true) #_lib-prefix #_buffer-lc #_input-lc #_preferred-aliases #_project-libspecs #_expected + + ;; Some basic examples. I chose 'donkey' as a unique alias. Note that `project-libspecs` is {}, so a real project analysis is performed + ;; (Which is why I chose a unique ns segment) + "donkey" "clj" "clj" [] {} ["[donkey.jvm :as donkey]"] + "donkey" "cljs" "cljs" [] {} ["[donkeyscript :as donkey]"] + "donkey" "cljc" "clj" [] {} ["#?(:clj [donkey.jvm :as donkey])"] + "donkey" "cljc" "cljs" [] {} ["#?(:cljs [donkeyscript :as donkey])"] + "donkey" "cljc" "cljc" [] {} ["#?(:clj [donkey.jvm :as donkey]\n :cljs [donkeyscript :as donkey])"] + + ;; A set of examples, similar to the previous set. However the result will always be `clojure.test`, because cljs.test is now less recommended/usual, + ;; so we shouldn't suggest reader conditionals when something simpler will do: "test" "clj" "clj" [] {} ["[clojure.test :as test]"] - "test" "cljs" "cljs" [] {} ["[cljs.test :as test]"] - "test" "cljc" "clj" [] {} ["#?(:clj [clojure.test :as test])"] - "test" "cljc" "cljs" [] {} ["#?(:cljs [cljs.test :as test])"] - "test" "cljc" "cljc" [] {} ["#?(:clj [clojure.test :as test]\n :cljs [cljs.test :as test])"] + "test" "cljs" "cljs" [] {} ["[clojure.test :as test]"] + "test" "cljc" "clj" [] {} ["[clojure.test :as test]"] + "test" "cljc" "cljs" [] {} ["[clojure.test :as test]"] + "test" "cljc" "cljc" [] {} ["[clojure.test :as test]"] + ;; Example story 1: "set" "cljc" "cljc" [["set" "clojure.set"]] {} ["[clojure.set :as set]"] ;; Story 2 - preferred-aliases are disregarded if the libspecs found in the project differ: diff --git a/test/test_aliases_sample.cljc b/test/test_aliases_sample.cljc index 9354afea..df2585a6 100644 --- a/test/test_aliases_sample.cljc +++ b/test/test_aliases_sample.cljc @@ -1,6 +1,7 @@ (ns test-aliases-sample "Supports `refactor-nrepl.ns.suggest-libspecs-test`." (:require + #?(:clj [donkey.jvm :as donkey] :cljs [donkeyscript :as donkey]) #?(:clj [clojure.test :as test] :cljs [cljs.test :as test]))) (test/deftest foo From 88389913e79d9eaeaa87230fd085aa0696929ab8 Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 4 Jul 2023 15:46:56 +0200 Subject: [PATCH 12/15] Don't emit partial code --- src/refactor_nrepl/ns/suggest_libspecs.clj | 12 ++++-------- test/refactor_nrepl/ns/suggest_libspecs_test.clj | 10 ++++++---- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/refactor_nrepl/ns/suggest_libspecs.clj b/src/refactor_nrepl/ns/suggest_libspecs.clj index 9f145ec7..37aa0c8b 100644 --- a/src/refactor_nrepl/ns/suggest_libspecs.clj +++ b/src/refactor_nrepl/ns/suggest_libspecs.clj @@ -137,14 +137,10 @@ other-platform (if (= :clj platform) :cljs :clj)] - (if i-cljc? - (build-reader-conditional platform ns-sym - other-platform "" - as-alias) - (format "#?(%s [%s :as %s])" - platform - ns-sym - as-alias)))) + (format "#?(%s [%s :as %s])" + platform + ns-sym + as-alias))) (defn add-cljc-key [{:keys [clj cljs] :as m} as-alias] (let [left (get clj as-alias) diff --git a/test/refactor_nrepl/ns/suggest_libspecs_test.clj b/test/refactor_nrepl/ns/suggest_libspecs_test.clj index 4a9ea757..fd4ce4a7 100644 --- a/test/refactor_nrepl/ns/suggest_libspecs_test.clj +++ b/test/refactor_nrepl/ns/suggest_libspecs_test.clj @@ -96,8 +96,10 @@ "io" "cljc" "clj" [["io" "clojure.java.io" :only :clj]] '{:clj {io [test-clj-ns]}} ["#?(:clj [test-clj-ns :as io])"] ;; The difference here is that test-cljs-ns is backed by a .cljs extension and therefore is not a valid .cljc or :clj suggestion "io" "cljc" "cljc" [["io" "clojure.java.io" :only :clj]] '{:cljs {io [test-cljs-ns]}} ["#?(:clj [clojure.java.io :as io]\n :cljs [test-cljs-ns :as io])"] - ;; Returns an empty form for the :clj branch when there's no valid suggestion for it: - "io" "cljc" "cljc" [] '{:cljs {io [test-cljs-ns]}} ["#?(:clj [ :as io]\n :cljs [test-cljs-ns :as io])"] + ;; Returns no form for the :clj branch when there's no valid suggestion for it. + ;; (Note: that can emit invalid code, but there are no other good alternatives. Particularly considering that cljs workflows often recompile on save, + ;; i.e. we cannot emit `:clj [ :as io]` and let the user complete the form) + "io" "cljc" "cljc" [] '{:cljs {io [test-cljs-ns]}} ["#?(:cljs [test-cljs-ns :as io])"] "io" "cljc" "cljc" [] '{:cljs {io [test-cljc-ns]}} ["[test-cljc-ns :as io]"] ;; https://github.com/clojure-emacs/refactor-nrepl/issues/384#issuecomment-1221622306 extra cases ;; discards user preference, and offers both cljc choices individually and as a reader conditional (only one reader conditional! switching its branches makes less sense) @@ -108,8 +110,8 @@ "io" "cljc" "cljc" [] '{:clj {io [test-clj-ns]} :cljs {io [test-cljs-ns]}} ["#?(:clj [test-clj-ns :as io]\n :cljs [test-cljs-ns :as io])"] "io" "cljc" "cljc" [] '{:cljs {io [test-cljs-ns - test-cljs-ns-2]}} ["#?(:clj [ :as io]\n :cljs [test-cljs-ns :as io])" - "#?(:clj [ :as io]\n :cljs [test-cljs-ns-2 :as io])"] + test-cljs-ns-2]}} ["#?(:cljs [test-cljs-ns :as io])" + "#?(:cljs [test-cljs-ns-2 :as io])"] "io" "cljc" "cljc" [] '{:clj {io [test-cljc-ns]} :cljs {io [test-cljs-ns]}} ["[test-cljc-ns :as io]" "#?(:clj [test-cljc-ns :as io]\n :cljs [test-cljs-ns :as io])"] From 02a80e08c766ef88ebc94e1ce8dcbf056d969ce5 Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 4 Jul 2023 15:59:13 +0200 Subject: [PATCH 13/15] Strengthen `refactor-nrepl.artifacts-test` --- test/refactor_nrepl/artifacts_test.clj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/refactor_nrepl/artifacts_test.clj b/test/refactor_nrepl/artifacts_test.clj index 57b49896..637f4ad4 100644 --- a/test/refactor_nrepl/artifacts_test.clj +++ b/test/refactor_nrepl/artifacts_test.clj @@ -30,8 +30,8 @@ (f) (catch Exception e ;; give Maven a break: - (Thread/sleep 12000) - (if (< attempts 4) + (Thread/sleep 18000) + (if (< attempts 7) (retry-flaky f (inc attempts)) (throw e)))))) From b3b460f64a0bbf189d8300565ae08befbe850606 Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 4 Jul 2023 16:22:54 +0200 Subject: [PATCH 14/15] kondo --- src/refactor_nrepl/ns/suggest_libspecs.clj | 9 +++------ test/test_aliases_sample.cljc | 1 + 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/refactor_nrepl/ns/suggest_libspecs.clj b/src/refactor_nrepl/ns/suggest_libspecs.clj index 37aa0c8b..24d997dd 100644 --- a/src/refactor_nrepl/ns/suggest_libspecs.clj +++ b/src/refactor_nrepl/ns/suggest_libspecs.clj @@ -129,14 +129,11 @@ :cljs right as-alias))]))) -(defn build-partial-reader-conditional [ns-sym as-alias i-cljc?] +(defn build-partial-reader-conditional [ns-sym as-alias] (let [files (-> ns-sym meta :files (doto (assert "No :files found"))) platform (if (some #{".clj"} (filenames->extensions files)) :clj - :cljs) - other-platform (if (= :clj platform) - :cljs - :clj)] + :cljs)] (format "#?(%s [%s :as %s])" platform ns-sym @@ -297,7 +294,7 @@ (and b-cljc? (false? (some-> candidate meta :files valid-cljc-files?))) - (build-partial-reader-conditional candidate as-alias i-cljc?) + (build-partial-reader-conditional candidate as-alias) :else ;; it's data, format it: (format "[%s :as %s]" diff --git a/test/test_aliases_sample.cljc b/test/test_aliases_sample.cljc index df2585a6..6f96a67e 100644 --- a/test/test_aliases_sample.cljc +++ b/test/test_aliases_sample.cljc @@ -1,5 +1,6 @@ (ns test-aliases-sample "Supports `refactor-nrepl.ns.suggest-libspecs-test`." + {:clj-kondo/config {:linters {:unused-namespace {:level :off}}}} (:require #?(:clj [donkey.jvm :as donkey] :cljs [donkeyscript :as donkey]) #?(:clj [clojure.test :as test] :cljs [cljs.test :as test]))) From 686aaf987e380d4acf5795da872cad894c61e8ea Mon Sep 17 00:00:00 2001 From: vemv Date: Tue, 4 Jul 2023 16:58:35 +0200 Subject: [PATCH 15/15] 3.7.0 --- CHANGELOG.md | 5 +++++ Makefile | 2 +- README.md | 8 ++++---- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 098e4898..8b9a9eec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +## 3.7.0 + +* Implement new middleware op: `suggest-libspecs` + * Supports a beta clj-refactor.el feature. + ## 3.6.0 * [#387](https://github.com/clojure-emacs/refactor-nrepl/issues/387): extend clj-kondo `:unused-namespace` integration. Now namespace local configuration is also taken into account. diff --git a/Makefile b/Makefile index 078cd528..1454c707 100644 --- a/Makefile +++ b/Makefile @@ -39,7 +39,7 @@ deploy: check-env .inline-deps jar: .inline-deps lein with-profile -user,+$(VERSION),+plugin.mranderson/config jar -# Usage: PROJECT_VERSION=3.2.1 make install +# Usage: PROJECT_VERSION=3.7.0 make install # PROJECT_VERSION is needed because it's not computed dynamically install: check-install-env .inline-deps lein with-profile -user,+$(VERSION),+plugin.mranderson/config install diff --git a/README.md b/README.md index 33f57af4..9bda67cc 100644 --- a/README.md +++ b/README.md @@ -26,8 +26,8 @@ Be aware that this isn't the case if you connect to an already running REPL proc Add the following, either in your project's `project.clj`, or in the `:user` profile found at `~/.lein/profiles.clj`: ```clojure -:plugins [[refactor-nrepl "3.6.0"] - [cider/cider-nrepl "0.28.3"]] +:plugins [[refactor-nrepl "3.7.0"] + [cider/cider-nrepl "0.31.0"]] ``` ### Embedded nREPL @@ -365,7 +365,7 @@ When you want to release locally to the following: And here's how to deploy to Clojars: ```bash -git tag -a v3.6.0 -m "3.6.0" +git tag -a v3.7.0 -m "3.7.0" git push --tags ``` @@ -375,7 +375,7 @@ An extensive changelog is available [here](CHANGELOG.md). ## License -Copyright © 2013-2022 Benedek Fazekas, Magnar Sveen, Alex Baranosky, Lars Andersen, Bozhidar Batsov +Copyright © 2013-2023 Benedek Fazekas, Magnar Sveen, Alex Baranosky, Lars Andersen, Bozhidar Batsov Distributed under the Eclipse Public License, the same as Clojure.