From 6a3442b9d2bb3e6a6abacd15889560057463f165 Mon Sep 17 00:00:00 2001 From: jmks Date: Fri, 7 Oct 2016 14:09:12 -0400 Subject: [PATCH] [Fix #3542] Add IgnoreCopDirectives option to Metrics/LineLength The IgnoreCopDirectives option of Metrics/LineLength ignores directives like '# rubocop:disable ...' when calculating the line's length. --- CHANGELOG.md | 2 + config/default.yml | 3 + lib/rubocop/cop/metrics/line_length.rb | 52 +++++++++-- spec/rubocop/cli/cli_auto_gen_config_spec.rb | 13 ++- spec/rubocop/config_loader_spec.rb | 6 +- spec/rubocop/cop/metrics/line_length_spec.rb | 98 ++++++++++++++++++++ 6 files changed, 160 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6e9a9480ae..05589a170d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 @@ -2410,3 +2411,4 @@ [@logicminds]: https://github.com/logicminds [@abrom]: https://github.com/abrom [@thegedge]: https://github.com/thegedge +[@jmks]: https://github.com/jmks/ diff --git a/config/default.yml b/config/default.yml index cd70a80b7e1..2356fc14752 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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? diff --git a/lib/rubocop/cop/metrics/line_length.rb b/lib/rubocop/cop/metrics/line_length.rb index 7bac41e5671..d07dac4653e 100644 --- a/lib/rubocop/cop/metrics/line_length.rb +++ b/lib/rubocop/cop/metrics/line_length.rb @@ -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) @@ -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 @@ -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 diff --git a/spec/rubocop/cli/cli_auto_gen_config_spec.rb b/spec/rubocop/cli/cli_auto_gen_config_spec.rb index a08dce0471e..a3a1f16f012 100644 --- a/spec/rubocop/cli/cli_auto_gen_config_spec.rb +++ b/spec/rubocop/cli/cli_auto_gen_config_spec.rb @@ -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', @@ -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', @@ -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', @@ -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. @@ -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', diff --git a/spec/rubocop/config_loader_spec.rb b/spec/rubocop/config_loader_spec.rb index dbf38a0c095..b2a3463ae10 100644 --- a/spec/rubocop/config_loader_spec.rb +++ b/spec/rubocop/config_loader_spec.rb @@ -200,7 +200,8 @@ 'Max' => 77, 'AllowHeredoc' => true, 'AllowURI' => true, - 'URISchemes' => %w(http https) + 'URISchemes' => %w(http https), + 'IgnoreCopDirectives' => false }, 'Metrics/MethodLength' => { 'Description' => @@ -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 } ) diff --git a/spec/rubocop/cop/metrics/line_length_spec.rb b/spec/rubocop/cop/metrics/line_length_spec.rb index 5cfbeccd025..300c331dbe6 100644 --- a/spec/rubocop/cop/metrics/line_length_spec.rb +++ b/spec/rubocop/cop/metrics/line_length_spec.rb @@ -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