Generify IRubyObject.toJava to make it more pleasant to use.

This change allow callers to skip the cast when calling toJava on a Ruby object. It was initially done to assist work on #4781 by allowing a simple "throw rubyException.toJava(Throwable.class). I tested binary compatibility in two ways: * A full recompile with all generification in place plus testing standard JRuby commands. This confirms that classes recompiled against the API but without other use of generics still function properly. * A partial recompile with only the generified classes rebuilt. This tests that classes not compiled against the API still function properly. Of course any external JRuby extensions (readline, openssl) will have been tested by running those standard commands (e.g. irb, gem install).
jruby · Feb 27, 2018 · ff88982 · ff88982
1 parent 6a0bb07
commit ff88982
Showing 19 changed files with 128 additions and 141 deletions.
diff --git a/core/src/main/java/org/jruby/RubyArray.java b/core/src/main/java/org/jruby/RubyArray.java
@@ -4969,7 +4969,7 @@ public Object[] toArray(final Object[] arg) {
     }
 
     @Override
-    public Object toJava(Class target) {
+    public <T> T toJava(Class<T> target) {
         if (target.isArray()) {
             Class type = target.getComponentType();
             Object rawJavaArray = Array.newInstance(type, realLength);
@@ -4978,7 +4978,7 @@ public Object toJava(Class target) {
             } catch (ArrayIndexOutOfBoundsException ex) {
                 throw concurrentModification(getRuntime(), ex);
             }
-            return rawJavaArray;
+            return target.cast(rawJavaArray);
         } else {
             return super.toJava(target);
         }

diff --git a/core/src/main/java/org/jruby/RubyBasicObject.java b/core/src/main/java/org/jruby/RubyBasicObject.java
@@ -856,7 +856,7 @@ public IRubyObject checkArrayType() {
      * @see IRubyObject#toJava
      */
     @Override
-    public Object toJava(Class target) {
+    public <T> T toJava(Class<T> target) {
         return defaultToJava(target);
     }
 

diff --git a/core/src/main/java/org/jruby/RubyBoolean.java b/core/src/main/java/org/jruby/RubyBoolean.java
@@ -161,6 +161,19 @@ public static IRubyObject false_xor(IRubyObject f, IRubyObject oth) {
         public static RubyString false_to_s(IRubyObject f) {
             return RubyString.newStringShared(f.getRuntime(), FALSE_BYTES);
         }
+
+        /*
+        Because Boolean objects can't cast to boolean via Class.cast, and you can't return
+        a primitive from a generified method, this impl remains ungenerified.
+         */
+        @Override
+        public Object toJava(Class target) {
+            if (target.isAssignableFrom(Boolean.class) | target.equals(boolean.class)) {
+                return Boolean.FALSE;
+            } else {
+                return super.toJava(target);
+            }
+        }
     }
 
     static final ByteList TRUE_BYTES = new ByteList(new byte[] { 't','r','u','e' }, USASCIIEncoding.INSTANCE);
@@ -192,6 +205,19 @@ public static IRubyObject true_xor(IRubyObject t, IRubyObject oth) {
         public static RubyString true_to_s(IRubyObject t) {
             return RubyString.newStringShared(t.getRuntime(), TRUE_BYTES);
         }
+
+        /*
+        Because Boolean objects can't cast to boolean via Class.cast, and you can't return
+        a primitive from a generified method, this impl remains ungenerified.
+         */
+        @Override
+        public Object toJava(Class target) {
+            if (target.isAssignableFrom(Boolean.class) | target.equals(boolean.class)) {
+                return Boolean.TRUE;
+            } else {
+                return super.toJava(target);
+            }
+        }
     }
 
     @JRubyMethod(name = "hash")
@@ -221,16 +247,5 @@ public IRubyObject taint(ThreadContext context) {
     public void marshalTo(MarshalStream output) throws java.io.IOException {
         output.write(isTrue() ? 'T' : 'F');
     }
-
-    @Override
-    public Object toJava(Class target) {
-        if (target.isAssignableFrom(Boolean.class) || target.equals(boolean.class)) {
-            if (isFalse()) return Boolean.FALSE;
-
-            return Boolean.TRUE;
-        } else {
-            return super.toJava(target);
-        }
-    }
 }
 
diff --git a/core/src/main/java/org/jruby/RubyClass.java b/core/src/main/java/org/jruby/RubyClass.java
@@ -1886,7 +1886,7 @@ public synchronized void addClassAnnotation(Class annotation, Map fields) {
     }
 
     @Override
-    public Object toJava(final Class target) {
+    public <T> T toJava(Class<T> target) {
         if (target == Class.class) {
             if (reifiedClass == null) reifyWithAncestors(); // possibly auto-reify
             // Class requested; try java_class or else return nearest reified class
@@ -1895,13 +1895,13 @@ public Object toJava(final Class target) {
             if ( ! javaClass.isNil() ) return javaClass.toJava(target);
 
             Class reifiedClass = nearestReifiedClass(this);
-            if ( reifiedClass != null ) return reifiedClass;
+            if ( reifiedClass != null ) return target.cast(reifiedClass);
             // should never fall through, since RubyObject has a reified class
         }
 
         if (target.isAssignableFrom(RubyClass.class)) {
             // they're asking for something RubyClass extends, give them that
-            return this;
+            return target.cast(this);
         }
 
         return defaultToJava(target);

diff --git a/core/src/main/java/org/jruby/RubyFile.java b/core/src/main/java/org/jruby/RubyFile.java
@@ -2028,10 +2028,10 @@ private static RubyString checkHome(ThreadContext context) {
     }
 
     @Override
-    public Object toJava(Class target) {
-        if (target == java.io.File.class) {
+    public <T> T toJava(Class<T> target) {
+        if (target == File.class) {
             final String path = getPath();
-            return path == null ? null : new java.io.File(path);
+            return path == null ? null : target.cast(new File(path));
         }
         return super.toJava(target);
     }

diff --git a/core/src/main/java/org/jruby/RubyIO.java b/core/src/main/java/org/jruby/RubyIO.java
@@ -4752,14 +4752,14 @@ public boolean getBOM() {
     }
 
     @Override
-    public Object toJava(Class target) {
+    public <T> T toJava(Class<T> target) {
         if (target == java.io.InputStream.class) {
             getOpenFile().checkReadable(getRuntime().getCurrentContext());
-            return getInStream();
+            return target.cast(getInStream());
         }
         if (target == java.io.OutputStream.class) {
             getOpenFile().checkWritable(getRuntime().getCurrentContext());
-            return getOutStream();
+            return target.cast(getOutStream());
         }
         return super.toJava(target);
     }

diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -4718,7 +4718,7 @@ public void setCacheProxy(boolean cacheProxy) {
     }
 
     @Override
-    public Object toJava(final Class target) {
+    public <T> T toJava(Class<T> target) {
         if (target == Class.class) { // try java_class for proxy modules
             final ThreadContext context = getRuntime().getCurrentContext();
             IRubyObject javaClass = JavaClass.java_class(context, this);

diff --git a/core/src/main/java/org/jruby/RubyNil.java b/core/src/main/java/org/jruby/RubyNil.java
@@ -249,24 +249,24 @@ public static IRubyObject rationalize(ThreadContext context, IRubyObject recv, I
     }
 
     @Override
-    public Object toJava(Class target) {
+    public <T> T toJava(Class<T> target) {
         if (target.isPrimitive()) {
             if (target == Boolean.TYPE) {
-                return Boolean.FALSE;
+                return target.cast(false);
             } else if (target == Byte.TYPE) {
-                return (byte)0;
+                return target.cast((byte)0);
             } else if (target == Short.TYPE) {
-                return (short)0;
+                return target.cast((short)0);
             } else if (target == Character.TYPE) {
-                return (char)0;
+                return target.cast((char)0);
             } else if (target == Integer.TYPE) {
-                return 0;
+                return target.cast(0);
             } else if (target == Long.TYPE) {
-                return (long)0;
+                return target.cast(0L);
             } else if (target == Float.TYPE) {
-                return (float)0;
+                return target.cast(0F);
             } else if (target == Double.TYPE) {
-                return (double)0;
+                return target.cast(0.0);
             }
         }
         return null;

diff --git a/core/src/main/java/org/jruby/RubyNumeric.java b/core/src/main/java/org/jruby/RubyNumeric.java
@@ -1337,7 +1337,7 @@ public IRubyObject conjugate(ThreadContext context) {
     }
 
     @Override
-    public Object toJava(Class target) {
+    public <T> T toJava(Class<T> target) {
         return JavaUtil.getNumericConverter(target).coerce(this, target);
     }
 

diff --git a/core/src/main/java/org/jruby/RubyString.java b/core/src/main/java/org/jruby/RubyString.java
@@ -5624,18 +5624,18 @@ public static ByteList encodeBytelist(CharSequence value, Encoding encoding) {
     }
 
     @Override
-    public Object toJava(Class target) {
+    public <T> T toJava(Class<T> target) {
         if (target.isAssignableFrom(String.class)) {
-            return decodeString();
+            return target.cast(decodeString());
         }
         if (target.isAssignableFrom(ByteList.class)) {
-            return value;
+            return target.cast(value);
         }
         if (target == Character.class || target == Character.TYPE) {
             if ( strLength() != 1 ) {
                 throw getRuntime().newArgumentError("could not coerce string of length " + strLength() + " (!= 1) into a char");
             }
-            return decodeString().charAt(0);
+            return target.cast(decodeString().charAt(0));
         }
         return super.toJava(target);
     }

diff --git a/core/src/main/java/org/jruby/RubySymbol.java b/core/src/main/java/org/jruby/RubySymbol.java
@@ -646,8 +646,8 @@ public static RubySymbol unmarshalFrom(UnmarshalStream input) throws java.io.IOE
     }
 
     @Override
-    public Object toJava(Class target) {
-        if (target == String.class || target == CharSequence.class) return symbol;
+    public <T> T toJava(Class<T> target) {
+        if (target == String.class || target == CharSequence.class) return target.cast(symbol);
 
         return super.toJava(target);
     }

diff --git a/core/src/main/java/org/jruby/RubyTime.java b/core/src/main/java/org/jruby/RubyTime.java
@@ -1285,27 +1285,27 @@ public static RubyTime load(ThreadContext context, IRubyObject recv, IRubyObject
     }
 
     @Override
-    public Object toJava(Class target) {
+    public <T> T toJava(Class<T> target) {
         if (target == Date.class) {
-            return getJavaDate();
+            return target.cast(getJavaDate());
         }
         if (target == Calendar.class || target == GregorianCalendar.class) {
-            return dt.toGregorianCalendar();
+            return target.cast(dt.toGregorianCalendar());
         }
         if (target == DateTime.class) {
-            return this.dt;
+            return target.cast(this.dt);
         }
         if (target == java.sql.Date.class) {
-            return new java.sql.Date(dt.getMillis());
+            return target.cast(new java.sql.Date(dt.getMillis()));
         }
         if (target == java.sql.Time.class) {
-            return new java.sql.Time(dt.getMillis());
+            return target.cast(new java.sql.Time(dt.getMillis()));
         }
         if (target == java.sql.Timestamp.class) {
-            return new java.sql.Timestamp(dt.getMillis());
+            return target.cast(new java.sql.Timestamp(dt.getMillis()));
         }
         if (target.isAssignableFrom(Date.class)) {
-            return getJavaDate();
+            return target.cast(getJavaDate());
         }
         return super.toJava(target);
     }

diff --git a/core/src/main/java/org/jruby/java/proxies/ConcreteJavaProxy.java b/core/src/main/java/org/jruby/java/proxies/ConcreteJavaProxy.java
@@ -210,7 +210,7 @@ public IRubyObject id() {
 
     @Override
     @SuppressWarnings("unchecked")
-    public Object toJava(final Class type) {
+    public <T> T toJava(Class<T> type) {
         final Object object = getObject();
         final Class clazz = object.getClass();
 
@@ -222,16 +222,16 @@ public Object toJava(final Class type) {
                  object instanceof Boolean && type == Boolean.TYPE ) {
                 // FIXME in more permissive call paths, like invokedynamic, this can allow
                 // precision-loading downcasts to happen silently
-                return object;
+                return (T) object;
             }
         }
         else if ( type.isAssignableFrom(clazz) ) {
             if ( Java.OBJECT_PROXY_CACHE || metaClass.getCacheProxy() ) {
                 getRuntime().getJavaSupport().getObjectProxyCache().put(object, this);
             }
-            return object;
+            return type.cast(object);
         }
-        else if ( type.isAssignableFrom(getClass()) ) return this; // e.g. IRubyObject.class
+        else if ( type.isAssignableFrom(getClass()) ) return type.cast(this); // e.g. IRubyObject.class
 
         throw getRuntime().newTypeError("failed to coerce " + clazz.getName() + " to " + type.getName());
     }

diff --git a/core/src/main/java/org/jruby/java/proxies/JavaProxy.java b/core/src/main/java/org/jruby/java/proxies/JavaProxy.java
@@ -462,12 +462,12 @@ private RubyMethod getRubyMethod(ThreadContext context, String name, Class... ar
 
     @Override
     @SuppressWarnings("unchecked")
-    public Object toJava(final Class type) {
+    public <T> T toJava(Class<T> type) {
         final Object object = getObject();
         final Class<?> clazz = object.getClass();
 
-        if ( type.isAssignableFrom(clazz) ) return object;
-        if ( type.isAssignableFrom(getClass()) ) return this; // e.g. IRubyObject.class
+        if ( type.isAssignableFrom(clazz) ) return type.cast(object);
+        if ( type.isAssignableFrom(getClass()) ) return type.cast(this); // e.g. IRubyObject.class
 
         throw getRuntime().newTypeError("failed to coerce " + clazz.getName() + " to " + type.getName());
     }

diff --git a/core/src/main/java/org/jruby/javasupport/JavaAccessibleObject.java b/core/src/main/java/org/jruby/javasupport/JavaAccessibleObject.java
@@ -169,9 +169,10 @@ public String toString() {
     }
 
     @Override
-    public Object toJava(Class target) {
-        if ( AccessibleObject.class.isAssignableFrom(target) ) {
-            return accessibleObject();
+    public <T> T toJava(Class<T> target) {
+        AccessibleObject accessibleObject = accessibleObject();
+        if (target.isAssignableFrom(accessibleObject.getClass())) {
+            return target.cast(accessibleObject);
         }
         return super.toJava(target);
     }

diff --git a/core/src/main/java/org/jruby/javasupport/JavaObject.java b/core/src/main/java/org/jruby/javasupport/JavaObject.java
@@ -301,12 +301,12 @@ public IRubyObject marshal_load(ThreadContext context, IRubyObject str) {
 
     @Override
     @SuppressWarnings("unchecked")
-    public Object toJava(final Class target) {
+    public <T> T toJava(Class<T> target) {
         final Object value = getValue();
         if ( value == null ) return null;
 
         if ( target.isAssignableFrom( value.getClass() ) ) {
-            return value;
+            return target.cast(value);
         }
         return super.toJava(target);
     }

diff --git a/core/src/main/java/org/jruby/javasupport/JavaPackage.java b/core/src/main/java/org/jruby/javasupport/JavaPackage.java
@@ -295,9 +295,9 @@ public IRubyObject sealed_p(ThreadContext context) {
 
     @Override
     @SuppressWarnings("unchecked")
-    public Object toJava(final Class target) {
+    public <T> T toJava(Class<T> target) {
         if ( target.isAssignableFrom( Package.class ) ) {
-            return Package.getPackage(packageName);
+            return target.cast(Package.getPackage(packageName));
         }
         return super.toJava(target);
     }

diff --git a/core/src/main/java/org/jruby/javasupport/JavaUtil.java b/core/src/main/java/org/jruby/javasupport/JavaUtil.java
@@ -264,7 +264,7 @@ public static <T> T convertProcToInterface(ThreadContext context, RubyBasicObjec
         return (T) javaObject.getValue();
     }
 
-    public static NumericConverter getNumericConverter(Class target) {
+    public static <T> NumericConverter<T> getNumericConverter(Class<T> target) {
         final NumericConverter converter = NUMERIC_CONVERTERS.get(target);
         return converter == null ? NUMERIC_TO_OTHER : converter;
     }
@@ -587,8 +587,8 @@ public static abstract class JavaConverter {
         public String toString() {return type.getName() + " converter";}
     }
 
-    public interface NumericConverter {
-        public Object coerce(RubyNumeric numeric, Class target);
+    public interface NumericConverter<T> {
+        T coerce(RubyNumeric numeric, Class<T> target);
     }
 
     private static IRubyObject trySimpleConversions(Ruby runtime, Object object) {
@@ -910,90 +910,61 @@ public void set(Ruby runtime, Object array, int i, IRubyObject value) {
         JAVA_CONVERTERS.put(BigInteger.class, JAVA_BIGINTEGER_CONVERTER);
     }
 
-    private static final NumericConverter NUMERIC_TO_BYTE = new NumericConverter() {
-        public Object coerce(RubyNumeric numeric, Class target) {
-            final long value = numeric.getLongValue();
-            if ( isLongByteable(value) ) return (byte) value;
-            throw numeric.getRuntime().newRangeError("too big for byte: " + numeric);
-        }
-    };
-    private static final NumericConverter NUMERIC_TO_SHORT = new NumericConverter() {
-        public Object coerce(RubyNumeric numeric, Class target) {
-            final long value = numeric.getLongValue();
-            if ( isLongShortable(value) ) return (short) value;
-            throw numeric.getRuntime().newRangeError("too big for short: " + numeric);
-        }
-    };
-    private static final NumericConverter NUMERIC_TO_CHARACTER = new NumericConverter() {
-        public Object coerce(RubyNumeric numeric, Class target) {
-            final long value = numeric.getLongValue();
-            if ( isLongCharable(value) ) return (char) value;
-            throw numeric.getRuntime().newRangeError("too big for char: " + numeric);
-        }
+    private static final NumericConverter<Byte> NUMERIC_TO_BYTE = (numeric, target) -> {
+        final long value = numeric.getLongValue();
+        if ( isLongByteable(value) ) return (byte) value;
+        throw numeric.getRuntime().newRangeError("too big for byte: " + numeric);
     };
-    private static final NumericConverter NUMERIC_TO_INTEGER = new NumericConverter() {
-        public Object coerce(RubyNumeric numeric, Class target) {
-            final long value = numeric.getLongValue();
-            if ( isLongIntable(value) ) return (int) value;
-            throw numeric.getRuntime().newRangeError("too big for int: " + numeric);
-        }
+    private static final NumericConverter<Short> NUMERIC_TO_SHORT = (numeric, target) -> {
+        final long value = numeric.getLongValue();
+        if ( isLongShortable(value) ) return (short) value;
+        throw numeric.getRuntime().newRangeError("too big for short: " + numeric);
     };
-    private static final NumericConverter NUMERIC_TO_LONG = new NumericConverter() {
-        public Object coerce(RubyNumeric numeric, Class target) {
-            return numeric.getLongValue();
-        }
+    private static final NumericConverter<Character> NUMERIC_TO_CHARACTER = (numeric, target) -> {
+        final long value = numeric.getLongValue();
+        if ( isLongCharable(value) ) return (char) value;
+        throw numeric.getRuntime().newRangeError("too big for char: " + numeric);
     };
-    private static final NumericConverter NUMERIC_TO_FLOAT = new NumericConverter() {
-        public Object coerce(RubyNumeric numeric, Class target) {
-            final double value = numeric.getDoubleValue();
-            // many cases are ok to convert to float; if not one of these, error
-            if ( isDoubleFloatable(value) ) return (float) value;
-            throw numeric.getRuntime().newTypeError("too big for float: " + numeric);
-        }
+    private static final NumericConverter<Integer> NUMERIC_TO_INTEGER = (numeric, target) -> {
+        final long value = numeric.getLongValue();
+        if ( isLongIntable(value) ) return (int) value;
+        throw numeric.getRuntime().newRangeError("too big for int: " + numeric);
     };
-    private static final NumericConverter NUMERIC_TO_DOUBLE = new NumericConverter() {
-        public Object coerce(RubyNumeric numeric, Class target) {
-            return numeric.getDoubleValue();
-        }
+    private static final NumericConverter<Long> NUMERIC_TO_LONG = (numeric, target) -> numeric.getLongValue();
+    private static final NumericConverter<Float> NUMERIC_TO_FLOAT = (numeric, target) -> {
+        final double value = numeric.getDoubleValue();
+        // many cases are ok to convert to float; if not one of these, error
+        if ( isDoubleFloatable(value) ) return (float) value;
+        throw numeric.getRuntime().newTypeError("too big for float: " + numeric);
     };
-    private static final NumericConverter NUMERIC_TO_BIGINTEGER = new NumericConverter() {
-        public Object coerce(RubyNumeric numeric, Class target) {
-            return numeric.getBigIntegerValue();
-        }
-    };
-    private static final NumericConverter NUMERIC_TO_OBJECT = new NumericConverter() {
-        public Object coerce(RubyNumeric numeric, Class target) {
-            // for Object, default to natural wrapper type
-            if (numeric instanceof RubyFixnum) {
-                long value = numeric.getLongValue();
-                return Long.valueOf(value);
-            } else if (numeric instanceof RubyFloat) {
-                double value = numeric.getDoubleValue();
-                return Double.valueOf(value);
-            } else if (numeric instanceof RubyBignum) {
-                return ((RubyBignum)numeric).getValue();
-            } else if (numeric instanceof RubyBigDecimal) {
-                return ((RubyBigDecimal)numeric).getValue();
-            } else {
-                return NUMERIC_TO_OTHER.coerce(numeric, target);
-            }
-        }
-    };
-    private static final NumericConverter NUMERIC_TO_OTHER = new NumericConverter() {
-        public Object coerce(RubyNumeric numeric, Class target) {
-            if (target.isAssignableFrom(numeric.getClass())) {
-                // just return as-is, since we can't do any coercion
-                return numeric;
-            }
-            // otherwise, error; no conversion available
-            throw numeric.getRuntime().newTypeError("could not coerce " + numeric.getMetaClass() + " to " + target);
+    private static final NumericConverter<Double> NUMERIC_TO_DOUBLE = (numeric, target) -> numeric.getDoubleValue();
+    private static final NumericConverter<BigInteger> NUMERIC_TO_BIGINTEGER = (numeric, target) -> numeric.getBigIntegerValue();
+
+    private static final NumericConverter NUMERIC_TO_OTHER = (numeric, target) -> {
+        if (target.isAssignableFrom(numeric.getClass())) {
+            // just return as-is, since we can't do any coercion
+            return numeric;
         }
+        // otherwise, error; no conversion available
+        throw numeric.getRuntime().newTypeError("could not coerce " + numeric.getMetaClass() + " to " + target);
     };
-    private static final NumericConverter NUMERIC_TO_VOID = new NumericConverter() {
-        public Object coerce(RubyNumeric numeric, Class target) {
-            return null;
+    private static final NumericConverter<Object> NUMERIC_TO_OBJECT = (numeric, target) -> {
+        // for Object, default to natural wrapper type
+        if (numeric instanceof RubyFixnum) {
+            long value = numeric.getLongValue();
+            return Long.valueOf(value);
+        } else if (numeric instanceof RubyFloat) {
+            double value = numeric.getDoubleValue();
+            return Double.valueOf(value);
+        } else if (numeric instanceof RubyBignum) {
+            return ((RubyBignum)numeric).getValue();
+        } else if (numeric instanceof RubyBigDecimal) {
+            return ((RubyBigDecimal)numeric).getValue();
+        } else {
+            return NUMERIC_TO_OTHER.coerce(numeric, target);
         }
     };
+    private static final NumericConverter NUMERIC_TO_VOID = (numeric, target) -> null;
     private static boolean isDoubleFloatable(double value) {
         return true;
     }

diff --git a/core/src/main/java/org/jruby/runtime/builtin/IRubyObject.java b/core/src/main/java/org/jruby/runtime/builtin/IRubyObject.java
@@ -275,7 +275,7 @@ public interface IRubyObject {
      *
      * @param type The target type to which the object should be converted.
      */
-    Object toJava(Class type);
+    <T> T toJava(Class<T> type);
 
     /**
      * RubyMethod dup.