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

Style/EmptyLiteral and frozen_string_literal generate broken code #9773

Closed
adfoster-r7 opened this issue May 5, 2021 · 2 comments · Fixed by #9774
Closed

Style/EmptyLiteral and frozen_string_literal generate broken code #9773

adfoster-r7 opened this issue May 5, 2021 · 2 comments · Fixed by #9774

Comments

@adfoster-r7
Copy link


Expected behavior

Example made up program showing the issue:

result = String.new
amount = gets.chomp.to_i
amount.times do
  result << "hello world\n"
end

puts result

Usage:

ruby foo.rb
2
hello world
hello world

Actual behavior

The Style/EmptyLiteral rule combined with the frozen_string_literal generates an invalid program:

# frozen_string_literal: true

result = ''
amount = gets.chomp.to_i
amount.times do
  result << "hello world\n"
end

puts result

Program error:

$ ruby foo.rb 
5
Traceback (most recent call last):
	2: from foo.rb:5:in `<main>'
	1: from foo.rb:5:in `times'
foo.rb:6:in `block in <main>': can't modify frozen String: "" (FrozenError)

Example rules:

foo.rb:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
result = String.new
^
foo.rb:1:10: C: [Corrected] Style/EmptyLiteral: Use string literal '' instead of String.new.
result = String.new
         ^^^^^^^^^^
foo.rb:2:1: C: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
result = ''
^

Interestingly this problem only arises if the file doesn't already have the frozen_string_literal rule, so having the frozen string literal comment beforehand doesn't actually break the program as the Style/EmptyLiteral rule doesn't run.

# frozen_string_literal: true

result = String.new
amount = gets.chomp.to_i
amount.times do
  result << "hello world\n"
end

puts result

Steps to reproduce the problem

Create a file with the example text above, and run rubocop -A foo.rb

RuboCop version

$ rubocop -V
warning: parser/current is loading parser/ruby27, which recognizes
warning: 2.7.3-compliant syntax, but you are running 2.7.2.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
1.12.1 (using Parser 3.0.1.0, rubocop-ast 1.4.1, running on ruby 2.7.2 x86_64-darwin19)
@adfoster-r7
Copy link
Author

The current Style/EmptyLiteral rule is great for fixing uses of Hash.new/Array.new into their literal vaules, but it would be great if the rule allowed more granular configuration value for the string literal functionality to be disabled. Is that something that makes sense to send a pull request for?

@dvandersluis
Copy link
Member

@adfoster-r7 thanks, I opened a PR to fix this.

dvandersluis added a commit to dvandersluis/rubocop that referenced this issue May 5, 2021
…for `String.new` when `Style/FrozenStringLiteral` is enabled.
@koic koic closed this as completed in #9774 May 5, 2021
koic added a commit that referenced this issue May 5, 2021
[Fix #9773] Fix `Style/EmptyLiteral` to not register offenses for `String.new` when `Style/FrozenStringLiteral` is enabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants