jruby · May 13, 2018 · May 13, 2018 · May 13, 2018
Showing with 55 additions and 39 deletions.

+38 −37 core/src/main/java/org/jruby/RubyModule.java

+14 −0 core/src/main/java/org/jruby/RubySymbol.java

+3 −2 core/src/main/java/org/jruby/javasupport/JavaPackage.java
diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -211,7 +211,7 @@ public static IRubyObject autoload(ThreadContext context, IRubyObject self, IRub
         @JRubyMethod(name = "autoload?")
         public static IRubyObject autoload_p(ThreadContext context, IRubyObject self, IRubyObject symbol) {
             final Ruby runtime = context.runtime;
-            final String name = symbol.asJavaString();
+            final String name = TypeConverter.checkID(symbol).idString();
 
             RubyModule mod = RubyKernel.getModuleForAutoload(runtime, self);
             for (/* RubyModule mod = (RubyModule) self */; mod != null; mod = mod.getSuperClass()) {
@@ -1967,7 +1967,7 @@ private void addAccessor(ThreadContext context, RubySymbol identifier, Visibilit
      */
     public void setMethodVisibility(IRubyObject[] methods, Visibility visibility) {
         for (int i = 0; i < methods.length; i++) {
-            exportMethod(methods[i].asJavaString(), visibility);
+            exportMethod(TypeConverter.checkID(methods[i]).idString(), visibility);
         }
     }
 
@@ -2682,12 +2682,12 @@ public RubyArray public_instance_methods19(IRubyObject[] args) {
 
     @JRubyMethod(name = "instance_method", required = 1)
     public IRubyObject instance_method(IRubyObject symbol) {
-        return newMethod(null, symbol.asJavaString(), false, null);
+        return newMethod(null, TypeConverter.checkID(symbol).idString(), false, null);
     }
 
     @JRubyMethod(name = "public_instance_method", required = 1)
     public IRubyObject public_instance_method(IRubyObject symbol) {
-        return newMethod(null, symbol.asJavaString(), false, PUBLIC);
+        return newMethod(null, TypeConverter.checkID(symbol).idString(), false, PUBLIC);
     }
 
     /** rb_class_protected_instance_methods
@@ -2962,26 +2962,26 @@ public IRubyObject method_undefined(ThreadContext context, IRubyObject nothing)
 
     @JRubyMethod(name = "method_defined?", required = 1)
     public RubyBoolean method_defined_p(ThreadContext context, IRubyObject symbol) {
-        return isMethodBound(symbol.asJavaString(), true) ? context.tru : context.fals;
+        return isMethodBound(TypeConverter.checkID(symbol).idString(), true) ? context.tru : context.fals;
     }
 
     @JRubyMethod(name = "public_method_defined?", required = 1)
     public IRubyObject public_method_defined(ThreadContext context, IRubyObject symbol) {
-        DynamicMethod method = searchMethod(symbol.asJavaString());
+        DynamicMethod method = searchMethod(TypeConverter.checkID(symbol).idString());
 
         return context.runtime.newBoolean(!method.isUndefined() && method.getVisibility() == PUBLIC);
     }
 
     @JRubyMethod(name = "protected_method_defined?", required = 1)
     public IRubyObject protected_method_defined(ThreadContext context, IRubyObject symbol) {
-        DynamicMethod method = searchMethod(symbol.asJavaString());
+        DynamicMethod method = searchMethod(TypeConverter.checkID(symbol).idString());
 
         return context.runtime.newBoolean(!method.isUndefined() && method.getVisibility() == PROTECTED);
     }
 
     @JRubyMethod(name = "private_method_defined?", required = 1)
     public IRubyObject private_method_defined(ThreadContext context, IRubyObject symbol) {
-        DynamicMethod method = searchMethod(symbol.asJavaString());
+        DynamicMethod method = searchMethod(TypeConverter.checkID(symbol).idString());
 
         return context.runtime.newBoolean(!method.isUndefined() && method.getVisibility() == PRIVATE);
     }
@@ -3018,7 +3018,9 @@ public RubyModule alias_method(ThreadContext context, IRubyObject newId, IRubyOb
     @JRubyMethod(name = "undef_method", rest = true)
     public RubyModule undef_method(ThreadContext context, IRubyObject[] args) {
         for (int i=0; i<args.length; i++) {
-            undef(context, args[i].asJavaString());
+            RubySymbol name = TypeConverter.checkID(args[i]);
+
+            undef(context, name.idString());
         }
         return this;
     }
@@ -3285,46 +3287,44 @@ private RubyModule proceedWithPrepend(RubyModule insertBelow, RubyModule moduleT
 
     @JRubyMethod(name = "class_variable_defined?", required = 1)
     public IRubyObject class_variable_defined_p(ThreadContext context, IRubyObject var) {
-        String internedName = validateClassVariable(var, var.asJavaString().intern());
-        RubyModule module = this;
-        do {
-            if (module.hasClassVariable(internedName)) {
-                return context.tru;
-            }
-        } while ((module = module.getSuperClass()) != null);
+        String id = validateClassVariable(context.runtime, var);
+
+        for (RubyModule module = this; module != null; module = module.getSuperClass()) {
+            if (module.hasClassVariable(id)) return context.tru;
+        }
 
         return context.fals;
     }
 
     /** rb_mod_cvar_get
      *
      */
-    public IRubyObject class_variable_get(IRubyObject var) {
-        return getClassVar(var, validateClassVariable(var, var.asJavaString()).intern());
+    public IRubyObject class_variable_get(IRubyObject name) {
+        return getClassVar(name, validateClassVariable(getRuntime(), name));
     }
 
     @JRubyMethod(name = "class_variable_get")
-    public IRubyObject class_variable_get19(IRubyObject var) {
-        return class_variable_get(var);
+    public IRubyObject class_variable_get19(IRubyObject name) {
+        return class_variable_get(name);
     }
 
     /** rb_mod_cvar_set
      *
      */
-    public IRubyObject class_variable_set(IRubyObject var, IRubyObject value) {
-        return setClassVar(validateClassVariable(var, var.asJavaString()).intern(), value);
+    public IRubyObject class_variable_set(IRubyObject name, IRubyObject value) {
+        return setClassVar(validateClassVariable(getRuntime(), name), value);
     }
 
     @JRubyMethod(name = "class_variable_set")
-    public IRubyObject class_variable_set19(IRubyObject var, IRubyObject value) {
-        return class_variable_set(var, value);
+    public IRubyObject class_variable_set19(IRubyObject name, IRubyObject value) {
+        return class_variable_set(name, value);
     }
 
     /** rb_mod_remove_cvar
      *
      */
     public IRubyObject remove_class_variable(ThreadContext context, IRubyObject name) {
-        return removeClassVariable(name.asJavaString());
+        return removeClassVariable(validateClassVariable(context.runtime, name));
     }
 
     @JRubyMethod(name = "remove_class_variable")
@@ -3440,8 +3440,8 @@ public IRubyObject const_get_2_0(ThreadContext context, IRubyObject[] args) {
         boolean inherit = args.length == 1 || ( ! args[1].isNil() && args[1].isTrue() );
 
         final IRubyObject symbol = args[0];
-        final String fullName = symbol.asJavaString();
-        String name = fullName;
+        RubySymbol fullName = TypeConverter.checkID(symbol);
+        String name = fullName.idString();
 
         int sep = name.indexOf("::");
         // symbol form does not allow ::
@@ -3458,7 +3458,7 @@ public IRubyObject const_get_2_0(ThreadContext context, IRubyObject[] args) {
 
         // Bare ::
         if (name.length() == 0) {
-            throw runtime.newNameError("wrong constant name ", fullName);
+            throw runtime.newNameError("wrong constant name ", fullName.idString());
         }
 
         while ( ( sep = name.indexOf("::") ) != -1 ) {
@@ -3794,15 +3794,6 @@ public boolean fastIsClassVarDefined(String internedName) {
         return isClassVarDefined(internedName);
     }
 
-    /** rb_mod_remove_cvar
-     *
-     * @deprecated - use {@link #removeClassVariable(String)}
-     */
-    @Deprecated
-    public IRubyObject removeCvar(IRubyObject name) {
-        return removeClassVariable(name.asJavaString());
-    }
-
     public IRubyObject removeClassVariable(String name) {
         String javaName = validateClassVariable(name);
         IRubyObject value;
@@ -4334,6 +4325,16 @@ protected final String validateClassVariable(IRubyObject nameObj, String name) {
         throw getRuntime().newNameError("`%1$s' is not allowed as a class variable name", this, nameObj);
     }
 
+    protected String validateClassVariable(Ruby runtime, IRubyObject object) {
+        RubySymbol name = TypeConverter.checkID(object);
+
+        if (!name.validClassVariableName()) {
+            throw getRuntime().newNameError(str(runtime, "`", ids(runtime, name), "' is not allowed as a class variable name"), this, object);
+        }
+
+        return name.idString();
+    }
+
     protected final void ensureClassVariablesSettable() {
         checkAndRaiseIfFrozen();
     }

diff --git a/core/src/main/java/org/jruby/RubySymbol.java b/core/src/main/java/org/jruby/RubySymbol.java
@@ -251,6 +251,20 @@ public boolean validInstanceVariableName() {
         return valid && getBytes().length() >= 2; // FIXME: good enough on length check?  Trying to avoid counter.
     }
 
+    /**
+     * Is the string this constant represents a valid constant identifier name.
+     */
+    public boolean validClassVariableName() {
+        boolean valid = ByteListHelper.eachCodePoint(getBytes(), (int index, int codepoint, Encoding encoding) ->
+                index == 0 && codepoint == '@' ||
+                        index == 1 && codepoint == '@' ||
+                        index == 2 && (!encoding.isDigit(codepoint)) && (encoding.isAlnum(codepoint) || !Encoding.isAscii(codepoint) || codepoint == '_') ||
+                        index > 2 && (encoding.isAlnum(codepoint) || !Encoding.isAscii(codepoint) || codepoint == '_'));
+
+        return valid && getBytes().length() >= 2; // FIXME: good enough on length check?  Trying to avoid counter.
+    }
+
+
     public boolean validLocalVariableName() {
         boolean valid =  ByteListHelper.eachCodePoint(getBytes(), (int index, int codepoint, Encoding encoding) ->
                 index == 0 && (!encoding.isDigit(codepoint) && (encoding.isAlnum(codepoint) || !Encoding.isAscii(codepoint) || codepoint == '_')) ||

diff --git a/core/src/main/java/org/jruby/javasupport/JavaPackage.java b/core/src/main/java/org/jruby/javasupport/JavaPackage.java
@@ -35,6 +35,7 @@
 import org.jruby.RubyClass;
 import org.jruby.RubyModule;
 import org.jruby.RubyString;
+import org.jruby.RubySymbol;
 import org.jruby.anno.JRubyClass;
 import org.jruby.anno.JRubyMethod;
 import org.jruby.exceptions.RaiseException;
@@ -207,9 +208,9 @@ public IRubyObject respond_to_p(final ThreadContext context, IRubyObject name, I
     }
 
     private IRubyObject respond_to(final ThreadContext context, IRubyObject mname, final boolean includePrivate) {
-        String name = mname.asJavaString();
+        RubySymbol name = TypeConverter.checkID(mname);
 
-        if (getMetaClass().respondsToMethod(name, !includePrivate)) return context.tru;
+        if (getMetaClass().respondsToMethod(name.idString(), !includePrivate)) return context.tru;
         /*
         if ( ( name = BlankSlateWrapper.handlesMethod(name) ) != null ) {
             RubyBoolean bound = checkMetaClassBoundMethod(context, name, includePrivate);