Skip to content

Introduce :insert-newline-after-require #303

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 3 commits into from
Jun 27, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Unreleased

* [clojure-emacs/clj-refactor.el#459](https://github.com/clojure-emacs/clj-refactor.el/issues/459): `clean-ns` should conform to the style guide: `(:require` in the ns form should be followed by a newline.
* You can opt out via the new `:insert-newline-after-require` configuration option.
* [#294](https://github.com/clojure-emacs/refactor-nrepl/pull/294): Properly skip uneval nodes when looking for the first/last sexp
* From now on, if you set the `clojure.tools.namespace.repl/refresh-dirs`, files outside said `refresh-dirs` won't be analyzed, resulting in safer, more efficient analysis.

Expand Down
12 changes: 6 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@
VERSION ?= 1.10

.inline-deps:
lein inline-deps
lein with-profile -user inline-deps
touch .inline-deps

inline-deps: .inline-deps

test: .inline-deps
lein with-profile +$(VERSION),+plugin.mranderson/config test
lein with-profile -user,+$(VERSION),+plugin.mranderson/config test

cljfmt:
lein with-profile +$(VERSION),+cljfmt,+lein-plugin cljfmt check
lein with-profile -user,+$(VERSION),+cljfmt,+lein-plugin cljfmt check

eastwood:
lein with-profile +$(VERSION),+eastwood eastwood
lein with-profile -user,+$(VERSION),+eastwood eastwood

kondo:
lein with-profile -dev,+$(VERSION),+clj-kondo run -m clj-kondo.main --lint src
Expand All @@ -27,14 +27,14 @@ kondo:
BUMP ?= patch

release:
lein with-profile +$(VERSION),+lein-plugin release $(BUMP)
lein with-profile -user,+$(VERSION),+lein-plugin release $(BUMP)

# Deploying requires the caller to set environment variables as
# specified in project.clj to provide a login and password to the
# artifact repository.

deploy: .inline-deps
lein with-profile +$(VERSION),+plugin.mranderson/config,+lein-plugin deploy clojars
lein with-profile -user,+$(VERSION),+plugin.mranderson/config,+lein-plugin deploy clojars

clean:
lein clean
Expand Down
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ Configuration settings are passed along with each msg, currently the recognized
;; Should `clean-ns` favor prefix forms in the ns macro?
:prefix-rewriting true

;; Should `pprint-ns` place a newline after the `:require` and `:import` tokens?
:insert-newline-after-require true

;; Some libspecs are side-effecting and shouldn't be pruned by `clean-ns`
;; even if they're otherwise unused.
;; This seq of strings will be used as regexp patterns to match
Expand Down
4 changes: 2 additions & 2 deletions project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@
with-debug-bindings [[:inner 0]]
merge-meta [[:inner 0]]
try-if-let [[:block 1]]}}}]
:eastwood {:plugins [[jonase/eastwood "0.4.0"]]
:eastwood {:plugins [[jonase/eastwood "0.6.0"]]
;; TODO: add :test-paths
:eastwood {:namespaces [:source-paths]
;; vendored - shouldn't be tweaked for satisfying linters:
:exclude-namespaces [refactor-nrepl.ns.slam.hound.regrow]
:exclude-linters [:unused-ret-vals]}}
:clj-kondo [:test
{:dependencies [[clj-kondo "2021.03.31"]]}]}
{:dependencies [[clj-kondo "2021.06.18"]]}]}
:jvm-opts ["-Djava.net.preferIPv4Stack=true"])
3 changes: 3 additions & 0 deletions src/refactor_nrepl/config.clj
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
;; Should `clean-ns` favor prefix forms in the ns macro?
:prefix-rewriting true

;; Should `pprint-ns` place a newline after the `:require` and `:import` tokens?
:insert-newline-after-require true

;; Some libspecs are side-effecting and shouldn't be pruned by `clean-ns`
;; even if they're otherwise unused.
;; This seq of strings will be used as regexp patterns to match
Expand Down
12 changes: 10 additions & 2 deletions src/refactor_nrepl/ns/pprint.clj
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
[pprint :refer [pprint]]
[string :as str]]
[refactor-nrepl
[config :refer [*config*]]
[core :as core :refer [prefix-form?]]
[util :as util :refer [replace-last]]])

Expand Down Expand Up @@ -34,9 +35,15 @@
(printf "%s " libspec))))))
ordered-libspecs))))

(defn insert-clause-delimiter []
(if (:insert-newline-after-require *config*)
(println)
(print " ")))

(defn pprint-require-form
[[_ & libspecs]]
(print "(:require\n")
(print "(:require")
(insert-clause-delimiter)
(dorun
(map-indexed
(fn [idx libspec]
Expand Down Expand Up @@ -107,7 +114,8 @@

(defn- pprint-import-form
[[_ & imports]]
(printf "(:import ")
(print "(:import")
(insert-clause-delimiter)
(dorun
(map-indexed
(fn [idx import]
Expand Down
12 changes: 7 additions & 5 deletions test/refactor_nrepl/ns/clean_ns_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,12 @@
(:import java.util.Date)))

(deftest test-pprint-artifact-ns
(let [path (.getAbsolutePath (File. "test/resources/artifacts_pprinted"))
actual (pprint-ns (with-meta artifact-ns nil))
expected (slurp path)]
(is (= expected actual))))
(are [setting filename] (let [actual (config/with-config {:insert-newline-after-require setting}
(pprint-ns (with-meta artifact-ns nil)))
expected (-> filename File. .getAbsolutePath slurp)]
(= expected actual))
true "test/resources/artifacts_pprinted"
false "test/resources/artifacts_pprinted_traditional_newline"))

(deftest handles-imports-when-only-enum-is-used
(let [new-ns (clean-ns ns2)
Expand Down Expand Up @@ -231,7 +233,7 @@

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

(deftest fallback-to-relative-path
(is (= (pprint-ns (clean-ns ns1))
Expand Down
3 changes: 2 additions & 1 deletion test/resources/artifacts_pprinted
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@
[transport :as transport]]
[org.httpkit.client :as http]
[refactor-nrepl.externs :refer [add-dependencies]])
(:import java.util.Date))
(:import
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the treatment of :import homogeneous relative to that of :require, as most likely users never want an asymmetry.

(It's also what https://github.com/bbatsov/clojure-style-guide/tree/e6cb6dab0925bcfabcd4ec96a2f69f41bf9c698e#line-break-ns-declaration suggests in its example)

java.util.Date))
13 changes: 13 additions & 0 deletions test/resources/artifacts_pprinted_traditional_newline
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
(ns refactor-nrepl.artifacts
(:require [clojure
[edn :as edn]
[string :as str]]
[clojure.data.json :as json]
[clojure.java.io :as io]
[nrepl
[middleware :refer [set-descriptor!]]
[misc :refer [response-for]]
[transport :as transport]]
[org.httpkit.client :as http]
[refactor-nrepl.externs :refer [add-dependencies]])
(:import java.util.Date))