:require :rename is not supported
seancorfield opened this issue · 10 comments
We use :rename
in a handful of places to create better names for functions. Slamhound gets confused by this. Not sure whether this is even possible to fix or just a limitation with looking up names? Example:
[image-resizer.rotate :as rotate
:refer [rotate-90-counter-clockwise-fn rotate-270-counter-clockwise-fn]
:rename {rotate-90-counter-clockwise-fn rotate-right rotate-270-counter-clockwise-fn rotate-left}]
Slamhound says:
java.lang.Exception: Couldn't resolve rotate-left, got as far as {:alias {clojure.string str, clojure.java.io io}, :import #{javax.imageio.plugins.jpeg.JPEGImageWriteParam javax.imageio.ImageIO javax.imageio.stream.FileImageOutputStream javax.imageio.IIOImage}, :refer {worldsingles.data.core #{execute}, date-clj #{subtract today}, worldsingles.data.ws #{worldsingles-db}, clojure.java.jdbc #{query}}, :old {:load nil, :exclude {}, :xrefer #{}, :require #{}, :refer-all #{}, :verbose #{}, :rename {}, :alias {image-resizer.rotate rotate, clojure.string str, clojure.java.io io}, :reload #{}, :reload-all #{}, :gen-class nil, :import #{javax.imageio.plugins.jpeg.JPEGImageWriteParam javax.imageio.ImageIO javax.imageio.stream.FileImageOutputStream javax.imageio.IIOImage}, :refer {worldsingles.data.ws #{worldsingles-db}, image-resizer.rotate #{rotate-90-counter-clockwise-fn rotate-270-counter-clockwise-fn}, clojure.java.jdbc #{query}, date-clj #{subtract today}, worldsingles.data.core #{execute get-by-id}}}, :meta nil, :name worldsingles.admin.photos}
regrow.clj:384 slam.hound.regrow/regrow
hound.clj:13 slam.hound/reconstruct
/Developer/workspace/worldsingles/ws/model/clojure/worldsingles/src/worldsingles/admin/photos.clj:1 worldsingles.admin.photos/eval47051
:)
I've been waiting for this moment: fc7e887
I'll get this hooked up today; the hardest part will actually be getting these long libspecs to print attractively.
Thanks!
Sorry :)
We use this in eight files, mostly to rename clojure.string functions so we don't have to use a namespace alias / prefix (yeah, counter-intuitive... I know) but there are cases like the one above where the original name is just so horrible we want to avoid it completely.
Renames are a bit of a gray area in the ns macro; the docstrings for ns
and require
don't mention it at all. It's only documented in the refer
docs (and therefore implicitly by use
). Of course, all of these functions are really just frontends for load-lib
, so everything pretty much works everywhere.
I suspect, however, that if I can't implement this :rename support cleanly, @technomancy might NAK on this feature.
BTW, you say you are using Light Table with Slamhound. Is there an official plugin for this? I'd love to link to it in the README.
LightTable plugin: https://github.com/chadhq/slamhound-lt
FWIW, I just removed most of our :rename
s where they didn't add much to the code. We can live without it if it's too much of a pain to implement (but some libraries just use really long, sucky names!).
Yeah, part of the original philosophy behind slamhound is to use automation to encourage regularity in ns
declarations, so stuff like renaming string vars is definitely not a sufficient use case.
Renaming functions with bad names that you don't have control over is more of a grey area; I probably wouldn't add support for it myself, but I wouldn't turn it down if someone else wanted to take a crack at it and could come up with something reasonable. =)
Okay, I've implemented rename support in the rename-support
branch. Here's a diff against master:
https://github.com/technomancy/slamhound/compare/rename-support
As you can see, it's almost entirely a clean accretion of code. The :rename path of the regrow routine is ONLY taken if a missing reference matches a renamed var in the original ns map (searching for possible renames is otherwise impossible).
If it works for you, and @technomancy thinks it's okay, we'll merge it in!
I tested it on the file that caused me to open this issue and it worked beautifully (it removed the :as
alias on that required namespace since we weren't using it - so it cleaned things up too).
Thank you for your swift response to this!
Nice work! Let's bring it in.
Thank you for releasing 1.5.3. Running it across all our code now to see what it picks up.
Finding some issues around handling of :import
for which I'll open separate tickets but otherwise it's looking good and finding us some nice cleanups.