Merge pull request #1078 from barunio/block_alignment_autocorrect

Add auto-correct to BlockAlignment cop

CHANGELOG.md

@@ -14,6 +14,8 @@
 * `Debugger` cop now checks for `binding.pry_remote`. ([@yous][])
 * [#1238](https://github.com/bbatsov/rubocop/issues/1238): Add `MinBodyLength` option to `Next` cop. ([@bbatsov][])
 * [#1241](https://github.com/bbatsov/rubocop/issues/1241): `TrailingComma` cop does auto-correction. ([@yous][])
+* [#1078](https://github.com/bbatsov/rubocop/pull/1078): New cop `BlockEndNewline` checks if the end statement of a multiline block is on its own line. ([@barunio][])
+* [#1078](https://github.com/bbatsov/rubocop/pull/1078): `BlockAlignment` cop does auto-correction. ([@barunio][])
 
 ### Changes
 

config/enabled.yml

@@ -62,6 +62,10 @@ Style/BlockComments:
   Description: 'Do not use block comments.'
   Enabled: true
 
+Style/BlockEndNewline:
+  Description: 'Put end statement of multiline block on its own line.'
+  Enabled: true
+
 Style/Blocks:
   Description: >-
                 Avoid using {...} for multi-line blocks (multiline chaining is

lib/rubocop.rb

@@ -122,6 +122,7 @@
 require 'rubocop/cop/style/bare_percent_literals'
 require 'rubocop/cop/style/begin_block'
 require 'rubocop/cop/style/block_comments'
+require 'rubocop/cop/style/block_end_newline'
 require 'rubocop/cop/style/blocks'
 require 'rubocop/cop/style/braces_around_hash_parameters'
 require 'rubocop/cop/style/case_equality'

lib/rubocop/cop/lint/block_alignment.rb

@@ -119,7 +119,7 @@ def check_block_alignment(start_node, block_node)
           indentation_of_do_line = match.begin(0)
           return unless end_loc.column != indentation_of_do_line
 
-          add_offense(nil,
+          add_offense(block_node,
                       end_loc,
                       format(MSG, end_loc.line, end_loc.column,
                              start_loc.source.lines.to_a.first.chomp,
@@ -147,6 +147,29 @@ def already_processed_node?(node)
         def block_is_on_next_line?(begin_node, block_node)
           begin_node.loc.line != block_node.loc.line
         end
+
+        def autocorrect(node)
+          key = node.children.first
+          source = node.loc.expression.source_buffer
+
+          @corrections << lambda do |corrector|
+            start_col = key.loc.expression.column
+            starting_position_of_block_end = node.loc.end.begin_pos
+            end_col = node.loc.end.column
+
+            if end_col < start_col
+              delta = start_col - end_col
+              corrector.insert_before(node.loc.end, ' ' * delta)
+            elsif end_col > start_col
+              delta = start_col - end_col
+              range_start = starting_position_of_block_end + delta
+              range_end = range_start - delta
+
+              range = Parser::Source::Range.new(source, range_start, range_end)
+              corrector.remove(range)
+            end
+          end
+        end
       end
     end
   end

lib/rubocop/cop/style/block_end_newline.rb

@@ -0,0 +1,56 @@
+# encoding: utf-8
+
+module RuboCop
+  module Cop
+    module Style
+      # This cop checks whether the end statement of a do end blocks
+      # is on its own line.
+      #
+      # @example
+      #   # bad
+      #   blah do |i|
+      #     foo(i) end
+      #
+      #   # good
+      #   blah do |i|
+      #     foo(i)
+      #   end
+      #
+      #   # bad
+      #   blah { |i|
+      #     foo(i) }
+      #
+      #   # good
+      #   blah { |i|
+      #     foo(i)
+      #   }
+      class BlockEndNewline < Cop
+        MSG = 'Expression at %d, %d should be on its own line.'
+
+        def on_block(node)
+          end_loc = node.loc.end
+          do_loc = node.loc.begin # Actually it's either do or {.
+          return if do_loc.line == end_loc.line # Ignore one-liners.
+
+          # If the end is on its own line, there is no offense
+          return if /^\s*#{end_loc.source}/.match(end_loc.source_line)
+
+          msg = format(MSG, end_loc.line, end_loc.column + 1)
+          add_offense(node, end_loc, msg)
+        end
+
+        def autocorrect(node)
+          @corrections << lambda do |corrector|
+            indentation = indentation_of_block_start_line(node)
+            corrector.insert_before(node.loc.end, "\n" + (' ' * indentation))
+          end
+        end
+
+        def indentation_of_block_start_line(node)
+          match = /\S.*/.match(node.loc.begin.source_line)
+          match.begin(0)
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/lint/block_alignment_spec.rb

@@ -6,13 +6,44 @@
 describe RuboCop::Cop::Lint::BlockAlignment do
   subject(:cop) { described_class.new }
 
-  it 'registers an offense for mismatched block end' do
-    inspect_source(cop,
-                   ['test do |ala|',
-                    '  end'
-                   ])
-    expect(cop.messages)
-      .to eq(['`end` at 2, 2 is not aligned with `test do |ala|` at 1, 0'])
+  context 'when the block has no arguments' do
+    it 'registers an offense for mismatched block end' do
+      inspect_source(cop,
+                     ['test do',
+                      '  end'
+                     ])
+      expect(cop.messages)
+        .to eq(['`end` at 2, 2 is not aligned with `test do` at 1, 0'])
+    end
+
+    it 'auto-corrects alignment' do
+      new_source = autocorrect_source(cop, ['test do',
+                                            '  end'
+                                           ])
+
+      expect(new_source).to eq(['test do',
+                                'end'].join("\n"))
+    end
+  end
+
+  context 'when the block has arguments' do
+    it 'registers an offense for mismatched block end' do
+      inspect_source(cop,
+                     ['test do |ala|',
+                      '  end'
+                     ])
+      expect(cop.messages)
+        .to eq(['`end` at 2, 2 is not aligned with `test do |ala|` at 1, 0'])
+    end
+
+    it 'auto-corrects alignment' do
+      new_source = autocorrect_source(cop, ['test do |ala|',
+                                            '  end'
+                                           ])
+
+      expect(new_source).to eq(['test do |ala|',
+                                'end'].join("\n"))
+    end
   end
 
   it 'acepts a block end that does not begin its line' do
@@ -61,6 +92,17 @@
                      ])
       expect(cop.offenses).to be_empty
     end
+
+    it 'auto-corrects alignment to the block start' do
+      new_source = autocorrect_source(cop,
+                                      ['a = b = c = test do |ala|',
+                                       '    end'
+                                      ])
+
+      expect(new_source).to eq(['a = b = c = test do |ala|',
+                                '            end'
+                               ].join("\n"))
+    end
   end
 
   context 'and the block is an operand' do
@@ -105,6 +147,23 @@
         .to eq(['`end` at 4, 0 is not aligned with `a_long_method_that_dont_fit_on_the_line ' \
                 'do |v|` at 2, 2'])
     end
+
+    it 'auto-corrects alignment' do
+      new_source = autocorrect_source(
+        cop,
+        ['variable =',
+         '  a_long_method_that_dont_fit_on_the_line do |v|',
+         '    v.foo',
+         'end'
+        ])
+
+      expect(new_source)
+        .to eq(['variable =',
+                '  a_long_method_that_dont_fit_on_the_line do |v|',
+                '    v.foo',
+                '  end'
+               ].join("\n"))
+    end
   end
 
   context 'when the method part is a call chain that spans several lines' do
@@ -181,6 +240,36 @@
       inspect_source(cop, src)
       expect(cop.offenses).to be_empty
     end
+
+    it 'auto-corrects misaligned ends with the start of the expression' do
+      src = ['def foo(bar)',
+             '  bar.get_stuffs',
+             '      .reject do |stuff|',
+             '        stuff.with_a_very_long_expression_that_doesnt_fit_the_line',
+             '        end.select do |stuff|',
+             '        stuff.another_very_long_expression_that_doesnt_fit_the_line',
+             '    end',
+             '      .select do |stuff|',
+             '        stuff.another_very_long_expression_that_doesnt_fit_the_line',
+             '        end',
+             'end']
+
+      aligned_src = [
+        'def foo(bar)',
+        '  bar.get_stuffs',
+        '      .reject do |stuff|',
+        '        stuff.with_a_very_long_expression_that_doesnt_fit_the_line',
+        '  end.select do |stuff|',
+        '        stuff.another_very_long_expression_that_doesnt_fit_the_line',
+        '  end',
+        '      .select do |stuff|',
+        '        stuff.another_very_long_expression_that_doesnt_fit_the_line',
+        '  end',
+        'end'].join("\n")
+
+      new_source = autocorrect_source(cop, src)
+      expect(new_source).to eq(aligned_src)
+    end
   end
 
   context 'when variables of a mass assignment spans several lines' do
@@ -202,6 +291,16 @@
       expect(cop.messages)
         .to eq(['`end` at 4, 4 is not aligned with `e,` at 1, 0 or `f = [5, 6].map do |i|` at 2, 0'])
     end
+
+    it 'can not auto-correct' do
+      src = ['e,',
+             'f = [5, 6].map do |i|',
+             '  i - 5',
+             '    end']
+
+      new_source = autocorrect_source(cop, src)
+      expect(new_source).to eq(src.join("\n"))
+    end
   end
 
   it 'accepts end aligned with an instance variable' do

spec/rubocop/cop/style/block_end_newline_spec.rb

@@ -0,0 +1,61 @@
+# encoding: utf-8
+
+require 'spec_helper'
+
+describe RuboCop::Cop::Style::BlockEndNewline do
+  subject(:cop) { described_class.new }
+
+  it 'does not register an offense for a one-liner' do
+    inspect_source(cop, ['test do foo end'])
+    expect(cop.messages).to be_empty
+  end
+
+  it 'does not register an offense for multiline blocks with newlines before '\
+     'the end' do
+    inspect_source(cop,
+                   ['test do',
+                    '  foo',
+                    'end'])
+    expect(cop.messages).to be_empty
+  end
+
+  it 'registers an offense when multiline block end is not on its own line' do
+    inspect_source(cop,
+                   ['test do',
+                    '  foo end'
+                   ])
+    expect(cop.messages)
+      .to eq(['Expression at 2, 7 should be on its own line.'])
+  end
+
+  it 'registers an offense when multiline block } is not on its own line' do
+    inspect_source(cop,
+                   ['test {',
+                    '  foo }'
+                   ])
+    expect(cop.messages)
+      .to eq(['Expression at 2, 7 should be on its own line.'])
+  end
+
+  it 'autocorrects a do/end block where the end is not on its own line' do
+    src =  ['test do',
+            '  foo end']
+
+    new_source = autocorrect_source(cop, src)
+
+    expect(new_source).to eq(['test do',
+                              '  foo ',
+                              'end'].join("\n"))
+  end
+
+  it 'autocorrects a {} block where the } is not on its own line' do
+    src =  ['test {',
+            '  foo }']
+
+    new_source = autocorrect_source(cop, src)
+
+    expect(new_source).to eq(['test {',
+                              '  foo ',
+                              '}'].join("\n"))
+  end
+end