Skip to content

Commit

Permalink
[Fix rubocop#7929] Accept frozen_string_literal anywhere in leading c…
Browse files Browse the repository at this point in the history
…omment lines

The frozen_string_literal comment can exist anywhere in the
"leading comment lines" (i.e. before the first line of ruby
code). If it appears after any ruby code it is ignored which
does not cause an errors but could cause the absence of an
expected error which is bad.

The previous implementation was checking if this comment was
one of the first three comments which had two issues:
- The comments it was checking could be anywhere, even after
  ruby code
- Since it was only checking three comments, it's possible
  that you can have an appropriate frozen_string_literal but
  this cop would show an error.

This commit updates the logic to look at all leading comment
lines, which may be more or less than three depending on where
the first non-comment token is.
  • Loading branch information
jeffcarbs committed May 7, 2020
1 parent 21a1b2b commit 748d989
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -12,6 +12,7 @@

### Bug fixes

* [#7929](https://github.com/rubocop-hq/rubocop/issues/7929): Fix `Style/FrozenStringLiteralComment` to accept frozen_string_literal anywhere in leading comment lines. ([@jeffcarbs][])
* [#7882](https://github.com/rubocop-hq/rubocop/pull/7882): Fix `Style/CaseEquality` when `AllowOnConstant` is `true` and the method receiver is implicit. ([@rafaelfranca][])
* [#7790](https://github.com/rubocop-hq/rubocop/issues/7790): Fix `--parallel` and `--ignore-parent-exclusion` combination. ([@jonas054][])
* [#7881](https://github.com/rubocop-hq/rubocop/issues/7881): Fix `--parallel` and `--force-default-config` combination. ([@jonas054][])
Expand Down Expand Up @@ -4485,3 +4486,4 @@
[@saurabhmaurya15]: https://github.com/saurabhmaurya15
[@DracoAter]: https://github.com/DracoAter
[@diogoosorio]: https://github.com/diogoosorio
[@jeffcarbs]: https://github.com/jeffcarbs
11 changes: 10 additions & 1 deletion lib/rubocop/cop/mixin/frozen_string_literal.rb
Expand Up @@ -46,7 +46,16 @@ def frozen_string_literal_specified?
end

def leading_comment_lines
processed_source.comments.first(3).map(&:text)
first_non_comment_token = processed_source.tokens.find do |token|
!token.comment?
end

if first_non_comment_token
# `line` is 1-indexed so we need to subtract 1 to get the array index
processed_source.lines[0...first_non_comment_token.line - 1]
else
processed_source.lines
end
end
end
end
Expand Down
118 changes: 118 additions & 0 deletions spec/rubocop/cop/style/frozen_string_literal_comment_spec.rb
Expand Up @@ -249,6 +249,39 @@
RUBY
end

it 'accepts a frozen string literal comment after other comments' do
expect_no_offenses(<<~RUBY)
#!/usr/bin/env ruby
# encoding: utf-8
#
# These are more comments
# frozen_string_literal: false
puts 1
RUBY
end

it 'registers an offense for having a frozen string literal comment ' \
'under ruby code' do
expect_offense(<<~RUBY)
# encoding: utf-8
^ Missing frozen string literal comment.
puts 1
# frozen_string_literal: true
RUBY

# Since we're only looking at the leading comments, a
# `frozen_string_literal` comment elsewhere in the code is invisible
# to this cop so the autocorrect won't remove it.
expect_correction(<<~RUBY)
# encoding: utf-8
# frozen_string_literal: true
puts 1
# frozen_string_literal: true
RUBY
end

it 'registers an offense for an extra first empty line' do
pending 'There is a flaw that skips adding caret symbol in this case, ' \
'making it impossible to use `expect_offense` matcher'
Expand Down Expand Up @@ -463,6 +496,36 @@
puts 1
RUBY
end

it 'registers an offense for having a frozen string literal comment ' \
'after other comments' do
expect_offense(<<~RUBY)
#!/usr/bin/env ruby
# encoding: utf-8
#
# These are more comments
# frozen_string_literal: false
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary frozen string literal comment.
puts 1
RUBY

expect_correction(<<~RUBY)
#!/usr/bin/env ruby
# encoding: utf-8
#
# These are more comments
puts 1
RUBY
end

it 'accepts a frozen string literal comment under ruby code' do
expect_no_offenses(<<~RUBY)
# encoding: utf-8
puts 1
# frozen_string_literal: true
RUBY
end
end

context 'always_true' do
Expand Down Expand Up @@ -959,6 +1022,61 @@
RUBY
end

it 'accepts a frozen string literal comment after other comments' do
expect_no_offenses(<<~RUBY)
#!/usr/bin/env ruby
# encoding: utf-8
#
# These are more comments
# frozen_string_literal: true
puts 1
RUBY
end

it 'registers an offense for a disabled frozen string literal comment ' \
'after other comments' do
expect_offense(<<~RUBY)
#!/usr/bin/env ruby
# encoding: utf-8
#
# These are more comments
# frozen_string_literal: false
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Frozen string literal comment must be set to `true`.
puts 1
RUBY

expect_correction(<<~RUBY)
#!/usr/bin/env ruby
# encoding: utf-8
#
# These are more comments
# frozen_string_literal: true
puts 1
RUBY
end

it 'registers an offense for having a frozen string literal comment ' \
'under ruby code' do
expect_offense(<<~RUBY)
# encoding: utf-8
^ Missing magic comment `# frozen_string_literal: true`.
puts 1
# frozen_string_literal: true
RUBY

# Since we're only looking at the leading comments, a
# `frozen_string_literal` comment elsewhere in the code is invisible
# to this cop so the autocorrect won't remove it.
expect_correction(<<~RUBY)
# encoding: utf-8
# frozen_string_literal: true
puts 1
# frozen_string_literal: true
RUBY
end

it 'registers an offense for a disabled frozen string literal comment ' \
'under shebang with no other code' do
expect_offense(<<~RUBY)
Expand Down

0 comments on commit 748d989

Please sign in to comment.