Merge pull request #178 from jonase/replace

Replace

CHANGELOG.md

@@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. This change
 
 ## [0.1.3] / 2016-11-21
 ### Additions
+* Automatic replacement of suggestions (`--replace` and `--interactive` cli arguments)
 * Enabled Emacs' next error function to go to next Kibit suggestion. See the updated code in the README for the change.
 * #172 Kibit can now handle sets without crashing!
 * #152 Send exceptions to STDERR instead of STDOUT

README.md

@@ -44,10 +44,10 @@ If you want to know how the Kibit rule system works there are some slides availa
 If `lein kibit` returns any suggestions to forms then it's exit code will be 1. Otherwise it will exit 0. This can be useful to add in a build step for automated testing.
 
 
-    $lein kibit
+    $ lein kibit
     ... suggestions follow
 
-    $echo $?
+    $ echo $?
     1
 
 ## Automatically rerunning when files change
@@ -56,7 +56,7 @@ You can use [lein-auto](https://github.com/weavejester/lein-auto) to run kibit a
 lein-auto's README for installation instructions. Note that this will run kibit over all of your files, not just the
 ones that have changed.
 
-    $lein auto kibit
+    $ lein auto kibit
     auto> Files changed: project.clj, [...]
     auto> Running: lein kibit
     ... suggestions follow
@@ -66,6 +66,37 @@ ones that have changed.
     ... suggestions follow
     auto> Failed.
 
+## Automatically replacing suggestions in source file
+
+You can have kibit automatically apply suggestions to your source files.
+
+Given a file:
+
+```clojure
+(ns example)
+
+(+ 1 a)
+```
+
+    $ lein kibit --replace
+
+will rewrite the file as:
+
+```clojure
+(ns example)
+
+(+ 1 a)
+```
+
+Replacement can also be run interactively:
+
+    $ lein kibit --replace --interactive
+     Would you like to replace
+       (+ 1 a)
+      with
+       (inc a)
+     in example.clj:3? [yes/no]
+
 ## Reporters
 
 Kibit comes with two reporters, the default plaintext reporter, and a GitHub Flavoured Markdown reporter. To specify a reporter, use the `-r` or `--reporter` commandline argument. For example:

project.clj

@@ -7,7 +7,8 @@
             :comments "Contact if any questions"}
   :dependencies [[org.clojure/clojure "1.8.0"]
                  [org.clojure/core.logic "0.8.11"]
-                 [org.clojure/tools.cli "0.3.5"]]
+                 [org.clojure/tools.cli "0.3.5"]
+                 [rewrite-clj "0.4.12"]]
   :profiles {:dev {:dependencies [[lein-marginalia "0.9.0"]]
                    :resource-paths ["test/resources"]}}
   :deploy-repositories [["releases" :clojars]]

src/kibit/driver.clj

@@ -5,14 +5,20 @@
             [clojure.tools.cli :refer [cli]]
             [kibit
              [check :refer [check-file]]
+             [replace :refer [replace-file]]
              [reporters :refer :all]
-             [rules :refer [all-rules]]
-             [monkeypatch :refer :all]])
+             [rules :refer [all-rules]]])
   (:import java.io.File))
 
 (def cli-specs [["-r" "--reporter"
                  "The reporter used when rendering suggestions"
-                 :default "text"]])
+                 :default "text"]
+                ["-e" "--replace"
+                 "Automatially apply suggestions to source file"
+                 :flag true]
+                ["-i" "--interactive"
+                 "Interactively prompt before replacing suggestions in source file (Requires `--replace`)"
+                 :flag true]])
 
 (defn ends-with?
   "Returns true if the java.io.File ends in any of the strings in coll"
@@ -35,28 +41,38 @@
   (sort-by #(.getAbsolutePath ^File %)
            (filter clojure-file? (file-seq dir))))
 
-(defn run
+(defn- run-replace [source-files rules options]
+  (doseq [file source-files]
+    (replace-file file
+                  :rules (or rules all-rules)
+                  :interactive (:interactive options))))
+
+(defn- run-check [source-files rules {:keys [reporter]}]
+  (mapcat (fn [file] (try (check-file file
+                                      :reporter (name-to-reporter reporter
+                                                                  cli-reporter)
+                                      :rules (or rules all-rules))
+                          (catch Exception e
+                            (binding [*out* *err*]
+                              (println "Check failed -- skipping rest of file")
+                              (println (.getMessage e))))))
+          source-files))
+
+(defn run [source-paths rules & args]
   "Runs the kibit checker against the given paths, rules and args.
 
   Paths is expected to be a sequence of io.File objects.
 
   Rules is either a collection of rules or nil. If Rules is nil, all of kibit's checkers are used.
 
-  Optionally accepts a :reporter keyword argument, defaulting to \"text\"."
-  [source-paths rules & args]
+  Optionally accepts a :reporter keyword argument, defaulting to \"text\"
+  If :replace is provided in options, suggested replacements will be performed automatically."
   (let [[options file-args usage-text] (apply (partial cli args) cli-specs)
         source-files                   (mapcat #(-> % io/file find-clojure-sources-in-dir)
                                                (if (empty? file-args) source-paths file-args))]
-    (mapcat (fn [file]
-              (with-monkeypatches kibit-redefs
-                (check-file file
-                            :reporter (name-to-reporter (:reporter options) cli-reporter)
-                            :rules    (or rules all-rules))
-                (catch Exception e
-                  (binding [*out* *err*]
-                    (println "Check failed -- skipping rest of file")
-                    (println (.getMessage e))))))
-            source-files)))
+    (if (:replace options)
+      (run-replace source-files rules options)
+      (run-check source-files rules options))))
 
 (defn external-run
   "Used by lein-kibit to count the results and exit with exit-code 1 if results are found"

src/kibit/replace.clj

@@ -0,0 +1,131 @@
+(ns kibit.replace
+  (:require [clojure.string :as str]
+            [clojure.java.io :as io]
+            [rewrite-clj.zip :as rewrite.zip]
+            [rewrite-clj.node :as rewrite.node]
+            [kibit.check :as check]
+            [kibit.reporters :as reporters]))
+
+(defn- prompt
+  "Create a yes/no prompt using the given message.
+
+  From `leiningen.ancient.console`."
+  [& msg]
+  (let [msg (str (str/join msg) " [yes/no] ")]
+    (locking *out*
+      (loop [i 0]
+        (when (= (mod i 4) 2)
+          (println "*** please type in one of 'yes'/'y' or 'no'/'n' ***"))
+        (print msg)
+        (flush)
+        (let [r (or (read-line) "")
+              r (.toLowerCase ^String r)]
+          (case r
+            ("yes" "y") true
+            ("no" "n")  false
+            (recur (inc i))))))))
+
+(defn- report-or-prompt
+  ""
+  [file interactive? {:keys [line expr alt]}]
+  (if interactive?
+    (prompt (with-out-str
+              (println "Would you like to replace")
+              (reporters/pprint-code expr)
+              (println " with")
+              (reporters/pprint-code alt)
+              (print (format "in %s:%s?" file line))))
+    (do
+      (println "Replacing")
+      (reporters/pprint-code expr)
+      (println " with")
+      (reporters/pprint-code alt)
+      (println (format "in %s:%s" file line))
+
+      true)))
+
+(def ^:private expr? (comp not rewrite.node/printable-only? rewrite.zip/node))
+
+(defn- map-zipper
+  "Apply `f` to all code forms in `zipper0`"
+  [f zipper0]
+  (let [zipper (if (expr? zipper0)
+                 (rewrite.zip/postwalk zipper0
+                                       expr?
+                                       f)
+                 zipper0)]
+    (if (rewrite.zip/rightmost? zipper)
+      zipper
+      (recur f (rewrite.zip/right zipper)))))
+
+(defn- replace-zipper*
+  ""
+  [zipper reporter kw-opts]
+  (if-let [check-map (apply check/check-expr
+                            (rewrite.zip/sexpr zipper)
+                            :resolution
+                            :subform
+                            kw-opts)]
+    (if (reporter (assoc check-map
+                         :line
+                         (-> zipper rewrite.zip/node meta :row)))
+      (recur (rewrite.zip/edit zipper
+                               (fn -replace-zipper [sexpr]
+                                 (vary-meta (:alt check-map)
+                                            (fn -remove-loc [m]
+                                              (dissoc m
+                                                      :line
+                                                      :column)))))
+             reporter
+             kw-opts)
+      zipper)
+    zipper))
+
+(defn- replace-zipper
+  ""
+  [zipper & kw-opts]
+  (let [options (apply hash-map kw-opts)]
+    ;; TODO use (:reporter options) to determine format?
+    (replace-zipper* zipper
+                     (partial report-or-prompt
+                              (:file options)
+                              (:interactive options))
+                     kw-opts)))
+
+(defn replace-expr
+  "Apply any suggestions to `expr`.
+
+  `expr`    - Code form to check and replace in
+  `kw-opts` - any valid option for `check/check-expr`, as well as:
+              - `:file` current filename
+              - `:interactive` prompt for confirmation before replacement or not
+
+  Returns a string of the replaced form"
+  [expr & kw-opts]
+  (->> (str expr)
+       rewrite.zip/of-string
+       (map-zipper (fn -replace-expr [node]
+                     (apply replace-zipper
+                            node
+                            kw-opts)))
+       rewrite.zip/root
+       rewrite.node/sexpr))
+
+(defn replace-file
+  "Apply any suggestions to `file`.
+
+  `file`    - File to check and replace in
+  `kw-opts` - any valid option for `check/check-expr`, as well as:
+              - `:interactive` prompt for confirmation before replacement or not
+
+  Modifies `file`, returns `nil`"
+  [file & kw-opts]
+  (->> (slurp file)
+       rewrite.zip/of-string
+       (map-zipper (fn -replace-zipper [node]
+                     (apply replace-zipper
+                            node
+                            :file (str file)
+                            kw-opts)))
+       rewrite.zip/root-string
+       (spit file)))

test/kibit/test/replace.clj

@@ -0,0 +1,52 @@
+(ns kibit.test.replace
+  (:require [kibit.check :as check]
+            [kibit.rules :as rules]
+            [kibit.replace :as replace])
+  (:use [clojure.test])
+  (:import java.io.File
+           java.io.StringWriter))
+
+(defmacro discard-output
+  "Like `with-out-str`, but discards was was written to *out*"
+  [& body]
+  `(binding [*out* (StringWriter.)]
+     ~@body))
+
+(deftest replace-expr-are
+  (are [expected-form test-form]
+      (= expected-form
+         (discard-output
+          (replace/replace-expr test-form)))
+
+    '(inc a)
+    '(+ 1 a)
+
+    '(defn "Documentation" ^{:my-meta 1} [a]
+       ;; a comment
+       (inc a))
+    '(defn "Documentation" ^{:my-meta 1} [a]
+       ;; a comment
+       (+ 1 a))))
+
+(deftest replace-file-are
+  (are [expected-form test-form]
+      (= expected-form
+         (let [file (doto (File/createTempFile "replace-file" ".clj")
+                      (.deleteOnExit)
+                      (spit test-form))]
+           (discard-output (replace/replace-file file))
+           (slurp file)))
+
+    "(inc a)"
+    "(+ 1 a)"
+
+    "(ns replace-file)
+
+     (defn \"Documentation\" ^{:my-meta 1} [a]
+       ;; a comment
+       (inc a))"
+    "(ns replace-file)
+
+     (defn \"Documentation\" ^{:my-meta 1} [a]
+       ;; a comment
+       (+ 1 a))"))