Skip to content

Commit

Permalink
Metrics/AbcSize: Fix assignment counts for various cases.
Browse files Browse the repository at this point in the history
`var += ...` was counted twice
`obj.method += ...` was not counted
`Const ||= ...` was crashing [#8742]
  • Loading branch information
marcandre committed Sep 20, 2020
1 parent 66f8c07 commit fbf1966
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@
* [#8730](https://github.com/rubocop-hq/rubocop/issues/8730): Fix an error for `Lint/UselessTimes` when there is a blank line in the method definition. ([@koic][])
* [#8740](https://github.com/rubocop-hq/rubocop/issues/8740): Fix a false positive for `Style/HashAsLastArrayItem` when the hash is in an implicit array. ([@dvandersluis][])
* [#8739](https://github.com/rubocop-hq/rubocop/issues/8739): Fix an error for `Lint/UselessTimes` when using empty block argument. ([@koic][])
* [#8742](https://github.com/rubocop-hq/rubocop/issues/8742): Fix some assignment counts for `Metrics/AbcSize`. ([@marcandre][])

### Changes

Expand Down
35 changes: 25 additions & 10 deletions lib/rubocop/cop/metrics/utils/abc_size_calculator.rb
Expand Up @@ -81,21 +81,37 @@ def else_branch?(node)
private

def assignment?(node)
return compound_assignment(node) if node.masgn_type? || node.shorthand_asgn?

node.for_type? ||
node.op_asgn_type? ||
(node.respond_to?(:setter_method?) && node.setter_method?) ||
(simple_assignment?(node) && capturing_variable?(node.children.first))
simple_assignment?(node) ||
argument?(node)
end

def simple_assignment?(node)
return false if node.masgn_type?
def compound_assignment(node)
# Methods setter can not be detected for multiple assignments
# and shorthand assigns, so we'll count them here instead
children = node.masgn_type? ? node.children[0].children : node.children

if node.equals_asgn?
reset_on_lvasgn(node) if node.lvasgn_type?
return true
will_be_miscounted = children.count do |child|
child.respond_to?(:setter_method?) &&
!child.setter_method?
end
@assignment += will_be_miscounted

false
end

argument?(node)
def simple_assignment?(node)
if !node.equals_asgn?
false
elsif node.lvasgn_type?
reset_on_lvasgn(node)
capturing_variable?(node.children.first)
else
true
end
end

def capturing_variable?(name)
Expand All @@ -106,9 +122,8 @@ def branch?(node)
BRANCH_NODES.include?(node.type)
end

# TODO: move to rubocop-ast
def argument?(node)
ARGUMENT_TYPES.include?(node.type)
ARGUMENT_TYPES.include?(node.type) && capturing_variable?(node.children.first)
end

def condition?(node)
Expand Down
53 changes: 53 additions & 0 deletions spec/rubocop/cop/metrics/utils/abc_size_calculator_spec.rb
Expand Up @@ -16,6 +16,27 @@ def method_name
it { is_expected.to eq '<0, 3, 0>' }
end

context 'with +=' do
let(:source) { <<~RUBY }
def method_name
x = nil
x += 1
end
RUBY

it { is_expected.to eq '<2, 0, 0>' }
end

context 'with += for setters' do
let(:source) { <<~RUBY }
def method_name
foo.bar += 1
end
RUBY

it { is_expected.to eq '<1, 2, 0>' }
end

context 'with ||=' do
let(:source) { <<~RUBY }
def method_name
Expand All @@ -27,6 +48,16 @@ def method_name
it { is_expected.to eq '<2, 0, 1>' }
end

context 'with ||= on a constant' do
let(:source) { <<~RUBY }
def method_name
self::FooModule ||= Mod
end
RUBY

it { is_expected.to eq '<1, 0, 1>' }
end

context 'with &&=' do
let(:source) { <<~RUBY }
def method_name
Expand Down Expand Up @@ -142,6 +173,28 @@ def method_name
it { is_expected.to eq '<3, 1, 0>' }
end

context 'multiple assignment with method setters' do
let(:source) { <<~RUBY }
def method_name
self.a, foo.b, bar[42] = nil
end
RUBY

it { is_expected.to eq '<3, 5, 0>' }
end

context 'equivalent to multiple assignment with method setters' do
let(:source) { <<~RUBY }
def method_name
self.a = nil # 1, 1, 0
foo.b = nil # 1, +2, 0
bar[42] = nil # 1, +2, 0
end
RUBY

it { is_expected.to eq '<3, 5, 0>' }
end

context 'if and arithmetic operations' do
let(:source) { <<~RUBY }
def method_name
Expand Down

0 comments on commit fbf1966

Please sign in to comment.