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

Suppress "literal string will be frozen in the future" warning #1019

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

tikkss
Copy link
Contributor

@tikkss tikkss commented Oct 14, 2024

Before change:

$ ruby -W -I lib -e 'require "irb"; IRB.setup(nil); IRB::Irb.new.build_statement("1 + 2")'
/Users/zzz/src/github.com/ruby/irb/lib/irb.rb:1135: warning: literal string will be frozen in the future

After change:

$ ruby -W -I lib -e 'require "irb"; IRB.setup(nil); IRB::Irb.new.build_statement("1 + 2")'

Before change:

```console
$ ruby -W -I lib -e 'require "irb"; IRB.setup(nil); IRB::Irb.new.build_statement("1 + 2")'
/Users/zzz/src/github.com/ruby/irb/lib/irb.rb:1135: warning: literal string will be frozen in the future
```

After change:

```console
$ ruby -W -I lib -e 'require "irb"; IRB.setup(nil); IRB::Irb.new.build_statement("1 + 2")'
```
@tompng
Copy link
Member

tompng commented Oct 14, 2024

Actually, warning in IRB::Irb.new.build_statement("1 + 2") is not a problem because literal string is not passed to build_statement (this is a internal API) in ruby/irb.

However, making build_statement not modify the given argument improves readability and code quality. 👍

lib/irb.rb Outdated
@@ -1132,7 +1132,7 @@ def build_statement(code)
return Statement::EmptyInput.new
end

code.force_encoding(@context.io.encoding)
code.dup.force_encoding(@context.io.encoding)
Copy link
Member

@tompng tompng Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement has no effect. We need to change the encoding of code.
maybe code = code.dup.force_encoding(@context.io.encoding)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review and suggestion! I fixed them at
37fe153.

By the way, I met the warning in SciRuby/iruby's test code, SciRuby/iruby's code.

Because improves readability and code quality.

Co-authored-by: tomoya ishida <[email protected]>
Copy link
Member

@tompng tompng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 👍

@tompng tompng merged commit 3da04b9 into ruby:master Oct 18, 2024
30 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request Oct 18, 2024
warning
(ruby/irb#1019)

* Suppress "literal string will be frozen in the future" warning

Before change:

```console
$ ruby -W -I lib -e 'require "irb"; IRB.setup(nil); IRB::Irb.new.build_statement("1 + 2")'
/Users/zzz/src/github.com/ruby/irb/lib/irb.rb:1135: warning: literal string will be frozen in the future
```

After change:

```console
$ ruby -W -I lib -e 'require "irb"; IRB.setup(nil); IRB::Irb.new.build_statement("1 + 2")'
```

* Making build_statement not modify the given argument

Because improves readability and code quality.

Co-authored-by: tomoya ishida <[email protected]>

---------

ruby/irb@3da04b9786

Co-authored-by: tomoya ishida <[email protected]>
@tikkss tikkss deleted the suppress-frozen-string-literal-warning branch October 18, 2024 21:13
@tikkss
Copy link
Contributor Author

tikkss commented Oct 18, 2024

I was deeply impressed by your keynote at RubyKaigi 2024 in person.
I'm happy to collaborate with you on code. Thanks!

@st0012 st0012 added the bug Something isn't working label Oct 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

3 participants