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.

Sign up for GitHub

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

Jump to bottom

Enable frozen string literal #1039

Merged
merged 5 commits into from Sep 28, 2020
Merged

Enable frozen string literal #1039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
83e5654
Avoid unexpected mutation of strings
aliismayilov Sep 10, 2020
47f79fd
Run tests with frozen strings ruby opt
aliismayilov Sep 10, 2020
e5e2b84
Exclude `enable-frozen-string-literal` cases for older rails versions
aliismayilov Sep 10, 2020
37ae2e4
Suppress ruby warning when testing output
aliismayilov Sep 11, 2020
5e1b014
Changing tests to get around a JRuby issue
HamptonMakes Sep 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 40 additions & 1 deletion .travis.yml
View file
Open in desktop
Expand Up @@ -9,6 +9,9 @@ rvm:
- 2.6
- 2.5
- jruby-9.2
env:
- RUBYOPT="--enable-frozen-string-literal"
- RUBYOPT=""
gemfile:
- test/gemfiles/Gemfile.rails-6.0.x
- test/gemfiles/Gemfile.rails-5.2.x
Expand All @@ -26,24 +29,60 @@ matrix:
gemfile: test/gemfiles/Gemfile.rails-4.1.x
- rvm: 2.5
gemfile: test/gemfiles/Gemfile.rails-4.2.x
- rvm: 2.5
gemfile: test/gemfiles/Gemfile.rails-5.0.x
env: RUBYOPT="--enable-frozen-string-literal"
- rvm: 2.5
gemfile: test/gemfiles/Gemfile.rails-5.0.x.erubi
env: RUBYOPT="--enable-frozen-string-literal"
- rvm: 2.5
gemfile: test/gemfiles/Gemfile.rails-5.1.x
env: RUBYOPT="--enable-frozen-string-literal"
- rvm: 2.6
gemfile: test/gemfiles/Gemfile.rails-4.0.x
- rvm: 2.6
gemfile: test/gemfiles/Gemfile.rails-4.1.x
- rvm: 2.6
gemfile: test/gemfiles/Gemfile.rails-4.2.x
- rvm: 2.6
gemfile: test/gemfiles/Gemfile.rails-5.0.x
env: RUBYOPT="--enable-frozen-string-literal"
- rvm: 2.6
gemfile: test/gemfiles/Gemfile.rails-5.0.x.erubi
env: RUBYOPT="--enable-frozen-string-literal"
- rvm: 2.6
gemfile: test/gemfiles/Gemfile.rails-5.1.x
env: RUBYOPT="--enable-frozen-string-literal"
- rvm: 2.7
gemfile: test/gemfiles/Gemfile.rails-4.0.x
- rvm: 2.7
gemfile: test/gemfiles/Gemfile.rails-4.1.x
- rvm: 2.7
gemfile: test/gemfiles/Gemfile.rails-4.2.x
- rvm: 2.7
gemfile: test/gemfiles/Gemfile.rails-5.0.x
env: RUBYOPT="--enable-frozen-string-literal"
- rvm: 2.7
gemfile: test/gemfiles/Gemfile.rails-5.0.x.erubi
env: RUBYOPT="--enable-frozen-string-literal"
- rvm: 2.7
gemfile: test/gemfiles/Gemfile.rails-5.1.x
env: RUBYOPT="--enable-frozen-string-literal"
- rvm: ruby-head
gemfile: test/gemfiles/Gemfile.rails-4.0.x
- rvm: ruby-head
gemfile: test/gemfiles/Gemfile.rails-4.1.x
- rvm: ruby-head
gemfile: test/gemfiles/Gemfile.rails-4.2.x
- rvm: jruby-9.2
gemfile: test/gemfiles/Gemfile.rails-5.0.x
env: RUBYOPT="--enable-frozen-string-literal"
- rvm: jruby-9.2
gemfile: test/gemfiles/Gemfile.rails-5.0.x.erubi
env: RUBYOPT="--enable-frozen-string-literal"
- rvm: jruby-9.2
gemfile: test/gemfiles/Gemfile.rails-5.1.x
env: RUBYOPT="--enable-frozen-string-literal"
include:
- rvm: 2.7
gemfile: test/gemfiles/Gemfile.rails-edge
Expand All @@ -69,4 +108,4 @@ before_install:
- ./cc-test-reporter before-build
script: "bundle exec rake submodules test"
after_script:
- ./cc-test-reporter after-build --exit-code $TRAVIS_TEST_RESULT
- ./cc-test-reporter after-build --exit-code $TRAVIS_TEST_RESULT
2 changes: 1 addition & 1 deletion haml.gemspec
View file
Open in desktop
Expand Up @@ -32,7 +32,7 @@ Gem::Specification.new do |spec|
spec.add_development_dependency 'rbench'
spec.add_development_dependency 'minitest', '>= 4.0'
spec.add_development_dependency 'nokogiri'
spec.add_development_dependency 'simplecov', '0.17.1' # Locked to this version due to https://github.com/codeclimate/test-reporter/issues/418
spec.add_development_dependency 'simplecov'

spec.description = <<-END
Haml (HTML Abstraction Markup Language) is a layer on top of HTML or XML that's
Expand Down
2 changes: 1 addition & 1 deletion lib/haml/util.rb
View file
Open in desktop
Expand Up @@ -213,7 +213,7 @@ def unescape_interpolation(str, escape_html = nil)
scan.scan(/\w+/)
end
content = eval("\"#{interpolated}\"")
content.prepend(char) if char == '@' || char == '$'
content = "#{char}#{content}" if char == '@' || char == '$'
Copy link
Member

Choose a reason for hiding this comment

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

I'd be curious if there are any performance issues here...?

content = "Haml::Helpers.html_escape((#{content}))" if escape_html

res << "\#{#{content}}"
Expand Down
14 changes: 12 additions & 2 deletions test/cases/engine_internals_test.rb
View file
Open in desktop
Expand Up @@ -18,6 +18,10 @@ def test_engine_reflects_defaults
Haml::Options.defaults.replace(defaults)
end

# This test is here due to https://github.com/haml/haml/issues/972
# where we used to trust in the #inspect of `true` and `false` to return
# "true" and "false". This tests isolates and executes a whole ruby interpreter
# to catch the problem.
def test_engine_inspect_monkeypatch
Tempfile.open(['haml-test', '.rb']) do |f|
f.puts <<-HAML
Expand All @@ -37,9 +41,15 @@ def inspect
print Haml::Engine.new('%div{ foo: true, bar: false }').render
HAML
f.close
out = IO.popen([RbConfig.ruby, f.path], &:read)
out = IO.popen([RbConfig.ruby, '-W0', f.path], &:read)
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a documentation note here on why the -W0 is there and what it's used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here the I attempted to set ruby interpreter warning level to 0 (=silence), with hopes that jruby warning wouldn't be printed out. I see that you've already merged the PR, I guess, I can leave it as is.

assert_equal true, $?.success?
assert_equal "<div foo></div>\n", out
# JRuby can sometimes print warnings that are printed to STDOUT
# which can cause a 'polluted' output. Here, instead of looking
# for a perfectly clean output, we ensure the expected output
# is included – but not that in this invocation-style that it's
# perfectly clean. An error that truly broke rendering would
# be caught by many other lines
assert out.include?("<div foo></div>\n")
end
end

Expand Down
2 changes: 1 addition & 1 deletion test/cases/nesting_test.rb
View file
Open in desktop
Expand Up @@ -160,7 +160,7 @@ def test_silent_end_with_stuff
b
a
HTML
- str = "abcde"
- str = +"abcde"
Copy link
Member

Choose a reason for hiding this comment

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

Wow, I had no idea about this feature... it took me some serious digging to learn what it even was. Great work!

- if true
= str.slice!(-1).chr
- end until str.empty?
Expand Down
4 changes: 2 additions & 2 deletions test/cases/ruby_multiline_test.rb
View file
Open in desktop
Expand Up @@ -187,11 +187,11 @@ def test_ruby_multiline_with_punctuated_methods_is_continuation
<p>bar</p>
HTML
= ["bar",
" ".strip!,
" ".strip,
"".empty?,
"bang"].join(", ")
%p foo
%p bar
HAML
end
end
end