Project

General

Profile

Actions

Patch #31004

closed

Decode hexadecimal-encoded literals in order to be frozen string literals friendly

Added by Go MAEDA about 5 years ago. Updated about 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Code cleanup/refactoring
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

Currently, hexadecimal-encoded literals are used in many places in the source code of Redmine. I propose to rewrite those literals to UTF-8.

The reason is that those hexadecimal-encoded strings increase the work required to support frozen-string-literal which will be default in Ruby 3.0. For example, the following code (test/unit/mail_handler_test.rb:714) is not frozen-string-literal ready because you cannot call String#force_encoding for frozen strings.

  def test_add_issue_with_japanese_subject
    issue = submit_email(
              'subject_japanese_1.eml',
              :issue => {:project => 'ecookbook'}
            )
    assert_kind_of Issue, issue
    ja = "\xe3\x83\x86\xe3\x82\xb9\xe3\x83\x88".force_encoding('UTF-8')
    assert_equal ja, issue.subject
  end

You will see the following error If you run the test.

Error:
MailHandlerTest#test_add_issue_with_japanese_subject:
FrozenError: can't modify frozen String
    test/unit/mail_handler_test.rb:721:in `force_encoding'
    test/unit/mail_handler_test.rb:721:in `test_add_issue_with_japanese_subject'

So, we have to update the code in the near future, at the latest until Ruby 3.0 is released. The patch provided by Pavel Rosický in #26561#note-9 fixes the code as follows:

diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb
index adc4b71cf..75ebe3a30 100644
--- a/test/unit/mail_handler_test.rb
+++ b/test/unit/mail_handler_test.rb
@@ -1,3 +1,4 @@
+# frozen-string-literal: true
 # encoding: utf-8
 #
 # Redmine - project management software
@@ -717,7 +718,7 @@ class MailHandlerTest < ActiveSupport::TestCase
               :issue => {:project => 'ecookbook'}
             )
     assert_kind_of Issue, issue
-    ja = "\xe3\x83\x86\xe3\x82\xb9\xe3\x83\x88".force_encoding('UTF-8')
+    ja = (+"\xe3\x83\x86\xe3\x82\xb9\xe3\x83\x88").force_encoding('UTF-8')
     assert_equal ja, issue.subject
   end

The code does not have any problem but there is an easier solution. We can make the code frozen-string-literal ready by just stopping using hexadecimal-encoded strings like the following. Even better, "テスト" that means "test" in Japanese is easier to read than "\xe3\x83\x86\xe3\x82\xb9\xe3\x83\x88". Of course, I can understand "テスト" in a moement but cannot understand "\xe3\x83\x86...".

diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb
index adc4b71cf..c6ded6bcf 100644
--- a/test/unit/mail_handler_test.rb
+++ b/test/unit/mail_handler_test.rb
@@ -1,3 +1,4 @@
+# frozen-string-literal: true
 # encoding: utf-8
 #
 # Redmine - project management software
@@ -717,8 +718,7 @@ class MailHandlerTest < ActiveSupport::TestCase
               :issue => {:project => 'ecookbook'}
             )
     assert_kind_of Issue, issue
-    ja = "\xe3\x83\x86\xe3\x82\xb9\xe3\x83\x88".force_encoding('UTF-8')
-    assert_equal ja, issue.subject
+    assert_equal 'テスト', issue.subject
   end

   def test_add_issue_with_korean_body

There is no disadvantage at all to change hexadecimal-encoded strings to UTF-8 strings. I think we should complete this before working on #26561 (frozen-string-literal).


Files


Related issues

Related to Redmine - Feature #26561: Enable frozen string literalsClosedGo MAEDA

Actions
Actions #1

Updated by Go MAEDA about 5 years ago

Actions #2

Updated by Go MAEDA about 5 years ago

  • Tracker changed from Defect to Feature
Actions #3

Updated by Pavel Rosický about 5 years ago

thanks for poiting this out. I would definitely prefer this solution.

Actions #4

Updated by Go MAEDA about 5 years ago

  • Subject changed from Change hexadecimal-encoded strings to UTF-8 string to Decode hexadecimal-encoded literals in order to be frozen string literals friendly
Actions #5

Updated by Go MAEDA about 5 years ago

  • Assignee set to Go MAEDA
Actions #6

Updated by Yuichi HARADA about 5 years ago

Most of the hexadecimal-encoded literals are under the test directory. Replaced UTF-8's hexadecimal-encoded literals in the test directory with string literals.
Hexadecimal-encoded literals other than UTF-8 are mutable using String#+@ (https://docs.ruby-lang.org/en/2.6.0/String.html#method-i-2B-40) .
I attached a patch.

Actions #7

Updated by Go MAEDA about 5 years ago

  • Status changed from New to Closed
  • Target version set to 4.1.0
  • Resolution set to Fixed

Committed the patch. Thanks.

Actions #8

Updated by Go MAEDA about 5 years ago

  • Tracker changed from Feature to Patch
Actions

Also available in: Atom PDF