From d26f8bad36ce7e00a351e05c08ce755bdf2dc5db Mon Sep 17 00:00:00 2001 From: Lars Andersen Date: Sun, 27 Jun 2021 19:17:58 +0200 Subject: [PATCH] [Fix #239] Teach clean-ns about more metadata The performance hit in considering literally every piece of metadata shouldn't be too detrimental so we'll just err on the side of caution for now. --- src/refactor_nrepl/find/symbols_in_file.clj | 57 ++++++++++++------- src/refactor_nrepl/ns/clean_ns.clj | 9 +-- test/refactor_nrepl/ns/clean_ns_test.clj | 7 ++- test/resources/ns_with_lots_of_meta.clj | 13 ++++- test/resources/ns_with_lots_of_meta_clean.clj | 3 +- 5 files changed, 57 insertions(+), 32 deletions(-) diff --git a/src/refactor_nrepl/find/symbols_in_file.clj b/src/refactor_nrepl/find/symbols_in_file.clj index 38d598ba..a19bbf34 100644 --- a/src/refactor_nrepl/find/symbols_in_file.clj +++ b/src/refactor_nrepl/find/symbols_in_file.clj @@ -34,6 +34,12 @@ (.substring s 0 (dec (.length s))) s)))) +(defn- strip-meta-from-reader + [metadata] + (-> metadata + (dissoc :line :column :end-line :end-column) + not-empty)) + (defn symbols-in-file "Return a set of all the symbols occurring in the file at path. @@ -63,24 +69,33 @@ (let [rdr (-> path slurp core/file-content-sans-ns readers/indexing-push-back-reader) rdr-opts {:read-cond :allow :features #{dialect} :eof :eof} - syms (atom #{}) - collect-symbol (fn [form] - ;; Regular symbol - (when (symbol? form) - (swap! syms conj (normalize-ctor-call form))) - ;; Classes used in typehints - (when-let [t (:tag (meta form))] - (swap! syms conj t)) - (when (and (keyword? form) - (core/fully-qualified? form)) - (swap! syms conj - (symbol (core/prefix form) - (core/suffix form)))) - form)] - (loop [form (reader/read rdr-opts rdr)] - (when (not= form :eof) - (walk/prewalk collect-symbol form) - (recur (reader/read rdr-opts rdr)))) - (->> @syms - (map (partial fix-ns-of-backquoted-symbols (dialect parsed-ns))) - set))))))) + syms (atom #{})] + (letfn [(collect-symbols [form] + (let [minimal-meta (strip-meta-from-reader (meta form))] + ;; metadata used in metadata-based protocol extensions + ;; see #239 for an example + (when minimal-meta + (->> minimal-meta + seq + flatten + (run! collect-symbols))) + ;; Regular symbol + (when (symbol? form) + (swap! syms conj (normalize-ctor-call form))) + ;; Classes used in typehints + (when-let [t (:tag minimal-meta)] + (swap! syms conj t)) + (when (and (keyword? form) + (core/fully-qualified? form)) + (swap! syms conj + (symbol (core/prefix form) + (core/suffix form))))) + form)] + (loop [form (reader/read rdr-opts rdr)] + (when (not= form :eof) + (walk/prewalk collect-symbols form) + (recur (reader/read rdr-opts rdr)))) + (->> @syms + (map + (partial fix-ns-of-backquoted-symbols (dialect parsed-ns))) + set)))))))) diff --git a/src/refactor_nrepl/ns/clean_ns.clj b/src/refactor_nrepl/ns/clean_ns.clj index 64123687..adc90975 100644 --- a/src/refactor_nrepl/ns/clean_ns.clj +++ b/src/refactor_nrepl/ns/clean_ns.clj @@ -43,10 +43,11 @@ (assert (core/source-file? path)) ;; Prefix notation not supported in cljs. ;; We also turn it off for cljc for reasons of symmetry - (config/with-config {:prefix-rewriting (if (or (core/cljs-file? path) - (core/cljc-file? path)) - false - (:prefix-rewriting config/*config*))} + (config/with-config {:prefix-rewriting + (if (or (core/cljs-file? path) + (core/cljc-file? path)) + false + (:prefix-rewriting config/*config*))} (let [ns-form (validate (core/read-ns-form-with-meta path)) deps-preprocessor (if (get config/*config* :prune-ns-form) #(prune-dependencies % path) diff --git a/test/refactor_nrepl/ns/clean_ns_test.clj b/test/refactor_nrepl/ns/clean_ns_test.clj index c6a367d7..f41e7134 100644 --- a/test/refactor_nrepl/ns/clean_ns_test.clj +++ b/test/refactor_nrepl/ns/clean_ns_test.clj @@ -235,9 +235,10 @@ (is (= expected actual)))) (deftest preserves-all-meta - (let [actual (pprint-ns (clean-ns ns-with-lots-of-meta)) - expected (slurp (:path ns-with-lots-of-meta-clean))] - (is (= expected actual)))) + (config/with-config {:prefix-rewriting false} + (let [actual (pprint-ns (clean-ns ns-with-lots-of-meta)) + expected (slurp (:path ns-with-lots-of-meta-clean))] + (is (= expected actual))))) (deftest does-not-remove-dollar-sign-if-valid-symbol (let [cleaned (pprint-ns (clean-ns ns-using-dollar))] diff --git a/test/resources/ns_with_lots_of_meta.clj b/test/resources/ns_with_lots_of_meta.clj index 6b1ebef5..f16c7014 100644 --- a/test/resources/ns_with_lots_of_meta.clj +++ b/test/resources/ns_with_lots_of_meta.clj @@ -1,12 +1,19 @@ -(ns ^:md1 ^:md2 ^{:longhand "as well" :really? "yes" :ok true} ns-with-gen-class-methods-meta +(ns ^:md1 ^:md2 ^{:longhand "as well" :really? "yes" :ok true} + ns-with-gen-class-methods-meta (:gen-class :methods [^:static [foo [String] String] ^{:test true} [bar [String] String] ^{:other "text"} [baz [String] String]] :name Name) - (:require [clojure.string :as s] - [clojure.pprint :refer [fresh-line]])) + (:require + [clojure.string :as s] + [clojure.pprint :refer [fresh-line]] + [clojure.edn :as edn])) (defn useit [] (fresh-line)) + +(defn issue-239-metadata-based-protocol-extension [] + ;; https://github.com/clojure-emacs/refactor-nrepl/issues/239 + ^{`s/whatever-goes true} {}) diff --git a/test/resources/ns_with_lots_of_meta_clean.clj b/test/resources/ns_with_lots_of_meta_clean.clj index 11339ecb..332bf8c1 100644 --- a/test/resources/ns_with_lots_of_meta_clean.clj +++ b/test/resources/ns_with_lots_of_meta_clean.clj @@ -7,4 +7,5 @@ ^{:other "text"} [baz [String] String]] :name Name) (:require - [clojure.pprint :refer [fresh-line]])) + [clojure.pprint :refer [fresh-line]] + [clojure.string :as s]))