Return nil from Row#field when the offset is past the end - #366
Conversation
Array#[start..-1] returns [] when start == length but nil once start is
greater, so the slice in Row#field and Row#index vanishes and the
:assoc / :index send lands on nil:
row = CSV::Row.new(%w{A B C A A}, %w{1 2 3 4 5})
row.field("A", 5) # => nil
row.field("A", 6) # => NoMethodError: undefined method `assoc' for nil
The docs promise nil unconditionally ("Returns +nil+ if the header does
not exist", "Returns +nil+ if +index+ is out of range"), and offset 5 on
a 5-field row already returns nil, so offset 6 returning nil is the only
self-consistent behaviour. Negative offsets past the start break the
same way.
Five entry points reach it: #field, its alias #[], #values_at, #index,
and #delete.
The existing assertions stop at the last offset that happens to work
(field("A", 5)), one short of the bug.
|
Could you share your motivation for this? You don't have any real-world problem of this, right? |
|
No, I don't have a real-world problem. I should say that plainly rather than invent one. I found it by checking documented contracts against behaviour, not from a production incident. I also went looking for a natural way to reach it before answering you, and did not find a convincing one — ragged CSV does not do it, because So it takes an offset the caller supplied or computed against different data. What is left, on Two things I would still argue for, and then I'll drop it:
If neither of those moves you, close it — it is a small thing and you have the better view of what CSV users actually hit. For transparency: I use AI assistance to find and prepare these, and I run and verify everything before sending it. |
The problem
CSV::Row#field(header, offset)raisesNoMethodErroronce the offset goes one past the end of the row, where the docs promisenil.Five public entry points reach it —
#field, its alias#[],#values_at,#index, and#delete:Cause
lib/csv/row.rb:206Array#[start..-1]returns[]whenstart == length, butniloncestartis greater — so the receiver ofassoc/[]disappears.lib/csv/row.rb:575has the same shape and is what#indexand#deletego through:Why this is the code and not the docs
The documented contract is unconditional:
and the neighbouring offset already behaves that way: on a 5-field row,
field("A", 5)returnsnil. Offset 6 returningnilis the only self-consistent reading.Why it was not caught
test/csv/test_row.rb:70-75walks the offsets up to exactly the last one that works:The row has 5 fields, so it stops one short of the bug.
The change
Guard the slice at both sites. Assertions added to the existing
test_fieldblock for offset 6, a far-past offset, and the#[]/#index/#values_atpaths.Verification
Full suite: 527 tests, 4047 assertions, 0 failures, 0 errors — before and after.
Reverting only
lib/csv/row.rband keeping the new assertions fails withNoMethodError: undefined method 'assoc' for nil:NilClass, so they exercise this bug rather than#fieldin general.Disclosure: this patch was prepared with AI assistance. The reproduction, the red/green check and the suite runs above were executed against this branch; happy to adjust anything on request.