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

[Fix #4633] Make metrics cops aware of define_method #5049

Merged
merged 1 commit into from
Nov 13, 2017
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
* [#5011](https://github.com/bbatsov/rubocop/pull/5011): Remove `SupportedStyles` from "Configuration parameters" in `.rubocop_todo.yml`. ([@pocke][])
* `Lint/RescueWithoutErrorClass` has been replaced by `Style/RescueStandardError`. ([@rrosenblum][])
* [#5042](https://github.com/bbatsov/rubocop/pull/5042): Make offense locations of metrics cops to contain whole a method. ([@pocke][])
* [#4633](https://github.com/bbatsov/rubocop/issues/4633): Make metrics cops aware of `define_method`. ([@pocke][])

## 0.51.0 (2017-10-18)

Expand Down
35 changes: 28 additions & 7 deletions lib/rubocop/cop/mixin/method_complexity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,46 @@ module Cop
# This module handles measurement and reporting of complexity in methods.
module MethodComplexity
include ConfigurableMax
extend NodePattern::Macros

def on_def(node)
check_complexity(node, node.method_name)
end
alias on_defs on_def

def on_block(node)
define_method?(node) do |name|
check_complexity(node, name)
end
end

private

def_node_matcher :define_method?, <<-PATTERN
(block
(send nil? :define_method ({sym str} $_))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also apply this to Object#define_singleton_method? 🙂

args
_)
PATTERN

def check_complexity(node, method_name)
# Accepts empty methods always.
return unless node.body

max = cop_config['Max']
complexity = complexity(node)
complexity = complexity(node.body)

return unless complexity > max

msg = format(self.class::MSG, node.method_name, complexity, max)
msg = format(self.class::MSG, method_name, complexity, max)

add_offense(node, message: msg) do
self.max = complexity.ceil
end
end
alias on_defs on_def

private

def complexity(node)
node.each_node(*self.class::COUNTED_NODES).reduce(1) do |score, n|
def complexity(body)
body.each_node(*self.class::COUNTED_NODES).reduce(1) do |score, n|
score + complexity_score_for(n)
end
end
Expand Down
16 changes: 16 additions & 0 deletions spec/rubocop/cop/metrics/abc_size_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ def method_name
RUBY
end

it 'accepts an empty `define_method`' do
expect_no_offenses(<<-RUBY.strip_indent)
define_method :method_name do
end
RUBY
end

it 'registers an offense for an if modifier' do
inspect_source(<<-RUBY.strip_indent)
def method_name
Expand Down Expand Up @@ -70,6 +77,15 @@ def method_name
'high. [6.4/0]']) # sqrt(1*1 + 6*6 + 2*2) => 6.4
end

it 'registers an offense for a `define_method`' do
expect_offense(<<-RUBY.strip_indent)
define_method :method_name do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Assignment Branch Condition size for method_name is too high. [1/0]
x = 1
end
RUBY
end

context 'target_ruby_version >= 2.3', :ruby23 do
it 'treats safe navigation method calls like regular method calls' do
expect_offense(<<-RUBY.strip_indent) # sqrt(0 + 2*2 + 0) => 2
Expand Down
23 changes: 23 additions & 0 deletions spec/rubocop/cop/metrics/cyclomatic_complexity_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,20 @@ def method_name
RUBY
end

it 'accepts an empty method' do
expect_no_offenses(<<-RUBY.strip_indent)
def method_name
end
RUBY
end

it 'accepts an empty `define_method`' do
expect_no_offenses(<<-RUBY.strip_indent)
define_method :method_name do
end
RUBY
end

it 'accepts complex code outside of methods' do
expect_no_offenses(<<-RUBY.strip_indent)
def method_name
Expand Down Expand Up @@ -197,6 +211,15 @@ def method_name_2
end
RUBY
end

it 'registers an offense for a `define_method`' do
expect_offense(<<-RUBY.strip_indent)
define_method :method_name do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Cyclomatic complexity for method_name is too high. [2/1]
call_foo if some_condition
end
RUBY
end
end

context 'when Max is 2' do
Expand Down
23 changes: 23 additions & 0 deletions spec/rubocop/cop/metrics/perceived_complexity_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,20 @@ def method_name
RUBY
end

it 'accepts an empty method' do
expect_no_offenses(<<-RUBY.strip_indent)
def method_name
end
RUBY
end

it 'accepts an empty `define_method`' do
expect_no_offenses(<<-RUBY.strip_indent)
define_method :method_name do
end
RUBY
end

it 'accepts complex code outside of methods' do
expect_no_offenses(<<-RUBY.strip_indent)
def method_name
Expand Down Expand Up @@ -215,6 +229,15 @@ def method_name_2
end
RUBY
end

it 'registers an offense for a `define_method`' do
expect_offense(<<-RUBY.strip_indent)
define_method :method_name do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Perceived complexity for method_name is too high. [2/1]
call_foo if some_condition
end
RUBY
end
end

context 'when Max is 2' do
Expand Down