Suppress warnings when clojure.core symbols are explicitly replaced with "use" or "refer"

Question

Problem: Libraries that provide DSLs (such as core.matrix) often replace or extend functions in core (such as "+", "==", "zero?"), since it is desirable to use the best / most idiomatic names.

Currently importing such libraries with "use" causes unwanted warnings like "WARNING: + already refers to: #'clojure.core/+ in namespace: test.blank, being replaced by: #'clojure.core.matrix/+".

Avoiding these warnings requires extra user effort and boilerplate code, which is frustrating for users since they have already explicitly asked for the full library to be imported into the current namespace (i.e. via "use" or ":refer :all").

Proposed solution is to introduce a new var warn-on-replace similar to warn-on-reflection which allows this warning to be controlled.

Answer 1

Comment made by: mikera

Basic patch and test attached.

Answer 2

Comment made by: jafingerhut

I have no idea whether this idea will be vetted or not, but if it is, I have some comments on the proposed patch.

The new symbol warn-on-replace should have doc and metadata on it. See add-doc-and-meta for warn-on-reflection in core.clj for an example to copy and edit.

You check WARN_ON_REFLECTION before issuing the warning in Namespace.java, instead of WARN_ON_REPLACE.

Possible typo in the test description in ns_libs.clj: Maybe "symbol in clojure.core" instead of "symbol is clojure.core"?

If someone wants warnings from :use statements in ns forms, it seems the only way to do that with patch clj-1257.diff would be to do (set! warn-on-replace true) in a file before the ns form. That would not work well with the current version of tools.namespace, which assumes that if there is an ns form, it is the first form in the file. One can argue that tools.namespace should not make such an assumption, but it does right now.

Perhaps there should be a command line option clojure.compile.warn-on-replace like there is for clojure.compile.warn-on-reflection (search for warn-on-replace in Compile.java)?

Answer 3

Comment made by: mikera

Thanks Andy for the feedback! I'll post an updated patch shortly.

It occurs to me that we should probably implement a more general approach to warnings in Clojure. Adding new vars and command line options for each warning doesn't seem like a good long-term strategy. I think that's beyond the scope of this patch though. :-)

Answer 4

Comment made by: jafingerhut

Actually, there is something called compiler-options (search for the variations compiler-options, COMPILER_OPTIONS, and compiler_options for all related occurrences) that is a map where each key/value pair is a different option. That might be preferable for warn-on-replace, if it is in fact desired.

Answer 5

Comment made by: mikera

Updated patch attached.

Compiler-options looks like it may indeed be a better place for this, if that is the preferred strategy for controlling warnings. But I'll wait for more feedback / confirmation on the approach before making that change.

Answer 6

Comment made by: alexmiller

Is (:refer-clojure :exclude (link: + = zero?)) a valid workaround? Or are you really concerned with the consumers of the library?

Answer 7

Comment made by: mikera

I'm mainly concerned with consumers of the library.

So while (:refer-clojure :exclude (link: + = zero?)) is possible as a temporary workaround, it's very inconvenient for users. We should really fix this in Clojure itself. Users have enough trouble with ns forms already without adding to their woes :-)

As an alternative solution: I personally wouldn't mind it if the library author could add some metadata to symbols (e.g. "^:replace-symbol") to signal that a library function is intended to replace something in core. That's a slightly different approach (and I think a bit trickier to implement) but it should also work.

Answer 8

Comment made by: mikera

Example issue reported by a user because of this:

https://github.com/mikera/vectorz-clj/issues/23

Answer 9

Comment made by: jafingerhut

As before, I can't comment on whether there is interest in this ticket or these patches, but I can say that all patches dated Sep 7 2013 and earlier no longer applied cleanly to latest master after some commits were made to Clojure on Aug 29, 2014. They did apply cleanly before that day.

I have not checked how easy or difficult it might be to update this patch.

Answer 10

Comment made by: mikera

I'm happy to update the patch, just need feedback on which approach / solution to this problem is preferred.

I'd really like to see this in 1.7!

Answer 11

Comment made by: alexmiller

Linking to CLJ-1746 as related.

Answer 12

Comment made by: jmromrell

Did anything ever come of this? I am trying to resolve similar warnings from monger, among other libraries.

Answer 13

Comment made by: alexmiller

It doesn't look like the patch was ever updated from the last discussion about the current patch, so it's never been screened. I think I would follow the path of **warn-on-reflection** rather than compiler options.

Answer 14

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