Skip to content

Commit

Permalink
Merge pull request #162 from MrJoy/fix_file_rewriting_with_rewriters
Browse files Browse the repository at this point in the history
`ruby-rewrite` fails to actually rewrite the input file when a rewriter is loaded.  This fixes the issue.
  • Loading branch information
whitequark committed Aug 22, 2014
2 parents 56ecc86 + b621b2b commit 21ce3d5
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lib/parser/runner/ruby_rewrite.rb
Expand Up @@ -43,6 +43,7 @@ def load_and_discover(file)

def process(initial_buffer)
buffer = initial_buffer
original_name = buffer.name

@rewriters.each do |rewriter_class|
@parser.reset
Expand Down Expand Up @@ -81,8 +82,8 @@ def process(initial_buffer)
buffer = new_buffer
end

if File.exist?(buffer.name)
File.open(buffer.name, 'w') do |file|
if File.exist?(original_name)
File.open(original_name, 'w') do |file|
file.write buffer.source
end
else
Expand Down
3 changes: 3 additions & 0 deletions test/bug_163/fixtures/input.rb
@@ -0,0 +1,3 @@
if(true)
puts "Hello, world!"
end
3 changes: 3 additions & 0 deletions test/bug_163/fixtures/output.rb
@@ -0,0 +1,3 @@
if true
puts "Hello, world!"
end
18 changes: 18 additions & 0 deletions test/bug_163/rewriter.rb
@@ -0,0 +1,18 @@
class Rewriter < Parser::Rewriter
def on_if(node)
# Crude, totally-not-usable-in-the-real-world code to remove optional
# parens from control keywords.
#
# In a perfect test scenario we'd simply make this a no-op, to demonstrate
# that the bug happens when any rewriter is loaded regardless of whether it
# actually changes anything but that makes assertions much harder to get
# right. It's much easier to just show that the file did, or did not
# get changed.
if node.children[0].type == :begin
replace node.children[0].loc.begin, ' '
remove node.children[0].loc.end
end

super
end
end
35 changes: 35 additions & 0 deletions test/bug_163/test_runner_rewrite.rb
@@ -0,0 +1,35 @@
require 'pathname'
require 'fileutils'
require 'shellwords'

BASE_DIR = Pathname.new(__FILE__) + '../..'
require (BASE_DIR + 'helper').expand_path

class TestRunnerRewrite < Minitest::Test
def setup
@ruby_rewrite = BASE_DIR.expand_path + '../bin/ruby-rewrite'
@test_dir = BASE_DIR + 'bug_163'
@fixtures_dir = @test_dir + 'fixtures'
end

def test_rewriter
Dir.mktmpdir(nil, BASE_DIR.expand_path.to_s) do |tmp_dir|
tmp_dir = Pathname.new(tmp_dir)
sample_file = tmp_dir + 'bug_163.rb'
sample_file_expanded = sample_file.expand_path
expected_file = @fixtures_dir + 'output.rb'

FileUtils.cp(@fixtures_dir + 'input.rb', tmp_dir + 'bug_163.rb')
FileUtils.cd @test_dir do
exit_code = system %Q{
#{Shellwords.escape(@ruby_rewrite.to_s)} --modify \
-l rewriter.rb \
#{Shellwords.escape(sample_file_expanded.to_s)}
}
end

assert File.read(expected_file.expand_path) == File.read(sample_file),
"#{sample_file} should be identical to #{expected_file}"
end
end
end
1 change: 1 addition & 0 deletions test/helper.rb
@@ -1,4 +1,5 @@
require 'tempfile'
require 'minitest/test'

require 'simplecov'
require 'coveralls'
Expand Down

0 comments on commit 21ce3d5

Please sign in to comment.