Inconsistency of IndexError vs nil for unknown capture group

After matching with a String, StringScanner#[:unknown_capture_group] returns nil:

$ irb -rstrscan
irb(main):001> s=StringScanner.new("abc")
=> #<StringScanner 0/3 @ "abc">
irb(main):002> s.scan("a")
=> "a"
irb(main):003> s[:foo]
=> nil

After matching with a Regexp, StringScanner#[:unknown_capture_group] raises IndexError:
(using a new process to be sure to not be affected by #135)

irb -rstrscan
irb(main):001> s=StringScanner.new("abc")
=> #<StringScanner 0/3 @ "abc">
irb(main):002> s.scan(/./)
=> "a"
irb(main):003> s[:foo]
(irb):3:in `[]': undefined group name reference: foo (IndexError)

It would be best if the behavior is consistent.

There is a test for the nil case in https://github.com/ruby/strscan/blob/v3.1.2/test/strscan/test_stringscanner.rb#L830-L840
And there are some tests for the IndexError cases in https://github.com/ruby/strscan/blob/v3.1.2/test/strscan/test_stringscanner.rb#L443-L444 and in more places.

The implementation is

strscan/ext/strscan/strscan.c

Line 1764 in 6968b11

if (!RTEST(p->regex)) return Qnil;

This is proving problematic on TruffleRuby.
We would like to upstream our pure-Ruby implementation of StringScanner, but to do so we want to avoid exposing internals as much as possible.
Things like p->regex is something I don't want to expose to code living outside the truffleruby repository.
Having to workaround by keeping extra state for whether the last match was with a String or a Regexp feels very messy and wrong semantically.

Could the behavior be unified for both of these cases?

Given the Regexp-matching methods predate the String ones, I think it would be best to have IndexError in both cases.
Using a String pattern should be equivalent to a Regexp in StringScanner & transparent.

Though returning nil for both cases can also be implemented easily on TruffleRuby.

The worse is the mix of both which exposes internal details of the StringScanner implementation.

BTW this inconsistency seems to have been introduced in a0ff956 which is ruby/ruby@90311f3 and https://bugs.ruby-lang.org/issues/13759

@naitoh

OK. Let's use IndexError for String too.

@naitoh Do you want to work on this?

Do you want to work on this?

Yes, I would like to try this issue.

Wondering how #[name] should behave when a String pattern is used but matching failed - to raise IndexError or to return nil.

When a Regexp pattern is used #[name] returns nil:

s = StringScanner.new("abc")
s.scan(/\d/) # => nil
s[:a] # => nil

Returning nil looks consistent with handling a Regexp pattern. But is inconsistent with handling a String pattern after successful matching (when IndexError is raised).

I think the logic should be:

There is always a "result of the last StringScanner match" (some kind of MatchData internally or nil) whether matched against String pattern or Regexp pattern
If the last match failed, the last result is nil, then [] returns nil for any group
If the last match succeeded, then use the semantics of MatchData#[] or equivalent, which raises IndexError on missing group. For a String pattern there are of course no named groups.

Let's use the same behavior as a Regexp pattern.

Fix a bug that inconsistency of IndexError vs nil for unknown capture…

[ruby/strscan] Fix a bug that inconsistency of IndexError vs nil for

Navigation Menu

Repository navigation

Inconsistency of IndexError vs nil for unknown capture group #139

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Inconsistency of IndexError vs nil for unknown capture group #139

Description

Activity

eregon commented on Feb 18, 2025

kou commented on Feb 19, 2025

naitoh commented on Feb 19, 2025

andrykonchin commented on Feb 21, 2025

eregon commented on Feb 21, 2025

kou commented on Feb 22, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions