Defect #38145
closedUnreachable branch in ApplicationHelper#format_object due to the use of the deprecated Fixnum class
0%
Description
There is an unreachable branch in ApplicationHelper#format_object. This is because a deprecated class name `Fixnum` is given to the `when` keyword.
Reference:
Unify Fixnum and Bignum into Integer
https://bugs.ruby-lang.org/issues/12005
While the branch is unreachable, fortunately, the result of the code below `when 'Fixnum'` is the same as the result of the code for the unknown class below `else`, so no problem is occurring for now.
The following patch fixes the issue.
Index: app/helpers/application_helper.rb
===================================================================
--- app/helpers/application_helper.rb (revision 22022)
+++ app/helpers/application_helper.rb (working copy)
@@ -263,7 +263,7 @@
format_time(object)
when 'Date'
format_date(object)
- when 'Fixnum'
+ when 'Integer'
object.to_s
when 'Float'
sprintf "%.2f", object
</diff>
Files
Updated by Thomas Löber almost 3 years ago
I think the problem (or "code smell") is that the case argument is object.class.name and not object. If the code was like this, using Fixnum would be fine because Fixnum equals Integer:
case object
when Array
formatted_objects = object.map {|o| format_object(o, html)}
html ? safe_join(formatted_objects, ', ') : formatted_objects.join(', ')
when Time
format_time(object)
when Date
format_date(object)
when Fixnum
object.to_s
when Float
sprintf "%.2f", object
Updated by Go MAEDA almost 3 years ago
- File 38145.patch 38145.patch added
Thomas Löber wrote:
I think the problem (or "code smell") is that the
caseargument isobject.class.nameand notobject. If the code was like this, usingFixnumwould be fine becauseFixnumequalsInteger:
Thank you for suggesting the improved code. But we should not use Fixnum because the constant was removed in Ruby 3.2.
https://github.com/ruby/ruby/blob/ruby_3_2/NEWS.md#removed-constants
Attaching an updated patch.
Updated by Thomas Löber almost 3 years ago
I agree with the use of Integer instead of Fixnum.
However, I don't think the patch works, because the case expression is object.class, but it should be object.
The case statement calls === on the expression(s) that come after when, with the case expression as a parameter.
>> object = 1 => 1 >> Integer === object # Integer.===(object) => true >> Integer === object.class # Integer.===(object.class) => false
Updated by Go MAEDA almost 3 years ago
- File 38145-v2.patch 38145-v2.patch added
Thomas Löber wrote:
However, I don't think the patch works, because the
caseexpression isobject.class, but it should beobject.
Thank you for pointing it out, I attached the wrong patch.
I have updated the patch.
Updated by Go MAEDA almost 3 years ago
- Target version changed from Candidate for next major release to 5.1.0
Setting the target version to 5.1.0.
Updated by Go MAEDA almost 3 years ago
- Status changed from New to Closed
- Assignee set to Go MAEDA
- Resolution set to Fixed
Committed the fix.