Fix #235. allow gen-class methods metadata extraction (#236)

* [Fix #235] streamline metadata extraction process

(extract-ns-meta) will return a map with both the ns metadata, and
the (:gen-class :methods) metadata. All in longhand notation

* [Fix #235] make (extract-ns-meta) return nil values if no metadata

it was returning {:ns-meta {}, :gc-methods-meta nil}

* [Fix #235] adapt pprint-meta

only for the top level meta information

* [Fix #235] adapt (pprint-gen-class-form) to print metadata

* [Fix #235] fix gc-methods-meta : keys with different value...

can conflict. ^{:dynamic false} and ^:dynamic would conflict and it
would only take into account one of them.

* [Fix #235] allow mixed type notation in metadata

both at the top level, and for gen-class methods

add relevant tests

Author: Carlos Requena López

README.md

@@ -158,6 +158,8 @@ The `clean-ns` op will perform the following cleanups on an ns form:
 * Remove any unused namespaces, referred symbols or imported classes.
 * Remove any duplication in the :require and :import form.
 * Prune or remove any `:rename` clause.
+* Use the shorthand version of metadata found if possible, and sort it
+  alphabetically
 
 The `clean-ns` requires a `path` which is the path to the file containing the `ns` to be operated upon.
 

src/refactor_nrepl/core.clj

@@ -250,24 +250,40 @@
 
       :else (-> fully-qualified-name str (.split "\\.") last))))
 
+(defn extract-gen-class-methods-meta
+  "Retrieve the metadata relative to :methods in the :gen-class top
+  level component.
+
+  Returns nil if there is no :gen-class, or if there is no :methods
+  inside :gen-class
+
+  .indexOf returns -1 if not found, and since the structure we are
+  looking for comes after, by 'incing' by default we can just check
+  for zero?"
+  [ns-form]
+  (let [gen-class (get-ns-component ns-form :gen-class)
+        methods_index (inc (if-not (nil? gen-class)
+                             (.indexOf gen-class :methods)
+                             -1))]
+    (if-not (zero? methods_index)
+      (apply merge (map #(meta %) (nth gen-class methods_index)))
+      nil)))
+
 (defn extract-ns-meta
   "Retrieve the metadata for the ns, if there is any.
 
-  This is preferable to just doing (meta the-ns-form) because the
-  compiler, as well as libraries like clojure.test, add quite a bit of
-  metadata which shouldn't be printed back out."
+  By parsing the ns as a string, and reading the metadata off it, all
+  the metadata introduced by the compiler or clojure.test is not
+  printed back out"
   [file-content]
   (let [ns-string (sexp/get-first-sexp file-content)
-        meta? (-> ns-string (.replaceFirst "\\^\\{" "\\{")
-                  (StringReader.)
-                  (PushbackReader.)
-                  parse/read-ns-decl
-                  second)
-        shorthand-meta?  (re-seq #"\^:([^\s]+)\s" ns-string)]
-    (cond
-      (map? meta?) meta?
-      shorthand-meta? {::shorthand-meta-coll (map (comp keyword second) shorthand-meta?)}
-      :else nil)))
+        ns-form (-> ns-string
+                    (StringReader.)
+                    (PushbackReader.)
+                    parse/read-ns-decl)
+        ns-meta (meta (second ns-form))]
+    {:top-level-meta ns-meta
+     :gc-methods-meta (extract-gen-class-methods-meta ns-form)}))
 
 (defn read-ns-form-with-meta
   "Read the ns form found at PATH.

src/refactor_nrepl/ns/pprint.clj

@@ -3,7 +3,9 @@
             [clojure
              [pprint :refer [pprint]]
              [string :as str]]
-            [refactor-nrepl.core :as core :refer [prefix-form?]])
+            [refactor-nrepl
+             [core :as core :refer [prefix-form?]]
+             [util :as util :refer [replace-last]]])
 
   (:import java.util.regex.Pattern))
 
@@ -52,17 +54,55 @@
   (and (sequential? form)
        (= (first form) type)))
 
+(defn pprint-meta
+  "Given some metadata m, print the shorthand metadata first, and the
+  longhand metadata second, trying to convert to shorthand notation if
+  possible
+
+  If newlines is true, it prints a newline after each piece of
+  longhand metadata"
+  [m & {:keys [newlines]
+        :or {newlines false}}]
+  (let [short? #(= (str %) "true")
+        shorthand (sort (filter (fn [[k v]] (short? v)) m))
+        longhand (remove (fn [[k v]] (short? v)) m)]
+    (doseq [[k v] shorthand]
+      (print "^" (str k) ""))
+    (when-not (empty? longhand)
+      (printf "^{")
+      (doseq [[k v] longhand]
+        (print k)
+        (if newlines
+          (print (with-out-str (pprint v)))
+          (print (replace-last (with-out-str (pprint v)) #"\s" ""))))
+      (if newlines
+        (println "}")
+        (print "}")))))
+
 (defn- pprint-gen-class-form
-  [[_ & elems]]
+  "Prints the gen class form and :methods metadata (if any)."
+  [[_ & elems] metadata]
   (if (empty? elems)
     (println "(:gen-class)")
     (println "(:gen-class"))
   (dorun
    (map-indexed
     (fn [idx [key val]]
-      (if (= idx (dec (count (partition 2 elems))))
-        (printf "%s %s)\n" key val)
-        (println key val)))
+      (if (= key :methods)
+        (do
+          (print key "[")
+          (doseq [method val]           ;val are all the methods
+            (pprint-meta (filter (fn [[k v]]
+                                   (contains? metadata k))
+                                 (meta method)))
+            (print method)
+            (when-not (= method (last val))
+              (println)))
+          (print "]"))
+        (print key val))
+      (when (= idx (dec (count (partition 2 elems))))
+        (print ")"))
+      (println))
     (partition 2 elems))))
 
 (defn- pprint-import-form
@@ -76,25 +116,17 @@
         (println import)))
     imports)))
 
-(defn pprint-meta
-  [m]
-  (if-let [shorthand-meta-coll (::core/shorthand-meta-coll m)]
-    (doseq [shorthand-meta shorthand-meta-coll]
-      (print (str "^" shorthand-meta " ")))
-    (printf (.replaceAll (str "^" (into (sorted-map) m) "\n")
-                         ", " "\n"))))
-
 (defn pprint-ns
   [[_ name & more :as ns-form]]
   (let [docstring? (when (string? (first more)) (first more))
         attrs? (when (map? (second more)) (second more))
         forms (cond (and docstring? attrs?) (nthrest more 2)
                     (not (or docstring? attrs?)) more
                     :else (rest more))
-        ns-meta (meta ns-form)]
+        ns-meta (:top-level-meta (meta ns-form))]
     (-> (with-out-str
           (printf "(ns ")
-          (when (seq ns-meta) (pprint-meta ns-meta))
+          (when (seq ns-meta) (pprint-meta ns-meta :newlines true))
           (print name)
           (if (or docstring? attrs? forms)
             (println)
@@ -116,11 +148,11 @@
                         (str/trim-newline
                          (with-out-str
                            (cond (form-is? form :require) (pprint-require-form form)
-                                 (form-is? form :gen-class) (pprint-gen-class-form form)
+                                 (form-is? form :gen-class) (pprint-gen-class-form form (:gc-methods-meta (meta ns-form)))
                                  (form-is? form :import) (pprint-import-form form)
                                  :else (pprint form)))))
                 (cond (form-is? form :require) (pprint-require-form form)
-                      (form-is? form :gen-class) (pprint-gen-class-form form)
+                      (form-is? form :gen-class) (pprint-gen-class-form form (:gc-methods-meta (meta ns-form)))
                       (form-is? form :import) (pprint-import-form form)
                       :else (pprint form))))
             forms)))

src/refactor_nrepl/util.clj

@@ -1,4 +1,5 @@
 (ns refactor-nrepl.util
+  (:require [clojure.string :as string])
   (:import java.util.regex.Pattern))
 
 (defn normalize-to-unix-path
@@ -41,3 +42,13 @@
     (if (seq xs)
       (apply conj coll xs)
       coll)))
+
+(defn replace-last
+  "Replaces the last instance of match from the string s with rep"
+  [s match rep]
+  (as-> s _
+    (reverse _)
+    (apply str _)
+    (string/replace-first _ match rep)
+    (reverse _)
+    (apply str _)))

test/refactor_nrepl/ns/clean_ns_test.clj

@@ -38,8 +38,11 @@
 (def cljc-ns-same-clj-cljs-cleaned (core/read-ns-form-with-meta (absolute-path "test/resources/cljcns_same_clj_cljs_cleaned.cljc")))
 
 (def ns-with-shorthand-meta (clean-msg "test/resources/ns_with_shorthand_meta.clj"))
-
 (def ns-with-multiple-shorthand-meta (clean-msg "test/resources/ns_with_multiple_shorthand_meta.clj"))
+(def ns-with-gen-class-methods-meta (clean-msg "test/resources/ns_with_gen_class_methods_meta.clj"))
+(def ns-with-gen-class-methods-meta-clean (clean-msg "test/resources/ns_with_gen_class_methods_meta_clean.clj"))
+(def ns-with-lots-of-meta (clean-msg "test/resources/ns_with_lots_of_meta.clj"))
+(def ns-with-lots-of-meta-clean (clean-msg "test/resources/ns_with_lots_of_meta_clean.clj"))
 
 (def ns-with-inner-classes (clean-msg "test/resources/ns_with_inner_classes.clj"))
 
@@ -199,11 +202,21 @@
   (let [cleaned (pprint-ns (clean-ns ns-with-shorthand-meta))]
     (is (re-find #"\^:automation" cleaned))))
 
-(deftest preservres-multiple-shortand-meta
+(deftest preserves-multiple-shortand-meta
   (let [cleaned (pprint-ns (clean-ns ns-with-multiple-shorthand-meta))]
     (is (re-find #"\^:automation" cleaned))
     (is (re-find #"\^:multiple" cleaned))))
 
+(deftest preserves-gen-class-methods-meta
+  (let [actual (pprint-ns (clean-ns ns-with-gen-class-methods-meta))
+        expected (slurp (:path ns-with-gen-class-methods-meta-clean))]
+    (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))))
+
 (deftest does-not-remove-dollar-sign-if-valid-symbol
   (let [cleaned (pprint-ns (clean-ns ns-using-dollar))]
     (is (re-find #"\[\$\]" cleaned))))

test/resources/ns_with_gen_class_methods_meta.clj

@@ -0,0 +1,12 @@
+(ns 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]]))
+
+(defn useit
+  []
+  (fresh-line))

test/resources/ns_with_gen_class_methods_meta_clean.clj

@@ -0,0 +1,7 @@
+(ns ns-with-gen-class-methods-meta
+  (:gen-class
+   :methods [^:static [foo [String] String]
+             ^:test [bar [String] String]
+             ^{:other "text"} [baz [String] String]]
+   :name Name)
+  (:require [clojure.pprint :refer [fresh-line]]))

test/resources/ns_with_lots_of_meta.clj

@@ -0,0 +1,12 @@
+(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]]))
+
+(defn useit
+  []
+  (fresh-line))

test/resources/ns_with_lots_of_meta_clean.clj

@@ -0,0 +1,9 @@
+(ns ^:md1 ^:md2 ^:ok ^{:longhand "as well"
+                       :really? "yes"}
+ ns-with-gen-class-methods-meta
+  (:gen-class
+   :methods [^:static [foo [String] String]
+             ^:test [bar [String] String]
+             ^{:other "text"} [baz [String] String]]
+   :name Name)
+  (:require [clojure.pprint :refer [fresh-line]]))