technomancy / slamhound

Slamhound rips your namespace form apart and reconstructs it.
Other
473 stars 38 forks source link

Fix var conflicts among different namespaces with improved :require-as matching #41

Closed guns closed 11 years ago

guns commented 11 years ago

Hello,

This branch fixes an issue where slamhound chooses the wrong :require-as libspec when two namespaces contain the same var:

;; alpha/beta.clj
(ns alpha.beta)

(defn hello [])

;; alpha/gamma.clj
(ns alpha.gamma)

(defn hello [])

;; alpha/core.clj
(ns alpha.core
  (:require [alpha.beta :as beta]
            [alpha.gamma :as gamma]))

(beta/hello)
(gamma/hello)

Running slamhound on alpha/core.clj produces the erroneous ns form:

(ns alpha.core
  (:require [alpha.beta :as beta]
            [alpha.beta :as gamma]))

This is due to in-originals-pred erroneously returning a true value for the candidate [alpha.beta :as gamma] with the original requires above.

Matching the full candidate against a regular set of fully qualified symbols and libspecs avoids this error, assuming the candidate is a regular, fully qualified libspec itself.

The commit messages go into a little more detail if this is a bit unclear.

This branch also addresses issue #35, at least @kenrestivo's second comment about slamhound not respecting the original requires. I couldn't reproduce his initial example.

Please tell me if you would like me to squash the branch into a single commit or if you would like to see any changes.

Cheers, Sung Pae

guns commented 11 years ago

Update:

Missing require-as libspecs are sorted by last-segment-matches?

i.e.

(ns foo)

(string/join)

Previously this would produce the ns form

(ns foo
  (:require [clojure.set :as string]))

since clojure.set has a var named join and it is shorter than clojure.string.

If the user's require-as matches the last segment of the fully qualified lib, then that require-as takes precedence. In the above example, [clojure.string :as string] would be preferred.

technomancy commented 11 years ago

Sorry for the delay here. Thanks for the patches and the nice tests and descriptions that made them really easy to understand! I just released version 1.4.0 with these improvements.

guns commented 11 years ago

Thanks @technomancy! I knew you'd get to these patches in time.