[GR-33155] Implement #instance_variables and #methods on foreign objects and remove #keys, respond_to?(:keys || :class)

PullRequest: truffleruby/2879

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:
 
@@ -47,6 +48,8 @@ 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.
+* `foreign_object.respond_to?(:class)` is now always true (before it was only for Java classes), since the method is always defined.
 
 # 21.2.0
 

doc/contributor/interop.md

@@ -113,8 +113,6 @@ runtime object instance.
 
 `foreign_object.kind_of?` works like `foreign_object.is_a?`.
 
-`object.respond_to?(:class)` calls `Truffle::Interop.java_class?(object)`.
-
 `object.respond_to?(name)` for other names returns `false`.
 
 `object.__send__(name, *args)` works in the same way as literal method call on

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,10 +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.
 
 Where boolean value is expected (e.g., in `if` conditions) the foreign value is converted to boolean if possible or considered to be true.

spec/truffle/interop/foreign_inspect_to_s_spec.rb

@@ -53,18 +53,18 @@
       end
     end
 
-    describe "Java j.l.Class instance" do
+    describe "Java java.lang.Class instance" do
       it "gives a similar representation to Ruby" do
         foreign = Truffle::Interop.java_type("java.math.BigInteger")[:class]
-        foreign.inspect.should =~ /\A#<Polyglot::ForeignInstantiable\[Java\] java.lang.Class:0x\h+ java.math.BigInteger>\z/
+        foreign.inspect.should =~ /\A#<Polyglot::ForeignInstantiable\[Java\] java.lang.Class:0x\h+ java.math.BigInteger static={...}>\z/
         foreign.to_s.should == "#<Polyglot::ForeignInstantiable[Java] java.math.BigInteger>"
       end
     end
 
     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

@@ -62,18 +62,4 @@
       Truffle::Interop.java_array(1, 2, 3).should.respond_to?(:is_a?)
     end
   end
-
-  describe "for :class" do
-    it "and a Java class returns true" do
-      Truffle::Debug.java_class.should.respond_to?(:class)
-    end
-
-    it "and a Java object returns false" do
-      Truffle::Debug.java_object.should_not.respond_to?(:class)
-    end
-
-    it "and a Java array returns false" do
-      Truffle::Interop.java_array(1, 2, 3).should_not.respond_to?(:class)
-    end
-  end
 end

spec/truffle/interop/respond_to_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

@@ -77,7 +77,6 @@ def self.foreign_inspect_nonrecursive(object)
 
         show_members = true
         if Truffle::Interop.java_class?(object) # a java.lang.Class instance, treat it like a regular object
-          show_members = false
           string << " #{Truffle::Interop.to_display_string(object)}"
         end
         if Truffle::Interop.has_array_elements?(object)
@@ -88,7 +87,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 +130,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)