Merge pull request #7238 from kares/map-key-check

enebo · web-flow · commit fe30f82f7f9f · 2022-06-21T14:46:26.000-05:00
[ji] proper key? checking semantics for Map
diff --git a/core/src/main/java/org/jruby/RubyDir.java b/core/src/main/java/org/jruby/RubyDir.java
@@ -1106,7 +1106,7 @@ public static Optional<String> getHomeFromEnv(Ruby runtime) {
         final RubyString homeKey = RubyString.newStringShared(runtime, HOME);
         final RubyHash env = runtime.getENV();
 
-        if (env.has_key_p(homeKey).isFalse()) {
+        if (!env.hasKey(homeKey)) {
             return Optional.empty();
         } else {
             return Optional.of(env.op_aref(runtime.getCurrentContext(), homeKey).toString());
diff --git a/core/src/main/java/org/jruby/RubyHash.java b/core/src/main/java/org/jruby/RubyHash.java
@@ -1384,12 +1384,21 @@ public IRubyObject fetch(ThreadContext context, IRubyObject key, IRubyObject _de
      */
     @JRubyMethod(name = {"has_key?", "key?", "include?", "member?"}, required = 1)
     public RubyBoolean has_key_p(ThreadContext context, IRubyObject key) {
-        return internalGetEntry(key) == NO_ENTRY ? context.fals : context.tru;
+        return hasKey(key) ? context.tru : context.fals;
     }
 
+    @Deprecated
     public RubyBoolean has_key_p(IRubyObject key) {
-        Ruby runtime = metaClass.runtime;
-        return internalGetEntry(key) == NO_ENTRY ? runtime.getFalse() : runtime.getTrue();
+        return has_key_p(metaClass.runtime.getCurrentContext(), key);
+    }
+
+    /**
+     * A Java API to test the presence of a (Ruby) key in the Hash
+     * @param key the native (Ruby) key
+     * @return true if the hash contains the provided key
+     */
+    public boolean hasKey(IRubyObject key) {
+        return internalGetEntry(key) != NO_ENTRY;
     }
 
     private static class Found extends RuntimeException {
diff --git a/core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java b/core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java
@@ -259,15 +259,24 @@ public <T> void visitAll(ThreadContext context, VisitorWithState visitor, T stat
         }
 
         @Override
+        public boolean hasKey(IRubyObject key) {
+            return mapDelegate().containsKey(key);
+        }
+
+        @Override
+        public RubyBoolean has_key_p(ThreadContext context, IRubyObject key) {
+            return mapDelegate().containsKey(key.toJava(Object.class)) ? context.tru : context.fals;
+        }
+
+        @Override
+        @Deprecated
         public RubyBoolean has_key_p(IRubyObject key) {
-            final Object convertedKey = key.toJava(Object.class);
-            return getRuntime().newBoolean( mapDelegate().containsKey(convertedKey) );
+            return has_key_p(getRuntime().getCurrentContext(), key);
         }
 
         @Override
         public RubyBoolean has_value_p(ThreadContext context, IRubyObject val) {
-            final Object convertedVal = val.toJava(Object.class);
-            return getRuntime().newBoolean( mapDelegate().containsValue(convertedVal) );
+            return mapDelegate().containsValue(val.toJava(Object.class)) ? context.tru : context.fals;
         }
 
         @Override
diff --git a/spec/java_integration/types/map_spec.rb b/spec/java_integration/types/map_spec.rb
@@ -77,6 +77,22 @@
     expect( m.empty? ).to be true
   end
 
+  it "properly handles key? when key included in the map" do
+    map = java.util.HashMap.new
+    map[:foo] = 'bar'
+    map[:baz] = nil
+    expect( map.key?(:foo) ).to be true
+    expect( map.key?(:baz) ).to be true
+    expect( map.key?(:bar) ).to be false
+    expect( map.key?('bar') ).to be false
+
+    map = java.util.LinkedHashMap.new 'foo' => nil
+    expect( map.include?('foo') ).to be true
+    expect( map.has_key?('foo') ).to be true
+    expect( map.member?('foo') ).to be true
+    expect( map.include?('bar') ).to be false
+  end
+
   it "supports Hash-like operations" do
     h = java.util.HashMap.new
     test_ok(h.kind_of? java.util.Map)
