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

Get tests passing with frozen-string-literals enabled. #354

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/parser/lexer/literal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ def coerce_encoding(string)
end

def clear_buffer
@buffer = ''
@buffer = ''.dup
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use String.new for these to avoid the extra allocation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've recently discovered via @koic that Ruby isn't quite consistent with String.new. String.new.encoding is ASCII-8BIT, whereas String.new('').encoding (which doesn't avoid the extra allocation) is UTF-8.

I'm happy to change the PR to use String.new('') - I presume UTF-8 is the preferred encoding? - I've just found that some library maintainers (particularly Rails) prefer dup.


# Prime the buffer with lexer encoding; otherwise,
# concatenation will produce varying results.
Expand Down
2 changes: 1 addition & 1 deletion lib/parser/source/buffer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def column_for_position(position)
def source_lines
@lines ||= begin
lines = @source.lines.to_a
lines << '' if @source.end_with?("\n".freeze)
lines << ''.dup if @source.end_with?("\n".freeze)

lines.each do |line|
line.chomp!("\n".freeze)
Expand Down
2 changes: 1 addition & 1 deletion lib/parser/source/rewriter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ def merge_actions!(action, existing)
end

def merge_replacements(actions)
result = ''
result = ''.dup
prev_act = nil

actions.each do |act|
Expand Down
2 changes: 1 addition & 1 deletion test/test_diagnostic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def test_verifies_levels
end

def test_freezes
string = 'foo'
string = 'foo'.dup
highlights = [@range2]

diag = Parser::Diagnostic.new(:error, :escape_eof, @range1, highlights)
Expand Down
2 changes: 1 addition & 1 deletion test/test_lexer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def setup

def utf(str)
if str.respond_to?(:force_encoding)
str.force_encoding(Encoding::UTF_8)
str.dup.force_encoding(Encoding::UTF_8)
else
str
end
Expand Down
6 changes: 3 additions & 3 deletions test/test_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5091,7 +5091,7 @@ def test_file_line_non_literals
def test_bom
assert_parses(
s(:int, 1),
%Q{\xef\xbb\xbf1}.force_encoding(Encoding::BINARY),
%Q{\xef\xbb\xbf1}.dup.force_encoding(Encoding::BINARY),
%q{},
%w(1.9 2.0 2.1))
end
Expand All @@ -5103,7 +5103,7 @@ def test_magic_encoding_comment
s(:send, nil, :puts, s(:lvar, :"проверка"))),
%Q{# coding:koi8-r
\xd0\xd2\xcf\xd7\xc5\xd2\xcb\xc1 = 42
puts \xd0\xd2\xcf\xd7\xc5\xd2\xcb\xc1}.
puts \xd0\xd2\xcf\xd7\xc5\xd2\xcb\xc1}.dup.
force_encoding(Encoding::BINARY),
%q{},
%w(1.9 2.0 2.1))
Expand All @@ -5116,7 +5116,7 @@ def test_regexp_encoding
s(:str, "\\xa8"),
s(:regopt, :n)),
s(:str, "")),
%q{/\xa8/n =~ ""}.force_encoding(Encoding::UTF_8),
%q{/\xa8/n =~ ""}.dup.force_encoding(Encoding::UTF_8),
%{},
SINCE_1_9)
end
Expand Down