[GR-33155] [GR-21765] Do not treat foreign_object.class specially and add Polyglot::{HashTrait,IterableTrait,IteratorTrait}

PullRequest: truffleruby/2876

Author: eregon

CHANGELOG.md

@@ -7,6 +7,8 @@ New features:
 * Add `Polyglot::InnerContext` to eval code in any available language in an inner isolated context (#2169).
 * Foreign objects now have a dynamically-generated class based on their interop traits like `ForeignArray` and are better integrated with Ruby objects (#2149).
 * 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).
 
 Bug fixes:
 
@@ -42,6 +44,8 @@ Performance:
 
 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.
 
 # 21.2.0
 

doc/contributor/interop_implicit_api.md

@@ -8,12 +8,15 @@ In the documentation below:
 
 Format: `Ruby code` sends `InteropLibrary message`
 
+- `foreign_object[key]` sends `readHashValue(foreign_object, key)` if `hasHashEntries(foreign_object)`
 - `foreign_object[name]` sends `readMember(foreign_object, name)`
 - `foreign_object[index]` sends `readArrayElement(foreign_object, index)`
+- `foreign_object[key] = value` sends `writeHashEntry(foreign_object, key)` if `hasHashEntries(foreign_object)`
 - `foreign_object[name] = value` sends `writeMember(foreign_object, name, value)`
 - `foreign_object[index] = value` sends `writeArrayElement(foreign_object, index, value)`
 - `foreign_object.name = value` sends `writeMember(foreign_object, name, value)`
 - `foreign_object.name = *arguments` sends `writeMember(foreign_object, name, arguments)`
+- `foreign_object.delete(key)` sends `removeHashEntry(foreign_object, key)` if `hasHashEntries(foreign_object)`
 - `foreign_object.delete(name)` sends `removeMember(foreign_object, name)`
 - `foreign_object.delete(index)` sends `removeArrayElement(foreign_object, index)`
 - `foreign_object.call(*arguments)` sends `execute(foreign_object, *arguments)`
@@ -26,8 +29,6 @@ Format: `Ruby code` sends `InteropLibrary message`
 - `foreign_object.method_name(*arguments)` sends `invokeMember(foreign_object, method_name, *arguments)`
 - `foreign_object.method_name(*arguments, &block)` sends `invokeMember(foreign_object, method_name, *arguments, block)`
 - `foreign_object.new(*arguments)` sends `instantiate(foreign_object, *arguments)`
-- `foreign_object.class` sends `readMember(foreign_object, "class")` when `foreign_object` is a `java.lang.Class`
-- `foreign_object.class` sends `getMetaObject(foreign_object)`
 - `foreign_object.inspect` returns a Ruby-style `#inspect` string showing members, array elements, etc
 - `foreign_object.to_s` sends `asString(foreign_object)` when `isString(foreign_object)` is true
 - `foreign_object.to_s` sends `toDisplayString(foreign_object)` otherwise
@@ -45,10 +46,10 @@ Format: `Ruby code` sends `InteropLibrary message`
 - `foreign_object.__id__` uses `System.identityHashCode()` otherwise (which might not be unique)
 - `foreign_object.hash` sends `identityHashCode(foreign_object)` when `hasIdentity()` is true (which might not be unique)
 - `foreign_object.hash` uses `System.identityHashCode()` otherwise (which might not be unique)
+- `foreign_object.map` and other `Enumerable` methods work for foreign arrays
+- `foreign_object.map` and other `Enumerable` methods work for foreign iterables (`hasIterator()`)
 
 Use `.respond_to?` for calling `InteropLibrary` predicates:
-- `foreign_object.respond_to?(:inspect)` is always true
-- `foreign_object.respond_to?(:to_s)` is always true
 - `foreign_object.respond_to?(:to_str)` sends `isString(foreign_object)`
 - `foreign_object.respond_to?(:to_a)` sends `hasArrayElements(foreign_object)`
 - `foreign_object.respond_to?(:to_ary)` sends `hasArrayElements(foreign_object)`
@@ -58,4 +59,3 @@ Use `.respond_to?` for calling `InteropLibrary` predicates:
 - `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)`
-- `foreign_object.respond_to?(:is_a?)` is always true

doc/user/jruby-migration.md

@@ -354,7 +354,7 @@ frame.setVisible(true)
 ```
 
 Instead of using Ruby metaprogramming to simulate a Java package name, we explicitly import classes.
-`Java.import` is similar to JRuby's `java_import`, and does `::ClassName = Java.type('package.ClassName')`.
+`Java.import` is similar to JRuby's `java_import`, and does `ClassName = Java.type('package.ClassName')`.
 
 Constants are read by reading properties of the class rather than using Ruby notation.
 
@@ -367,7 +367,7 @@ This is only available in GraalVM - not in the standalone distribution installed
 
 In JRuby, Java classes can either be referenced in the `Java` module, such as `Java::ComFoo::Bar`, or if they have a common TLD they can be referenced as `com.foo.Bar`. `java_import com.foo.Bar` will define `Bar` as a top-level constant.
 
-In TruffleRuby, Java classes are referred to using either `Java.type('com.foo.Bar')`, which you would then normally assign to a constant, or you can use `Java.import 'com.foo.Bar'` to have `Bar` defined as a top-level constant.
+In TruffleRuby, Java classes are referred to using either `Java.type('com.foo.Bar')`, which you would then normally assign to a constant, or you can use `Java.import 'com.foo.Bar'` to have `Bar` defined in the enclosing module.
 
 ### Wildcard Package Imports
 

doc/user/polyglot.md

@@ -102,8 +102,6 @@ If you want to pass a Ruby object to another language for fields to be read and
 
 `object.new(*args)` will create a new object from the foreign object (as if it is some kind of class).
 
-`object.class` on a Java `Class` object will give you an object on which you can call instance methods, rather than static methods.
-
 `object.respond_to?(:size)` will tell you if the foreign object has a size or length.
 
 `object.nil?` will tell you if the foreign object represents the language's equivalent of `null` or `nil`.
@@ -128,11 +126,13 @@ It is easier to use Java interoperability in JVM mode (`--jvm`). Java interopera
 See [here](https://www.graalvm.org/reference-manual/embed-languages/#build-native-images-from-polyglot-applications)
 for more details.
 
-`Java.type('name')` returns a Java class object, given a name such as `java.lang.Integer` or `int[]`. With the class object, `.new` will create an instance, `.foo` will call the static method `foo`, `[:FOO]` will read the field
+`Java.type('name')` returns a Java type, given a name such as `java.lang.Integer` or `int[]`.
+With the type object, `.new` will create an instance, `.foo` will call the static method `foo`, `[:FOO]` will read the static field
 `FOO`, and so on.
-To access instance methods use `.class`, such as `MyClass.class.getName`.
+To access methods of the `java.lang.Class` instance, use `[:class]`, such as `MyClass[:class].getName`.
+You can also go from the `java.lang.Class` instance to the Java type by using `[:static]`.
 
-To import a Java class as a top-level constant, use `Java.import 'name'`.
+To import a Java class in the enclosing module, use `MyClass = Java.type 'java.lang.MyClass'` or `Java.import 'java.lang.MyClass'`.
 
 ## Embedding in Java
 

spec/tags/truffle/interop/java_import_tags.txt

@@ -40,15 +40,23 @@
         foreign = Truffle::Interop.to_java_map(hash)
 
         hash.to_s.should == '{:a=>1, :b=>2, :c=>3}' # for comparison
-        foreign.inspect.should =~ /\A#<Polyglot::ForeignObject\[Java\] java\.util\.HashMap:0x\h+ {"a"=>1, "b"=>2, "c"=>3}>\z/
-        foreign.to_s.should == "#<Polyglot::ForeignObject[Java] {a=1, b=2, c=3}>"
+        foreign.inspect.should =~ /\A#<Polyglot::ForeignHash\[Java\] java\.util\.HashMap:0x\h+ {"a"=>1, "b"=>2, "c"=>3}>\z/
+        foreign.to_s.should == "#<Polyglot::ForeignHash[Java] {a=1, b=2, c=3}>"
       end
     end
 
-    describe "Java class" do
+    describe "Java type" do
       it "gives a similar representation to Ruby" do
         foreign = Truffle::Interop.java_type("java.math.BigInteger")
-        foreign.inspect.should == "#<Polyglot::ForeignInstantiable[Java] class java.math.BigInteger>"
+        foreign.inspect.should == "#<Polyglot::ForeignInstantiable[Java] type java.math.BigInteger>"
+        foreign.to_s.should == "#<Polyglot::ForeignInstantiable[Java] type java.math.BigInteger>"
+      end
+    end
+
+    describe "Java j.l.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.to_s.should == "#<Polyglot::ForeignInstantiable[Java] java.math.BigInteger>"
       end
     end
@@ -64,8 +72,7 @@
 
   describe "recursive array" do
     it "gives a similar representation to Ruby" do
-      x = [1, 2, 3]
-      foreign = Truffle::Debug.foreign_array_from_java(Truffle::Interop.to_java_array(x))
+      foreign = Truffle::Debug.foreign_array
       foreign[0] = foreign
       foreign.inspect.should =~ /\A#<Polyglot::ForeignArray:0x\h+ \[\[...\], 2, 3\]>\z/
       foreign.to_s.should == "#<Polyglot::ForeignArray [foreign array]>"
@@ -96,37 +103,42 @@
     end
   end
 
-  guard -> { !TruffleRuby.native? } do
-    describe "array" do
-      it "gives a similar representation to Ruby" do
-        foreign = Truffle::Debug.foreign_array_from_java(Truffle::Interop.to_java_array([1, 2, 3]))
-        foreign.inspect.should =~ /\A#<Polyglot::ForeignArray:0x\h+ \[1, 2, 3\]>\z/
-        foreign.to_s.should == "#<Polyglot::ForeignArray [foreign array]>"
-      end
+  describe "array" do
+    it "gives a similar representation to Ruby" do
+      foreign = Truffle::Debug.foreign_array
+      foreign.inspect.should =~ /\A#<Polyglot::ForeignArray:0x\h+ \[1, 2, 3\]>\z/
+      foreign.to_s.should == "#<Polyglot::ForeignArray [foreign array]>"
+    end
 
-      it "gives a similar representation to Ruby, even if it is also a pointer" do
-        foreign = Truffle::Debug.foreign_pointer_array_from_java(Truffle::Interop.to_java_array([1, 2, 3]))
-        foreign.inspect.should =~ /\A#<Polyglot::ForeignArrayPointer 0x0 \[1, 2, 3\]>\z/
-        foreign.to_s.should == "#<Polyglot::ForeignArrayPointer [foreign pointer array]>"
-      end
+    it "gives a similar representation to Ruby, even if it is also a pointer" do
+      foreign = Truffle::Debug.foreign_pointer_array
+      foreign.inspect.should =~ /\A#<Polyglot::ForeignArrayPointer 0x0 \[1, 2, 3\]>\z/
+      foreign.to_s.should == "#<Polyglot::ForeignArrayPointer [foreign pointer array]>"
     end
   end
 
-  guard -> { !TruffleRuby.native? } do
-    describe "object without members" do
-      it "gives a similar representation to Ruby" do
-        foreign = Truffle::Debug.foreign_object
-        foreign.inspect.should =~ /\A#<Polyglot::ForeignObject:0x\h+>\z/
-        foreign.to_s.should == "#<Polyglot::ForeignObject [foreign object]>"
-      end
+  describe "hash" do
+    it "gives a similar representation to Ruby" do
+      foreign = Truffle::Debug.foreign_hash
+      { a: 1, b: 2 }.inspect.should == "{:a=>1, :b=>2}"
+      foreign.inspect.should =~ /\A#<Polyglot::ForeignHash:0x\h+ {:a=>1, :b=>2}>\z/
+      foreign.to_s.should == "#<Polyglot::ForeignHash [foreign hash]>"
     end
+  end
 
-    describe "object with members" do
-      it "gives a similar representation to Ruby" do
-        foreign = Truffle::Debug.foreign_object_from_map(Truffle::Interop.to_java_map({a: 1, b: 2, c: 3}))
-        foreign.inspect.should =~ /\A#<Polyglot::ForeignObject:0x\h+ a=1, b=2, c=3>\z/
-        foreign.to_s.should == "#<Polyglot::ForeignObject [foreign object with members]>"
-      end
+  describe "object without members" do
+    it "gives a similar representation to Ruby" do
+      foreign = Truffle::Debug.foreign_object
+      foreign.inspect.should =~ /\A#<Polyglot::ForeignObject:0x\h+>\z/
+      foreign.to_s.should == "#<Polyglot::ForeignObject [foreign object]>"
+    end
+  end
+
+  describe "object with members" do
+    it "gives a similar representation to Ruby" do
+      foreign = Truffle::Debug.foreign_object_with_members
+      foreign.inspect.should =~ /\A#<Polyglot::ForeignObject:0x\h+ a=1, b=2, c=3>\z/
+      foreign.to_s.should == "#<Polyglot::ForeignObject [foreign object with members]>"
     end
   end
 end

spec/truffle/interop/foreign_inspect_to_s_spec.rb

@@ -10,38 +10,37 @@
 
 guard -> { !TruffleRuby.native? } do
   describe "Java.import" do
-
-    it "imports a class to the top-level" do
-      ruby_exe(%{
+    it "imports a class under the enclosing module" do
+      module TruffleJavaImportSpecs1
         Java.import 'java.math.BigInteger'
-        puts BigInteger.new('1234').toString
-      }).should == "1234\n"
+        BigInteger.new('1234').toString.should == "1234"
+      end
+
+      TruffleJavaImportSpecs1.should.const_defined?('BigInteger', false)
+      Object.should_not.const_defined?('BigInteger')
     end
 
     it "returns the imported class" do
-      ruby_exe(%{
-        puts Java.import('java.math.BigInteger').class.getName
-      }).should == "java.math.BigInteger\n"
+      module TruffleJavaImportSpecs2
+        Java.import('java.math.BigInteger')[:class].getName.should == "java.math.BigInteger"
+      end
     end
 
     it "can import classes twice" do
-      ruby_exe(%{
+      module TruffleJavaImportSpecs3
         Java.import 'java.math.BigInteger'
         Java.import 'java.math.BigInteger'
-        puts BigInteger.new('1234').toString
-      }).should == "1234\n"
+        BigInteger.new('1234').toString.should == "1234"
+      end
     end
 
     it "raises an error if the constant is already defined" do
-      ruby_exe(%{
+      module TruffleJavaImportSpecs4
         BigInteger = '14'
-        begin
+        -> {
           Java.import 'java.math.BigInteger'
-        rescue NameError => e
-          puts e
-        end
-      }).should == "constant BigInteger already set\n"
+        }.should raise_error(NameError, "constant BigInteger already set")
+      end
     end
-
   end
 end

spec/truffle/interop/java_import_spec.rb

@@ -13,27 +13,27 @@
   describe "Truffle::Interop.java_type" do
 
     it "returns a Java class for a known primitive name" do
-      Truffle::Interop.java_type("int").class.getName.should == "int"
+      Truffle::Interop.java_type("int")[:class].getName.should == "int"
     end
 
     it "returns a Java class for known primitive name as an array" do
-      Truffle::Interop.java_type("int[]").class.getName.should == "[I"
+      Truffle::Interop.java_type("int[]")[:class].getName.should == "[I"
     end
 
     it "returns a Java class for a known class name " do
-      Truffle::Interop.java_type("java.math.BigInteger").class.getName.should == "java.math.BigInteger"
+      Truffle::Interop.java_type("java.math.BigInteger")[:class].getName.should == "java.math.BigInteger"
     end
 
     it "returns a Java class for known class name as an array" do
-      Truffle::Interop.java_type("java.math.BigInteger[]").class.getName.should == "[Ljava.math.BigInteger;"
+      Truffle::Interop.java_type("java.math.BigInteger[]")[:class].getName.should == "[Ljava.math.BigInteger;"
     end
 
     it "throws RuntimeError for unknown class names" do
       -> { Truffle::Interop.java_type("does.not.Exist") }.should raise_error(RuntimeError)
     end
 
     it "works with symbols" do
-      Truffle::Interop.java_type(:int).class.getName.should == "int"
+      Truffle::Interop.java_type(:int)[:class].getName.should == "int"
     end
 
   end

spec/truffle/interop/java_type_spec.rb

@@ -541,8 +541,7 @@ def array_element_predicate(message, predicate, insert_on_true_case)
               end,
               Test.new("fails with StopIterationException if the hash is empty", :hash, :polyglot_hash) do |subject|
                 iterator = Truffle::Interop.hash_entries_iterator(subject)
-                # TODO introduce StopIterationException in Ruby
-                -> { Truffle::Interop.iterator_next_element(iterator) }.should raise_error(Exception)
+                -> { Truffle::Interop.iterator_next_element(iterator) }.should raise_error(StopIteration)
               end,
               unsupported_test { |subject| Truffle::Interop.hash_entries_iterator(subject) }],
       Message[:getHashKeysIterator,
@@ -558,8 +557,7 @@ def array_element_predicate(message, predicate, insert_on_true_case)
               end,
               Test.new("fails with StopIterationException if the hash is empty", :hash, :polyglot_hash) do |subject|
                 iterator = Truffle::Interop.hash_keys_iterator(subject)
-                # TODO introduce StopIterationException in Ruby
-                -> { Truffle::Interop.iterator_next_element(iterator) }.should raise_error(Exception)
+                -> { Truffle::Interop.iterator_next_element(iterator) }.should raise_error(StopIteration)
               end,
               unsupported_test { |subject| Truffle::Interop.hash_entries_iterator(subject) }],
       Message[:getHashValuesIterator,
@@ -575,8 +573,7 @@ def array_element_predicate(message, predicate, insert_on_true_case)
               end,
               Test.new("fails with StopIterationException if the hash is empty", :hash, :polyglot_hash) do |subject|
                 iterator = Truffle::Interop.hash_values_iterator(subject)
-                # TODO introduce StopIterationException in Ruby
-                -> { Truffle::Interop.iterator_next_element(iterator) }.should raise_error(Exception)
+                -> { Truffle::Interop.iterator_next_element(iterator) }.should raise_error(StopIteration)
               end,
               unsupported_test { |subject| Truffle::Interop.hash_entries_iterator(subject) }],