Implement #instance_variables and #methods on foreign objects and remove the old #keys

Author: eregon

CHANGELOG.md

@@ -9,6 +9,7 @@ New features:
 * Foreign arrays now have all methods of Ruby `Enumerable` and many methods of `Array` (#2149).
 * Foreign hashes now have all methods of Ruby `Enumerable` and many methods of `Hash` (#2149).
 * Foreign iterables (`InteropLibrary#hasIterator`) now have all methods of Ruby `Enumerable` (#2149).
+* Foreign objects now implement `#instance_variables` (readable non-invocable members) and `#methods` (invocable members + Ruby methods).
 
 Bug fixes:
 
@@ -46,6 +47,7 @@ Changes:
 
 * `foreign_object.class` on foreign objects is no longer special and uses `Kernel#class` (it used to return the `java.lang.Class` object for a Java type or `getMetaObject()`, but that is too incompatible with Ruby code).
 * `Java.import name` imports a Java class in the enclosing module instead of always as a top-level constant.
+* `foreign_object.keys` no longer returns members, use `foreign_object.instance_variables` or `foreign_object.methods` instead.
 
 # 21.2.0
 

doc/contributor/interop_implicit_api.md

@@ -22,7 +22,8 @@ Format: `Ruby code` sends `InteropLibrary message`
 - `foreign_object.call(*arguments)` sends `execute(foreign_object, *arguments)`
 - `foreign_object.nil?` sends `isNull(foreign_object)`
 - `foreign_object.size` sends `getArraySize(foreign_object)`
-- `foreign_object.keys` sends `getMembers(foreign_object)`
+- `foreign_object.instance_variables` sends `getMembers(foreign_object)` and returns readable non-invocable members
+- `foreign_object.methods` sends `getMembers(foreign_object)` and returns invocable members merged with available Ruby methods
 - `foreign_object.method_name` sends `invokeMember(foreign_object, method_name)` if member is invocable
 - `foreign_object.method_name` sends `readMember(foreign_object, method_name)` if member is readable but not invocable
 - `foreign_object.method_name` sends `readMember(foreign_object, method_name)` and raises if member is neither invocable nor readable
@@ -56,6 +57,5 @@ Use `.respond_to?` for calling `InteropLibrary` predicates:
 - `foreign_object.respond_to?(:to_f)` sends `fitsInDouble()`
 - `foreign_object.respond_to?(:to_i)` sends `fitsInLong()`
 - `foreign_object.respond_to?(:size)` sends `hasArrayElements(foreign_object)`
-- `foreign_object.respond_to?(:keys)` sends `hasMembers(foreign_object)`
 - `foreign_object.respond_to?(:call)` sends `isExecutable(foreign_object)`
 - `foreign_object.respond_to?(:new)` sends `isInstantiable(foreign_object)`

doc/user/polyglot.md

@@ -110,8 +110,6 @@ If you want to pass a Ruby object to another language for fields to be read and
 
 `object.respond_to?(:new)` will tell you if a foreign object can be used to create a new object (if it's a class).
 
-`object.respond_to?(:keys)` will tell you if a foreign object can give you a list of members.
-
 `object.respond_to?(:class)` will tell you if an object is a Java class.
 
 `Polyglot.as_enumerable(object)` will create a Ruby `Enumerable` from the foreign object, using its size or length, and reading from it.

spec/truffle/interop/foreign_inspect_to_s_spec.rb

@@ -64,7 +64,7 @@
     describe "Java object" do
       it "gives a similar representation to Ruby" do
         foreign = Truffle::Interop.java_type("java.math.BigInteger").new('14')
-        foreign.inspect.should =~ /\A#<Polyglot::ForeignObject\[Java\] java\.math\.BigInteger:0x\h+ .+>\z/
+        foreign.inspect.should =~ /\A#<Polyglot::ForeignObject\[Java\] java\.math\.BigInteger:0x\h+>\z/
         foreign.to_s.should == "#<Polyglot::ForeignObject[Java] 14>"
       end
     end

spec/truffle/interop/keys_spec.rb renamed to spec/truffle/interop/members_spec.rb

@@ -169,10 +169,18 @@
     -> { pfo.size }.should raise_error(NameError)
   end
 
-  it description['.keys', :getMembers] do
-    pfo, _, l = proxy[Object.new]
-    pfo.keys
+  it description['.instance_variables', :getMembers, [], 'and returns readable non-invocable members'] do
+    pfo, _, l = proxy[Truffle::Debug.foreign_object_with_members]
+    pfo.instance_variables.should == [:a, :b, :c]
+    l.log.should include(['getMembers', false])
+  end
+
+  it description['.methods', :getMembers, [], 'and returns invocable members merged with available Ruby methods'] do
+    pfo, _, l = proxy[Truffle::Debug.foreign_object_with_members]
+    methods = pfo.methods
     l.log.should include(['getMembers', false])
+    methods.last(2).should == [:method1, :method2]
+    methods.should include(*Object.public_instance_methods)
   end
 
   it description['.method_name', :invokeMember, ['method_name'], 'if member is invocable'] do
@@ -407,12 +415,6 @@
     l.log.should include(["hasArrayElements"])
   end
 
-  it description['.respond_to?(:keys)', :hasMembers] do
-    pfo, _, l = proxy[Object.new]
-    pfo.respond_to?(:keys)
-    l.log.should include(["hasMembers"])
-  end
-
   it description['.respond_to?(:call)', :isExecutable] do
     pfo, _, l = proxy[Object.new]
     pfo.respond_to?(:call)

spec/truffle/interop/special_forms_spec.rb

@@ -22,8 +22,11 @@
 import com.oracle.truffle.api.frame.Frame;
 import com.oracle.truffle.api.frame.FrameInstance;
 import com.oracle.truffle.api.frame.MaterializedFrame;
+import com.oracle.truffle.api.interop.ArityException;
 import com.oracle.truffle.api.interop.StopIterationException;
 import com.oracle.truffle.api.interop.UnknownKeyException;
+import com.oracle.truffle.api.interop.UnsupportedMessageException;
+import com.oracle.truffle.api.interop.UnsupportedTypeException;
 import com.oracle.truffle.api.nodes.RootNode;
 import com.oracle.truffle.api.object.DynamicObjectLibrary;
 import com.oracle.truffle.api.source.SourceSection;
@@ -596,25 +599,49 @@ protected boolean hasMembers() {
             @ExportMessage
             @TruffleBoundary
             protected Object getMembers(boolean includeInternal) {
-                return new ForeignArrayNode.ForeignArray("a", "b", "c");
+                return new ForeignArrayNode.ForeignArray("a", "b", "c", "method1", "method2");
             }
 
             @TruffleBoundary
             @ExportMessage
             protected boolean isMemberReadable(String member) {
-                return map.containsKey(member);
+                return map.containsKey(member) || isMemberInvocable(member);
             }
 
             @TruffleBoundary
             @ExportMessage
-            protected Object readMember(String key) throws UnknownIdentifierException {
-                final Object value = map.get(key);
+            protected boolean isMemberInvocable(String member) {
+                return "method1".equals(member) || "method2".equals(member);
+            }
+
+            @TruffleBoundary
+            @ExportMessage
+            protected Object readMember(String member) throws UnknownIdentifierException {
+                if (member.equals("method1")) {
+                    return new ForeignExecutableNode.ForeignExecutable(42);
+                } else if (member.equals("method2")) {
+                    return new ForeignExecutableNode.ForeignExecutable(44);
+                }
+
+                final Object value = map.get(member);
                 if (value == null) {
-                    throw UnknownIdentifierException.create(key);
+                    throw UnknownIdentifierException.create(member);
                 }
                 return value;
             }
 
+            @TruffleBoundary
+            @ExportMessage
+            protected Object invokeMember(String member, Object[] arguments) throws UnsupportedMessageException,
+                    ArityException, UnknownIdentifierException, UnsupportedTypeException {
+                if (!isMemberInvocable(member)) {
+                    throw UnknownIdentifierException.create(member);
+                }
+
+                return InteropLibrary.getUncached().execute(readMember("method1"));
+            }
+
+
             @ExportMessage
             protected String toDisplayString(boolean allowSideEffects) {
                 return "[foreign object with members]";

src/main/java/org/truffleruby/debug/TruffleDebugNodes.java

@@ -88,7 +88,7 @@ def self.foreign_inspect_nonrecursive(object)
           show_members = false
           string << " {#{object.map { |k, v| "#{basic_inspect_for k}=>#{basic_inspect_for v}" }.join(', ')}}"
         end
-        if show_members && Truffle::Interop.has_members?(object)
+        if show_members
           pairs = pairs_from_object(object)
           unless pairs.empty?
             string << " #{pairs.map { |k, v| "#{k}=#{basic_inspect_for v}" }.join(', ')}"
@@ -131,8 +131,7 @@ def self.java_type?(object)
     end
 
     def self.pairs_from_object(object)
-      readable_members = Truffle::Interop.members(object).select { |member| Truffle::Interop.member_readable?(object, member) }
-      readable_members.map { |key| [key, object[key]] }
+      object.instance_variables.map { |key| [key, Truffle::Interop.read_member(object, key)] }
     end
 
     def self.ruby_class_and_language(object)

src/main/ruby/truffleruby/core/truffle/interop_operations.rb

@@ -344,8 +344,6 @@ def inspect
   class ForeignObject < Object
     def respond_to?(name, include_all = false)
       case symbol = name.to_sym
-      when :keys
-        Truffle::Interop.has_members?(self)
       when :class
         Truffle::Interop.java_class?(self)
       else
@@ -389,8 +387,25 @@ def is_a?(klass)
     end
     alias_method :kind_of?, :is_a?
 
-    def keys
-      Truffle::Interop.members(self)
+    def instance_variables
+      return [] unless Truffle::Interop.has_members?(self)
+      Truffle::Interop.members_without_conversion(self).filter_map do |member|
+        # Ruby does not have the concept of non-readable members, ignore those
+        if Truffle::Interop.member_readable?(self, member) &&
+            !Truffle::Interop.member_invocable?(self, member)
+          member.to_s.to_sym
+        end
+      end
+    end
+
+    def methods(regular = true)
+      if regular
+        super() | Truffle::Interop.members_without_conversion(self).filter_map do |member|
+          member.to_s.to_sym if Truffle::Interop.member_invocable?(self, member)
+        end
+      else
+        super(regular)
+      end
     end
 
     def [](member)