CompilerException / IllegalStateException when overriding vars

Question

`
=> (ns foo)
nil
=> (def a 1)

'foo/a

=> (ns bar (:require [foo :refer :all]))
nil
=> (def a 2)
CompilerException java.lang.IllegalStateException: a already refers to: #'foo/a in namespace: bar, compiling:(NO_SOURCE_PATH:4:1)

clojure.lang.Compiler.analyzeSeq (Compiler.java:6745)
clojure.lang.Compiler.analyze (Compiler.java:6529)
clojure.lang.Compiler.analyze (Compiler.java:6490)
clojure.lang.Compiler.eval (Compiler.java:6801)
clojure.lang.Compiler.eval (Compiler.java:6760)
clojure.core/eval (core.clj:3079)
clojure.main/repl/read-eval-print--7095/fn--7098 (main.clj:240)
clojure.main/repl/read-eval-print--7095 (main.clj:240)
clojure.main/repl/fn--7104 (main.clj:258)
clojure.main/repl (main.clj:258)
clojure.main/repl-opt (main.clj:324)
clojure.main/main (main.clj:422)

Caused by:
IllegalStateException a already refers to: #'foo/a in namespace: bar

clojure.lang.Namespace.warnOrFailOnReplace (Namespace.java:88)
clojure.lang.Namespace.intern (Namespace.java:72)
clojure.lang.Compiler$DefExpr$Parser.parse (Compiler.java:534)
clojure.lang.Compiler.analyzeSeq (Compiler.java:6738)
clojure.lang.Compiler.analyze (Compiler.java:6529)

`

I would expect (at worst) a similar warning to the initial namespace loading, rather than an exception here.

Answer 1

Comment made by: bronsa

CLJ-1746 is relevant and I like that proposal much better than CLJ-1257

Answer 2

Comment made by: mikera

Hi Nicola, CLJ-1746 looks like a reasonable suggestion but still doesn't address the problem here, for the same reason that :exclude doesn't (see my comment to Alex)

The fundamental issue is that the current behaviour causes user code to break when libraries are upgraded to add new vars and requires changes to user code to fix / work around it. I consider that broken behaviour, or at least very bad design.

This is especially since we are encouraged to choose good names: I quote from the library coding standards(http://dev.clojure.org/display/community/Library Coding Standards)

"Use good names, and don't be afraid to collide with names in other namespaces. That's what the flexible namespace support is there for."

It isn't very flexible when user code keep breaking.....

Answer 3

Comment made by: bronsa

From the same page, just a two lines below:
"Be explicit and minimalist about dependencies on other packages. (Prefer :require :refer in 1.4+ or :use :only in 1.0-1.3)."

If users carelessly import a whole package, I'd say that's their fault. Since clojure >1.3 the popular consensus has been that :use/:refer :all are not a good idea and people have been moving away from that, preferring :require :refer or :require :as instead.

The few that still use :use/:refer :all do it mostly for backward compatibility or in tests (I myself do it in tools.reader for the former reason).

I really don't think this is such a bad design choice as you seem to think, and I think that most people in the community would agree that the current behaviour and drive towards using :require :refer/require :as in lieu of :use/:refer :all is way more beneficial than harmful

Answer 4

Comment made by: mikera

Hi Nicola, I have no issue with people using explicit :refer / :require, and I agree it is normal practice. It's good to be explicit and I generally do that myself (except in test / demo code).

However we aren't talking about that case : this issue is most relevant in those situations where users (for their own legitimate reasons) have decided to import a whole namespace. This is often convenient (e.g. REPL usage), sometimes it is valuable for specific purposes (e.g. testing).

I personally care about this mostly from the perspective of a library author: I want users to be able to use the library in whatever way is most convenient (which may include importing all vars), and I don't want user code to break randomly when I make a new release. Note that this also means I don't have control over user code so any solution that involves explicit requires, excludes, or some other manual workarounds is not a practical solution.

You can say "that's their fault" but this is currently supposed to be supported in Clojure so I think it ought to work sanely.

Answer 5

Comment made by: alexmiller

I think the library / evolution argument is a good one and I like that it reduces breakage due to evolution of 3rd party libraries. (I feel very strongly about this in clojure.core itself which is auto-referred, less strongly otherwise.)

On the flip side, if we remove this error, we should be explicit about what we are giving up to fully consider it. Presumably we are at least giving up a helpful error that occurs in the case of an accidental override. Are there other impacts?

I do not expect that we're going to change anything in 1.7.

Answer 6

Comment made by: stu

Recategorizing as a feature request, as the current behavior was the original intent.

Could a namespace-reflecting macro provide for this use case without requiring a change to core?

Answer 7

Reference: https://clojure.atlassian.net/browse/CLJ-1401 (reported by mikera)