Skip to content

Commit

Permalink
[Fix #3542] Add IgnoreCopDirectives option to Metrics/LineLength
Browse files Browse the repository at this point in the history
The IgnoreCopDirectives option of Metrics/LineLength ignores directives like
'# rubocop:disable ...' when calculating the line's length.
  • Loading branch information
jmks authored and bbatsov committed Oct 11, 2016
1 parent f689210 commit 6a3442b
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@
* [#3566](https://github.com/bbatsov/rubocop/issues/3566): Add new `Metric/BlockLength` cop to ensure blocks don't get too long. ([@savef][])
* [#3428](https://github.com/bbatsov/rubocop/issues/3428): Add support for configuring `Style/PreferredHashMethods` with either `short` or `verbose` style method names. ([@abrom][])
* [#3455](https://github.com/bbatsov/rubocop/issues/3455): Add new `Rails/DynamicFindBy` cop. ([@pocke][])
* [#3542](https://github.com/bbatsov/rubocop/issues/3542): Add a configuration option, `IgnoreCopDirectives`, to `Metrics/LineLength` to stop cop directives (`# rubocop:disable Metrics/AbcSize`) from being counted when considering line length. ([@jmks][])

### Bug fixes

Expand Down Expand Up @@ -2410,3 +2411,4 @@
[@logicminds]: https://github.com/logicminds
[@abrom]: https://github.com/abrom
[@thegedge]: https://github.com/thegedge
[@jmks]: https://github.com/jmks/
3 changes: 3 additions & 0 deletions config/default.yml
Expand Up @@ -1123,6 +1123,9 @@ Metrics/LineLength:
URISchemes:
- http
- https
# The IgnoreCopDirectives option causes the LineLength rule to ignore cop
# directives like '# rubocop: enable ...' when calculating a line's length.
IgnoreCopDirectives: false

Metrics/MethodLength:
CountComments: false # count full line comments?
Expand Down
52 changes: 45 additions & 7 deletions lib/rubocop/cop/metrics/line_length.rb
Expand Up @@ -19,17 +19,21 @@ def investigate(processed_source)
end
end

private

def check_line(line, index, heredocs)
return unless line.length > max
return if line.length <= max
if ignore_cop_directives? && directive_on_source_line?(index)
return check_directive_line(line, index)
end
return if heredocs &&
line_in_whitelisted_heredoc?(heredocs, index.succ)
return check_uri_line(line, index) if allow_uri?

if allow_uri?
uri_range = find_excessive_uri_range(line)
return if uri_range && allowed_uri_position?(line, uri_range)
end

offense(excess_range(uri_range, line, index), line)
offense(
source_range(processed_source.buffer, index + 1, 0...line.length),
line
)
end

def offense(loc, line)
Expand Down Expand Up @@ -81,6 +85,10 @@ def allow_uri?
cop_config['AllowURI']
end

def ignore_cop_directives?
cop_config['IgnoreCopDirectives']
end

def allowed_uri_position?(line, uri_range)
uri_range.begin < max && uri_range.end == line.length
end
Expand Down Expand Up @@ -111,6 +119,36 @@ def valid_uri?(uri_ish_string)
def uri_regexp
@regexp ||= URI.regexp(cop_config['URISchemes'])
end

def check_directive_line(line, index)
return if line_length_without_directive(line) <= max

range = max..(line_length_without_directive(line) - 1)
offense(source_range(processed_source.buffer, index + 1, range), line)
end

def directive_on_source_line?(index)
source_line_number = index + processed_source.buffer.first_line
comment =
processed_source
.comments
.detect { |e| e.location.line == source_line_number }

return false unless comment
comment.text.match(CommentConfig::COMMENT_DIRECTIVE_REGEXP)
end

def line_length_without_directive(line)
before_comment, = line.split('#')
before_comment.rstrip.length
end

def check_uri_line(line, index)
uri_range = find_excessive_uri_range(line)
return if uri_range && allowed_uri_position?(line, uri_range)

offense(excess_range(uri_range, line, index), line)
end
end
end
end
Expand Down
13 changes: 8 additions & 5 deletions spec/rubocop/cli/cli_auto_gen_config_spec.rb
Expand Up @@ -46,7 +46,7 @@ def abs(path)
expect(IO.readlines('.rubocop_todo.yml')[8..-1].map(&:chomp))
.to eq(['# Offense count: 1',
'# Configuration parameters: AllowHeredoc, AllowURI, ' \
'URISchemes.',
'URISchemes, IgnoreCopDirectives.',
'# URISchemes: http, https',
'Metrics/LineLength:',
' Max: 85',
Expand Down Expand Up @@ -81,7 +81,7 @@ def abs(path)
expect(IO.readlines('.rubocop_todo.yml')[8..-1].join)
.to eq(['# Offense count: 1',
'# Configuration parameters: AllowHeredoc, AllowURI, ' \
'URISchemes.',
'URISchemes, IgnoreCopDirectives.',
'# URISchemes: http, https',
'Metrics/LineLength:',
' Max: 81',
Expand Down Expand Up @@ -136,7 +136,8 @@ def abs(path)
'again.',
'',
'# Offense count: 2',
'# Configuration parameters: AllowHeredoc, AllowURI, URISchemes.',
'# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, '\
'IgnoreCopDirectives.',
'# URISchemes: http, https',
'Metrics/LineLength:',
' Max: 90',
Expand Down Expand Up @@ -224,7 +225,8 @@ def abs(path)
'again.',
'',
'# Offense count: 3',
'# Configuration parameters: AllowHeredoc, AllowURI, URISchemes.',
'# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, '\
'IgnoreCopDirectives.',
'# URISchemes: http, https',
'Metrics/LineLength:',
' Max: 90', # Offense occurs in 2 files, limit is 1, so no Exclude.
Expand Down Expand Up @@ -412,7 +414,8 @@ def abs(path)
'# versions of RuboCop, may require this file to be generated ' \
'again.',
'',
'# Configuration parameters: AllowHeredoc, AllowURI, URISchemes.',
'# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, '\
'IgnoreCopDirectives.',
'# URISchemes: http, https',
'Metrics/LineLength:',
' Max: 90',
Expand Down
6 changes: 4 additions & 2 deletions spec/rubocop/config_loader_spec.rb
Expand Up @@ -200,7 +200,8 @@
'Max' => 77,
'AllowHeredoc' => true,
'AllowURI' => true,
'URISchemes' => %w(http https)
'URISchemes' => %w(http https),
'IgnoreCopDirectives' => false
},
'Metrics/MethodLength' => {
'Description' =>
Expand Down Expand Up @@ -279,7 +280,8 @@
'Max' => 120, # overridden in line_length.yml
'AllowHeredoc' => false, # overridden in rubocop.yml
'AllowURI' => true,
'URISchemes' => %w(http https)
'URISchemes' => %w(http https),
'IgnoreCopDirectives' => false
}
)

Expand Down
98 changes: 98 additions & 0 deletions spec/rubocop/cop/metrics/line_length_spec.rb
Expand Up @@ -188,4 +188,102 @@
end
end
end

context 'when IgnoreCopDirectives is disabled' do
let(:cop_config) { { 'Max' => 80, 'IgnoreCopDirectives' => false } }

context 'and the source is acceptable length' do
let(:acceptable_source) { 'a' * 80 }

context 'with a trailing Rubocop directive' do
let(:cop_directive) { ' # rubcop:disable Metrics/SomeCop' }
let(:source) { acceptable_source + cop_directive }

it 'registers an offense for the line' do
inspect_source(cop, source)
expect(cop.offenses.size).to eq(1)
end

it 'highlights the excess directive' do
inspect_source(cop, source)
expect(cop.highlights).to eq([cop_directive])
end
end

context 'with an inline comment' do
let(:excess_comment) { ' ###' }
let(:source) { acceptable_source + excess_comment }

it 'highlights the excess comment' do
inspect_source(cop, source)
expect(cop.highlights).to eq([excess_comment])
end
end
end

context 'and the source is too long and has a trailing cop directive' do
let(:excess_with_directive) { 'b # rubocop:disable Metrics/AbcSize' }
let(:source) { 'a' * 80 + excess_with_directive }

it 'highlights the excess source and cop directive' do
inspect_source(cop, source)
expect(cop.highlights).to eq([excess_with_directive])
end
end
end

context 'when IgnoreCopDirectives is enabled' do
let(:cop_config) { { 'Max' => 80, 'IgnoreCopDirectives' => true } }

context 'and the Rubocop directive is excessively long' do
let(:source) { <<-END }
# rubocop:disable Metrics/SomeReallyLongMetricNameThatShouldBeMuchShorterAndNeedsANameChange
END

it 'accepts the line' do
inspect_source(cop, source)
expect(cop.offenses).to be_empty
end
end

context 'and the Rubocop directive causes an excessive line length' do
let(:source) { <<-END }
def method_definition_that_is_just_under_the_line_length_limit(foo, bar) # rubocop:disable Metrics/AbcSize
# complex method
end
END

it 'accepts the line' do
inspect_source(cop, source)
expect(cop.offenses).to be_empty
end

context 'and has explanatory text' do
let(:source) { <<-END }
def method_definition_that_is_just_under_the_line_length_limit(foo) # rubocop:disable Metrics/AbcSize inherently complex!
# complex
end
END

it 'accepts the line' do
inspect_source(cop, source)
expect(cop.offenses).to be_empty
end
end
end

context 'and the source is too long' do
let(:source) { 'a' * 80 + 'bcd' + ' # rubocop:enable Style/ClassVars' }

it 'registers an offense for the line' do
inspect_source(cop, source)
expect(cop.offenses.size).to eq(1)
end

it 'highlights only the non-directive part' do
inspect_source(cop, source)
expect(cop.highlights).to eq(['bcd'])
end
end
end
end

0 comments on commit 6a3442b

Please sign in to comment.