Defect #38145

Unreachable branch in ApplicationHelper#format_object due to the use of the deprecated Fixnum class

Added by Go MAEDA about 1 month ago. Updated 23 days ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:Code cleanup/refactoring
Target version:5.1.0
Resolution:Fixed Affected version:

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>

38145.patch Magnifier (1.62 KB) Go MAEDA, 2023-01-08 09:37

38145-v2.patch Magnifier (1.61 KB) Go MAEDA, 2023-01-11 15:43

Associated revisions

Revision 22041
Added by Go MAEDA 23 days ago

Unreachable branch in ApplicationHelper#format_object due to the use of the deprecated Fixnum class (#38145).

Patch by Go MAEDA.

History

#1 Updated by Thomas Löber 29 days 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

#2 Updated by Go MAEDA 27 days ago

Thomas Löber wrote:

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:

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.

#3 Updated by Thomas Löber 24 days 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

#4 Updated by Go MAEDA 23 days ago

Thomas Löber wrote:

However, I don't think the patch works, because the case expression is object.class, but it should be object.

Thank you for pointing it out, I attached the wrong patch.

I have updated the patch.

#5 Updated by Go MAEDA 23 days ago

  • Target version changed from Candidate for next major release to 5.1.0

Setting the target version to 5.1.0.

#6 Updated by Go MAEDA 23 days ago

  • Status changed from New to Closed
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the fix.

Also available in: Atom PDF