Fork me on GitHub
#clojure-dev
<
2019-07-31
>
andy.fingerhut20:07:21

There are only a few people who can answer that question as you asked it, and I am not one of them, unfortunately. You probably already know that it is intended behavior to munge dash characters in namespace symbols into underscores before looking for resources/files on the classpath, because the JVM does not support dashes. Given that, it seems that there might be unavoidable naming issues in this area, like what you have found? I haven't thought deeply about it, but perhaps such issues would be avoidable if *loaded-libs* only contained the post-munged names, but that sounds likely to cause upset for any Clojure tooling that peeks inside *loaded-libs*.

nathanmarz20:07:57

the context is we had a bug caused by unexpected redefinitions from a typo in a require for a namespace containing a -

andy.fingerhut20:07:44

I don't know the history here, but it surprises me a little that you can even successfully do a require using test_ns, when the ns form inside the file is test-ns. It seems reasonable that such a require should fail.

gfredericks20:07:40

you'd imagine that it loads the file and then errors because test_ns hasn't been defined as a namespace?

gfredericks20:07:50

that seems like a reasonable expectation to me

andy.fingerhut20:07:52

Although my quick REPL testing is not matching my memory of code that I thought was present that checks whether the namespace exists near the end of the require. Double-checking to see where my memory is going wrong here.

nathanmarz20:07:40

yea, that does seem like better behavior

nathanmarz20:07:03

which in our case would have caught the typo immediately rather than cause a very difficult debugging session

andy.fingerhut20:07:02

Ah, if you do require of test_ns with an :as clause, and the file has ns for test-ns, then you get an error. Without the :as, it does not.

andy.fingerhut20:07:45

The error checking is also enabled via use, but understood if you wouldn't want to go there.

gfredericks20:07:12

when you read a lot of this code you start wondering whether the term lib was (or still is?) used to mean something slightly different from either "namespace" or "source file"

gfredericks20:07:33

I think I mentioned that here once a year ago but can't remember if anybody said anything useful about that

gfredericks20:07:49

but I could imagine that being a barrier to assuming that the arg to require is a namespace symbol

gfredericks20:07:27

from the require (which describes its args as libs, not namespaces) docstring:

A 'lib' is a named set of resources in classpath whose contents define a
  library of Clojure code. Lib names are symbols and each lib is associated
  with a Clojure namespace and a Java package that share its name. A lib's
  name also locates its root directory within classpath using Java's
  package name to classpath-relative path mapping. All resources in a lib
  should be contained in the directory structure under its root directory.
  All definitions a lib makes should be in its associated namespace.

andy.fingerhut22:07:36

I do not recall whether linters like Eastwood or clj-kondo would catch that kind of thing.

borkdude22:07:14

summarized: the check should be: if there is an ns foo-bar, but you require with foo_bar, it should give a warning?

borkdude22:07:42

or give a warning when you're using underscores at all?

andy.fingerhut22:07:50

I would think the most accurate check would be: the symbol naming the namespace in a require/use, and the ns form name symbol, should be equal.

borkdude22:07:53

but what happens if foo_bar and foo-bar both exist?

andy.fingerhut22:07:09

Eastwood definitely has checks already for compatibility between ns form symbols and the file name it is inside of on the file system.

andy.fingerhut22:07:56

Both namespace symbols? They can't be both used in a single ns form, of course. And for require to work, they would both have to be in the same file.

andy.fingerhut22:07:17

Two ns forms in the same file is not illegal, but pretty strange in most code.

borkdude22:07:05

right and require is file based and files must always have namespaces

borkdude22:07:45

I mean, you cannot require a namespace which is not in a file, but defined in the REPL

borkdude22:07:25

oh you can.

borkdude22:07:33

I think I was confused by how it works in CLJS

borkdude22:07:56

so you could have either foo_bar with (ns foo-bar ..) or (ns foo_bar), so a check would have to know about that and then emit a warning. that's something clj-kondo could easily do.

andy.fingerhut22:07:57

Well, require with :as, or use, both give an error if loading the file it finds does not end up causing the target namespace to exist.

andy.fingerhut22:07:53

A file foo_bar with either of those ns forms are legal, and would work if you did a require + :as with a matching symbol to name the namespace.

borkdude22:07:23

this is a very rare case I would say. stylistically, I'd say: avoid underscores in namespaces names

andy.fingerhut22:07:39

The warning that would have saved time in the scenario above cannot be caught by looking at the contents of only one file, but across files.

andy.fingerhut22:07:22

Stylistically forbidding underscores in namespace names could be adopted as a project convention, of course.

borkdude22:07:22

this kind of check would probably be just a few lines of scripting using the data that clj-kondo spits out: https://github.com/borkdude/clj-kondo/tree/master/analysis#data

andy.fingerhut22:07:52

Project style/convention-wise, mandating :as for all requires would also prevent this.

andy.fingerhut22:07:09

or rather, catch it quickly.

borkdude22:07:11

mandating :as for all requires is not something I would agree with

borkdude22:07:37

there are good reasons to not have an alias

seancorfield22:07:26

I agree. We have a number of namespaces where :require ... :refer [...] is the appropriate usage (because they declare operator-like symbols and using an alias makes them harder to read).

andy.fingerhut22:07:34

There is of course the possibility of monkey-patching require in your local dev environment to do the strong checking even without :as. Just blue-skying here, at this point, I know.