search

clojure-emacs / refactor-nrepl

nREPL middleware to support refactorings in an editor agnostic way
Eclipse Public License 1.0
255 stars 69 forks source link

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

Closed RickMoynihan closed 3 years ago

RickMoynihan commented 3 years ago

The convention came about in this clj-kondo thread:

https://github.com/clj-kondo/clj-kondo/issues/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?

https://github.com/clojure-emacs/refactor-nrepl/blob/dbafd6e686402dc020d7026e21ee556812fc51da/src/refactor_nrepl/ns/prune_dependencies.clj#L118-L122

bbatsov commented 3 years ago

PR welcome!

expez commented 3 years ago

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 commented 3 years ago

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.