Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweaks to Metrics/PerceivedComplexity #8204

Merged
merged 3 commits into from Jul 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Expand Up @@ -32,8 +32,9 @@
* [#8376](https://github.com/rubocop-hq/rubocop/pull/8376): `Style/MethodMissingSuper` cop is removed in favor of new `Lint/MissingSuper` cop. ([@fatkodima][])
* [#8350](https://github.com/rubocop-hq/rubocop/pull/8350): Set default max line length to 120 for `Style/MultilineMethodSignature`. ([@koic][])
* [#8338](https://github.com/rubocop-hq/rubocop/pull/8338): **potentially breaking**. Config#for_department now returns only the config specified for that department; the 'Enabled' attribute is no longer calculated. ([@marcandre][])
* [#8037](https://github.com/rubocop-hq/rubocop/pull/8037): **(Breaking)** Cop `Metrics/AbcSize` now counts ||=, &&=, multiple assignments, for, yield, iterating blocks. `&.` now count as conditions too (unless repeated on the same variable). Default bumped from 15 to 16.5. ([@marcandre][])
* [#8037](https://github.com/rubocop-hq/rubocop/pull/8037): **(Breaking)** Cop `Metrics/AbcSize` now counts ||=, &&=, multiple assignments, for, yield, iterating blocks. `&.` now count as conditions too (unless repeated on the same variable). Default bumped from 15 to 17. Consider using `rubocop -a --disable-uncorrectable` to ease transition. ([@marcandre][])
* [#8276](https://github.com/rubocop-hq/rubocop/issues/8276): Cop `Metrics/CyclomaticComplexity` not longer counts `&.` when repeated on the same variable. ([@marcandre][])
* [#8204](https://github.com/rubocop-hq/rubocop/pull/8204): **(Breaking)** Cop `Metrics/PerceivedComplexity` now counts `else` in `case` statements, `&.`, `||=`, `&&=` and blocks known to iterate. Default bumped from 7 to 8. Consider using `rubocop -a --disable-uncorrectable` to ease transition. ([@marcandre][])

## 0.88.0 (2020-07-13)

Expand Down
2 changes: 1 addition & 1 deletion config/default.yml
Expand Up @@ -2015,7 +2015,7 @@ Metrics/PerceivedComplexity:
VersionAdded: '0.25'
VersionChanged: '0.81'
IgnoredMethods: []
Max: 7
Max: 8

################## Migration #############################

Expand Down
2 changes: 1 addition & 1 deletion docs/modules/ROOT/pages/cops_metrics.adoc
Expand Up @@ -472,6 +472,6 @@ end # 7 complexity points
| Array

| Max
| `7`
| `8`
| Integer
|===
4 changes: 2 additions & 2 deletions lib/rubocop/cop/lint/redundant_cop_disable_directive.rb
Expand Up @@ -135,7 +135,7 @@ def each_already_disabled(line_ranges, disabled_ranges, comments)
end
end

# rubocop:todo Metrics/CyclomaticComplexity
# rubocop:todo Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def find_redundant(comment, offenses, cop, line_range, next_line_range)
if all_disabled?(comment)
# If there's a disable all comment followed by a comment
Expand All @@ -153,7 +153,7 @@ def find_redundant(comment, offenses, cop, line_range, next_line_range)
cop if cop_offenses.none? { |o| line_range.cover?(o.line) }
end
end
# rubocop:enable Metrics/CyclomaticComplexity
# rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity

def all_disabled?(comment)
/rubocop\s*:\s*(?:disable|todo)\s+all\b/.match?(comment.text)
Expand Down
15 changes: 7 additions & 8 deletions lib/rubocop/cop/metrics/perceived_complexity.rb
Expand Up @@ -26,13 +26,11 @@ module Metrics
# do_something until a && b # 2
# end # ===
# end # 7 complexity points
class PerceivedComplexity < Base
include MethodComplexity

class PerceivedComplexity < CyclomaticComplexity
MSG = 'Perceived complexity for %<method>s is too high. ' \
'[%<complexity>d/%<max>d]'
COUNTED_NODES = %i[if case while until
for rescue and or].freeze

COUNTED_NODES = (CyclomaticComplexity::COUNTED_NODES - [:when] + [:case]).freeze

private

Expand All @@ -42,17 +40,18 @@ def complexity_score_for(node)
# If cond is nil, that means each when has an expression that
# evaluates to true or false. It's just an alternative to
# if/elsif/elsif... so the when nodes count.
nb_branches = node.when_branches.length + (node.else_branch ? 1 : 0)
if node.condition.nil?
node.when_branches.length
nb_branches
else
# Otherwise, the case node gets 0.8 complexity points and each
# when gets 0.2.
(0.8 + 0.2 * node.when_branches.length).round
(0.8 + 0.2 * nb_branches).round
end
when :if
node.else? && !node.elsif? ? 2 : 1
else
1
super
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/identical_conditional_branches.rb
Expand Up @@ -81,7 +81,7 @@ def on_case(node)

private

def check_branches(branches) # rubocop:todo Metrics/CyclomaticComplexity
def check_branches(branches) # rubocop:todo Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
# return if any branch is empty. An empty branch can be an `if`
# without an `else` or a branch that contains only comments.
return if branches.any?(&:nil?)
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/variable_force/variable.rb
Expand Up @@ -38,7 +38,7 @@ def referenced?
!@references.empty?
end

# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity
# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
def reference!(node)
reference = Reference.new(node, @scope)
@references << reference
Expand All @@ -63,7 +63,7 @@ def reference!(node)
end
end
end
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity

def in_modifier_if?(assignment)
parent = assignment.node.parent
Expand Down
37 changes: 37 additions & 0 deletions spec/rubocop/cop/metrics/perceived_complexity_spec.rb
Expand Up @@ -156,6 +156,22 @@ def method_name
RUBY
end

it 'counts else in a case with no argument' do
expect_offense(<<~RUBY)
def method_name
^^^^^^^^^^^^^^^ Perceived complexity for method_name is too high. [4/1]
case
when value == 1
call_foo
when value == 2
call_bar
else
call_baz
end
end
RUBY
end

it 'registers an offense for &&' do
expect_offense(<<~RUBY)
def method_name
Expand Down Expand Up @@ -226,6 +242,27 @@ def method_name_2
end
RUBY
end

it 'does not count unknown block calls' do
expect_no_offenses(<<~RUBY)
def method_name
bar.baz(:qux) do |x|
raise x
end
end
RUBY
end

it 'counts known iterating block' do
expect_offense(<<~RUBY)
def method_name
^^^^^^^^^^^^^^^ Perceived complexity for method_name is too high. [2/1]
ary.each do |x|
foo(x)
end
end
RUBY
end
end

context 'when method is in list of ignored methods' do
Expand Down