[Fix rubocop#3750] Account for an assignment that spans multiple line…

…s in ConditionalAssignment
rrosenblum · Dec 5, 2016 · 04ef6f7 · 04ef6f7
1 parent e37fa45
commit 04ef6f7
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 2 deletions.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -13,7 +13,7 @@ Metrics/AbcSize:
 # Offense count: 33
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 173
+  Max: 176
 
 # Offense count: 30
 Metrics/CyclomaticComplexity:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@
 
 * [#3751](https://github.com/bbatsov/rubocop/pull/3751): Avoid crash in `Rails/EnumUniqueness` cop. ([@pocke][])
 * [#3766](https://github.com/bbatsov/rubocop/pull/3766): Avoid crash in `Style/ConditionalAssignment` cop with masgn. ([@pocke][])
+* [#3750](https://github.com/bbatsov/rubocop/issues/3750): Register an offense in `Style/ConditionalAssignment` when the assignment spans multiple lines. ([@rrosenblum][])
 
 ## 0.46.0 (2016-11-30)
 

diff --git a/lib/rubocop/cop/style/conditional_assignment.rb b/lib/rubocop/cop/style/conditional_assignment.rb
@@ -410,7 +410,10 @@ def longest_line(node, assignment)
         end
 
         def longest_rhs(branches)
-          branches.map { |branch| branch.children.last.source.length }.max
+          line_lengths = branches.flat_map do |branch|
+            branch.children.last.source.split("\n").map(&:length)
+          end
+          line_lengths.max
         end
 
         def line_length_cop_enabled?

diff --git a/spec/rubocop/cop/style/conditional_assignment_assign_to_condition_spec.rb b/spec/rubocop/cop/style/conditional_assignment_assign_to_condition_spec.rb
@@ -588,6 +588,61 @@
     expect(cop.messages).to eq([described_class::MSG])
   end
 
+  it 'registers an offense for assignment in if else when the assignment ' \
+    'spans multiple lines' do
+    source = ['if foo',
+              '  foo = {',
+              '    a: 1,',
+              '    b: 2,',
+              '    c: 2,',
+              '    d: 2,',
+              '    e: 2,',
+              '    f: 2,',
+              '    g: 2,',
+              '    h: 2',
+              '  }',
+              'else',
+              '  foo = { }',
+              'end']
+    inspect_source(cop, source)
+
+    expect(cop.messages).to eq([described_class::MSG])
+  end
+
+  it 'autocorrects assignment in if else when the assignment ' \
+    'spans multiple lines' do
+    source = ['if foo',
+              '  foo = {',
+              '    a: 1,',
+              '    b: 2,',
+              '    c: 2,',
+              '    d: 2,',
+              '    e: 2,',
+              '    f: 2,',
+              '    g: 2,',
+              '    h: 2',
+              '  }',
+              'else',
+              '  foo = { }',
+              'end']
+    new_source = autocorrect_source(cop, source)
+
+    expect(new_source).to eq(['foo = if foo',
+                              '  {',
+                              '    a: 1,',
+                              '    b: 2,',
+                              '    c: 2,',
+                              '    d: 2,',
+                              '    e: 2,',
+                              '    f: 2,',
+                              '    g: 2,',
+                              '    h: 2',
+                              '  }',
+                              'else',
+                              '  { }',
+                              '      end'].join("\n"))
+  end
+
   context 'assignment as the last statement' do
     it 'allows more than variable assignment in if else' do
       source = ['if foo',