Initial refactor
This commit is contained in:
Jonas Enlund
parent
4860b24789
commit
40dd12fd62
4 changed files with 104 additions and 179 deletions
61
src/kibit/check.clj
Normal file
61
src/kibit/check.clj
Normal file
|
|
@ -0,0 +1,61 @@
|
|||
(ns kibit.check
|
||||
(:import [clojure.lang LineNumberingPushbackReader])
|
||||
(:require [kibit.core :as core]))
|
||||
|
||||
;; ### Important notes
|
||||
;; Feel free to contribute rules to [kibit's github repo](https://github.com/jonase/kibit)
|
||||
|
||||
;; Reading source files
|
||||
;; --------------------
|
||||
|
||||
;; `read-file` is intended to be used with a Clojure source file,
|
||||
;; read in by a LineNumberingPushbackReader. Expressions are
|
||||
;; extracted using the clojure reader (ala `read`).
|
||||
;; Line numbers are added as `:line` metadata to the forms.
|
||||
(defn read-file
|
||||
"Generate a lazy sequence of top level forms from a
|
||||
LineNumberingPushbackReader"
|
||||
[^LineNumberingPushbackReader r]
|
||||
(lazy-seq
|
||||
(let [form (read r false ::eof)]
|
||||
(when-not (= form ::eof)
|
||||
(cons form (read-file r))))))
|
||||
|
||||
;; `tree-seq` returns a lazy-seq of nodes for a tree.
|
||||
;; Given an expression, we can then match rules against its pieces.
|
||||
;; This is like using `clojure.walk` with `identity`:
|
||||
;;
|
||||
;; user=> (expr-seq '(if (pred? x) (inc x) x))
|
||||
;; ((if (pred? x) (inc x) x)
|
||||
;; if
|
||||
;; (pred? x)
|
||||
;; pred?
|
||||
;; x
|
||||
;; (inc x)
|
||||
;; inc
|
||||
;; x
|
||||
;; x)`
|
||||
;;
|
||||
(defn expr-seq
|
||||
"Given an expression (any piece of Clojure data), return a lazy (depth-first)
|
||||
sequence of the expr and all its sub-expressions"
|
||||
[expr]
|
||||
(tree-seq sequential?
|
||||
seq
|
||||
expr))
|
||||
|
||||
(defn- build-result-map [expr simplified-expr]
|
||||
(merge {:expr expr
|
||||
:line (-> expr meta :line)}
|
||||
(when (not= expr simplified-expr)
|
||||
{:alt simplified-expr})))
|
||||
|
||||
(defn check-toplevel-forms [reader rules]
|
||||
(for [expr (read-file (LineNumberingPushbackReader. reader))
|
||||
:let [simplified-expr (core/simplify expr rules)]]
|
||||
(build-result-map expr simplified-expr)))
|
||||
|
||||
(defn check-subforms [reader rules]
|
||||
(for [expr (mapcat expr-seq (read-file (LineNumberingPushbackReader. reader)))
|
||||
:let [simplified-expr (core/simplify-one expr rules)]]
|
||||
(build-result-map expr simplified-expr)))
|
||||
|
|
@ -1,28 +1,8 @@
|
|||
(ns kibit.core
|
||||
"Kibit's core functionality uses core.logic to suggest idiomatic
|
||||
replacements for patterns of code."
|
||||
(:require [clojure.java.io :as io]
|
||||
[clojure.walk :as walk]
|
||||
[clojure.core.logic :as logic]
|
||||
[kibit.rules :as core-rules]
|
||||
[kibit.reporters :as reporters])
|
||||
(:import [clojure.lang LineNumberingPushbackReader]))
|
||||
|
||||
;; ### Important notes
|
||||
;; Feel free to contribute rules to [kibit's github repo](https://github.com/jonase/kibit)
|
||||
|
||||
;; The rule sets
|
||||
;; -------------
|
||||
;;
|
||||
;; Rule sets are stored in individual files that have a top level
|
||||
;; `(defrules rules ...)`. The collection of rules are in the `rules`
|
||||
;; directory.
|
||||
;;
|
||||
;; Here, we logically prepare all the rules, by substituting in logic vars
|
||||
;; where necessary.
|
||||
;;
|
||||
;; For more information, see: [rules](#jonase.kibit.rules) namespace
|
||||
(def all-rules (map logic/prep core-rules/all-rules))
|
||||
(:require [clojure.walk :as walk]
|
||||
[clojure.core.logic :as logic]))
|
||||
|
||||
;; Building an alternative form
|
||||
;; ----------------------------
|
||||
|
|
@ -36,150 +16,22 @@
|
|||
(guard-fn expr))
|
||||
(check-guards expr rest)))
|
||||
|
||||
(defn simplify-one
|
||||
([expr]
|
||||
(simplify-one expr all-rules))
|
||||
([expr rules]
|
||||
(first (logic/run* [q]
|
||||
(logic/fresh [pat guards alt]
|
||||
(logic/membero [pat guards alt] rules)
|
||||
(logic/== pat expr)
|
||||
(check-guards expr guards)
|
||||
(logic/== q {:expr expr
|
||||
:alt alt
|
||||
:line (-> expr meta :line)}))))))
|
||||
|
||||
(defn simplify-multipass
|
||||
([expr]
|
||||
(simplify-multipass expr all-rules))
|
||||
([expr rules]
|
||||
(loop [expr expr
|
||||
simplify-map nil]
|
||||
(if-let [new-simplify-map (simplify-one expr rules)]
|
||||
(recur (:alt new-simplify-map)
|
||||
new-simplify-map)
|
||||
simplify-map))))
|
||||
|
||||
;; Guarding `simplify` allows for fine-grained control over what
|
||||
;; gets passed to a reporter. This allows those using kibit
|
||||
;; as a library or building out tool compatibility to shape
|
||||
;; the results prior to reporting.
|
||||
;;
|
||||
;; Normally, you'll only want to report an alternative form if it differs
|
||||
;; from the original expression form. You can use `identity` to short circuit
|
||||
;; the guard.
|
||||
;;
|
||||
;; Simplify-guards take a map and return a map or nil
|
||||
(defn unique-alt? [simplify-map]
|
||||
(let [{:keys [expr alt line]} simplify-map]
|
||||
(when-not (= alt expr)
|
||||
simplify-map)))
|
||||
|
||||
;; This walks across all the forms within an expression,
|
||||
;; checking each inner form. The outcome is a potential full alternative.
|
||||
;; We check to see if there is indeed a difference in the alternative,
|
||||
;; and if so, return a full simplify-map. See *Guarding simplify* above
|
||||
;;
|
||||
;; We build the simplify-map at the end because
|
||||
;; Clojure 1.3 munges the metadata in transients (so also in clojure.walk).
|
||||
(defn simplify
|
||||
([expr]
|
||||
(simplify expr all-rules unique-alt?))
|
||||
([expr rules]
|
||||
(simplify expr rules unique-alt?))
|
||||
([expr rules simplify-guard]
|
||||
(let [line-num (-> expr meta :line)
|
||||
simp-partial #(simplify-multipass %1 rules)
|
||||
alt (walk/postwalk #(or (-> % simp-partial :alt) %) expr)]
|
||||
(simplify-guard
|
||||
{:expr expr
|
||||
:alt alt
|
||||
:line line-num}))))
|
||||
|
||||
;; Reading source files
|
||||
;; --------------------
|
||||
|
||||
;; `read-file` is intended to be used with a Clojure source file,
|
||||
;; read in by a LineNumberingPushbackReader. Expressions are
|
||||
;; extracted using the clojure reader (ala `read`).
|
||||
;; Line numbers are added as `:line` metadata to the forms.
|
||||
(defn read-file
|
||||
"Generate a lazy sequence of top level forms from a
|
||||
LineNumberingPushbackReader"
|
||||
[^LineNumberingPushbackReader r]
|
||||
(lazy-seq
|
||||
(try
|
||||
(let [form (read r false ::eof)]
|
||||
(when-not (= form ::eof)
|
||||
(cons form (read-file r))))
|
||||
(catch Exception e
|
||||
(println "A form was skipped because it relies on active parsing/evaluating - issue #14")))))
|
||||
|
||||
;; `tree-seq` returns a lazy-seq of nodes for a tree.
|
||||
;; Given an expression, we can then match rules against its pieces.
|
||||
;; This is like using `clojure.walk` with `identity`:
|
||||
;;
|
||||
;; user=> (expr-seq '(if (pred? x) (inc x) x))
|
||||
;; ((if (pred? x) (inc x) x)
|
||||
;; if
|
||||
;; (pred? x)
|
||||
;; pred?
|
||||
;; x
|
||||
;; (inc x)
|
||||
;; inc
|
||||
;; x
|
||||
;; x)`
|
||||
;;
|
||||
(defn expr-seq
|
||||
"Given an expression (any piece of Clojure data), return a lazy (depth-first)
|
||||
sequence of the expr and all its sub-expressions"
|
||||
[expr]
|
||||
(tree-seq sequential?
|
||||
seq
|
||||
expr))
|
||||
|
||||
;; The reader is converted into a LineNumberingPushbackReader.
|
||||
;; Optional rule sets and verbosity can be set with keyword args,
|
||||
;; :rules, :verbose -
|
||||
;;
|
||||
;; `(check-via-reader my-reader :rules rule-set :verbose true)`
|
||||
;;
|
||||
;; Verbosity will report all the sub-expression substituions individually,
|
||||
;; in addition to reporting them all on the top-level form. This is an
|
||||
;; an ideal way to carefully inspect your code.
|
||||
(defn check-via-reader
|
||||
"Simplifies every expression (including sub-expressions) read in from
|
||||
a reader and returns a lazy sequence of the result of unification
|
||||
(`simplify` function)."
|
||||
([reader & kw-opts]
|
||||
(let [{:keys [rules simplify-guard verbose]
|
||||
:or {rules all-rules
|
||||
simplify-guard unique-alt?
|
||||
verbose false}} (apply hash-map kw-opts)]
|
||||
(if verbose
|
||||
(keep #(simplify-one % rules)
|
||||
(mapcat expr-seq (read-file (LineNumberingPushbackReader. reader))))
|
||||
(keep #(simplify % rules simplify-guard) (read-file (LineNumberingPushbackReader. reader)))))))
|
||||
|
||||
;; The results from simplify get passed to a `reporter`.
|
||||
;; A reporter can be any function that expects a single map.
|
||||
;; TODO - talk about optional args
|
||||
;;
|
||||
;; The entire sequence of simplify-maps from a given `source-file`
|
||||
;; are processed with `doseq`, since reporting is side-effect action.
|
||||
;;
|
||||
;; For more information on reporters, see: [reporters](#jonase.kibit.reporters) namespace
|
||||
(defn check-file
|
||||
"Checks every expression (including sub-expressions) in a clojure
|
||||
source file against the rules and processes them with a reporter"
|
||||
([source-file & kw-opts]
|
||||
(let [{:keys [rules simplify-guard verbose reporter]
|
||||
:or {rules all-rules
|
||||
simplify-guard unique-alt?
|
||||
verbose false
|
||||
reporter reporters/cli-reporter}} (apply hash-map kw-opts)]
|
||||
(with-open [reader (io/reader source-file)]
|
||||
(doseq [simplify-map (check-via-reader
|
||||
reader :rules rules :verbose verbose :simplify-guard simplify-guard)]
|
||||
(reporter simplify-map))))))
|
||||
;; Performs the first simplification found in the rules. If no rules
|
||||
;; apply the original expression is returned. Does not look at
|
||||
;; subforms.
|
||||
(defn simplify-one [expr rules]
|
||||
(let [alts (logic/run* [q]
|
||||
(logic/fresh [pat guards alt]
|
||||
(logic/membero [pat guards alt] rules)
|
||||
(logic/== pat expr)
|
||||
(check-guards expr guards)
|
||||
(logic/== q alt)))]
|
||||
(if (empty? alts) expr (first alts))))
|
||||
|
||||
;; Simplifies expr according to the rules until no more rules apply.
|
||||
(defn simplify [expr rules]
|
||||
(->> expr
|
||||
(iterate (partial walk/postwalk #(simplify-one % rules)))
|
||||
(partition 2 1)
|
||||
(drop-while #(apply not= %))
|
||||
(ffirst)))
|
||||
|
|
|
|||
|
|
@ -7,7 +7,9 @@
|
|||
[kibit.rules.collections :as coll]
|
||||
[kibit.rules.equality :as equality]
|
||||
[kibit.rules.performance :as perf]
|
||||
[kibit.rules.misc :as misc]))
|
||||
[kibit.rules.misc :as misc]
|
||||
[clojure.core.logic :as logic]))
|
||||
|
||||
|
||||
;; More information on rules
|
||||
;; -------------------------
|
||||
|
|
@ -36,5 +38,4 @@
|
|||
|
||||
;; TODO: Consider a refactor for this into a function
|
||||
;; `(defn rules-for-ns [& namespaces])`
|
||||
(def all-rules (apply concat (vals rule-map)))
|
||||
|
||||
(def all-rules (map logic/prep (apply concat (vals rule-map))))
|
||||
|
|
|
|||
|
|
@ -1,16 +1,27 @@
|
|||
(ns leiningen.kibit
|
||||
(:require [clojure.tools.namespace :as clj-ns]
|
||||
[clojure.java.io :as io]
|
||||
[kibit.core :as kibit]))
|
||||
[kibit.check :as kibit]
|
||||
[kibit.rules :as rules]
|
||||
[kibit.reporters :as reporters]))
|
||||
|
||||
(defn kibit
|
||||
"Suggest idiomatic replacements for patterns of code."
|
||||
[project]
|
||||
[project & opts]
|
||||
(let [paths (or (:source-paths project) [(:source-path project)])
|
||||
source-files (mapcat #(-> % io/file clj-ns/find-clojure-sources-in-dir)
|
||||
paths)]
|
||||
source-files (mapcat #(-> % io/file clj-ns/find-clojure-sources-in-dir) paths)
|
||||
kw-opts (apply hash-map (map read-string opts))
|
||||
verbose (or (:verbose kw-opts) false)
|
||||
check (if verbose kibit/check-subforms kibit/check-toplevel-forms)]
|
||||
(doseq [source-file source-files]
|
||||
(printf "== %s ==\n"
|
||||
(or (second (clj-ns/read-file-ns-decl source-file)) source-file))
|
||||
(kibit/check-file source-file)
|
||||
(flush))))
|
||||
(with-open [reader (io/reader source-file)]
|
||||
(printf "== %s ==\n" (or (second (clj-ns/read-file-ns-decl source-file)) source-file))
|
||||
(try
|
||||
(->> (check reader rules/all-rules)
|
||||
(filter #(contains? % :alt))
|
||||
(map reporters/cli-reporter)
|
||||
doall)
|
||||
(catch Exception e
|
||||
(println "Check failed -- skipping rest of file")
|
||||
(when verbose (.printStackTrace e))))))))
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue