Skip to content

Commit

Permalink
[Fix rubocop#3006] Handle merge called on a method on the receiver in…
Browse files Browse the repository at this point in the history
… RedundantMerge (rubocop#3035)
  • Loading branch information
rrosenblum authored and Neodelf committed Oct 15, 2016
1 parent 54ce492 commit da8cda4
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 8 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* [#2314](https://github.com/bbatsov/rubocop/pull/2314): Ignore `UnusedBlockArgument` for keyword arguments. ([@volkert][])
* [#2975](https://github.com/bbatsov/rubocop/issues/2975): Make comment indentation before `)` consistent with comment indentation before `}` or `]`. ([@jonas054][])
* [#3010](https://github.com/bbatsov/rubocop/issues/3010): Fix double reporting/correction of spaces after ternary operator colons (now only reported by `Style/SpaceAroundOperators`, and not `Style/SpaceAfterColon` too). ([@owst][])
* [#3006](https://github.com/bbatsov/rubocop/issues/3006): Handle non-local variables as receivers inside `each_with_object` in `Performance/RedundantMerge`. ([@lumeet][])
* [#3006](https://github.com/bbatsov/rubocop/issues/3006): Register an offense for calling `merge!` on a method on a variable inside `each_with_object` in `Performance/RedundantMerge`. ([@lumeet][], [@rrosenblum][])

### Changes

Expand Down
8 changes: 7 additions & 1 deletion lib/rubocop/cop/performance/redundant_merge.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,13 @@ def each_redundant_merge(node)
end

def value_used_inside_each_with_object?(node, receiver)
return false unless receiver.lvar_type?
while receiver.respond_to?(:send_type?) && receiver.send_type?
receiver, = *receiver
end

unless receiver.respond_to?(:lvar_type?) && receiver.lvar_type?
return false
end

parent = node.parent
grandparent = parent.parent if parent.begin_type?
Expand Down
40 changes: 34 additions & 6 deletions spec/rubocop/cop/performance/redundant_merge_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,40 @@
expect(cop.offenses).to be_empty
end

it 'does not crash when the receiver inside each_object is not a local' \
'variable' do
inspect_source(cop, ['foo.each_with_object(bar) do |f, hash|',
' hash[:a].merge!(b: "")',
'end'])
expect(cop.offenses).to be_empty
it 'autocorrects when receiver uses element reference to the object ' \
'built by each_with_object' do
source = ['foo.each_with_object(bar) do |f, hash|',
' hash[:a].merge!(b: "")',
'end']
new_source = autocorrect_source(cop, source)

expect(new_source).to eq(['foo.each_with_object(bar) do |f, hash|',
' hash[:a][:b] = ""',
'end'].join("\n"))
end

it 'autocorrects when receiver uses multiple element references to the ' \
'object built by each_with_object' do
source = ['foo.each_with_object(bar) do |f, hash|',
' hash[:a][:b].merge!(c: "")',
'end']
new_source = autocorrect_source(cop, source)

expect(new_source).to eq(['foo.each_with_object(bar) do |f, hash|',
' hash[:a][:b][:c] = ""',
'end'].join("\n"))
end

it 'autocorrects merge! called on any method on the object built ' \
'by each_with_object' do
source = ['foo.each_with_object(bar) do |f, hash|',
' hash.bar.merge!(c: "")',
'end']
new_source = autocorrect_source(cop, source)

expect(new_source).to eq(['foo.each_with_object(bar) do |f, hash|',
' hash.bar[:c] = ""',
'end'].join("\n"))
end
end

Expand Down

0 comments on commit da8cda4

Please sign in to comment.