Patch up some regressions in StringIO/IO#gets stuff.

* Only gets should set $_. * Only StringIO#each/each_line complain about limit=0. * Arity-split StringIO#each_line. * Remove LAST_LINE frame field requirement from StringIO#each and #each_line. * Misc tidying.
jruby · Aug 4, 2017 · 74d0588 · 74d0588
1 parent 4ca5a44
commit 74d0588
Showing 4 changed files with 78 additions and 56 deletions.
diff --git a/core/src/main/java/org/jruby/RubyEnumerator.java b/core/src/main/java/org/jruby/RubyEnumerator.java
@@ -139,7 +139,7 @@ public static IRubyObject enumeratorize(Ruby runtime, IRubyObject object, String
         return new RubyEnumerator(runtime, runtime.getEnumerator(), object, runtime.fastNewSymbol(method), new IRubyObject[] {arg});
     }
 
-    public static IRubyObject enumeratorize(Ruby runtime, IRubyObject object, String method, IRubyObject[] args) {
+    public static IRubyObject enumeratorize(Ruby runtime, IRubyObject object, String method, IRubyObject... args) {
         return new RubyEnumerator(runtime, runtime.getEnumerator(), object, runtime.fastNewSymbol(method), args); // TODO: make sure it's really safe to not to copy it
     }
 

diff --git a/core/src/main/java/org/jruby/RubyIO.java b/core/src/main/java/org/jruby/RubyIO.java
@@ -2268,7 +2268,11 @@ public IRubyObject gets(ThreadContext context, IRubyObject rs, IRubyObject limit
     private static final Getline.Callback<RubyIO, IRubyObject> GETLINE = new Getline.Callback<RubyIO, IRubyObject>() {
         @Override
         public IRubyObject getline(ThreadContext context, RubyIO self, IRubyObject rs, int limit, boolean chomp, Block block) {
-            return self.getlineInner(context, rs, limit, chomp, null);
+            IRubyObject result = self.getlineInner(context, rs, limit, chomp, null);
+
+            if (!result.isNil()) context.setLastLine(result);
+
+            return result;
         }
     };
 

diff --git a/core/src/main/java/org/jruby/ext/stringio/StringIO.java b/core/src/main/java/org/jruby/ext/stringio/StringIO.java
@@ -339,7 +339,7 @@ public IRubyObject each(ThreadContext context, IRubyObject arg0, IRubyObject arg
     }
 
     // MRI: strio_each
-    @JRubyMethod(name = "each", writes = FrameField.LASTLINE)
+    @JRubyMethod(name = "each")
     public IRubyObject each(ThreadContext context, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2, Block block) {
         if (!block.isGiven()) return enumeratorize(context.runtime, this, "each", Helpers.arrayOf(arg0, arg1, arg2));
 
@@ -363,19 +363,49 @@ public IRubyObject each(ThreadContext context, IRubyObject[] args, Block block)
         }
     }
 
-    private static boolean isLastArg0(final IRubyObject[] args) {
-        final int len = args.length;
-        return len > 0 &&
-            ! args[len - 1].isNil() &&
-            args[len - 1].checkStringType19().isNil() &&
-            RubyNumeric.num2long( args[len - 1] ) == 0 ;
+    @JRubyMethod(name = "each_line")
+    public IRubyObject each_line(ThreadContext context, Block block) {
+        if (!block.isGiven()) return enumeratorize(context.runtime, this, "each_line");
+
+        return each(context, block);
+    }
+
+    @JRubyMethod(name = "each_line")
+    public IRubyObject each_line(ThreadContext context, IRubyObject arg0, Block block) {
+        if (!block.isGiven()) return enumeratorize(context.runtime, this, "each_line", arg0);
+
+        return each(context, arg0, block);
+    }
+
+    @JRubyMethod(name = "each_line")
+    public IRubyObject each_line(ThreadContext context, IRubyObject arg0, IRubyObject arg1, Block block) {
+        if (!block.isGiven()) return enumeratorize(context.runtime, this, "each_line", arg0, arg1);
+
+        return each(context, arg0, arg1, block);
+    }
+
+    @JRubyMethod(name = "each_line")
+    public IRubyObject each_line(ThreadContext context, IRubyObject arg0, IRubyObject arg1, IRubyObject arg2, Block block) {
+        if (!block.isGiven()) return enumeratorize(context.runtime, this, "each_line", arg0, arg1, arg2);
+
+        return each(context, arg0, arg1, arg2, block);
     }
 
-    @JRubyMethod(name = "each_line", optional = 2, writes = FrameField.LASTLINE)
     public IRubyObject each_line(ThreadContext context, IRubyObject[] args, Block block) {
         if (!block.isGiven()) return enumeratorize(context.runtime, this, "each_line", args);
-
-        return each(context, args, block);
+        switch (args.length) {
+            case 0:
+                return each_line(context, block);
+            case 1:
+                return each_line(context, args[0], block);
+            case 2:
+                return each_line(context, args[0], args[1], block);
+            case 3:
+                return each_line(context, args[0], args[1], args[2], block);
+            default:
+                Arity.raiseArgumentError(context, args.length, 0, 3);
+                throw new RuntimeException("BUG");
+        }
     }
 
     @JRubyMethod(name = "lines", optional = 2)
@@ -557,7 +587,15 @@ public IRubyObject gets(ThreadContext context, IRubyObject[] args) {
     private static final Getline.Callback<StringIO, IRubyObject> GETLINE = new Getline.Callback<StringIO, IRubyObject>() {
         @Override
         public IRubyObject getline(ThreadContext context, StringIO self, IRubyObject rs, int limit, boolean chomp, Block block) {
-            return self.getline(context, rs, limit, chomp);
+            if (limit == 0) {
+                return RubyString.newEmptyString(context.runtime);
+            }
+
+            IRubyObject result = self.getline(context, rs, limit, chomp);
+
+            context.setLastLine(result);
+
+            return result;
         }
     };
 
@@ -566,6 +604,10 @@ public IRubyObject getline(ThreadContext context, StringIO self, IRubyObject rs,
         public StringIO getline(ThreadContext context, StringIO self, IRubyObject rs, int limit, boolean chomp, Block block) {
             IRubyObject line;
 
+            if (limit == 0) {
+                throw context.runtime.newArgumentError("invalid limit: 0 for each_line");
+            }
+
             while (!(line = self.getline(context, rs, limit, chomp)).isNil()) {
                 block.yieldSpecific(context, line);
             }
@@ -592,11 +634,7 @@ public RubyArray getline(ThreadContext context, StringIO self, IRubyObject rs, i
     private IRubyObject getline(ThreadContext context, final IRubyObject rs, int limit, boolean chomp) {
         Ruby runtime = context.runtime;
 
-        IRubyObject str = context.nil;
-
-        if (limit == 0) {
-            throw runtime.newArgumentError("invalid limit: 0 for each_line");
-        }
+        IRubyObject str;
 
         checkReadable();
 

diff --git a/core/src/main/java/org/jruby/util/io/Getline.java b/core/src/main/java/org/jruby/util/io/Getline.java
@@ -54,7 +54,7 @@ public static <Self, Return extends IRubyObject> Return getlineCall(ThreadContex
 
         boolean chomp = false;
         long limit;
-        IRubyObject rs, opt, optArg = context.nil, sepArg = null, limArg = null;
+        IRubyObject opt, optArg = context.nil, sepArg = null, limArg = null;
 
         switch (argc) {
             case 1:
@@ -85,67 +85,47 @@ public static <Self, Return extends IRubyObject> Return getlineCall(ThreadContex
             }
         }
 
-        rs = prepareGetsSeparator(context, sepArg, limArg, enc_io);
-        limit = prepareGetsLimit(context, sepArg, limArg);
-
-        Return result = getline.getline(context, self, rs, (int) limit, chomp, block);
-
-        if (!result.isNil()) context.setLastLine(result);
-
-        return result;
-    }
-
-    // MRI: prepare_getline_args, separator logic
-    private static IRubyObject prepareGetsSeparator(ThreadContext context, IRubyObject arg0, IRubyObject arg1, Encoding enc_io) {
         Ruby runtime = context.runtime;
         IRubyObject rs = runtime.getRecordSeparatorVar().get();
-        if (arg0 != null && arg1 == null) { // argc == 1
+        IRubyObject lim = context.nil;
+
+        if (sepArg != null && limArg == null) { // argc == 1
             IRubyObject tmp = context.nil;
 
-            if (arg0.isNil() || !(tmp = TypeConverter.checkStringType(runtime, arg0)).isNil()) {
+            if (sepArg.isNil() || !(tmp = TypeConverter.checkStringType(runtime, sepArg)).isNil()) {
                 rs = tmp;
+            } else {
+                lim = sepArg;
             }
-        } else if (arg0 != null && arg1 != null) { // argc >= 2
-            rs = arg0;
+        } else if (sepArg != null && limArg != null) { // argc >= 2
+            rs = sepArg;
             if (!rs.isNil()) {
                 rs = rs.convertToString();
             }
+            lim = limArg;
         }
+
+        // properly encode rs
         if (!rs.isNil()) {
             Encoding enc_rs;
 
-            enc_rs = ((RubyString)rs).getEncoding();
+            enc_rs = ((RubyString) rs).getEncoding();
             if (enc_io != enc_rs &&
-                    (((RubyString)rs).scanForCodeRange() != StringSupport.CR_7BIT ||
-                            (((RubyString)rs).size() > 0 && !enc_io.isAsciiCompatible()))) {
+                    (((RubyString) rs).scanForCodeRange() != StringSupport.CR_7BIT ||
+                            (((RubyString) rs).size() > 0 && !enc_io.isAsciiCompatible()))) {
                 if (rs == runtime.getGlobalVariables().getDefaultSeparator()) {
                     rs = RubyString.newStringLight(runtime, 0, enc_io);
-                    ((RubyString)rs).catAscii(NEWLINE_BYTES, 0, 1);
+                    ((RubyString) rs).catAscii(NEWLINE_BYTES, 0, 1);
                 }
                 else {
                     throw runtime.newArgumentError("encoding mismatch: " + enc_io + " IO with " + enc_rs + " RS");
                 }
             }
         }
-        return rs;
-    }
 
-    // MRI: prepare_getline_args, limit logic
-    private static long prepareGetsLimit(ThreadContext context, IRubyObject arg0, IRubyObject arg1) {
-        Ruby runtime = context.runtime;
-        IRubyObject lim = context.nil;
-        if (arg0 != null && arg1 == null) { // argc == 1
-            IRubyObject tmp = context.nil;
+        limit = lim.isNil() ? -1 : lim.convertToInteger().getLongValue();
 
-            if (arg0.isNil() || !(tmp = TypeConverter.checkStringType(runtime, arg0)).isNil()) {
-                // only separator logic
-            } else {
-                lim = arg0;
-            }
-        } else if (arg0 != null && arg1 != null) { // argc >= 2
-            lim = arg1;
-        }
-        return lim.isNil() ? -1 : lim.convertToInteger().getLongValue();
+        return getline.getline(context, self, rs, (int) limit, chomp, block);
     }
 
     private static final byte[] NEWLINE_BYTES = { (byte) '\n' };