Slamhound fails on files with load statements

[phillord]

I create a new project with lein, make core.clj like so...

(ns slamhound-test.core)
(load "core_extra.clj")
(defn -main [& args]
  (pprint args)
  (io/copy (ByteArrayInputStream. (.getBytes "hello"))
           (first args)))

and core_extra.clj like this.

(in-ns 'slamhound-test.core)
(defn temp [])

Which produces the following output:

lein slamhound src/slamhound_test/core.clj
Failed to reconstruct: #<File src/slamhound_test/core.clj>
Could not locate slamhound/core_extra.clj__init.class or slamhound/core_extra.clj.clj on classpath: 

Note the extra .clj on the end of core_extra!

This pattern is quite common for large namespaces (clojure.core for instance!)

[technomancy]

Looks like a dupe of #59. It's worth noting that the clojure.core namespace is very atypical for bootstrapping reasons and should not be considered an example of good style.

[phillord]

That's a shame. Splitting large namespaces can make them easier to manage. I suspect this is why clojure.core mostly does it, and is why I have. I understand the general issues with side-effects, but this appears to be a special case.

[guns]

One reason¹ clojure.core is so large is because it is designed to be mass-referred by the ns macro.

Applications and libraries OTOH really have no reason to fear splitting their functionality among many namespaces unless they want to offer the same convenience.

Of course, Slamhound is partly designed to break away from such anti-patterns. Explicit refers and ns aliases really provide a lot of clarity when reading a Clojure program, and Slamhound will dutifully insert these for us, so we can be just as lazy as before, while writing excellent and informative ns forms.

¹ Another reason very old clojure libraries are broken into several files is because the require function didn't exist until ~2008 (or something like that).

cf. http://clojure.com/blog/2012/02/17/clojure-governance.html

[phillord]

Maybe. In my case, I do want to support a single require/use statement, as managing namespace imports can be painful -- you obviously agree or you wouldn't work on slamhound. More over, I do not want to force people into to using an ever increasingly complex tool chain; being honest, after a year of using Clojure, I think it's been a good choice in many ways, but the tool chain is not so clean, nor that stable. I think it's the largest barrier to entry for the application I've spent my time on.

I have separated namespaces out where it makes sense. But still the main entry point of my library has got large; not surprising, really, because the data model I am working with is big also.

Regardless, to make slamhound work for me, I currently need to remove the multi-file approach that I took; not a step I am going to make rapidly (or work out how to fix slamhound for this purpose).

[guns]

being honest, after a year of using Clojure, I think it's been a good choice in many ways, but the tool chain is not so clean, nor that stable.

I smiled when I read this, as my background is C, Ruby, and JavaScript. Clojure + Leiningen feels like a Promethean gift from a higher plane in comparison :)

Regardless, to make slamhound work for me, I currently need to remove the multi-file approach that I took; not a step I am going to make rapidly (or work out how to fix slamhound for this purpose).

You know, if the load-file paths are specified in the :load ns subform, I don't believe there's any reason why Slamhound can't concat the load paths into a single unit before starting reconstruction ― like a compilation unit in C.

We would have to make sure only the root file is actually modified and that files that lack proper ns forms are excluded when reconstructing directory trees.

The only trick would be doing this elegantly, since it's not clear that technomancy wants to make major alterations for a case like this. (I bet he won't mind if the additions are non-obtrusive)

I'll be happy to try in the future, but I'm taking a little break from Slamhound as I've addressed all of my personal issues with the project and feel quite satisfied!

That said, Slamhound is very hackable; I imagine you could accomplish adding this feature within an evening if you did not wish to wait for someone else to take a stab at it.

[technomancy]

I wouldn't object to adding this as long as it sticks to only supporting :load as an ns clause like you said. I don't want to start handling bare top-level calls to things like load or require, which are low-level functions designed for repl usage.

[phillord]

On the tool chain, I got it to work, but then I've been using Emacs for 20 years; even then, for me, we have had slime/swank, then nrepl (which broke evertything then repaired it) and now cider (again, everything broken, now repairing). And my use case is to provide what I call "a textual UI" for people some of who are not programmers primarily.

:load on the namespace doesn't address my problem, unless I bung in a load of forward-declarations, which I am rather loathe to do after I went to quite a lot of effort to take them out (actually, it wouldn't work even then, as one of my subsidiary files uses a macro defined in the entry point file).

If you don't want to support this, then that's your decision, of course. I don't believe my use of load is either low-level nor an anti-pattern.

[technomancy]

Actually kind of curious about this; does Slime no longer work?

[guns]

:load on the namespace doesn't address my problem, unless I bung in a load of forward-declarations, which I am rather loathe to do after I went to quite a lot of effort to take them out (actually, it wouldn't work even then, as one of my subsidiary files uses a macro defined in the entry point file).

The entry point of your namespace should be the first file in :load. Actually, the root file could contain nothing but the :load clause. This would be easier to follow than mentally weaving together your source files.

[phillord]

@technomancy Slime probably does still work, but I was going to make the shift to nrepl; for I had colleagues using both clojure-mode 1 and 2, slime/swank and nrepl. I also have (emacs) code base developed against first slime/swank, then nrepl, now nrepl-client. None of this has been a show stopper, but it has meant quite a bit of co-ordinated change of development environment for myself and my co-workers.

@guns Yes, that's possible, although it would also require a significant reworking of my code based as well (ironically) of the creation of more subsidiary files. Perhaps I would not go this route again (I started with a single file, then split it when I found out how!), but I am where I am.

As are you; if you do not want to support this, fair enough. From what @technomancy says, you wouldn't take support for naked load even as a PR?

[technomancy]

I want to keep Slamhound focused on doing one thing well, namely rewriting ns forms.

[guns]

@phillord

As are you; if you do not want to support this, fair enough. From what @technomancy says, you wouldn't take support for naked load even as a PR?

This is @technomancy's project, but it happens that I fully agree with him.

The details of namespace construction in Clojure are unpleasantly imperative and the implementation of the ns macro is surprisingly facile, but the contract of the ns form is at least declarative.

An implicit goal of Slamhound is to encourage the use of this declarative subset of the ns form, as this greatly simplifies the human interpretation of a Clojure source file.

Interleaved code and load statements generally make human reading more complicated. It may feel simpler to the author, since he can group related functions together in a file. However, as each included file has an implicit dependency on the files that have been loaded before it, a simple find-word within a buffer is no longer likely to reveal the source of a symbol, nor every use of it within a namespace.

IMHO, it is better to stick to one file per namespace and lean on your tools¹ to help deal with the cognitive load of having distinct groups of vars within your file.

Or even better, split these var groups out into implementation namespaces, and simply keep a list of defs in the root file that map the top-level vars to their implementations. This requires maintenance of these var links, but has the advantage of making the contract of your namespace crystal clear.

All that said, I understand your POV. Clojure is pragmatic and permissible by design, and if inline (load …) is easier for you, then we are both happy.

I hope that you can see, in turn, that adding special case branches to Slamhound to support a namespace construction style that the project is trying to discourage is not very appealing.

Cheers!

¹ Light Table abstracts files in this way for instance, and both Emacs and Vim can "narrow in" on arbitrary subsections of a file.

[phillord]

"However, as each included file has an implicit dependency on the files that have been loaded before it, a simple find-word within a buffer is no longer likely to reveal the source of a symbol, nor every use of it within a namespace."

Nothing in life is simple, and most things involves compromise, and what you say is true. It is, why, for instance, I don't want to split my code up (that much) further.

"Or even better, split these var groups out into implementation namespaces, and simply keep a list of defs in the root file that map the top-level vars to their implementations"

In my case, this isn't really the problem; I have a single namespace that is just implementing something complex (the java package that implements the same data model has 150ish interface classes).

"I hope that you can see, in turn, that adding special case branches to Slamhound to support a namespace construction style that the project is trying to discourage is not very appealing."

It's all good. I sent a bug report in the hope that it would be useful for you, and it's been a valuable discussion for me. I certainly understand your reasons. In the end, I wouldn't expect you to extend your code just because I say it's a good idea.