Merge pull request #2567 from jonas054/2565_fix_infinite_loop

[Fix #2565] Let SpaceAroundOperators leave => to AlignHash

CHANGELOG.md

@@ -99,6 +99,7 @@
 * [#2546](https://github.com/bbatsov/rubocop/issues/2546): Report when two `rubocop:disable` comments (not the single line kind) for a given cop apppear in a file with no `rubocop:enable` in between. ([@jonas054][])
 * [#2552](https://github.com/bbatsov/rubocop/issues/2552): `Style/Encoding` can auto-correct files with a blank first line. ([@alexdowad][])
 * [#2556](https://github.com/bbatsov/rubocop/issues/2556): `Style/SpecialGlobalVariables` generates auto-config correctly. ([@alexdowad][])
+* [#2565](https://github.com/bbatsov/rubocop/issues/2565): Let `Style/SpaceAroundOperators` leave spacing around `=>` to `Style/AlignHash`. ([@jonas054][])
 
 ### Changes
 

lib/rubocop.rb

@@ -6,6 +6,7 @@
 require 'English'
 require 'set'
 require 'astrolabe/sexp'
+require 'powerpack/array/butfirst'
 require 'powerpack/enumerable/drop_last'
 require 'powerpack/hash/symbolize_keys'
 require 'powerpack/string/blank'
@@ -54,6 +55,7 @@
 require 'rubocop/cop/mixin/empty_lines_around_body'
 require 'rubocop/cop/mixin/end_keyword_alignment'
 require 'rubocop/cop/mixin/first_element_line_break'
+require 'rubocop/cop/mixin/hash_node'
 require 'rubocop/cop/mixin/if_node'
 require 'rubocop/cop/mixin/negative_conditional'
 require 'rubocop/cop/mixin/on_method_def'

lib/rubocop/cli.rb

@@ -100,7 +100,7 @@ def print_cops_of_type(cops, type, show_all)
         puts '# Supports --auto-correct' if cop.new.support_autocorrect?
         puts "#{cop.cop_name}:"
         cnf = @config_store.for(Dir.pwd.to_s).for_cop(cop)
-        puts cnf.to_yaml.lines.to_a[1..-1].map { |line| '  ' + line }
+        puts cnf.to_yaml.lines.to_a.butfirst.map { |line| '  ' + line }
         puts
       end
     end

lib/rubocop/cop/lint/format_parameter_mismatch.rb

@@ -75,7 +75,7 @@ def node_with_splat_args?(node)
 
           _receiver_node, _method_name, *args = *node
 
-          args[1..-1].any? { |arg| arg.type == :splat }
+          args.butfirst.any? { |arg| arg.type == :splat }
         end
 
         def heredoc?(node)

lib/rubocop/cop/mixin/hash_node.rb

@@ -0,0 +1,14 @@
+# encoding: utf-8
+
+module RuboCop
+  module Cop
+    # Common functionality for checking hash nodes.
+    module HashNode
+      def any_pairs_on_the_same_line?(node)
+        node.children.butfirst.any? do |pair|
+          !Util.begins_its_line?(pair.loc.expression)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/style/align_hash.rb

@@ -31,6 +31,8 @@ def deltas(first_pair, current_pair)
         # Common functionality for the styles where not only keys, but also
         # values are aligned.
         class AlignmentOfValues
+          include HashNode # any_pairs_on_the_same_line?
+
           def checkable_layout(node)
             !any_pairs_on_the_same_line?(node) && all_have_same_separator?(node)
           end
@@ -56,15 +58,9 @@ def separator_delta(first_pair, current_separator, key_delta)
             end
           end
 
-          def any_pairs_on_the_same_line?(node)
-            node.children[1..-1].any? do |pair|
-              !Util.begins_its_line?(pair.loc.expression)
-            end
-          end
-
           def all_have_same_separator?(node)
             first_separator = node.children.first.loc.operator.source
-            node.children[1..-1].all? do |pair|
+            node.children.butfirst.all? do |pair|
               pair.loc.operator.is?(first_separator)
             end
           end

lib/rubocop/cop/style/space_around_block_parameters.rb

@@ -75,7 +75,7 @@ def last_end_pos_inside_pipes(pos)
         end
 
         def check_each_arg(args)
-          args.children[1..-1].each do |arg|
+          args.children.butfirst.each do |arg|
             expr = arg.loc.expression
             check_no_space(range_with_surrounding_space(expr, :left).begin_pos,
                            expr.begin_pos - 1, 'Extra space before')

lib/rubocop/cop/style/space_around_operators.rb

@@ -7,12 +7,17 @@ module Style
       # which should not have surrounding space.
       class SpaceAroundOperators < Cop
         include PrecedingFollowingAlignment
+        include HashNode # any_pairs_on_the_same_line?
 
         def on_pair(node)
-          if node.loc.operator.is?('=>')
-            _, right = *node
-            check_operator(node.loc.operator, right.loc.expression)
-          end
+          return unless node.loc.operator.is?('=>')
+
+          align_hash_config = config.for_cop('Style/AlignHash')
+          return if align_hash_config['EnforcedHashRocketStyle'] == 'table' &&
+                    !any_pairs_on_the_same_line?(node.parent)
+
+          _, right = *node
+          check_operator(node.loc.operator, right.loc.expression)
         end
 
         def on_if(node)

spec/rubocop/cli_spec.rb

@@ -106,6 +106,42 @@ def abs(path)
         expect(IO.read('example.rb')).to eq(source.join("\n") + "\n")
       end
 
+      it 'does not correct SpaceAroundOperators in a hash that would be ' \
+         'changed back' do
+        create_file('.rubocop.yml', ['Style/HashSyntax:',
+                                     '  EnforcedStyle: hash_rockets',
+                                     '',
+                                     'Style/AlignHash:',
+                                     '  EnforcedHashRocketStyle: table'])
+        source = ['a = { 1=>2, a => b }',
+                  'hash = {',
+                  '  :alice => {',
+                  '    :age  => 23,',
+                  "    :role => 'Director'",
+                  '  },',
+                  '  :bob   => {',
+                  '    :age  => 25,',
+                  "    :role => 'Consultant'",
+                  '  }',
+                  '}']
+        create_file('example.rb', source)
+        expect(cli.run(['--auto-correct'])).to eq(1)
+
+        # 1=>2 is changed to 1 => 2. The rest is unchanged.
+        # SpaceAroundOperators leaves it to AlignHash when the style is table.
+        expect(IO.read('example.rb')).to eq(['a = { 1 => 2, a => b }',
+                                             'hash = {',
+                                             '  :alice => {',
+                                             '    :age  => 23,',
+                                             "    :role => 'Director'",
+                                             '  },',
+                                             '  :bob   => {',
+                                             '    :age  => 25,',
+                                             "    :role => 'Consultant'",
+                                             '  }',
+                                             '}'].join("\n") + "\n")
+      end
+
       it 'corrects IndentationWidth, RedundantBegin, and ' \
          'RescueEnsureAlignment offenses' do
         source = ['def verify_section',

spec/rubocop/cop/style/space_around_operators_spec.rb

@@ -2,10 +2,13 @@
 
 require 'spec_helper'
 
-describe RuboCop::Cop::Style::SpaceAroundOperators, :config do
+describe RuboCop::Cop::Style::SpaceAroundOperators do
   subject(:cop) { described_class.new(config) }
-
-  let(:cop_config) { {} }
+  let(:config) do
+    RuboCop::Config
+      .new('Style/AlignHash' => { 'EnforcedHashRocketStyle' => hash_style })
+  end
+  let(:hash_style) { 'key' }
 
   it 'accepts operator surrounded by tabs' do
     inspect_source(cop, "a\t+\tb")
@@ -279,10 +282,52 @@ def check_modifier(keyword)
         ['Surrounding space missing for operator `=`.'])
     end
 
-    it 'registers an offense for a hash rocket without spaces' do
-      inspect_source(cop, '{ 1=>2, a: b }')
-      expect(cop.messages).to eq(
-        ['Surrounding space missing for operator `=>`.'])
+    context 'when a hash literal is on a single line' do
+      before(:each) { inspect_source(cop, '{ 1=>2, a: b }') }
+
+      context 'and Style/AlignHash:EnforcedHashRocketStyle is key' do
+        let(:hash_style) { 'key' }
+
+        it 'registers an offense for a hash rocket without spaces' do
+          expect(cop.messages)
+            .to eq(['Surrounding space missing for operator `=>`.'])
+        end
+      end
+
+      context 'and Style/AlignHash:EnforcedHashRocketStyle is table' do
+        let(:hash_style) { 'table' }
+
+        it 'registers an offense for a hash rocket without spaces' do
+          expect(cop.messages)
+            .to eq(['Surrounding space missing for operator `=>`.'])
+        end
+      end
+    end
+
+    context 'when a hash literal is on multiple lines' do
+      before(:each) do
+        inspect_source(cop, ['{',
+                             '  1=>2,',
+                             '  a: b',
+                             '}'])
+      end
+
+      context 'and Style/AlignHash:EnforcedHashRocketStyle is key' do
+        let(:hash_style) { 'key' }
+
+        it 'registers an offense for a hash rocket without spaces' do
+          expect(cop.messages)
+            .to eq(['Surrounding space missing for operator `=>`.'])
+        end
+      end
+
+      context 'and Style/AlignHash:EnforcedHashRocketStyle is table' do
+        let(:hash_style) { 'table' }
+
+        it "doesn't register an offense for a hash rocket without spaces" do
+          expect(cop.offenses).to be_empty
+        end
+      end
     end
 
     it 'registers an offense for match operators without space' do