From 88dace536666d683cf9acfaf8f496d8c392816df Mon Sep 17 00:00:00 2001 From: Ben Hadley-Evans Date: Sat, 8 Oct 2016 00:17:32 +0100 Subject: [PATCH] [Fix #3566] Add new `Metrics/BlockLength` cop 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. --- CHANGELOG.md | 1 + config/default.yml | 8 ++ config/enabled.yml | 4 + lib/rubocop.rb | 1 + lib/rubocop/cop/metrics/block_length.rb | 30 ++++++ lib/rubocop/cop/mixin/code_length.rb | 2 +- spec/rubocop/cop/metrics/block_length_spec.rb | 100 ++++++++++++++++++ 7 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 lib/rubocop/cop/metrics/block_length.rb create mode 100644 spec/rubocop/cop/metrics/block_length_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a2b078a865f..bc01c3e4cf97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/config/default.yml b/config/default.yml index 5d765a1324fb..df144726585d 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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 diff --git a/config/enabled.yml b/config/enabled.yml index e7afa303c988..bc900f8855db 100644 --- a/config/enabled.yml +++ b/config/enabled.yml @@ -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' diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 490d1e4ac2a3..a6e416665dde 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -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' diff --git a/lib/rubocop/cop/metrics/block_length.rb b/lib/rubocop/cop/metrics/block_length.rb new file mode 100644 index 000000000000..b30a67d7c8f5 --- /dev/null +++ b/lib/rubocop/cop/metrics/block_length.rb @@ -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 diff --git a/lib/rubocop/cop/mixin/code_length.rb b/lib/rubocop/cop/mixin/code_length.rb index 542f1bf1ad5e..20c701e0a99b 100644 --- a/lib/rubocop/cop/mixin/code_length.rb +++ b/lib/rubocop/cop/mixin/code_length.rb @@ -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 diff --git a/spec/rubocop/cop/metrics/block_length_spec.rb b/spec/rubocop/cop/metrics/block_length_spec.rb new file mode 100644 index 000000000000..64fc162a2eb3 --- /dev/null +++ b/spec/rubocop/cop/metrics/block_length_spec.rb @@ -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