Skip to content

Commit

Permalink
[Fix rubocop#2464] Style/ParallelAssignment understands aref/attribut…
Browse files Browse the repository at this point in the history
…e assignments
  • Loading branch information
alexdowad committed Dec 10, 2015
1 parent ed9e46b commit 8779ee2
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
* [#2471](https://github.com/bbatsov/rubocop/issues/2471): `Style/MethodName` doesn't choke on methods which are defined inside methods. ([@alexdowad][])
* [#2449](https://github.com/bbatsov/rubocop/issues/2449): `Style/StabbyLambdaParentheses` only checks lambdas in the arrow form. ([@lumeet][])
* [#2456](https://github.com/bbatsov/rubocop/issues/2456): `Lint/NestedMethodDefinition` doesn't register offenses for method definitions inside an eval block (either `instance_eval`, `class_eval`, or `module_eval`). ([@alexdowad][])
* [#2464](https://github.com/bbatsov/rubocop/issues/2464): `Style/ParallelAssignment` understands aref and attribute assignments, and doesn't warn if they can't be correctly rearranged into a series of single assignments. ([@alexdowad][])

### Changes

Expand Down
19 changes: 18 additions & 1 deletion lib/rubocop/cop/style/parallel_assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class AssignmentSorter

def_node_matcher :var_name, '{(casgn _ $_) (_ $_)}'
def_node_search :uses_var?, '{({lvar ivar cvar gvar} %) (const _ %)}'
def_node_search :matching_calls, '(send %1 %2 $...)'

def initialize(assignments)
@assignments = assignments
Expand All @@ -115,7 +116,23 @@ def tsort_each_child(assignment)

@assignments.each do |other|
_other_lhs, other_rhs = *other
yield other if uses_var?(other_rhs, var_name(my_lhs))
if ((var = var_name(my_lhs)) && uses_var?(other_rhs, var)) ||
(my_lhs.asgn_method_call? && accesses?(other_rhs, my_lhs))
yield other
end
end
end

# `lhs` is an assignment method call like `obj.attr=` or `ary[idx]=`.
# Does `rhs` access the same value which is assigned by `lhs`?
def accesses?(rhs, lhs)
if lhs.method_name == :[]=
matching_calls(rhs, lhs.receiver, :[]).any? do |args|
args == lhs.method_args
end
else
access_method = lhs.method_name.to_s.chop.to_sym
matching_calls(rhs, lhs.receiver, access_method).any?
end
end
end
Expand Down
17 changes: 11 additions & 6 deletions spec/rubocop/cop/style/parallel_assignment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,12 @@
it_behaves_like('offenses', ['if true',
' a, b = 1, 2',
'end'].join("\n"))
it_behaves_like('offenses',
'a, b = Float::INFINITY, Float::INFINITY')
it_behaves_like('offenses',
'Float::INFINITY, Float::INFINITY = 1, 2')
it_behaves_like('offenses', 'a, b = Float::INFINITY, Float::INFINITY')
it_behaves_like('offenses', 'Float::INFINITY, Float::INFINITY = 1, 2')
it_behaves_like('offenses', 'a[0], a[1] = a[1], a[2]')
it_behaves_like('offenses', 'obj.attr1, obj.attr2 = obj.attr3, obj.attr1')
it_behaves_like('offenses', 'obj.attr1, ary[0] = ary[1], obj.attr1')
it_behaves_like('offenses', 'a[0], a[1] = a[1], b[0]')

shared_examples('allowed') do |source|
it "allows assignment of: #{source}" do
Expand Down Expand Up @@ -85,8 +87,11 @@
'a, b, c = foo'].join("\n"))
it_behaves_like('allowed', ['array = [1, 2, 3]',
'a, = array'].join("\n"))
it_behaves_like('allowed',
'a, b = Float::INFINITY')
it_behaves_like('allowed', 'a, b = Float::INFINITY')
it_behaves_like('allowed', 'a[0], a[1] = a[1], a[0]')
it_behaves_like('allowed', 'obj.attr1, obj.attr2 = obj.attr2, obj.attr1')
it_behaves_like('allowed', 'obj.attr1, ary[0] = ary[0], obj.attr1')
it_behaves_like('allowed', 'ary[0], ary[1], ary[2] = ary[1], ary[2], ary[0]')

it 'hightlights the entire expression' do
inspect_source(cop, 'a, b = 1, 2')
Expand Down

0 comments on commit 8779ee2

Please sign in to comment.