Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Literal string will be frozen deprecation warning in Ruby 3.4 #536

Open
NGMarmaduke opened this issue Dec 30, 2024 · 1 comment
Open

Literal string will be frozen deprecation warning in Ruby 3.4 #536

NGMarmaduke opened this issue Dec 30, 2024 · 1 comment

Comments

@NGMarmaduke
Copy link
Contributor

Using Ruby version 3.4.1 and add Warning[:deprecated] = true to spec_helper.rb results in numerous deprecation warnings due to the mutation of literal strings.

One example:

/Users/nick/projects/phony/lib/phony/country_codes.rb:158: warning: literal string will be frozen in the future (run with --debug-frozen-string-literal for more information)

Looking at the majority of warnings they are due to the use of String#slice!.

I started to work on a patch to fix the warnings however in some cases it seems as though the gem is relying on the mutation of arguments. I've attached a patch for one example in the 'trunk_code.rb'. Updating split to not mutate national_number causes qed test failures.

From 5ddb8251fcf7b910850a3c7ce3fd9641f280e2b0 Mon Sep 17 00:00:00 2001
From: Nick Maher <[email protected]>
Date: Mon, 30 Dec 2024 10:07:08 +0000
Subject: [PATCH] Trunk code example

---
 lib/phony/trunk_code.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/phony/trunk_code.rb b/lib/phony/trunk_code.rb
index c1dfae7..cd14eb7 100644
--- a/lib/phony/trunk_code.rb
+++ b/lib/phony/trunk_code.rb
@@ -29,8 +29,10 @@ module Phony
     # its parts.
     #
     def split(national_number)
-      national_number.gsub! @trunk_code_replacement, EMPTY_STRING if @split
-      [self, national_number]
+      return [self, national_number] unless @split
+
+      without_trunk = national_number.gsub @trunk_code_replacement, EMPTY_STRING
+      [self, without_trunk]
     end
 
     # Normalize normalizes the given national number.
-- 
2.42.1
@floere
Copy link
Owner

floere commented Jan 3, 2025

Hi @NGMarmaduke! Thanks so much for your efforts! 🙇🏻‍♂️ I've applied your patch and have pushed c5d105e to make the in-place replace explicit (so I or you can continue work on this). Even with the goal of saving resources, Phony should not rely on implicitly modifying a passed parameter.

Happy New Year! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants