https://www.redmine.org/https://www.redmine.org/favicon.ico?16793021292023-01-05T10:13:31ZRedmineRedmine - Defect #38145: Unreachable branch in ApplicationHelper#format_object due to the use of the deprecated Fixnum classhttps://www.redmine.org/issues/38145?journal_id=1090032023-01-05T10:13:31ZThomas Löber
<ul></ul><p>I think the problem (or "code smell") is that the <code>case</code> argument is <code>object.class.name</code> and not <code>object</code>. If the code was like this, using <code>Fixnum</code> would be fine because <code>Fixnum</code> equals <code>Integer</code>:</p>
<pre>
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
</pre> Redmine - Defect #38145: Unreachable branch in ApplicationHelper#format_object due to the use of the deprecated Fixnum classhttps://www.redmine.org/issues/38145?journal_id=1090182023-01-08T08:38:08ZGo MAEDA
<ul><li><strong>File</strong> <a href="/attachments/30099">38145.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/30099/38145.patch">38145.patch</a> added</li></ul><p>Thomas Löber wrote:</p>
<blockquote>
<p>I think the problem (or "code smell") is that the <code>case</code> argument is <code>object.class.name</code> and not <code>object</code>. If the code was like this, using <code>Fixnum</code> would be fine because <code>Fixnum</code> equals <code>Integer</code>:</p>
</blockquote>
<p>Thank you for suggesting the improved code. But we should not use <code>Fixnum</code> because the constant was removed in Ruby 3.2.<br /><a class="external" href="https://github.com/ruby/ruby/blob/ruby_3_2/NEWS.md#removed-constants">https://github.com/ruby/ruby/blob/ruby_3_2/NEWS.md#removed-constants</a></p>
<p>Attaching an updated patch.</p> Redmine - Defect #38145: Unreachable branch in ApplicationHelper#format_object due to the use of the deprecated Fixnum classhttps://www.redmine.org/issues/38145?journal_id=1090352023-01-10T10:21:11ZThomas Löber
<ul></ul><p>I agree with the use of <code>Integer</code> instead of <code>Fixnum</code>.</p>
<p>However, I don't think the patch works, because the <code>case</code> expression is <code>object.class</code>, but it should be <code>object</code>.</p>
<p>The <code>case</code> statement calls <code>===</code> on the expression(s) that come after <code>when</code>, with the <code>case</code> expression as a parameter.</p>
<pre>
>> object = 1
=> 1
>> Integer === object # Integer.===(object)
=> true
>> Integer === object.class # Integer.===(object.class)
=> false
</pre> Redmine - Defect #38145: Unreachable branch in ApplicationHelper#format_object due to the use of the deprecated Fixnum classhttps://www.redmine.org/issues/38145?journal_id=1090612023-01-11T14:43:46ZGo MAEDA
<ul><li><strong>File</strong> <a href="/attachments/30112">38145-v2.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/30112/38145-v2.patch">38145-v2.patch</a> added</li></ul><p>Thomas Löber wrote:</p>
<blockquote>
<p>However, I don't think the patch works, because the <code>case</code> expression is <code>object.class</code>, but it should be <code>object</code>.</p>
</blockquote>
<p>Thank you for pointing it out, I attached the wrong patch.</p>
<p>I have updated the patch.</p> Redmine - Defect #38145: Unreachable branch in ApplicationHelper#format_object due to the use of the deprecated Fixnum classhttps://www.redmine.org/issues/38145?journal_id=1090622023-01-11T14:44:01ZGo MAEDA
<ul><li><strong>Target version</strong> changed from <i>Candidate for next major release</i> to <i>5.1.0</i></li></ul><p>Setting the target version to 5.1.0.</p> Redmine - Defect #38145: Unreachable branch in ApplicationHelper#format_object due to the use of the deprecated Fixnum classhttps://www.redmine.org/issues/38145?journal_id=1090682023-01-12T04:04:36ZGo MAEDA
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Closed</i></li><li><strong>Assignee</strong> set to <i>Go MAEDA</i></li><li><strong>Resolution</strong> set to <i>Fixed</i></li></ul><p>Committed the fix.</p>