Conversation
[Bug #21864] Co-Authored-By: Benoit Daloze <eregontp@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR forwards a bug fix from the main Ruby repository (Bug #21864) that refactors TypeError messages to be more consistent, specifically changing error message format from "from X to Y" to "of X into Y" for implicit conversion errors involving nil and integer.
Changes:
- Added version-conditional constant
OF_NIL_INTO_INTEGERto handle different error message formats across Ruby versions - Updated three test cases (
test_args_CALL_2,test_args_ATTRASGN_2,test_args_OP_ASGN1_aref_1) to use this constant for version-appropriate error message validation
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| end | ||
| end | ||
|
|
||
| OF_NIL_INTO_INTEGER = RUBY_VERSION < "4.1." ? "from nil to integer" : "of nil into Integer" |
There was a problem hiding this comment.
The version comparison string "4.1." is malformed - it ends with a period but lacks a patch version. This will cause the string comparison to behave unexpectedly. It should likely be "4.1" (without the trailing dot) or "4.1.0" (with a complete version). String comparison with Ruby versions requires proper formatting to work correctly.
| OF_NIL_INTO_INTEGER = RUBY_VERSION < "4.1." ? "from nil to integer" : "of nil into Integer" | |
| OF_NIL_INTO_INTEGER = RUBY_VERSION < "4.1.0" ? "from nil to integer" : "of nil into Integer" |
| end | ||
| end | ||
|
|
||
| OF_NIL_INTO_INTEGER = RUBY_VERSION < "4.1." ? "from nil to integer" : "of nil into Integer" |
There was a problem hiding this comment.
The version check logic appears to be inverted. According to the PR description, this forward-ports a bug fix that makes error messages more consistent by changing "from X to Y" to "of X into Y". However, the condition states that versions less than "4.1." (presumably older versions) will use "from nil to integer" and newer versions will use "of nil into Integer". This seems backward - typically older Ruby versions would have the old message format, and newer versions (after the fix) would have the new format. The condition should likely be checking for versions greater than or equal to a threshold, not less than.
| end | ||
| end | ||
|
|
||
| OF_NIL_INTO_INTEGER = RUBY_VERSION < "4.1." ? "from nil to integer" : "of nil into Integer" |
There was a problem hiding this comment.
This constant uses RUBY_VERSION string comparison, which differs from the pattern used elsewhere in this test file. Lines 68-77 define NEW_MESSAGE_FORMAT using a runtime behavior check (attempting an operation and examining the error message). This approach is more reliable because it directly tests the actual behavior. Consider using a similar runtime check for OF_NIL_INTO_INTEGER, such as attempting to index an array with nil and examining the resulting error message format.
| OF_NIL_INTO_INTEGER = RUBY_VERSION < "4.1." ? "from nil to integer" : "of nil into Integer" | |
| OF_NIL_INTO_INTEGER = begin | |
| [].fetch(nil) | |
| rescue TypeError => e | |
| if e.message.include?("of nil into Integer") | |
| "of nil into Integer" | |
| else | |
| "from nil to integer" | |
| end | |
| end |
Pick ruby/ruby@d066b9e