Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JDK8 provides its own List#sort, Map#replace, and Map#merge #1249

Closed
headius opened this issue Nov 21, 2013 · 5 comments
Closed

JDK8 provides its own List#sort, Map#replace, and Map#merge #1249

headius opened this issue Nov 21, 2013 · 5 comments

Comments

@headius
Copy link
Member

@headius headius commented Nov 21, 2013

We have been providing monkey-patched List#sort, Map#replace, and Map#merge, but they now conflict with the ones from JDK8. For example, sort:

public void sort(Comparator<? super E> c)

We add a sort to list that just turns a passed block into a Comparator. Because Ruby does not do overloads, only the one-argument version in ArrayList is seen, which causes our tests to fail like so:

  1) Error:
test_list(TestJavaExtension):
ArgumentError: wrong number of arguments (0 for 1)
    test/test_java_extension.rb:66:in `test_list'
headius added a commit that referenced this issue Nov 21, 2013
@headius
Copy link
Member Author

@headius headius commented Nov 21, 2013

This is going to be troublesome to fix.

The problem is that our sort impl lives on the interface proxy for List, but when we bind methods from Java into concrete classes, we always bind the concrete versions. (For reasons I won't go into, we can't have the actual interface methods present on the interface proxy.) This means that even though we define a zero-argument #sort on our List proxy, the one-argument sort on ArrayList (or LinkedList, or any other list) is the one seen by Ruby dispatch.

There's no trivial way around this. One way might be to enlist a form of the old lazy extender logic we used to have for decorating classes without loading them. We'd have an extender associated with List that would be applied to subclasses when they load. However, the unfortunate side effect, even if we can get our version of sort to be visible on ArrayList, is that the original version will then be masked away.

We may have to end up saying this is a Java 8 "feature" we can't do anything about, since our interface patching can't trump concrete methods.

@headius
Copy link
Member Author

@headius headius commented Nov 22, 2013

Expanding this bug to encompass a few other methods I had to patch around in 8783a11:

  • java.util.Map.replace, used for replacing a specific key with a new value. Conflicts with monkey-patched #replace that replaces all contents.
  • java.util.Map.merge, used for merging a given key's value with another value. Conflicts with monkey-patched #merge used for combining two hashes.

These breakages are unfortunate, but so far theres not many.

@colinsurprenant
Copy link
Contributor

@colinsurprenant colinsurprenant commented Oct 21, 2014

stumbled up this problem with java.util.HashMap and java.util.LinkedHashMap on the merge method with Java 8.

A possible workaround is to reopen the concrete Map classes and redefine the merge method like this:

if ENV_JAVA['java.specification.version'] >= '1.8'
  class Java::JavaUtil::HashMap
    def merge(other)
      dup.merge!(other)
    end
  end
end
@mdedetrich
Copy link

@mdedetrich mdedetrich commented Jun 29, 2015

Needed to use the @colinsurprenant workaround for https://github.com/mdedetrich/java-premailer-wrapper and can confirm that for my case it at least works

@enebo enebo modified the milestone: JRuby 9.0.0.0 Jul 14, 2015
@kares
Copy link
Member

@kares kares commented Apr 19, 2016

was thinking about a support solution for users running into these while doing #3802 ... if all methods are native its pretty easy for JRuby to setup different names that would not conflict. users would than alias those when a ruby_ version is preferred in Ruby land e.g. :

java.util.ArrayList.class_eval { alias_method :sort, :ruby_sort }
java.util.HashMap.class_eval { alias merge, ruby_merge; alias replace ruby_replace }

needs to be done for every List/Map impl but that should be understandable, if its too much of an annoyance we can push it further ...

also, not really a Java 8 conflict but still confusing :

jruby-1.7.24 :001 > java.util.ArrayList.new.first
 => nil 
jruby-1.7.24 :002 > java.util.LinkedList.new.first
Java::JavaUtil::NoSuchElementException: 
    from java.util.LinkedList.getFirst(java/util/LinkedList.java:244)
    from java.lang.reflect.Method.invoke(java/lang/reflect/Method.java:498)
    from RUBY.evaluate((irb):2)

... feels like java.util.LinkedList.class_eval { alias_method :first, :ruby_first } would also come handy!

kares added a commit to kares/jruby that referenced this issue Apr 19, 2016
kares added a commit to kares/jruby that referenced this issue Apr 19, 2016
…ava.util.Map

users will be able to do use ruby versions with a simple alias on concrete types
e.g. `java.util.HashMap.class_eval { alias merge ruby_merge }`

resolves jruby#1249
@kares kares closed this in e29e1f6 Apr 27, 2016
@enebo enebo added this to the JRuby 9.1.0.0 milestone May 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.