#330 def/declare do not reset metadata unless init-expr is passed

Signed-off-by: Stuart Halloway <stu@thinkrelevance.com>

5 files changed, 97 insertions, 30 deletions

diff --git a/src/jvm/clojure/lang/Compiler.java b/src/jvm/clojure/lang/Compiler.java
index 2b382989..40acd58d 100644
--- a/src/jvm/clojure/lang/Compiler.java
+++ b/src/jvm/clojure/lang/Compiler.java
@@ -338,7 +338,19 @@ static class DefExpr implements Expr{
 		this.initProvided = initProvided;
 	}
 
-	public Object eval() throws Exception{
+    private boolean includesExplicitMetadata(MapExpr expr) {
+        for(int i=0; i < expr.keyvals.count(); i += 2)
+            {
+                Keyword k  = ((KeywordExpr) expr.keyvals.nth(i)).k;
+                if ((k != RT.FILE_KEY) &&
+                    (k != RT.DECLARED_KEY) &&
+                    (k != RT.LINE_KEY))
+                    return true;
+            }
+        return false;
+    }
+
+    public Object eval() throws Exception{
 		try
 			{
 			if(initProvided)
@@ -350,7 +362,9 @@ static class DefExpr implements Expr{
 				}
 			if(meta != null)
 				{
-				var.setMeta((IPersistentMap) meta.eval());
+                IPersistentMap metaMap = (IPersistentMap) meta.eval();
+                if (initProvided || includesExplicitMetadata((MapExpr) meta))
+				    var.setMeta((IPersistentMap) meta.eval());
 				}
 			return var;
 			}
@@ -367,10 +381,13 @@ static class DefExpr implements Expr{
 		objx.emitVar(gen, var);
 		if(meta != null)
 			{
-			gen.dup();
-			meta.emit(C.EXPRESSION, objx, gen);
-			gen.checkCast(IPERSISTENTMAP_TYPE);
-			gen.invokeVirtual(VAR_TYPE, setMetaMethod);
+            if (initProvided || includesExplicitMetadata((MapExpr) meta))
+                {
+                gen.dup();
+                meta.emit(C.EXPRESSION, objx, gen);
+                gen.checkCast(IPERSISTENTMAP_TYPE);
+                gen.invokeVirtual(VAR_TYPE, setMetaMethod);
+                }
 			}
 		if(initProvided)
 			{
diff --git a/src/jvm/clojure/lang/RT.java b/src/jvm/clojure/lang/RT.java
index 44ad1474..e1016f57 100644
--- a/src/jvm/clojure/lang/RT.java
+++ b/src/jvm/clojure/lang/RT.java
@@ -183,6 +183,7 @@ final static public Var ASSERT = Var.intern(CLOJURE_NS, Symbol.create("*assert*"
 final static public Var MATH_CONTEXT = Var.intern(CLOJURE_NS, Symbol.create("*math-context*"), null);
 static Keyword LINE_KEY = Keyword.intern(null, "line");
 static Keyword FILE_KEY = Keyword.intern(null, "file");
+static Keyword DECLARED_KEY = Keyword.intern(null, "declared");
 final static public Var USE_CONTEXT_CLASSLOADER =
 		Var.intern(CLOJURE_NS, Symbol.create("*use-context-classloader*"), T);
 //final static public Var CURRENT_MODULE = Var.intern(Symbol.create("clojure.core", "current-module"),
diff --git a/test/clojure/test_clojure/helpers.clj b/test/clojure/test_clojure/helpers.clj
new file mode 100644
index 00000000..42e88047
--- /dev/null
+++ b/test/clojure/test_clojure/helpers.clj
@@ -0,0 +1,27 @@
+;   The use and distribution terms for this software are covered by the
+;   Eclipse Public License 1.0 (http://opensource.org/licenses/eclipse-1.0.php)
+;   which can be found in the file epl-v10.html at the root of this distribution.
+;   By using this software in any fashion, you are agreeing to be bound by
+;   the terms of this license.
+;   You must not remove this notice, or any other, from this software.
+
+; Author: Stuart Halloway
+
+(ns clojure.test-clojure.helpers
+  (:use clojure.test))
+
+(defn temp-ns
+  "Create and return a temporary ns, using clojure.core + uses"
+  [& uses]
+  (binding [*ns* *ns*]
+    (in-ns (gensym))
+    (apply clojure.core/use 'clojure.core uses)
+    *ns*))
+
+(defmacro eval-in-temp-ns [& forms]
+  `(binding [*ns* *ns*]
+     (in-ns (gensym))
+     (clojure.core/use 'clojure.core)
+     (eval
+      '(do ~@forms))))
+
diff --git a/test/clojure/test_clojure/metadata.clj b/test/clojure/test_clojure/metadata.clj
index ce93912a..62eea0bf 100644
--- a/test/clojure/test_clojure/metadata.clj
+++ b/test/clojure/test_clojure/metadata.clj
@@ -9,7 +9,8 @@
 ; Authors: Stuart Halloway, Frantisek Sodomka
 
 (ns clojure.test-clojure.metadata
-  (:use clojure.test))
+  (:use clojure.test
+        [clojure.test-clojure.helpers :only (eval-in-temp-ns)]))
 
 (def public-namespaces
   '[clojure.core
@@ -37,3 +38,38 @@
 
 (deftest public-vars-with-docstrings-have-added
   (is (= [] (remove (comp :added meta) public-vars-with-docstrings))))
+
+(deftest interaction-of-def-with-metadata
+  (testing "initial def sets metadata"
+    (let [v (eval-in-temp-ns
+             (def ^{:a 1} foo 0)
+             #'foo)]
+      (is (= 1 (-> v meta :a)))))
+  (testing "subsequent declare doesn't overwrite metadata"
+    (let [v (eval-in-temp-ns
+             (def ^{:b 2} bar 0)
+             (declare bar)
+             #'bar)]
+      (is (= 2 (-> v meta :b))))
+    (testing "when compiled"
+      (let [v (eval-in-temp-ns
+               (def ^{:c 3} bar 0)
+               (defn declare-bar []
+                 (declare bar))
+               (declare-bar)
+               #'bar)]
+        (is (= 3 (-> v meta :c))))))
+  (testing "subsequent def with init-expr *does* overwrite metadata"
+    (let [v (eval-in-temp-ns
+             (def ^{:d 4} quux 0)
+             (def quux 1)
+             #'quux)]
+      (is (nil? (-> v meta :d))))
+    (testing "when compiled"
+      (let [v (eval-in-temp-ns
+               (def ^{:e 5} quux 0)
+               (defn def-quux []
+                 (def quux 1))
+               (def-quux)
+               #'quux)]
+        (is (nil? (-> v meta :e)))))))
diff --git a/test/clojure/test_clojure/rt.clj b/test/clojure/test_clojure/rt.clj
index e72d4e06..dd0e90ca 100644
--- a/test/clojure/test_clojure/rt.clj
+++ b/test/clojure/test_clojure/rt.clj
@@ -9,7 +9,7 @@
 ; Author: Stuart Halloway
 
 (ns clojure.test-clojure.rt
-  (:use clojure.test))
+  (:use clojure.test clojure.test-clojure.helpers))
 
 (defmacro with-err-print-writer
   "Evaluate with err pointing to a temporary PrintWriter, and
@@ -30,27 +30,13 @@
        ~@body
        (str s#))))
 
-(defn temp-ns
-  "Create and return a temporary ns, using clojure.core + uses"
-  [& uses]
-  (binding [*ns* *ns*]
-    (in-ns (gensym))
-    (apply clojure.core/use 'clojure.core uses)
-    *ns*))
-
-(defmacro eval-in-temp-ns [form]
-  `(binding [*ns* *ns*]
-     (in-ns (gensym))
-     (clojure.core/use 'clojure.core)
-     (eval ~form)))
-
 (defmacro should-print-err-message
   "Turn on all warning flags, and test that error message prints
    correctly for all semi-reasonable bindings of *err*."
   [msg-re form]
-  (binding [*warn-on-reflection* true]
-    (is (re-matches msg-re (with-err-string-writer (eval-in-temp-ns form))))
-    (is (re-matches msg-re (with-err-print-writer (eval-in-temp-ns form))))))
+  `(binding [*warn-on-reflection* true]
+    (is (re-matches ~msg-re (with-err-string-writer (eval-in-temp-ns ~form))))
+    (is (re-matches ~msg-re (with-err-print-writer (eval-in-temp-ns ~form))))))
 
 (deftest error-messages
   (testing "binding a core var that already refers to something"
@@ -59,19 +45,19 @@
      (defn prefers [] (throw (RuntimeException. "rebound!")))))
   (testing "reflection cannot resolve field"
     (should-print-err-message
-     #"Reflection warning, clojure/test_clojure/rt.clj:\d+ - reference to field blah can't be resolved.\n"
+     #"Reflection warning, NO_SOURCE_PATH:\d+ - reference to field blah can't be resolved.\n"
      (defn foo [x] (.blah x))))
   (testing "reflection cannot resolve instance method"
     (should-print-err-message
-     #"Reflection warning, clojure/test_clojure/rt.clj:\d+ - call to zap can't be resolved.\n"
+     #"Reflection warning, NO_SOURCE_PATH:\d+ - call to zap can't be resolved.\n"
      (defn foo [x] (.zap x 1))))
   (testing "reflection cannot resolve static method"
     (should-print-err-message
-     #"Reflection warning, clojure/test_clojure/rt.clj:\d+ - call to valueOf can't be resolved.\n"
+     #"Reflection warning, NO_SOURCE_PATH:\d+ - call to valueOf can't be resolved.\n"
      (defn foo [] (Integer/valueOf #"boom"))))
-  (testing "reflection cannot resolved constructor"
+  (testing "reflection cannot resolve constructor"
     (should-print-err-message
-     #"Reflection warning, clojure/test_clojure/rt.clj:\d+ - call to java.lang.String ctor can't be resolved.\n"
+     #"Reflection warning, NO_SOURCE_PATH:\d+ - call to java.lang.String ctor can't be resolved.\n"
      (defn foo [] (String. 1 2 3)))))
 
 (def example-var)