technomancy / slamhound

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

Always assumes any capitalized symbol is a class... #27

Closed AlexBaranosky closed 11 years ago

AlexBaranosky commented 11 years ago

For better or for worse we have some vars that are capitalized, and clj-schema assumes they are classes.

guns commented 11 years ago

Hi @AlexBaranosky,

It appears to me that you fixed this issue in:

https://github.com/technomancy/slamhound/commit/e2f7d95d#L2R28

Since failure-details returns [:import :require-refer], for capitalized symbols, the search starts with matching imports then to matching vars, which I think is the correct order of precedence.

Therefore given:

;; foo.clj
(ns foo)
(def Foo nil)

;; bar.clj
(ns bar)
(def bar Foo)

Slamhound correctly regrows bar as:

(ns bar
  (:require [foo :refer [Foo]]))

So long as Foo is not the name of a class on the classpath.

Could you please confirm this is fixed, or am I misunderstanding something?

Thanks!

guns commented 11 years ago

Ah, nevermind. I see the issue is that if some.package.Foo and a var named Foo both exist, the class Foo is always chosen irrespective of any existing ns :import specs.

Hence the fix to #26 should fix this as well. I'll work on this next.

guns commented 11 years ago

Fixed by #53