Merge pull request #5181 from deprecate-subclasses

deprecate jruby/core_ext Class#subclasses
jruby · May 23, 2018 · f574f9d · f574f9d
2 parents 389eea3 + 5f45b43
commit f574f9d
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 31 deletions.
diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java
@@ -936,6 +936,16 @@ public void addModule(RubyModule module) {
             allModules.add(module);
         }
     }
+
+    public void eachModule(Consumer<RubyModule> func) {
+        synchronized (allModules) {
+            for (RubyModule module : allModules) {
+                func.accept(module);
+            }
+        }
+    }
+
+    @Deprecated
     public void eachModule(Function1<Object, IRubyObject> func) {
         synchronized (allModules) {
             for (RubyModule module : allModules) {

diff --git a/core/src/main/java/org/jruby/RubyObjectSpace.java b/core/src/main/java/org/jruby/RubyObjectSpace.java
@@ -45,7 +45,6 @@
 import static org.jruby.runtime.Visibility.*;
 import org.jruby.runtime.builtin.IRubyObject;
 import org.jruby.util.collections.WeakValuedIdentityMap;
-import org.jruby.util.func.Function1;
 
 @JRubyModule(name="ObjectSpace")
 public class RubyObjectSpace {
@@ -129,17 +128,13 @@ public static IRubyObject each_objectInternal(final ThreadContext context, IRuby
         if (rubyClass == runtime.getClassClass() || rubyClass == runtime.getModule()) {
 
             final ArrayList<IRubyObject> modules = new ArrayList<>(96);
-            runtime.eachModule(new Function1<Object, IRubyObject>() {
-                public Object apply(IRubyObject arg1) {
-                    if (rubyClass.isInstance(arg1)) {
-                        if (arg1 instanceof IncludedModule) {
+            runtime.eachModule((module) -> {
+                    if (rubyClass.isInstance(module)) {
+                        if (!(module instanceof IncludedModule)) {
                             // do nothing for included wrappers or singleton classes
-                        } else {
-                            modules.add(arg1); // store the module to avoid concurrent modification exceptions
+                            modules.add(module); // store the module to avoid concurrent modification exceptions
                         }
                     }
-                    return null;
-                }
             });
 
             final int count = modules.size();
@@ -155,9 +150,8 @@ public Object apply(IRubyObject arg1) {
             block.yield(context, attached); int count = 1;
             if (attached instanceof RubyClass) {
                 for (RubyClass child : ((RubyClass) attached).subclasses(true)) {
-                    if (child instanceof IncludedModule) {
+                    if (!(child instanceof IncludedModule)) {
                         // do nothing for included wrappers or singleton classes
-                    } else {
                         count++; block.yield(context, child);
                     }
                 }

diff --git a/core/src/main/java/org/jruby/ext/jruby/JRubyLibrary.java b/core/src/main/java/org/jruby/ext/jruby/JRubyLibrary.java
@@ -37,21 +37,18 @@
 import org.jruby.anno.JRubyModule;
 import org.jruby.ast.Node;
 import org.jruby.ast.RootNode;
+import org.jruby.ast.util.ArgsUtil;
 import org.jruby.ir.IRBuilder;
 import org.jruby.ir.IRManager;
 import org.jruby.ir.IRScriptBody;
 import org.jruby.ir.targets.JVMVisitor;
 import org.jruby.ir.targets.JVMVisitorMethodContext;
 import org.jruby.javasupport.Java;
 import org.jruby.javasupport.JavaUtil;
-import org.jruby.runtime.Block;
-import org.jruby.runtime.DynamicScope;
-import org.jruby.runtime.ThreadContext;
-import org.jruby.runtime.Visibility;
+import org.jruby.runtime.*;
 import org.jruby.runtime.builtin.IRubyObject;
 import org.jruby.runtime.load.Library;
 import org.jruby.util.ByteList;
-import org.jruby.util.ClasspathLauncher;
 
 import java.io.ByteArrayInputStream;
 
@@ -307,4 +304,52 @@ public static IRubyObject load_string_ext(ThreadContext context, IRubyObject rec
         return context.nil;
     }
 
+    @JRubyMethod(module = true)
+    public static IRubyObject subclasses(ThreadContext context, IRubyObject recv, IRubyObject arg) {
+        return subclasses(context, recv, arg instanceof RubyClass ? (RubyClass) arg : arg.getMetaClass(), false);
+    }
+
+    @JRubyMethod(module = true)
+    public static IRubyObject subclasses(ThreadContext context, IRubyObject recv, IRubyObject arg, IRubyObject opts) {
+        boolean recurseAll = false;
+        opts = ArgsUtil.getOptionsArg(context.runtime, opts);
+        if (opts != context.nil) {
+            IRubyObject all = ((RubyHash) opts).fastARef(context.runtime.newSymbol("all"));
+            if (all != null) recurseAll = all.isTrue();
+        }
+        return subclasses(context, recv, arg instanceof RubyClass ? (RubyClass) arg : arg.getMetaClass(), recurseAll);
+    }
+
+    private static RubyArray subclasses(ThreadContext context, final IRubyObject recv,
+                                          final RubyClass klass, final boolean recurseAll) {
+
+        final RubyArray subclasses = RubyArray.newArray(context.runtime);
+
+        RubyClass singletonClass = ((RubyClass) klass).getSingletonClass();
+        RubyObjectSpace.each_objectInternal(context, recv, new IRubyObject[] { singletonClass },
+                new Block(new JavaInternalBlockBody(context.runtime, Signature.ONE_ARGUMENT) {
+
+                    @Override
+                    public IRubyObject yield(ThreadContext context, IRubyObject[] args) {
+                        return doYield(context, null, args[0]);
+                    }
+
+                    @Override
+                    protected IRubyObject doYield(ThreadContext context, Block block, IRubyObject value) {
+                        if (klass != value) {
+                            if (recurseAll) {
+                                return subclasses.append(value);
+                            }
+                            if (((RubyClass) value).superclass(context) == klass) {
+                                return subclasses.append(value);
+                            }
+                        }
+                        return context.nil;
+                    }
+
+                })
+        );
+        return subclasses;
+    }
+
 }
diff --git a/core/src/main/java/org/jruby/util/func/Function1.java b/core/src/main/java/org/jruby/util/func/Function1.java
@@ -29,6 +29,5 @@
 
 package org.jruby.util.func;
 
-public interface Function1<R,A0> {
-    public R apply(A0 a0);
-}
+@Deprecated
+public interface Function1<R, A0> extends java.util.function.Function<A0, R> {}
diff --git a/core/src/main/ruby/jruby/jruby.rb b/core/src/main/ruby/jruby/jruby.rb
@@ -38,6 +38,11 @@ def compile_ir(content = nil, filename = DEFAULT_FILENAME, extra_position_info =
     # @note implemented in *org.jruby.ext.jruby.JRubyLibrary*
     def compile(content, filename = '', extra_position_info = false); end if false
 
+    # Get all known subclasses of passed class.
+    # If recurse: true, include all (non-direct) descendants recursively.
+    # @return Enumerable[Class]
+    def subclasses_of(klass, recurse: false) end if false
+
   end
 
   # NOTE: This is not a public API and is subject to change at our whim.

diff --git a/lib/ruby/stdlib/jruby/core_ext/class.rb b/lib/ruby/stdlib/jruby/core_ext/class.rb
@@ -27,19 +27,10 @@ class Class
   private_constant :JClass
 
   ##
-  # Get an array of all known subclasses of this class. If recursive == true,
-  # include all descendants.
+  # @deprecated since JRuby 9.2, use `JRuby.subclasses_of(klass)`
   def subclasses(recursive = false)
-    subclasses = []
-    ObjectSpace.each_object(singleton_class) do |klass|
-      next if klass.equal? self
-      if recursive
-        subclasses << klass
-      else
-        subclasses << klass if klass.superclass.equal? self
-      end
-    end
-    subclasses
+    warn("klass.subclasses is deprecated, use JRuby.subclasses(klass) instead", uplevel: 1)
+    JRuby.subclasses(self, all: recursive)
   end
 
   ##