duck type RT.err #343

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

6 files changed, 94 insertions, 18 deletions

diff --git a/src/jvm/clojure/lang/Compile.java b/src/jvm/clojure/lang/Compile.java
index 26a417c5..81d1c811 100644
--- a/src/jvm/clojure/lang/Compile.java
+++ b/src/jvm/clojure/lang/Compile.java
@@ -31,7 +31,7 @@ private static final Var warn_on_reflection = RT.var("clojure.core", "*warn-on-r
 public static void main(String[] args) throws Exception{
 
 	OutputStreamWriter out = (OutputStreamWriter) RT.OUT.deref();
-	PrintWriter err = (PrintWriter) RT.ERR.deref();
+	PrintWriter err = RT.errPrintWriter();
 	String path = System.getProperty(PATH_PROP);
 	int count = args.length;
 
diff --git a/src/jvm/clojure/lang/Compiler.java b/src/jvm/clojure/lang/Compiler.java
index 1506cb27..be924769 100644
--- a/src/jvm/clojure/lang/Compiler.java
+++ b/src/jvm/clojure/lang/Compiler.java
@@ -923,9 +923,9 @@ static class InstanceFieldExpr extends FieldExpr implements AssignableExpr{
 		this.tag = tag;
 		if(field == null && RT.booleanCast(RT.WARN_ON_REFLECTION.deref()))
 			{
-			((PrintWriter) RT.ERR.deref())
-					.format("Reflection warning, %s:%d - reference to field %s can't be resolved.\n",
-							SOURCE_PATH.deref(), line, fieldName);
+			RT.errPrintWriter()
+		      .format("Reflection warning, %s:%d - reference to field %s can't be resolved.\n",
+					  SOURCE_PATH.deref(), line, fieldName);
 			}
 	}
 
@@ -1134,8 +1134,7 @@ static abstract class MethodExpr extends HostExpr{
 				}
 			catch(Exception e1)
 				{
-				e1.printStackTrace((PrintWriter) RT.ERR
-						.deref());  //To change body of catch statement use File | Settings | File Templates.
+				e1.printStackTrace(RT.errPrintWriter());
 				}
 
 			}
@@ -1199,9 +1198,9 @@ static class InstanceMethodExpr extends MethodExpr{
 
 		if(method == null && RT.booleanCast(RT.WARN_ON_REFLECTION.deref()))
 			{
-			((PrintWriter) RT.ERR.deref())
-					.format("Reflection warning, %s:%d - call to %s can't be resolved.\n",
-							SOURCE_PATH.deref(), line, methodName);
+			RT.errPrintWriter()
+		      .format("Reflection warning, %s:%d - call to %s can't be resolved.\n",
+					  SOURCE_PATH.deref(), line, methodName);
 			}
 	}
 
@@ -1348,9 +1347,9 @@ static class StaticMethodExpr extends MethodExpr{
 		method = (java.lang.reflect.Method) (methodidx >= 0 ? methods.get(methodidx) : null);
 		if(method == null && RT.booleanCast(RT.WARN_ON_REFLECTION.deref()))
 			{
-			((PrintWriter) RT.ERR.deref())
-					.format("Reflection warning, %s:%d - call to %s can't be resolved.\n",
-							SOURCE_PATH.deref(), line, methodName);
+			RT.errPrintWriter()
+              .format("Reflection warning, %s:%d - call to %s can't be resolved.\n",
+                      SOURCE_PATH.deref(), line, methodName);
 			}
 	}
 
@@ -2064,9 +2063,9 @@ public static class NewExpr implements Expr{
 		this.ctor = ctoridx >= 0 ? (Constructor) ctors.get(ctoridx) : null;
 		if(ctor == null && RT.booleanCast(RT.WARN_ON_REFLECTION.deref()))
 			{
-			((PrintWriter) RT.ERR.deref())
-					.format("Reflection warning, %s:%d - call to %s ctor can't be resolved.\n",
-							SOURCE_PATH.deref(), line, c.getName());
+			RT.errPrintWriter()
+              .format("Reflection warning, %s:%d - call to %s ctor can't be resolved.\n",
+                      SOURCE_PATH.deref(), line, c.getName());
 			}
 	}
 
diff --git a/src/jvm/clojure/lang/Namespace.java b/src/jvm/clojure/lang/Namespace.java
index 34a5c50d..383719f5 100644
--- a/src/jvm/clojure/lang/Namespace.java
+++ b/src/jvm/clojure/lang/Namespace.java
@@ -14,7 +14,6 @@ package clojure.lang;
 
 import java.io.ObjectStreamException;
 import java.io.Serializable;
-import java.io.PrintWriter;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -82,7 +81,7 @@ public Var intern(Symbol sym){
 }
 
 private void warnOnReplace(Symbol sym, Object o, Object v){
-	((PrintWriter) RT.ERR.deref()).println("WARNING: " + sym + " already refers to: " + o + " in namespace: " + name
+	RT.errPrintWriter().println("WARNING: " + sym + " already refers to: " + o + " in namespace: " + name
 		+ ", being replaced by: " + v);
 }
 
diff --git a/src/jvm/clojure/lang/RT.java b/src/jvm/clojure/lang/RT.java
index 4c9de96c..44ad1474 100644
--- a/src/jvm/clojure/lang/RT.java
+++ b/src/jvm/clojure/lang/RT.java
@@ -237,6 +237,16 @@ public static List<String> processCommandLine(String[] args){
 	return arglist;
 }
 
+// duck typing stderr plays nice with e.g. swank 
+public static PrintWriter errPrintWriter(){
+    Writer w = (Writer) ERR.deref();
+    if (w instanceof PrintWriter) {
+        return (PrintWriter) w;
+    } else {
+        return new PrintWriter(w);
+    }
+}
+
 static public final Object[] EMPTY_ARRAY = new Object[]{};
 static public final Comparator DEFAULT_COMPARATOR = new DefaultComparator();
 
@@ -337,7 +347,7 @@ public static void loadResourceScript(Class c, String name, boolean failIfNotFou
 }
 
 static public void init() throws Exception{
-	((PrintWriter) RT.ERR.deref()).println("No need to call RT.init() anymore");
+	RT.errPrintWriter().println("No need to call RT.init() anymore");
 }
 
 static public long lastModified(URL url, String libfile) throws Exception{
diff --git a/test/clojure/test_clojure.clj b/test/clojure/test_clojure.clj
index 81793ee5..04380914 100644
--- a/test/clojure/test_clojure.clj
+++ b/test/clojure/test_clojure.clj
@@ -55,6 +55,7 @@
       :annotations
       :pprint
       :serialization
+      :rt
       ])
 
 (def test-namespaces
diff --git a/test/clojure/test_clojure/rt.clj b/test/clojure/test_clojure/rt.clj
new file mode 100644
index 00000000..007bf811
--- /dev/null
+++ b/test/clojure/test_clojure/rt.clj
@@ -0,0 +1,67 @@
+;   Copyright (c) Rich Hickey. All rights reserved.
+;   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.rt
+  (:use clojure.test))
+
+(defmacro with-err-print-writer
+  "Evaluate with err pointing to a temporary PrintWriter, and
+   return err contents as a string."
+  [& body]
+  `(let [s# (java.io.StringWriter.)
+         p# (java.io.PrintWriter. s#)]
+     (binding [*err* p#]
+       ~@body
+       (str s#))))
+
+(defmacro with-err-string-writer
+  "Evaluate with err pointing to a temporary StringWriter, and
+   return err contents as a string."
+  [& body]
+  `(let [s# (java.io.StringWriter.)]
+     (binding [*err* s#]
+       ~@body
+       (str s#))))
+
+(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))))))
+
+(deftest error-messages
+  (testing "binding a var that already refers to something"
+    (should-print-err-message
+     #"WARNING: prefers already refers to: #'clojure.core/prefers in namespace: .*\n"
+     (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"
+     (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"
+     (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"
+     (defn foo [] (Integer/valueOf #"boom"))))
+  (testing "reflection cannot resolved constructor"
+    (should-print-err-message
+     #"Reflection warning, clojure/test_clojure/rt.clj:\d+ - call to java.lang.String ctor can't be resolved.\n"
+     (defn foo [] (String. 1 2 3)))))