Skip to content

Commit

Permalink
[Fix rubocop#4001] Fix bug in Metrics/LineLength
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
wkurniawan07 committed Mar 2, 2017
1 parent d31464e commit 7b5f7e2
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -2655,3 +2656,4 @@
[@dorian]: https://github.com/dorian
[@attilahorvath]: https://github.com/attilahorvath
[@droptheplot]: https://github.com/droptheplot
[@wkurniawan07]: https://github.com/wkurniawan07
2 changes: 1 addition & 1 deletion lib/rubocop/comment_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/metrics/line_length.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
32 changes: 32 additions & 0 deletions spec/rubocop/cop/metrics/line_length_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 7b5f7e2

Please sign in to comment.