Skip to content

Don't avoid 1-element vectors/lists #345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions src/refactor_nrepl/ns/clean_ns.clj
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,13 @@
* Remove any unused referred symbols.
* Prune, or remove if uneeded, the :rename clause
* Returns nil when nothing is changed, so the client knows not to do anything."
(:require [refactor-nrepl
[config :as config]
[core :as core]]
[refactor-nrepl.ns
[ns-parser :as ns-parser]
[prune-dependencies :refer [prune-dependencies]]
[rebuild :refer [rebuild-ns-form]]]
[clojure.java.io :as io]))
(:require
[clojure.java.io :as io]
[refactor-nrepl.config :as config]
[refactor-nrepl.core :as core]
[refactor-nrepl.ns.ns-parser :as ns-parser]
[refactor-nrepl.ns.prune-dependencies :refer [prune-dependencies]]
[refactor-nrepl.ns.rebuild :refer [rebuild-ns-form]]))

(defn- assert-no-exclude-clause
[use-form]
Expand All @@ -35,7 +34,9 @@
(assert-no-exclude-clause (core/get-ns-component ns-form :use))
ns-form)

(defn clean-ns [{:keys [path relative-path]}]
(defn clean-ns
"Returns nil if there's nothing to clean."
[{:keys [path relative-path]}]
(let [path (some (fn [p]
(when (and p (.exists (io/file p)))
p))
Expand Down
42 changes: 23 additions & 19 deletions src/refactor_nrepl/ns/rebuild.clj
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
(ns refactor-nrepl.ns.rebuild
(:require [clojure.string :as str]
[refactor-nrepl
[config :as config]
[util :as util]]
[refactor-nrepl.core :refer [prefix prefix-form? suffix]]))
(:require
[clojure.string :as str]
[refactor-nrepl.config :as config]
[refactor-nrepl.core :refer [prefix prefix-form? suffix]]
[refactor-nrepl.util :as util]))

(defn- assert-single-alias
[libspecs alias]
Expand Down Expand Up @@ -123,14 +123,17 @@
@libspecs-by-prefix))

(defn- create-libspec
[{:keys [ns as refer rename refer-macros] :as libspec}]
[{:keys [ns as refer rename refer-macros] :as libspec} vectorize?]
(let [all-flags #{:reload :reload-all :verbose :include-macros}
flags (util/filter-map #(all-flags (first %)) libspec)]
(if (and (not as) (not refer)
(empty? flags) (empty? rename) (empty? refer-macros))
ns
(into [ns]
(concat (when as [:as as])
flags (util/filter-map #(all-flags (first %)) libspec)
keep-as-is? (and (not as) (not refer)
(empty? flags) (empty? rename) (empty? refer-macros))]
(cond-> ns
(or vectorize? (not keep-as-is?))
vector

(not keep-as-is?)
(into (concat (when as [:as as])
(when refer
[:refer (if (sequential? refer)
(vec (sort-referred-symbols refer))
Expand All @@ -145,13 +148,14 @@
[libspecs]
(vec
(for [libspec libspecs]
(create-libspec (assoc libspec :ns (ns-suffix libspec))))))
(create-libspec (assoc libspec :ns (ns-suffix libspec))
false))))

(defn- create-libspec-vectors-with-prefix
[libspecs]
(vec
(for [libspec libspecs]
(create-libspec libspec))))
(create-libspec libspec true))))

(defn- create-prefixed-libspec-vectors
[[libspec & more :as libspecs]]
Expand Down Expand Up @@ -190,9 +194,9 @@

(defn- create-import-form
[prefix classes]
(if (= (count classes) 1)
(symbol (str prefix "." (first classes)))
(into [(symbol prefix)] (map symbol classes))))
(->> classes
(map symbol)
(apply list (symbol prefix))))

(defn- create-import-components
[classes-by-prefix]
Expand All @@ -203,8 +207,8 @@
[imports]
(->> imports
(map #(if (sequential? %)
(vec (cons (first %)
(sort dependency-comparator (rest %))))
(cons (first %)
(sort dependency-comparator (rest %)))
%))
(sort dependency-comparator)))

Expand Down
14 changes: 7 additions & 7 deletions test-resources/cljcns_cleaned.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,22 @@
{:author "Winnie the pooh"}
(:refer-clojure :exclude [macroexpand-1 read read-string])
#?@(:clj
[(:require clojure.data clojure.edn
[(:require [clojure.data] [clojure.edn]
[clojure.instant :as inst :reload true]
[clojure.pprint :refer [cl-format formatter get-pretty-writer]]
[clojure.string :refer :all :reload-all true]
[clojure.test :refer :all]
clojure.test.junit
[clojure.test.junit]
[clojure.walk :refer [postwalk prewalk]]
clojure.xml)
(:import [java.io Closeable FilenameFilter PushbackReader]
[java.util Calendar Date Random])]
[clojure.xml])
(:import (java.io Closeable FilenameFilter PushbackReader)
(java.util Calendar Date Random))]
:cljs
[(:require [cljs.pprint :as pprint]
[cljs.test :refer-macros [deftest is]]
[clojure.set :as set]
[clojure.string :refer [join split-lines]])
(:require-macros cljs.analyzer.api
(:require-macros [cljs.analyzer.api]
[cljs.analyzer.macros :as am]
[cljs.test :refer [run-tests testing]])
(:import goog.string)]))
(:import (goog string))]))
6 changes: 3 additions & 3 deletions test-resources/cljsns_cleaned.cljs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
(ns cljsns
(:require [cljs.pprint :as pprint]
[cljs.test :refer-macros [deftest is]]
cljsjs.js-yaml
[cljsjs.js-yaml]
[clojure.set :as set]
[clojure.string :refer [join split-lines]]
[js-literal-ns :as js-literal]
[keyword-ns :as kw])
(:require-macros cljs.analyzer.api
(:require-macros [cljs.analyzer.api]
[cljs.analyzer.macros :as am]
[cljs.test :refer [run-tests testing]])
(:import goog.string))
(:import (goog string)))
14 changes: 7 additions & 7 deletions test-resources/ns1_cleaned_and_pprinted
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@
:extends java.lang.Exception
:methods [[binomial [int int] double]])
(:require
clojure.data
clojure.edn
[clojure.data]
[clojure.edn]
[clojure.instant :as inst :reload true]
[clojure.pprint :refer [cl-format formatter get-pretty-writer]]
[clojure.string :refer :all :reload-all true]
[clojure.test :refer :all]
clojure.test.junit
[clojure.test.junit]
[clojure.walk :refer [postwalk prewalk]]
clojure.xml)
[clojure.xml])
(:import
[java.io Closeable FilenameFilter PushbackReader]
[java.util Calendar Date Random]
[refactor.nrepl SomeClass$InnerClass$InnerInnerClassOne SomeClass$InnerClass$InnerInnerClassTwo]))
(java.io Closeable FilenameFilter PushbackReader)
(java.util Calendar Date Random)
(refactor.nrepl SomeClass$InnerClass$InnerInnerClassOne SomeClass$InnerClass$InnerInnerClassTwo)))
8 changes: 4 additions & 4 deletions test-resources/ns1_cleaned_and_pprinted_prefix_notation
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
[string :refer :all :reload-all true]
[test :refer :all]
[walk :refer [postwalk prewalk]]]
clojure.test.junit)
[clojure.test.junit])
(:import
[java.io Closeable FilenameFilter PushbackReader]
[java.util Calendar Date Random]
[refactor.nrepl SomeClass$InnerClass$InnerInnerClassOne SomeClass$InnerClass$InnerInnerClassTwo]))
(java.io Closeable FilenameFilter PushbackReader)
(java.util Calendar Date Random)
(refactor.nrepl SomeClass$InnerClass$InnerInnerClassOne SomeClass$InnerClass$InnerInnerClassTwo)))
37 changes: 22 additions & 15 deletions test/refactor_nrepl/ns/clean_ns_test.clj
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
(ns refactor-nrepl.ns.clean-ns-test
(:require [clojure.test :refer [are deftest is]]
[refactor-nrepl.config :as config]
[refactor-nrepl.core :as core]
[refactor-nrepl.ns.clean-ns :refer [clean-ns]]
[refactor-nrepl.ns.pprint :refer [pprint-ns]]
[clojure.string :as str])
(:import java.io.File))
(:require
[clojure.string :as str]
[clojure.test :refer [are deftest is]]
[refactor-nrepl.config :as config]
[refactor-nrepl.core :as core]
[refactor-nrepl.ns.clean-ns :refer [clean-ns]]
[refactor-nrepl.ns.pprint :refer [pprint-ns]])
(:import
(java.io File)))

(defn- absolute-path [^String path]
(.getAbsolutePath (File. path)))
Expand Down Expand Up @@ -92,9 +94,15 @@
(clean-ns ns1)) :require)
imports (core/get-ns-component (clean-ns ns1) :import)
sorted-requires (core/get-ns-component ns1-cleaned :require)
sorted-imports (core/get-ns-component ns1-cleaned :import)]
(is (= sorted-requires requires))
(is (= sorted-imports imports))))
sorted-imports (core/get-ns-component ns1-cleaned :import)
collize (fn [coll transform-to]
(->> coll (map (fn [x]
(cond-> x
(and (not (coll? x))
(not (keyword? x)))
transform-to)))))]
(is (= (collize sorted-requires vector) requires))
(is (= (collize sorted-imports list) imports))))

(deftest throws-exceptions-for-unexpected-elements
(is (thrown? IllegalArgumentException
Expand Down Expand Up @@ -179,16 +187,16 @@
(deftest handles-imports-when-only-enum-is-used
(let [new-ns (clean-ns ns2)
imports (core/get-ns-component new-ns :import)]
(is (some #(= 'java.text.Normalizer %) imports))))
(is (some #(= '(java.text Normalizer) %) imports))))

(deftest keeps-referred-macros-around
(let [new-ns (clean-ns (clean-msg ns-referencing-macro))]
;; nil means no changes
(is (nil? new-ns))))

(deftest handles-clojurescript-files
(let [new-ns (clean-ns cljs-ns)]
(is (= cljs-ns-cleaned new-ns))))
(let [actual (clean-ns cljs-ns)]
(is (= cljs-ns-cleaned actual))))

(deftest handles-cljc-files
(let [new-ns (str (clean-ns cljc-ns))
Expand Down Expand Up @@ -245,8 +253,7 @@
(is (re-find #"\[\$\]" cleaned))))

(deftest does-not-break-import-for-inner-class
(let [cleaned (pprint-ns (clean-ns ns-with-inner-classes))]
(is (re-find #":import\n.*Line2D\$Double" cleaned))))
(is (nil? (clean-ns ns-with-inner-classes))))

(deftest fallback-to-relative-path
(is (= (pprint-ns (clean-ns ns1))
Expand Down