Rubocop: AutoCorrect parsing error and file corruption

[mdesantis]

rubocop --auto-correct reports a parsing error and corrupts the source file with a source file as described below. changing TargetRubyVersion produces the same results (I tried 2.1 and 2.3).

---

Expected behavior

The source code should be properly formatted.

Actual behavior

rubocop --auto-corrects reports a parsing error:

> rubocop --auto-correct test.rb 
Inspecting 1 file
E

Offenses:

test.rb:8:8: E: Lint/Syntax: unexpected token tIDENTIFIER
(Using Ruby 2.1 parser; configure using TargetRubyVersion parameter, under AllCops)
end    er ||= Rails.root.join('spec/support/samples')
       ^^
test.rb:14:49: C: [Corrected] Layout/BlockEndNewline: Expression at 14, 49 should be on its own line.
        ogg: [url_without_extension,  '.ogg'] } end
                                                ^^^
test.rb:17:49: C: [Corrected] Layout/BlockEndNewline: Expression at 17, 49 should be on its own line.
        ogg: [path_without_extension, '.ogg'] } end
                                                ^^^
test.rb:20:1: E: Lint/Syntax: unexpected token kEND
(Using Ruby 2.1 parser; configure using TargetRubyVersion parameter, under AllCops)
end
^^^

1 file inspected, 4 offenses detected, 2 offenses corrected

The source code is corrupted:

# frozen_string_literal: true

require 'spec_helper'

describe Audio, slow: true do
  def media_folder
    @media_
end    er ||= Rails.root.join('spec/support/samples')
  end

  describe '#save' do
    let(:media_type) { 'audio' }
    let(:urls)       do
      { m4a: [url_without_extension,  '.m4a'],
        ogg: [url_without_extension,  '.ogg'] } end
    let(:paths)      do
      { m4a: [path_without_extension, '.m4a'],
        ogg: [path_without_extension, '.ogg'] } end
  end
end

Steps to reproduce the problem

• Create a test.rb file with the following contents:

# frozen_string_literal: true

require 'spec_helper'

describe Audio, slow: true do
  def media_folder
    @media_folder ||= Rails.root.join('spec/support/samples')
  end

  describe '#save' do
    let(:media_type) { 'audio' }
    let(:urls)       do
      { m4a: [url_without_extension,  '.m4a'],
        ogg: [url_without_extension,  '.ogg'] } end
    let(:paths)      do
      { m4a: [path_without_extension, '.m4a'],
        ogg: [path_without_extension, '.ogg'] } end
  end
end

• Run rubocop --auto-correct test.rb
  

  RuboCop version

> rubocop -V
0.52.0 (using Parser 2.4.0.2, running on ruby 2.3.5 x86_64-linux)

[dblackwork]

I've seen the same thing, or something very closely related, in 0.52.1. My test file includes an intentional BlockEndNewline violation (in this particular case the corruption doesn't happen without the violation):

RSpec.describe Thing, type: :model do
  let!(:thing) {
    FactoryGirl.create(:thing) }
end

gets corrupted to this:

RSpec.describe Thing, type: :model do
  let!(
}  hing) {
    FactoryGirl.create(:thing) }
end

with this output from rubocop -a:

.tmp/rubocop_demo.rb:3:1: E: Lint/Syntax: unexpected token tRCURLY
(Using Ruby 2.3 parser; configure using TargetRubyVersion parameter, under AllCops)
}  hing) {
^
.tmp/rubocop_demo.rb:3:32: C: [Corrected] Layout/BlockEndNewline: Expression at 3, 32 should be on its own line.
    FactoryGirl.create(:thing) }
                               ^
.tmp/rubocop_demo.rb:4:32: E: Lint/Syntax: unexpected token tRCURLY
(Using Ruby 2.3 parser; configure using TargetRubyVersion parameter, under AllCops)
    FactoryGirl.create(:thing) }
                               ^

1 file inspected, 3 offenses detected, 1 offense corrected

[dblackwork]

(What seems to be happening is that when it does the replacement, it counts from the beginning of the enclosing block (the do/end), rather than the curly-braces block, and therefore overwrites the wrong substring.)

[dblackwork]

Interim (not PR-worthy) spike of changing block_end_newline.rb -- maybe it will inspire someone with deeper knowledge of the code base to whip up a real fix. (Not impossible that I will do that but this isn't it.) Diff follows.

45c45
<             new_block_end = "\n" + node.loc.end.source + (' ' * indentation)
---
>             new_block_end = "\n" + (' ' * indentation) + node.loc.end.source
65c65
<             node.source.length
---
>             index_of_delimiter_with_whitespaces(node) + node.closing_delimiter.length + 2
70c70
<           node.source =~ /\s*#{node.closing_delimiter}/
---
>           node.loc.end.source_buffer.source =~ /\s*#{node.closing_delimiter}\s*$/

[dblackwork]

And while I'm at it, a failing test:

diff --git a/spec/rubocop/cop/layout/block_end_newline_spec.rb b/spec/rubocop/cop/layout/block_end_newline_spec.rb
index 452ac37..3383b1f 100644
--- a/spec/rubocop/cop/layout/block_end_newline_spec.rb
+++ b/spec/rubocop/cop/layout/block_end_newline_spec.rb
@@ -60,4 +60,22 @@ RSpec.describe RuboCop::Cop::Layout::BlockEndNewline do
                               '}',
                               ''].join("\n"))
   end
+
+  it 'autocorrects a {} block nested in a do/end where the } is not on its own line' do
+    src = <<-RUBY.strip_indent
+      perform do
+        test {
+          foo  }
+      end
+    RUBY
+
+    new_source = autocorrect_source(src)
+
+    expect(new_source).to eq(['perform do',
+                              '  test {',
+                              '    foo',
+                              '  }',
+                              'end',
+                              ''].join("\n"))
+  end
 end

[bquorning]

I believe this issue has been fixed by #5590.