Skip to content

Accept separator, limit and keywords in ARGF line methods - #9784

Merged
headius merged 5 commits into
jruby:jruby-10.0from
zev:argf-enumerator-keywords
Oct 9, 2026
Merged

Accept separator, limit and keywords in ARGF line methods#9784
headius merged 5 commits into
jruby:jruby-10.0from
zev:argf-enumerator-keywords

Conversation

@zev

@zev zev commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Warning

Conflicts with #9785. Whichever merges second needs a rebase that keeps both sides of the RubyArgsFile.readlines forwarding and of shared/gets.rb. See the rebase note for the verified resolution.

Depends on #9783. This branch is stacked on io-enumerator-keywords and uses the RubyEnumerator.enumeratorize(context, object, method, args, keywords) overload it adds. Until #9783 merges, the diff here includes its commit. Only the last two commits are new (875ce40, 0011cfc).

ARGF.gets, readline, readlines, to_a, each and each_line checked their arity as 0..1 on the raw argument array, and the keyword Hash counted as one of the arguments. In CRuby they share rb_io_getline, which takes a separator and a limit plus keywords ("02:"). So in JRuby:

  • Passing a separator and a limit (ARGF.gets("\n", 1)), or a separator and a limit with chomp:, raised ArgumentError, with or without a block.
  • ARGF#each and ARGF#to_a weren't declared keywords = true, so chomp: was read as the limit and raised TypeError.
  • Without a block, ARGF.each_line and ARGF.each returned an Enumerator that passed keywords back as a positional Hash, the same bug Pass keyword arguments to IO.foreach, IO#each and IO#each_line enumerators #9783 fixes for IO. It raised TypeError when iterated.

With path a file containing "a\nb\n", each run as ruby -e '...' path:

Call CRuby 4.1 JRuby before JRuby after
ARGF.gets(chomp: true) "a" "a" "a"
ARGF.gets("\n", 1) "a" ArgumentError (given 2, expected 0..1) "a"
ARGF.gets("\n", 1, 2) ArgumentError (given 3, expected 0..2) ArgumentError (given 3, expected 0..1) ArgumentError (given 3, expected 0..2)
ARGF.readlines(chomp: true) ["a", "b"] ["a", "b"] ["a", "b"]
ARGF.readlines("\n", 1) ["a", "\n", "b", "\n"] ArgumentError ["a", "\n", "b", "\n"]
ARGF.to_a(chomp: true) ["a", "b"] TypeError: no implicit conversion of Hash into Integer ["a", "b"]
ARGF.each_line(chomp: true) { |l| a << l } ["a", "b"] ["a", "b"] ["a", "b"]
ARGF.each(chomp: true) { |l| a << l } ["a", "b"] TypeError ["a", "b"]
ARGF.each_line("\n", 1, chomp: true) { |l| a << l } ["a", "", "b", ""] ArgumentError (given 3, expected 0..1) ["a", "", "b", ""]
ARGF.each_line(chomp: true).to_a ["a", "b"] TypeError ["a", "b"]
ARGF.each(chomp: true).to_a ["a", "b"] TypeError ["a", "b"]
ARGF.each("\n", 1).to_a ["a", "\n", "b", "\n"] ArgumentError ["a", "\n", "b", "\n"]

"Before" is the same for unmodified jruby-10.0 and for #9783, since #9783 doesn't touch ARGF. "After" matches CRuby 4.1.0dev on every row.

When $stdin isn't an IO

When ARGV is empty and $stdin is replaced (by a StringIO, say), ARGF hands off to $stdin. There, each and each_line called $stdin.each with no arguments, and readlines passed keywords on as a positional Hash. MRI calls each_line and readlines with the original arguments and keywords. Each row runs with no ARGV and $stdin set to an object that reads from StringIO.new("a\nb\n") and records what it receives:

Call CRuby 4.1 JRuby before JRuby after
ARGF.to_a ["a\n", "b\n"] via readlines() ["a\n", "b\n"] via to_a() ["a\n", "b\n"] via readlines()
ARGF.readlines(chomp: true) ["a", "b"] via readlines(chomp: true) ["a", "b"] via readlines({chomp: true}) ["a", "b"] via readlines(chomp: true)
ARGF.readlines("\n", 1, chomp: true) ["a", "", "b", ""] via readlines("\n", 1, chomp: true) ArgumentError (given 3, expected 0..1) ["a", "", "b", ""] via readlines("\n", 1, chomp: true)
ARGF.each_line { |l| a << l } ["a\n", "b\n"] via each_line() ["a\n", "b\n"] via each() ["a\n", "b\n"] via each_line()
ARGF.each_line(chomp: true) { |l| a << l } ["a", "b"] via each_line(chomp: true) ["a\n", "b\n"] via each() (chomp ignored) ["a", "b"] via each_line(chomp: true)
ARGF.each("\n", 1) { |l| a << l } ["a", "\n", "b", "\n"] via each_line("\n", 1) ArgumentError (given 2, expected 0..1) ["a", "\n", "b", "\n"] via each_line("\n", 1)
ARGF.each_line("\n", 1, chomp: true).to_a ["a", "", "b", ""] via each_line("\n", 1, chomp: true) ArgumentError (given 3, expected 0..1) ["a", "", "b", ""] via each_line("\n", 1, chomp: true)

"Before" is again the same on jruby-10.0 and #9783.

ARGF.gets and ARGF.readline with a non-IO $stdin raise a ClassCastException on jruby-10.0. That's a separate crash, fixed in #9785.

Changes

  • RubyArgsFile, first commit (875ce40):
    • A new checkGetlineArity helper allows 0..2 positional arguments and doesn't count the keyword Hash. gets, readlines, each and each_line use it, and readline and to_a go through them. The annotations are now optional = 2. Method#arity stays -1.
    • each and each_line read callInfo before building the Enumerator and pass the keywords flag to it. Their block path is now a shared eachLineCommon. each is now keywords = true. Before, it wasn't, so the generated invoker zeroed callInfo and the each_line it called never saw keywords.
    • to_a calls readlines. In MRI they're the same C function (io.c: rb_define_method(rb_cARGF, "to_a", argf_readlines, -1)). The old to_a was a copy of readlines with two differences: it passed callInfo as 0, and for a non-IO $stdin it forwarded to_a where MRI forwards readlines. The fast path in argf_getline for no arguments with the default $/ depends only on args.length, so ARGF.to_a with no arguments still uses it.
  • RubyArgsFile, second commit (0011cfc):
    • With a non-IO $stdin, each and each_line call $stdin.each_line with the arguments and keywords, as MRI's argf_each_line does. I also dropped a redundant second next_argv call there, since the method has already called it.
    • readlines restores callInfo before forwarding to $stdin.readlines. That answers the old TODO the same way argf_getline already does for gets.
  • Specs: 14 new examples in the shared ARGF specs, plus an ARGFSpecs::Stdin fixture that records the arguments and keywords it receives. Each shared file is used by two methods, so that's 28 runs.
    • shared/each_line.rb (each, each_line): separator plus limit, chomp:, separator plus limit plus chomp:, both Enumerator forms, and non-IO $stdin with and without arguments.
    • shared/gets.rb (gets, readline): chomp:, separator plus limit, and separator plus chomp:.
    • shared/readlines.rb (readlines, to_a): chomp:, separator plus limit, and non-IO $stdin with and without arguments.
    • The non-IO $stdin examples use ARGF.class.new with no arguments, which is stream mode. MRI only supports a non-IO $stdin in stream mode: with "-" in ARGV it calls GetOpenFile on $stdin.

Testing

  • The new examples pass on CRuby 4.0.7 and 4.1.0dev. On Pass keyword arguments to IO.foreach, IO#each and IO#each_line enumerators #9783, 23 of the 28 runs fail. The other 5 cover things that already worked, such as ARGF.gets(chomp: true) and ARGF.readlines(chomp: true) on files.
  • spec/ruby/core/{io,file,enumerator,argf,kernel} and library/stringio with mspec ci: the same 2 failures as unmodified jruby-10.0 (Kernel#require ... c-extension file exists but loading fails, unrelated).

Not in this PR

🤖 Generated with Claude Code

@zev zev mentioned this pull request Oct 8, 2026
Merged
@zev

zev commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

⚠️ Rebase note: #9784 and #9785 conflict

Whichever of #9784 and #9785 merges second needs a rebase. I trial-merged argf-gets-non-io-stdin (#9785) into argf-enumerator-keywords (#9784). There are two conflicts, and the resolution is to keep both sides in each:

1. RubyArgsFile.readlines, forwarding to a non-IO $stdin. #9784 restores callInfo so keywords reach $stdin.readlines. #9785 switches the call to a public-only call site so a plain-object $stdin doesn't recurse into the private Kernel#readlines. The resolved code needs both:

        if (!(data.currentFile instanceof RubyIO)) {
            context.callInfo = callInfo; // restore callInfo for kwargs
            return sites(context).readlines.call(context, recv, data.currentFile, args);
        }

Don't keep only #9784's callMethod(context, "readlines", args). That brings back the StackOverflowError with $stdin = Object.new. Don't keep only #9785's line either, because keywords would then be passed as a positional Hash.

2. spec/ruby/core/argf/shared/gets.rb. Both PRs add examples at the same spot. Keep both sets: #9784's chomp/separator/limit examples, then #9785's when reading $stdin and $stdin is not an IO block.

ARGF.readline auto-merges correctly. It gets #9784's optional = 2 annotation and #9785's forwarding to $stdin.readline.

Leave as is: eachLineCommon in #9784 forwards each_line with callMethod, which can reach private methods. Unlike gets, readline and readlines, that's correct. MRI's argf_each_line uses rb_block_call_kw → iterate_method → CALL_FCALL (vm_eval.c), not forward_current's public call, so it shouldn't be switched to a NormalCachingCallSite.

Verified: with this resolution, the trial merge builds, all spec/ruby/core/argf specs pass on CRuby 4.1.0dev and JRuby (186 examples), and mspec ci over core/{io,file,enumerator,argf,kernel} and library/stringio shows only the 2 unrelated Kernel#require failures that jruby-10.0 also has.

headius
headius approved these changes Oct 8, 2026
View reviewed changes

@headius headius left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to see that this is a small change. Figures that it's functionality nobody has spec'ed and nobody has reported missing.

@headius headius added this to the JRuby 10.0.8.0 milestone Oct 8, 2026
zev and others added 5 commits October 8, 2026 19:47
@zev @claude
Pass arguments and keywords to IO.foreach, IO#each and IO#each_line e…
179dbd0 
…numerators

Without a block these methods returned an Enumerator that passed keyword
arguments back as a positional Hash, so iterating it raised TypeError
(foreach, each_line) or ignored them (each). IO#each also dropped its
positional arguments entirely.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zev @claude
Tag new IO enumerator specs as failing on Windows
7fbb5e0 
Windows checks out the spec fixtures with CRLF line endings, so these
fail there like the block forms of the same specs, which are already
tagged windows:. The separator-and-limit Enumerator spec for IO#each
was tagged fails: until this PR and is tagged windows: for IO#each_line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zev @claude
Accept separator, limit and keywords in ARGF line methods
0e06fcf 
ARGF.gets, readline, readlines, to_a, each and each_line counted the
keyword Hash as a positional argument and only allowed one, so passing a
separator and a limit, or a separator with chomp:, raised ArgumentError.
ARGF#each and ARGF#to_a were not declared to take keywords, and the
enumerators returned by each and each_line passed keywords back as a
positional Hash, raising TypeError when iterated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zev @claude
Pass ARGF arguments and keywords on to a $stdin that is not an IO
b2477d2 
When $stdin is not an IO (for example a StringIO), ARGF#each and
ARGF#each_line called $stdin.each with no arguments, dropping the
separator, limit and keywords. ARGF.readlines passed keywords to
$stdin.readlines as a positional Hash. Both now forward the arguments and
keywords, and each/each_line call each_line as MRI's argf_each_line does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zev @claude
Tag new ARGF separator and limit specs as failing on Windows
85ccd4a 
Windows checks out the spec fixtures with CRLF line endings, so the
chomp results keep a "\r" and these fail there, like the IO line specs
that are already tagged windows:.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zev
zev force-pushed the argf-enumerator-keywords branch from e3c2279 to 85ccd4a Compare October 9, 2026 00:49
@zev
zev marked this pull request as ready for review October 9, 2026 04:22
@headius
headius merged commit bcb8ec8 into jruby:jruby-10.0 Oct 9, 2026
126 of 127 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@headius headius headius approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

JRuby 10.0.8.0

Development

Successfully merging this pull request may close these issues.

2 participants

@zev @headius