more cleanup of string vs symbol usage.

jruby · Apr 16, 2018 · 070b78c · 070b78c
1 parent 61703bd
commit 070b78c
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 6 deletions.
diff --git a/core/src/main/java/org/jruby/RubyKernel.java b/core/src/main/java/org/jruby/RubyKernel.java
@@ -227,7 +227,7 @@ public static IRubyObject method_missing(ThreadContext context, IRubyObject recv
     }
 
     protected static IRubyObject methodMissingDirect(ThreadContext context, IRubyObject recv, RubySymbol symbol, Visibility lastVis, CallType lastCallType, IRubyObject[] args) {
-        return methodMissing(context, recv, symbol.toString(), lastVis, lastCallType, args, true);
+        return methodMissing(context, recv, symbol.idString(), lastVis, lastCallType, args, true);
     }
 
     public static IRubyObject methodMissing(ThreadContext context, IRubyObject recv, String name, Visibility lastVis, CallType lastCallType, IRubyObject[] args) {
@@ -774,8 +774,7 @@ public static RubyArray local_variables19(ThreadContext context, IRubyObject rec
         while (currentScope != null) {
             for (String id: currentScope.getStaticScope().getVariables()) {
                 RubySymbol name = runtime.newSymbol(id);
-                // FIXME: Technically a non-charset String will still not work here.
-                if (IdUtil.isLocal(name.toString()) && !encounteredLocalVariables.contains(name)) {
+                if (name.validLocalVariableName() && !encounteredLocalVariables.contains(name)) {
                     allLocalVariables.push(name);
                     encounteredLocalVariables.add(name);
                 }

diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -3012,7 +3012,7 @@ public IRubyObject module_exec(ThreadContext context, IRubyObject[] args, Block
     @JRubyMethod(name = "remove_method", rest = true, visibility = PRIVATE)
     public RubyModule remove_method(ThreadContext context, IRubyObject[] args) {
         for(int i=0;i<args.length;i++) {
-            removeMethod(context, TypeConverter.checkID(args[i]).toString());
+            removeMethod(context, TypeConverter.checkID(args[i]).idString());
         }
         return this;
     }
@@ -4407,7 +4407,7 @@ protected final String validateConstant(IRubyObject name) {
         return symbol.idString();
     }
 
-    @Deprecated
+    // FIXME: bytelist_love: This should really be working with symbol segments (errorName is FQN).
     protected final String validateConstant(String name, IRubyObject errorName) {
         if (IdUtil.isValidConstantName19(name)) return name;
 

diff --git a/core/src/main/java/org/jruby/RubySymbol.java b/core/src/main/java/org/jruby/RubySymbol.java
@@ -242,6 +242,14 @@ public boolean validInstanceVariableName() {
         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 == '_')) ||
+                        index != 0 && (encoding.isAlnum(codepoint) || !Encoding.isAscii(codepoint) || codepoint == '_'));
+
+        return valid && getBytes().length() >= 1;
+    }
+
     @Override
     public boolean isImmediate() {
     	return true;

diff --git a/core/src/main/java/org/jruby/RubyThread.java b/core/src/main/java/org/jruby/RubyThread.java
@@ -749,7 +749,7 @@ public static IRubyObject handle_interrupt(ThreadContext context, IRubyObject se
         public void visit(ThreadContext context, RubyHash self, IRubyObject key, IRubyObject value, int index, Object state) {
             if (value instanceof RubySymbol) {
                 RubySymbol sym = (RubySymbol) value;
-                switch (sym.toString()) {
+                switch (sym.idString()) {
                     case "immediate" : return;
                     case "on_blocking" : return;
                     case "never" : return;