Skip to content

Commit e4ceeee

Browse files
headiusheadius
committed
Tweaks to align java_import with other class-level methods
`java_import` has been defined on Object for many years, but after all that time it seems it really should have been defined on Module and toplevel only, similar to the visibility methods and `define_method`. In addition, it uses unnecessary evals to query and assign the constant, and if called against a non-Module it always defines the resulting constant on Object. This leads to peculiar behavior as reported in #6115. This change does the following: * Deprecate and warn for java_import called against arbitrary non-Module objects. * Move java_import to Module and define a top-level version that invokes it against Object. * Eliminate the eval and use const* methods to assign the value. This should be a bit faster (no evals) and provide a path toward removing Object#java_import. All spec:ji passes without any warnings, so at least we are not using Object#java_import there.
1 parent 3fc2993 commit e4ceeee

File tree

2 files changed

+72
-66
lines changed

2 files changed

+72
-66
lines changed

‎core/src/main/ruby/jruby/java/core_ext/module.rb‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,70 @@
22
class Module
33
private
44

5+
def java_import(*import_classes)
6+
import_classes = import_classes.each_with_object([]) do |classes, flattened|
7+
if classes.is_a?(Array)
8+
flattened.push(*classes)
9+
else
10+
flattened.push(classes)
11+
end
12+
end
13+
14+
import_classes.map do |import_class|
15+
case import_class
16+
when String
17+
unless JavaUtilities.valid_java_identifier?(import_class)
18+
raise ArgumentError.new "not a valid Java identifier: #{import_class}"
19+
end
20+
raise ArgumentError.new "must use jvm-style name: #{import_class}" if import_class.include?('::')
21+
import_class = JavaUtilities.get_proxy_class(import_class)
22+
when Module
23+
if import_class.respond_to? "java_class"
24+
# ok, it's a proxy
25+
else
26+
raise ArgumentError.new "not a Java class or interface: #{import_class}"
27+
end
28+
else
29+
raise ArgumentError.new "invalid Java class or interface: #{import_class}"
30+
end
31+
32+
java_class = import_class.java_class
33+
class_name = java_class.simple_name
34+
35+
if block_given?
36+
package = java_class.package
37+
38+
# package can be nil if it's default or no package was defined by the classloader
39+
if package
40+
package_name = package.name
41+
elsif java_class.canonical_name =~ /(.*)\.[^.]$/
42+
package_name = $1
43+
else
44+
package_name = ""
45+
end
46+
47+
constant = yield(package_name, class_name)
48+
else
49+
constant = class_name
50+
51+
# Inner classes are separated with $, get last element
52+
if constant =~ /\$([^$])$/
53+
constant = $1
54+
end
55+
end
56+
57+
unless constant =~ /^[A-Z].*/
58+
raise ArgumentError.new "cannot import class `" + java_class.name + "' as `" + constant + "'"
59+
end
60+
61+
if !const_defined?(constant) || const_get(constant) != import_class
62+
const_set(constant, import_class)
63+
end
64+
65+
import_class
66+
end
67+
end
68+
569
##
670
# Includes a Java package into this class/module. The Java classes in the
771
# package will become available in this class/module, unless a constant

‎core/src/main/ruby/jruby/java/core_ext/object.rb‎

Lines changed: 8 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -26,75 +26,17 @@ def java_kind_of?(other) # TODO: this can go away now, but people may be using i
2626
#
2727
# @!visibility public
2828
def java_import(*import_classes)
29-
import_classes = import_classes.each_with_object([]) do |classes, flattened|
30-
if classes.is_a?(Array)
31-
flattened.push(*classes)
32-
else
33-
flattened.push(classes)
34-
end
35-
end
36-
37-
import_classes.map do |import_class|
38-
case import_class
39-
when String
40-
unless JavaUtilities.valid_java_identifier?(import_class)
41-
raise ArgumentError.new "not a valid Java identifier: #{import_class}"
42-
end
43-
raise ArgumentError.new "must use jvm-style name: #{import_class}" if import_class.include?('::')
44-
import_class = JavaUtilities.get_proxy_class(import_class)
45-
when Module
46-
if import_class.respond_to? "java_class"
47-
# ok, it's a proxy
48-
else
49-
raise ArgumentError.new "not a Java class or interface: #{import_class}"
50-
end
51-
else
52-
raise ArgumentError.new "invalid Java class or interface: #{import_class}"
53-
end
54-
55-
java_class = import_class.java_class
56-
class_name = java_class.simple_name
57-
58-
if block_given?
59-
package = java_class.package
60-
61-
# package can be nil if it's default or no package was defined by the classloader
62-
if package
63-
package_name = package.name
64-
elsif java_class.canonical_name =~ /(.*)\.[^.]$/
65-
package_name = $1
66-
else
67-
package_name = ""
68-
end
69-
70-
constant = yield(package_name, class_name)
71-
else
72-
constant = class_name
73-
74-
# Inner classes are separated with $, get last element
75-
if constant =~ /\$([^$])$/
76-
constant = $1
77-
end
78-
end
79-
80-
unless constant =~ /^[A-Z].*/
81-
raise ArgumentError.new "cannot import class `" + java_class.name + "' as `" + constant + "'"
82-
end
83-
84-
# JRUBY-3453: Make import not complain if Java already has already imported the specific Java class
85-
# If no constant is defined, or the constant is not already set to the java_import, assign it
86-
eval_str = "if !defined?(#{constant}) || #{constant} != import_class; #{constant} = import_class; end"
87-
if Module === self
88-
class_eval(eval_str, __FILE__, __LINE__)
89-
else
90-
eval(eval_str, binding, __FILE__, __LINE__)
91-
end
92-
93-
import_class
94-
end
29+
warn "calling java_import on a non-Module object is deprecated"
30+
puts caller
31+
Object.send :java_import, *import_classes
9532
end
9633
private :java_import
9734

9835
alias :import :java_import unless respond_to?(:import)
9936

10037
end
38+
39+
# toplevel java_import is ok
40+
def java_import(*import_classes)
41+
Object.send :java_import, *import_classes
42+
end

0 commit comments

Comments
 (0)