From 7b5f7e2f779ab0bb4fa8aace03a7c737b1abc323 Mon Sep 17 00:00:00 2001 From: Wilson Kurniawan Date: Fri, 24 Feb 2017 04:34:15 +0800 Subject: [PATCH] [Fix #4001] Fix bug in Metrics/LineLength MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Metrics/LineLength had a bug when configured with “ignoreCopDirectives” set to true AND checking a line with both directive # and non-directive # present. For example: ```rb aaa … aaa # bbb … bbb # rubocop:disable CopName ``` where (aaa … aaa # bbb … bbb) exceeds the specified line length. The cop would correctly detect it as a violation, but would return the extraneous characters AND the directive in the error message. ```rb LARGE_DATA_STRING_PATTERN = %r{\A([A-Za-z0-9\+\/#]…)} # rubocop:disable CopName ``` The above line has # as part of regexp and the line is longer than the permissible line length before the directive. The cop did not detect it as a violation. --- CHANGELOG.md | 2 ++ lib/rubocop/comment_config.rb | 2 +- lib/rubocop/cop/metrics/line_length.rb | 2 +- spec/rubocop/cop/metrics/line_length_spec.rb | 32 ++++++++++++++++++++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65a61437edd4..a17ca4abd936 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ * [#4047](https://github.com/bbatsov/rubocop/issues/4047): Allow `find_zone` and `find_zone!` methods in `Rails/TimeZone`. ([@attilahorvath][]) * [#3457](https://github.com/bbatsov/rubocop/issues/3457): Clear a warning and prevent new warnings. ([@mikegee][]) * [#4066](https://github.com/bbatsov/rubocop/issues/4066): Register an offense in `Lint/ShadowedException` when an exception is shadowed and there is an implicit begin. ([@rrosenblum][]) +* [#4001](https://github.com/bbatsov/rubocop/issues/4001): Lint/UnneededDisable of Metrics/LineLength that isn't unneeded. ([@wkurniawan07][]) ## 0.47.1 (2017-01-18) @@ -2655,3 +2656,4 @@ [@dorian]: https://github.com/dorian [@attilahorvath]: https://github.com/attilahorvath [@droptheplot]: https://github.com/droptheplot +[@wkurniawan07]: https://github.com/wkurniawan07 diff --git a/lib/rubocop/comment_config.rb b/lib/rubocop/comment_config.rb index caa517822742..dc1fab2325b1 100644 --- a/lib/rubocop/comment_config.rb +++ b/lib/rubocop/comment_config.rb @@ -11,7 +11,7 @@ class CommentConfig COPS_PATTERN = "(all|#{COP_NAMES_PATTERN})".freeze COMMENT_DIRECTIVE_REGEXP = Regexp.new( - ('\A# rubocop : ((?:dis|en)able)\b ' + COPS_PATTERN).gsub(' ', '\s*') + ('# rubocop : ((?:dis|en)able)\b ' + COPS_PATTERN).gsub(' ', '\s*') ) CopAnalysis = Struct.new(:line_ranges, :start_line_number) diff --git a/lib/rubocop/cop/metrics/line_length.rb b/lib/rubocop/cop/metrics/line_length.rb index b028bc0ac834..6c85ef6a2553 100644 --- a/lib/rubocop/cop/metrics/line_length.rb +++ b/lib/rubocop/cop/metrics/line_length.rb @@ -159,7 +159,7 @@ def directive_on_source_line?(index) end def line_length_without_directive(line) - before_comment, = line.split('#') + before_comment, = line.split(CommentConfig::COMMENT_DIRECTIVE_REGEXP) before_comment.rstrip.length end diff --git a/spec/rubocop/cop/metrics/line_length_spec.rb b/spec/rubocop/cop/metrics/line_length_spec.rb index 62fe568512b0..40dbc66ef643 100644 --- a/spec/rubocop/cop/metrics/line_length_spec.rb +++ b/spec/rubocop/cop/metrics/line_length_spec.rb @@ -305,6 +305,38 @@ def method_definition_that_is_just_under_the_line_length_limit(foo) # rubocop:di inspect_source(cop, source) expect(cop.highlights).to eq(['bcd']) end + + context 'and the source contains non-directive # as comment' do + let(:source) { <<-END } + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # bbbbbbbbbbbbbb # rubocop:enable Style/ClassVars' + END + + 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(['bbbbbbb']) + end + end + + context 'and the source contains non-directive #s as non-comment' do + let(:source) { <<-END } + LARGE_DATA_STRING_PATTERN = %r{\A([A-Za-z0-9\+\/#]*\={0,2})#([A-Za-z0-9\+\/#]*\={0,2})#([A-Za-z0-9\+\/#]*\={0,2})\z} # rubocop:disable LineLength + END + + 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([']*={0,2})#([A-Za-z0-9+/#]*={0,2})z}']) + end + end end end end