httpclient icon indicating copy to clipboard operation
httpclient copied to clipboard

Address `warning: literal string will be frozen in the future` warnings

Open yahonda opened this issue 1 year ago • 4 comments

This commit addresses the warning: literal string will be frozen in the future warnings appeared at Rails CI https://buildkite.com/rails/rails-nightly/builds/1122#019263a3-2200-4e62-a6a8-028cffa60aaf

Here are the warnings appeared:

/usr/local/lib/ruby/gems/3.4.0+0/bundler/gems/httpclient-d57cc6d5ffee/lib/httpclient.rb:1256: warning: literal string will be frozen in the future
/usr/local/lib/ruby/gems/3.4.0+0/bundler/gems/httpclient-d57cc6d5ffee/lib/httpclient/http.rb:580: warning: literal string will be frozen in the future
/usr/local/lib/ruby/gems/3.4.0+0/bundler/gems/httpclient-d57cc6d5ffee/lib/httpclient/session.rb:954: warning: literal string will be frozen in the future
/usr/local/lib/ruby/gems/3.4.0+0/bundler/gems/httpclient-d57cc6d5ffee/lib/httpclient/util.rb:71: warning: literal string will be frozen in the future

There should be some warnings remained because Rails CI does not use all of httpclient code. I'll open some pull requests later to address remaining ones.

Ref: https://bugs.ruby-lang.org/issues/20205

yahonda avatar Oct 08 '24 12:10 yahonda

I think we should reasonably push back on these new warnings and the restriction itself.

I don't think they realized how big of an impact it will be while negatively impacting performance at least in the short term.

I have added a note: https://bugs.ruby-lang.org/issues/20205#note-55

hartator avatar Oct 08 '24 22:10 hartator

I also found warnings by stdlib with @yahonda:

/Users/hsbt/.local/share/rbenv/versions/ruby-dev/lib/ruby/3.4.0+0/open-uri.rb:455: warning: literal string will be frozen in the future
/Users/hsbt/.local/share/rbenv/versions/ruby-dev/lib/ruby/3.4.0+0/logger/log_device.rb:45: warning: literal string will be frozen in the future

@mame found open-uri warning caused by

def set_body_encoding
  if type = self.content_type
    OpenURI::Meta.init(o = '')
    o.meta_add_field('content-type', type)
    @body_encoding = o.encoding
  end
end

from https://github.com/nahi/httpclient/blob/master/lib/httpclient/http.rb#L241

It's hard to fix that from open-uri warnings.

hsbt avatar Oct 09 '24 02:10 hsbt

The warning isn't really from open-uri but from passing a literal to a method that mutate the argument.

The fix is to do: OpenURI::Meta.init(o = ''.dup)

byroot avatar Oct 09 '24 03:10 byroot

An easy way to solve these warnings is to run ruby with --enable-frozen-string-literal --debug-frozen-string-literal, this will include the location where the string was allocated in the FrozenError message.

byroot avatar Oct 09 '24 03:10 byroot

Covered other cases in #465

jeremy avatar Dec 17 '24 23:12 jeremy

Rebased master branch to make CI run.

yahonda avatar Feb 18 '25 04:02 yahonda

I recommend you add one Ruby with --enable-frozen-string-literal --debug-frozen-string-literal in the CI Matrix.

Also I silenced those warnings when I revamped CI, it should now be removed.

byroot avatar Feb 18 '25 08:02 byroot

Will a release be getting cut with these changes soon?

wyardley avatar Feb 20 '25 03:02 wyardley

We are currently planning a new release. Please wait a little longer.

takkanm avatar Feb 20 '25 23:02 takkanm