Skip to content
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

clean-ns should follow the convention to not prune namespace deps when they don't have a :refer or :as definition #292

Closed
RickMoynihan opened this issue Feb 8, 2021 · 3 comments

Comments

@RickMoynihan
Copy link
Contributor

RickMoynihan commented Feb 8, 2021

The convention came about in this clj-kondo thread:

clj-kondo/clj-kondo#241

Essentially the conclusion is to protect namespace declarations that are side-effecting from pruning (e.g. namespaces that load protocol extensions or multimethods etc) by not having them :refer anything or use an :as form.

clj-kondo and I believe some other tools already follow this convetion; it would be nice for refactor-nrepl to support this too.

I suspect it would be pretty simple to edit this function to support it?

(defn- libspec-should-never-be-pruned? [libspec]
(let [ns-name (str (:ns libspec))]
(some (fn [^String pattern]
(re-find (re-pattern pattern) ns-name))
(:libspec-whitelist config/*config*))))

@bbatsov
Copy link
Member

bbatsov commented Feb 16, 2021

PR welcome!

@expez
Copy link
Member

expez commented Feb 17, 2021

We've discussed this already in this repo and I consider it a non-issue for Clojure code. As I said over here I solve this by doing:

(ns my-ns
  (:require [clojure.string :as str]))

;; This has some really sweet side-effects that we want, because reasons
(require 'tick.timezone) 

This makes it super clear that this ns needs special consideration and requires no out-of-band knowledge about conventions.

Since people keep asking for this feature (and the solution above doesn't work for cljs) I agree with @bbatsov that a PR would be great, with the caveat that it has to be opt-in via a new setting. That way we don't create confusion for our existing user-base and leave the door open for the solution above that I find preferable when dealing with a Clojure-only project.

@vemv
Copy link
Member

vemv commented Sep 17, 2021

PR certainly welcome (with the mentioned constraints: should be opt-in)

Closing the issue as some time has passed, it's more manageable to have few issues in the tracker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants