Skip to content

Commit

Permalink
[Fix rubocop#3566] Add new Metrics/BlockLength cop
Browse files Browse the repository at this point in the history
This cop checks there aren't too many lines in blocks, much like `Metric/MethodLength` checks for too many lines in methods.

Also removed the splatted additional args from CodeLength#check_code_length, I don't think they've been necessary since the old `check` method was refactored into that method.
  • Loading branch information
savef committed Oct 8, 2016
1 parent 745f077 commit 88dace5
Show file tree
Hide file tree
Showing 7 changed files with 145 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* [#3370](https://github.com/bbatsov/rubocop/issues/3370): Add new `Rails/HttpPositionalArguments` cop to check your Rails 5 test code for existence of positional args usage. ([@logicminds][])
* [#3510](https://github.com/bbatsov/rubocop/issues/3510): Add a configuration option, `ConvertCodeThatCanStartToReturnNil`, to `Style/SafeNavigation` to check for code that could start returning `nil` if safe navigation is used. ([@rrosenblum][])
* Add a new `AllCops/StyleGuideBaseURL` setting that allows the use of relative paths and/or fragments within each cop's `StyleGuide` setting, to make forking of custom style guides easier. ([@scottmatthewman][])
* [#3566](https://github.com/bbatsov/rubocop/issues/3566): Add new `Metric/BlockLength` cop to ensure blocks don't get too long. ([@savef][])

### Bug fixes

Expand Down
8 changes: 8 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1122,6 +1122,14 @@ Metrics/MethodLength:
CountComments: false # count full line comments?
Max: 10

Metrics/BlockLength:
CountComments: false # count full line comments?
Max: 25
Exclude:
- 'Rakefile'
- '**/*.rake'
- 'spec/**/*.rb'

Metrics/ParameterLists:
Max: 5
CountKeywordArgs: true
Expand Down
4 changes: 4 additions & 0 deletions config/enabled.yml
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,10 @@ Metrics/MethodLength:
StyleGuide: '#short-methods'
Enabled: true

Metrics/BlockLength:
Description: 'Avoid long blocks with many lines.'
Enabled: true

Metrics/ParameterLists:
Description: 'Avoid parameter lists longer than three or four parameters.'
StyleGuide: '#too-many-params'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@

require 'rubocop/cop/metrics/cyclomatic_complexity'
require 'rubocop/cop/metrics/abc_size' # relies on cyclomatic_complexity
require 'rubocop/cop/metrics/block_length'
require 'rubocop/cop/metrics/block_nesting'
require 'rubocop/cop/metrics/class_length'
require 'rubocop/cop/metrics/line_length'
Expand Down
30 changes: 30 additions & 0 deletions lib/rubocop/cop/metrics/block_length.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Metrics
# This cop checks if the length of a block exceeds some maximum value.
# Comment lines can optionally be ignored.
# The maximum allowed length is configurable.
class BlockLength < Cop
include CodeLength

def on_block(node)
check_code_length(node)
end

private

def message(length, max_length)
format('Block has too many lines. [%d/%d]', length, max_length)
end

def code_length(node)
lines = node.source.lines.to_a[1..-2] || []

lines.count { |line| !irrelevant_line(line) }
end
end
end
end
end
2 changes: 1 addition & 1 deletion lib/rubocop/cop/mixin/code_length.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def count_comments?
cop_config['CountComments']
end

def check_code_length(node, *_)
def check_code_length(node)
length = code_length(node)
return unless length > max_length

Expand Down
100 changes: 100 additions & 0 deletions spec/rubocop/cop/metrics/block_length_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# frozen_string_literal: true

require 'spec_helper'

describe RuboCop::Cop::Metrics::BlockLength, :config do
subject(:cop) { described_class.new(config) }
let(:cop_config) { { 'Max' => 2, 'CountComments' => false } }

it 'rejects a block with more than 5 lines' do
inspect_source(cop, ['something do',
' a = 1',
' a = 2',
' a = 3',
'end'])
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line).sort).to eq([1])
expect(cop.config_to_allow_offenses).to eq('Max' => 3)
end

it 'reports the correct beginning and end lines' do
inspect_source(cop, ['something do',
' a = 1',
' a = 2',
' a = 3',
'end'])
offense = cop.offenses.first
expect(offense.location.first_line).to eq(1)
expect(offense.location.last_line).to eq(5)
end

it 'accepts a block with less than 3 lines' do
inspect_source(cop, ['something do',
' a = 1',
' a = 2',
'end'])
expect(cop.offenses).to be_empty
end

it 'does not count blank lines' do
inspect_source(cop, ['something do',
' a = 1',
'',
'',
' a = 4',
'end'])
expect(cop.offenses).to be_empty
end

it 'accepts empty blocks' do
inspect_source(cop, ['something do',
'end'])
expect(cop.offenses).to be_empty
end

it 'rejects brace blocks too' do
inspect_source(cop, ['something {',
' a = 1',
' a = 2',
' a = 3',
'}'])
expect(cop.offenses.size).to eq(1)
end

it 'properly counts nested blocks' do
inspect_source(cop, ['something do',
' something do',
' a = 2',
' a = 3',
' a = 4',
' a = 5',
' end',
'end'])
expect(cop.offenses.size).to eq(2)
expect(cop.offenses.map(&:line).sort).to eq([1, 2])
end

it 'does not count commented lines by default' do
inspect_source(cop, ['something do',
' a = 1',
' #a = 2',
' #a = 3',
' a = 4',
'end'])
expect(cop.offenses).to be_empty
end

context 'when CountComments is enabled' do
before { cop_config['CountComments'] = true }

it 'also counts commented lines' do
inspect_source(cop, ['something do',
' a = 1',
' #a = 2',
' a = 3',
'end'])
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line).sort).to eq([1])
end
end
end

0 comments on commit 88dace5

Please sign in to comment.