[jruby~main:dd7ad183] Multiple improvements to inner class support and other JI tweaks:

[nicksieger@kenai.com]

Project:    jruby
Repository: main
Revision:   dd7ad183d7c93a98d3b4a50d7862d6144b536af4
Author:     nicksieger
Date:       2010-03-26 08:21:47 UTC
Link:       

Log Message:
------------
Fix regression from 7f52685: reinstate ENOENT for bad file: URLs.
Multiple improvements to inner class support and other JI tweaks:
* Unify inner class logic for interfaces and classes
* Make it possible to access lower-case inner classes with a method (:: or .) 
(JRUBY-4666)
* Make it possible to import inner classes as their simple name
* Provide a better error for lower-case class name imports
* Improve arity error on package/class elements in dotted syntax
* Clean up java_import logic
* Make include_class call java_import instead of vice-versa


Revisions:
----------
9df875b410c332a15a5c456b3463fb866f81d296
dd7ad183d7c93a98d3b4a50d7862d6144b536af4


Modified Paths:
---------------
src/org/jruby/RubyFileStat.java
lib/ruby/site_ruby/shared/builtin/javasupport/core_ext/object.rb
lib/ruby/site_ruby/shared/builtin/javasupport/java.rb
src/org/jruby/javasupport/Java.java
src/org/jruby/javasupport/JavaClass.java


Diffs:
------
diff --git a/src/org/jruby/RubyFileStat.java b/src/org/jruby/RubyFileStat.java
index 3e995d5..a0aa3ca 100644
--- a/src/org/jruby/RubyFileStat.java
+++ b/src/org/jruby/RubyFileStat.java
@@ -109,7 +109,17 @@ public class RubyFileStat extends RubyObject {
         if (filename.startsWith("file:") && filename.indexOf('!') != -1) {
             // file: URL handling
             String zipFileEntry = filename.substring(filename.indexOf("!") + 
1);
-            if (zipFileEntry.charAt(0) == '/') zipFileEntry = 
zipFileEntry.substring(1);
+            if (zipFileEntry.length() > 0) {
+                if (zipFileEntry.charAt(0) == '/') {
+                    if (zipFileEntry.length() > 1) {
+                        zipFileEntry = zipFileEntry.substring(1);
+                    } else {
+                        throw getRuntime().newErrnoENOENTError("invalid 
jar/file URL: " + filename);
+                    }
+                }
+            } else {
+                throw getRuntime().newErrnoENOENTError("invalid jar/file 
URL: " + filename);
+            }
             filename = filename.substring(5, filename.indexOf("!"));
             
             try {
diff --git a/lib/ruby/site_ruby/shared/builtin/javasupport/core_ext/object.rb 
b/lib/ruby/site_ruby/shared/builtin/javasupport/core_ext/object.rb
index 8f1ee1d..5cc799c 100644
--- a/lib/ruby/site_ruby/shared/builtin/javasupport/core_ext/object.rb
+++ b/lib/ruby/site_ruby/shared/builtin/javasupport/core_ext/object.rb
@@ -22,65 +22,8 @@ class Object
   # using either its base name or by using a name returned from an optional 
block,
   # passing all specified classes in turn and providing the block package 
name
   # and base class name.
-  def include_class(include_class)
-    if include_class.respond_to? "java_class"
-      # FIXME: When I changed this user const_set instead of eval below 
Comparator got lost
-      # which means I am missing something.
-      constant = include_class.java_class.to_s.split(".").last
-      
-      # JRUBY-3453: Make import not complain if Java already has already 
imported the specific Java class
-      # If no constant is defined, or the constant is not already set to the 
include_class, assign it
-      eval_str = "if !defined?(#{constant}) || #{constant} != include_class; 
#{constant} = include_class; end"
-      if (Module === self)
-        return class_eval(eval_str, __FILE__, __LINE__)
-      else
-        return eval(eval_str, binding, __FILE__, __LINE__)
-      end
-    end
-    
-    if include_class.respond_to? "_name"
-      return self.class.instance_eval { import(include_class._name) }
-    end
-    
-    # else, pull in the class
-    class_names = [*include_class]
-
-    class_names.each do |full_class_name|
-      if !full_class_name.respond_to? :match
-        raise ArgumentError.new "Invalid java class/interface: 
#{full_class_name}"
-      end
-
-      package_name, class_name = 
full_class_name.match(/((.*)\.)?([^\.]*)/)[2,3]
-
-      if block_given?
-        constant = yield(package_name, class_name)
-      else
-        constant = class_name
-      end
-      
-      cls = self.kind_of?(Module) ? self : self.class
-
-         # Constant already exists...do not let proxy get created unless the 
collision is the proxy
-         # you are trying to include.
-      existing_constant = cls.instance_eval(constant) rescue nil
-      proxy = nil
-      if existing_constant
-        proxy = JavaUtilities.get_proxy_class(full_class_name)
-        warn "redefining #{constant}" unless existing_constant == proxy
-      end
-
-      if existing_constant && existing_constant == proxy
-        return proxy
-      end
-
-      # FIXME: When I changed this user const_set instead of eval below 
Comparator got lost
-      # which means I am missing something.
-      if (Module === self)
-        class_eval("#{constant} = 
JavaUtilities.get_proxy_class(\"#{full_class_name}\")", __FILE__, __LINE__)
-      else
-        eval("#{constant} = 
JavaUtilities.get_proxy_class(\"#{full_class_name}\")", binding, __FILE__, 
__LINE__)
-      end
-    end
+  def include_class(include_class, &block)
+    java_import(include_class, &block)
   end
   
   # TODO: this can go away now, but people may be using it
@@ -91,8 +34,55 @@ class Object
     return other.java_class.assignable_from?(self.java_class)
   end
 
-  def java_import(*args, &block)
-    include_class(*args, &block)
+  def java_import(import_class)
+    case import_class
+    when Array
+      import_class.each do |arg|
+        java_import(arg)
+      end
+      return
+    when String
+      # pull in the class
+      import_class = JavaUtilities.get_proxy_class(import_class);
+    when Module
+      # do nothing, assume we already have it
+    else
+      raise ArgumentError.new "Invalid java class/interface: #{import_class}"
+    end
+
+    full_name = import_class.java_class.name
+    package = import_class.java_class.package
+    # package can be nil if it's default or no package was defined by the 
classloader
+    package_name = package ? package.name : 
full_name[0...full_name.rindex('.')]
+    if package_name.length > 0
+      class_name = full_name[(package_name.length + 1)..-1]
+    else
+      class_name = full_name
+    end
+
+    if block_given?
+      constant = yield(package_name, class_name)
+    else
+      constant = class_name
+
+      # Inner classes are separated with $
+      if constant =~ /\$/
+        constant = constant.split(/\$/).last
+      end
+
+      if constant[0,1].upcase != constant[0,1]
+        raise ArgumentError.new "cannot import class `" + class_name + "' as 
`" + constant + "'"
+      end
+    end
+
+    # JRUBY-3453: Make import not complain if Java already has already 
imported the specific Java class
+    # If no constant is defined, or the constant is not already set to the 
include_class, assign it
+    eval_str = "if !defined?(#{constant}) || #{constant} != import_class; 
#{constant} = import_class; end"
+    if (Module === self)
+      return class_eval(eval_str, __FILE__, __LINE__)
+    else
+      return eval(eval_str, binding, __FILE__, __LINE__)
+    end
   end
   
   private :java_import
diff --git a/lib/ruby/site_ruby/shared/builtin/javasupport/java.rb 
b/lib/ruby/site_ruby/shared/builtin/javasupport/java.rb
index c11a5db..b72fdeb 100644
--- a/lib/ruby/site_ruby/shared/builtin/javasupport/java.rb
+++ b/lib/ruby/site_ruby/shared/builtin/javasupport/java.rb
@@ -10,7 +10,7 @@ module Java
    end
 
    def method_missing(sym, *args)
-     raise ArgumentError, "wrong number of arguments (#{args.length} for 0)" 
unless args.empty?
+      raise ArgumentError, "Java package `java' does not have a method 
`#{sym}'" unless args.empty?
      JavaUtilities.get_top_level_proxy_or_package sym
    end
  end
@@ -47,7 +47,7 @@ module JavaPackageModuleTemplate
     private :const_missing
     
     def method_missing(sym, *args)
-      raise ArgumentError, "wrong number of arguments (#{args.length} for 
0)" unless args.empty?
+      raise ArgumentError, "Java package `#{package_name}' does not have a 
method `#{sym}'" unless args.empty?
       JavaUtilities.get_proxy_or_package_under_package self, sym
     end
     private :method_missing
diff --git a/src/org/jruby/javasupport/Java.java 
b/src/org/jruby/javasupport/Java.java
index 9be0aeb..6563ede 100644
--- a/src/org/jruby/javasupport/Java.java
+++ b/src/org/jruby/javasupport/Java.java
@@ -95,6 +95,7 @@ import org.jruby.java.proxies.InterfaceJavaProxy;
 import org.jruby.java.proxies.JavaProxy;
 import org.jruby.java.proxies.RubyObjectHolderProxy;
 import org.jruby.util.CodegenUtils;
+import org.jruby.util.IdUtil;
 import org.jruby.util.SafePropertyAccessor;
 
 @JRubyModule(name = "Java")
@@ -488,10 +489,9 @@ public class Java implements Library {
 
     private static RubyClass createProxyClass(Ruby runtime, RubyClass 
baseType,
             JavaClass javaClass, boolean invokeInherited) {
-       // JRUBY-2938 the proxy class might already exist
-       RubyClass proxyClass = javaClass.getProxyClass();
-       if (proxyClass != null)
-           return proxyClass;
+        // JRUBY-2938 the proxy class might already exist
+        RubyClass proxyClass = javaClass.getProxyClass();
+        if (proxyClass != null) return proxyClass;
 
         // this needs to be split, since conditional calling #inherited 
doesn't fit standard ruby semantics
         RubyClass.checkInheritable(baseType);
@@ -737,23 +737,29 @@ public class Java implements Library {
     // package scheme 2: separate module for each full package name, 
constructed 
     // from the camel-cased package segments: Java::JavaLang::Object, 
     private static void addToJavaPackageModule(RubyModule proxyClass, 
JavaClass javaClass) {
+        Ruby runtime = proxyClass.getRuntime();
         Class<?> clazz = javaClass.javaClass();
         String fullName;
         if ((fullName = clazz.getName()) == null) {
             return;
         }
         int endPackage = fullName.lastIndexOf('.');
-        // we'll only map conventional class names to modules 
-        if (fullName.indexOf('$') != -1 || 
!Character.isUpperCase(fullName.charAt(endPackage + 1))) {
-            return;
+        RubyModule parentModule;
+        String className;
+
+        // inner classes must be nested
+        if (fullName.indexOf('$') != -1) {
+            parentModule = getProxyClass(runtime, 
(JavaClass)javaClass.declaring_class());
+            className = clazz.getSimpleName();
+        } else {
+            String packageString = endPackage < 0 ? "" : 
fullName.substring(0, endPackage);
+            parentModule = getJavaPackageModule(runtime, packageString);
+            className = parentModule == null ? fullName : 
fullName.substring(endPackage + 1);
         }
-        Ruby runtime = proxyClass.getRuntime();
-        String packageString = endPackage < 0 ? "" : fullName.substring(0, 
endPackage);
-        RubyModule packageModule = getJavaPackageModule(runtime, 
packageString);
-        if (packageModule != null) {
-            String className = fullName.substring(endPackage + 1);
-            if (packageModule.getConstantAt(className) == null) {
-                packageModule.const_set(runtime.newSymbol(className), 
proxyClass);
+        
+        if (parentModule != null && IdUtil.isConstant(className)) {
+            if (parentModule.getConstantAt(className) == null) {
+                parentModule.setConstant(className, proxyClass);
             }
         }
     }
@@ -968,11 +974,18 @@ public class Java implements Library {
         }
     }
 
-    private static void memoizePackageOrClass(RubyModule parentPackage, 
String name, final IRubyObject value) {
+    private static void memoizePackageOrClass(final RubyModule 
parentPackage, final String name, final IRubyObject value) {
         RubyClass singleton = parentPackage.getSingletonClass();
         singleton.addMethod(name, new 
org.jruby.internal.runtime.methods.JavaMethod(singleton, Visibility.PUBLIC) {
             public IRubyObject call(ThreadContext context, IRubyObject self, 
RubyModule clazz, String name, IRubyObject[] args, Block block) {
-                Arity.checkArgumentCount(context.getRuntime(), args, 0, 0);
+                if (args.length != 0) {
+                    throw context.getRuntime().newArgumentError(
+                            "Java package `"
+                            + parentPackage.callMethod("package_name")
+                            + "' does not have a method `"
+                            + name
+                            + "'");
+                }
                 return call(context, self, clazz, name);
             }
 
diff --git a/src/org/jruby/javasupport/JavaClass.java 
b/src/org/jruby/javasupport/JavaClass.java
index 921dc16..d4c0577 100644
--- a/src/org/jruby/javasupport/JavaClass.java
+++ b/src/org/jruby/javasupport/JavaClass.java
@@ -70,6 +70,7 @@ import org.jruby.anno.JRubyClass;
 import org.jruby.common.IRubyWarnings.ID;
 import org.jruby.exceptions.RaiseException;
 import org.jruby.internal.runtime.methods.DynamicMethod;
+import org.jruby.internal.runtime.methods.JavaMethod.JavaMethodZero;
 import org.jruby.java.addons.ArrayJavaAddons;
 import org.jruby.java.proxies.ArrayJavaProxy;
 import org.jruby.java.invokers.ConstructorInvoker;
@@ -579,7 +580,7 @@ public class JavaClass extends JavaObject {
         installClassFields(proxy);
         installClassMethods(proxy);
         installClassConstructors(proxy);
-        installClassConstants(javaClass, proxy);
+        installClassClasses(javaClass, proxy);
         
         // FIXME: bit of a kludge here (non-interface classes assigned to 
both
         // class and module fields). simplifies proxy extender code, will go 
away
@@ -657,20 +658,37 @@ public class JavaClass extends JavaObject {
         }
     }
 
-    private void installClassConstants(final Class<?> javaClass, final 
RubyClass proxy) {
+    private void installClassClasses(final Class<?> javaClass, final 
RubyModule proxy) {
         // setup constants for public inner classes
-        Class<?>[] classes = getClasses(javaClass);
+        Class<?>[] classes = getDeclaredClasses(javaClass);
 
-        for (int i = classes.length; --i >= 0;) {
+        for (int i = classes.length; --i >= 0; ) {
             if (javaClass == classes[i].getDeclaringClass()) {
                 Class<?> clazz = classes[i];
+
+                // no non-public inner classes
+                if (!Modifier.isPublic(clazz.getModifiers())) continue;
+                
                 String simpleName = getSimpleName(clazz);
-                if (simpleName.length() == 0) {
-                    continue;
-                }
-                // Ignore bad constant named inner classes pending JRUBY-697
-                if (IdUtil.isConstant(simpleName) && 
proxy.getConstantAt(simpleName) == null) {
-                    proxy.setConstant(simpleName, 
Java.get_proxy_class(JAVA_UTILITIES, get(getRuntime(), clazz)));
+                if (simpleName.length() == 0) continue;
+
+                final IRubyObject innerProxy = 
Java.get_proxy_class(JAVA_UTILITIES,get(getRuntime(),clazz));
+
+                if (IdUtil.isConstant(simpleName)) {
+                    if (proxy.getConstantAt(simpleName) == null) {
+                        proxy.const_set(getRuntime().newString(simpleName), 
innerProxy);
+                    }
+                } else {
+                    // lower-case name
+                    if (!proxy.respondsTo(simpleName)) {
+                        // define a class method
+                        proxy.getSingletonClass().addMethod(simpleName, new 
JavaMethodZero(proxy.getSingletonClass(), Visibility.PUBLIC) {
+                            @Override
+                            public IRubyObject call(ThreadContext context, 
IRubyObject self, RubyModule clazz, String name) {
+                                return innerProxy;
+                            }
+                        });
+                    }
                 }
             }
         }
@@ -872,22 +890,8 @@ public class JavaClass extends JavaObject {
         for (NamedInstaller installer : staticInstallers.values()) {
             installer.install(module);
         }
-        // setup constants for public inner classes
-        Class<?>[] classes = getClasses(javaClass);
 
-        for (int i = classes.length; --i >= 0; ) {
-            if (javaClass == classes[i].getDeclaringClass()) {
-                Class<?> clazz = classes[i];
-                String simpleName = getSimpleName(clazz);
-                if (simpleName.length() == 0) continue;
-                
-                // Ignore bad constant named inner classes pending JRUBY-697
-                if (IdUtil.isConstant(simpleName) && 
module.getConstantAt(simpleName) == null) {
-                    module.const_set(getRuntime().newString(simpleName),
-                        
Java.get_proxy_class(JAVA_UTILITIES,get(getRuntime(),clazz)));
-                }
-            }
-        }
+        installClassClasses(javaClass, module);
         
         this.proxyModule = module;
         applyProxyExtenders();
@@ -1786,6 +1790,14 @@ public class JavaClass extends JavaObject {
             return new Constructor[] {};
         }        
     }
+
+    private static Class<?>[] getDeclaredClasses(Class<?> javaClass) {
+        try {
+            return javaClass.getDeclaredClasses();
+        } catch (SecurityException e) {
+            return new Class<?>[] {};
+        }
+    }
     
     private static Class<?>[] getClasses(Class<?> javaClass) {
         try {