Namespace.java: addAlias exception could be more informative

Question

In https://github.com/clojure/clojure/blob/653b8465845a78ef7543e0a250078eea2d56b659/src/jvm/clojure/lang/Namespace.java#L224-L225, the ns argument is not included in the exception message.

This hinders debuggabiliity, especially when the .addAlias call didn't originate from intentful aliasing, but rather, as a result of re-evaluating a namespace (which is the AST-building strategy that tools such as https://github.com/jonase/eastwood use).

Have you considered improving the message?

Answer 1

Example? Proposed change?

Comments

Example desired message:

`Alias __ (intended to refer to __) already exists in namespace __, aliasing __`

Where `(intended to refer to __)` is the new part.

Proposed change:

use the `ns` argument of this Java method as part of the message.

Its practical usefulness being that it allows distinguishing two cases:

* the intended alias is simply overlapping with an existing, differently-named entry
* two java.lang.Namespace objects are being compared, representing the same namespace but not being equal (`.equals` comparison), due to incorrect code reloading (code-reloading issues,  `eval` issues)

---

I presume this is an example of the first case:

user=> (require 'clojure.set)
nil
user=> (require 'clojure.string)
nil
user=> (alias 's 'clojure.set)
nil
user=> (alias 's 'clojure.string)
Execution error (IllegalStateException) at user/eval151 (REPL:1).
Alias s already exists in namespace user, aliasing clojure.set

That seems pretty clear to me and I'm not sure it would help to mention the attempted alias namespace. Can you explain how you might get into the 2nd case?

---

Your example correctly describes what I call "[...] overlapping with an existing, differently-named entry".

As for the other case, it is reflected in https://github.com/clojure/tools.namespace/tree/e5a9a2abc5360cef57d6f97908b1ec9d5322597a#warnings-for-aliases , where there are two competing "com.example.foo" clojure.lang.Namespace objects.

---

Seems pretty clear in that case what the problem is too.

---

It's (somewhat) clear because that's a direct, fine-grained user interaction. I say "somewhat", because people don't necessarily know what a clojure.lang.Namespace object is, or that two of those might exist for the same file.

That's not always the case: tools such as Eastwood perform `eval` across a given project's top-level forms (less direct, less fine-grained).

So one can get this error, and be left wondering if the issue is caused by loading code in an incorrect order, or if Eastwood is in fact placing overlapping aliases.

So, debugging Eastwood issues was legitimately hindered by these incomplete error messages.

---

So this should have been your original question, the actual problem, rather than a proposed solution deep in the implementation.

What is the sequence of events with Eastwood that leads to the issue? Could Eastwood avoid or detect this scenario?

---

I understand the intent of your indication, but at the same time, this is an error that is not rare at all to obtain. It's relatively common to experience it at some point, to see people ask about it, etc.

Tools other than Eastwood also eval code with an analog strategy (see https://github.com/clojure-emacs/refactor-nrepl/tree/71e057c413933e25f2ae5921f6aad4a262ae90ce "launch-missiles" warning), so the more tools in one's stack, the easier it is to get these errors, and the more useful it is to have complete debugging information, allowing one to discard mistaken hypotheses.

FWIW, I could dive into Eastwood and obtain a fix (just as an internal fork for now): https://github.com/nedap/eastwood .

So, this ask.clojure.org request is not one for obtaining a tool-specific fix I already have; it's a request for avoiding similar pains whenever debugging similar issues (at a different time, or by different people).

---

Sorry if this feels like I'm being obstinate, but I am trying to be methodical in working the problem ("this is what happens, and this I am missing this necessary info") instead of starting from a solution ("just change some code") to ensure we are doing the right thing.

At this point, you have not yet described a detailed, reproducible scenario that replicates the actual problem. Without that, we have no way to a) consider other possible solutions or b) test and verify that a particular change solves the problem. You claim that this problem is both common and commonly reported. If so, it should be easy to point to one of these discussions or describe the scenario, and that's all I'm looking for.

I messed around with tools.namespace trying to reproduce this kind of an error, and the only way I got there was with a require at the repl where changing this message would not give me any additional info:

    user=> (require '[foo :as bar])
    Execution error (IllegalStateException) at user/eval1664 (REPL:1).
    Alias bar already exists in namespace user, aliasing foo

Note that in Clojure 1.10.0+, the top line of the stack is not printed here, so you do not see "Namespace" (the examples in tools.namespace readme are older). Saying "Alias bar (intended to refer to foo) already exists in namespace user, aliasing foo" gives no additional information in  either understanding or addressing the issue, does it?