Skip to content

Commit

Permalink
[Fix rubocop#7426] Add always_true to Style/FrozenStringLiteralComment
Browse files Browse the repository at this point in the history
This adds another style to Style/FrozenStringLiteralComment that
enforces that the comment is always enabled, that is, it requires that
string literals are always frozen. The default style, `always`,
enforces that the comment exists but does not enforce that the comment
is enabled.

This also differentiates the offense message for the `always` and
`always_true` styles.

It changes the offense message when using the `always` style to be
less specific, since the old version of the message implied that the
only acceptable value of the frozen_string_literal comment was
`true`. In fact, in the `always` style, either `true` or `false` is
acceptable. The new `always_true` style uses that preexisting message,
since when using that style it is required that the value of the
frozen_string_literal comment is `true`.
  • Loading branch information
parkerfinch committed Dec 2, 2019
1 parent e97eb5c commit b586912
Show file tree
Hide file tree
Showing 12 changed files with 736 additions and 49 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -2,6 +2,10 @@

## master (unreleased)

### New features

* [#7426](https://github.com/rubocop-hq/rubocop/issues/7426): Add `always_true` style to Style/FrozenStringLiteralComment. ([@parkerfinch][])

### Bug fixes

* [#7530](https://github.com/rubocop-hq/rubocop/issues/7530): Typo in `Style/TrivialAccessors`'s `AllowedMethods`. ([@movermeyer][])
Expand Down
4 changes: 4 additions & 0 deletions config/default.yml
Expand Up @@ -2754,6 +2754,10 @@ Style/FrozenStringLiteralComment:
# string literal. If you run code against multiple versions of Ruby, it is
# possible that this will create errors in Ruby 2.3.0+.
- always
# `always_true` will add the frozen string literal comment to a file,
# similarly to the `always` style, but will also change any disabled
# comments (e.g. `# frozen_string_literal: false`) to be enabled.
- always_true
# `never` will enforce that the frozen string literal comment does not
# exist in a file.
- never
Expand Down
6 changes: 6 additions & 0 deletions lib/rubocop/cop/mixin/frozen_string_literal.rb
Expand Up @@ -39,6 +39,12 @@ def frozen_string_literals_enabled?
end
end

def frozen_string_literal_specified?
leading_comment_lines.any? do |line|
MagicComment.parse(line).frozen_string_literal_specified?
end
end

def leading_comment_lines
comments = processed_source.comments

Expand Down
100 changes: 89 additions & 11 deletions lib/rubocop/cop/style/frozen_string_literal_comment.rb
Expand Up @@ -51,29 +51,65 @@ module Style
# module Baz
# # ...
# end
#
# @example EnforcedStyle: always_true
# # The `always_true` style enforces that the frozen string literal
# # comment is set to `true`. This is a stricter option than `always`
# # and forces projects to use frozen string literals.
# # bad
# # frozen_string_literal: false
#
# module Baz
# # ...
# end
#
# # bad
# module Baz
# # ...
# end
#
# # good
# # frozen_string_literal: true
#
# module Bar
# # ...
# end
class FrozenStringLiteralComment < Cop
include ConfigurableEnforcedStyle
include FrozenStringLiteral
include RangeHelp

MSG = 'Missing magic comment `# frozen_string_literal: true`.'
MSG_MISSING_TRUE = 'Missing magic comment `# frozen_string_literal: '\
'true`.'
MSG_MISSING = 'Missing magic "frozen_string_literal" comment.'
MSG_UNNECESSARY = 'Unnecessary frozen string literal comment.'
MSG_DISABLED = 'Frozen string literal comment must be set to `true`.'
SHEBANG = '#!'

def investigate(processed_source)
return if processed_source.tokens.empty?

if frozen_string_literal_comment_exists?
check_for_no_comment(processed_source)
case style
when :never
ensure_no_comment(processed_source)
when :always_true
ensure_enabled_comment(processed_source)
else
check_for_comment(processed_source)
ensure_comment(processed_source)
end
end

def autocorrect(node)
lambda do |corrector|
if style == :never
case style
when :never
remove_comment(corrector, node)
when :always_true
if frozen_string_literal_specified?
enable_comment(corrector)
else
insert_comment(corrector)
end
else
insert_comment(corrector)
end
Expand All @@ -82,12 +118,27 @@ def autocorrect(node)

private

def check_for_no_comment(processed_source)
unnecessary_comment_offense(processed_source) if style == :never
def ensure_no_comment(processed_source)
return unless frozen_string_literal_comment_exists?

unnecessary_comment_offense(processed_source)
end

def check_for_comment(processed_source)
offense(processed_source) unless style == :never
def ensure_comment(processed_source)
return if frozen_string_literal_comment_exists?

missing_offense(processed_source)
end

def ensure_enabled_comment(processed_source)
if frozen_string_literal_specified?
return if frozen_string_literals_enabled?

# The comment exists, but is not enabled.
disabled_offense(processed_source)
else # The comment doesn't exist at all.
missing_true_offense(processed_source)
end
end

def last_special_comment(processed_source)
Expand All @@ -111,11 +162,22 @@ def frozen_string_literal_comment(processed_source)
end
end

def offense(processed_source)
def missing_offense(processed_source)
last_special_comment = last_special_comment(processed_source)
range = source_range(processed_source.buffer, 0, 0)

add_offense(last_special_comment,
location: range,
message: MSG_MISSING)
end

def missing_true_offense(processed_source)
last_special_comment = last_special_comment(processed_source)
range = source_range(processed_source.buffer, 0, 0)

add_offense(last_special_comment, location: range)
add_offense(last_special_comment,
location: range,
message: MSG_MISSING_TRUE)
end

def unnecessary_comment_offense(processed_source)
Expand All @@ -127,11 +189,27 @@ def unnecessary_comment_offense(processed_source)
message: MSG_UNNECESSARY)
end

def disabled_offense(processed_source)
frozen_string_literal_comment =
frozen_string_literal_comment(processed_source)

add_offense(frozen_string_literal_comment,
location: frozen_string_literal_comment.pos,
message: MSG_DISABLED)
end

def remove_comment(corrector, node)
corrector.remove(range_with_surrounding_space(range: node.pos,
side: :right))
end

def enable_comment(corrector)
comment = frozen_string_literal_comment(processed_source)

corrector.replace(line_range(comment.line),
FROZEN_STRING_LITERAL_ENABLED)
end

def insert_comment(corrector)
comment = last_special_comment(processed_source)

Expand Down
27 changes: 26 additions & 1 deletion manual/cops_style.md
Expand Up @@ -2328,12 +2328,37 @@ module Baz
# ...
end
```
#### EnforcedStyle: always_true

```ruby
# The `always_true` style enforces that the frozen string literal
# comment is set to `true`. This is a stricter option than `always`
# and forces projects to use frozen string literals.
# bad
# frozen_string_literal: false
module Baz
# ...
end
# bad
module Baz
# ...
end
# good
# frozen_string_literal: true
module Bar
# ...
end
```

### Configurable attributes

Name | Default value | Configurable values
--- | --- | ---
EnforcedStyle | `always` | `always`, `never`
EnforcedStyle | `always` | `always`, `always_true`, `never`

## Style/GlobalVars

Expand Down
6 changes: 3 additions & 3 deletions spec/fixtures/html_formatter/expected.html
Expand Up @@ -418,7 +418,7 @@ <h1 class="title">RuboCop Inspection Report</h1>
<div class="meta">
<span class="location">Line #1</span>
<span class="severity convention">convention:</span>
<span class="message">Style/FrozenStringLiteralComment: Missing magic comment <code># frozen_string_literal: true</code>.</span>
<span class="message">Style/FrozenStringLiteralComment: Missing magic "frozen_string_literal" comment.</span>
</div>

<pre><code><span class="highlight convention">c</span>lass ApplicationController &lt; ActionController::Base</code></pre>
Expand Down Expand Up @@ -461,7 +461,7 @@ <h1 class="title">RuboCop Inspection Report</h1>
<div class="meta">
<span class="location">Line #1</span>
<span class="severity convention">convention:</span>
<span class="message">Style/FrozenStringLiteralComment: Missing magic comment <code># frozen_string_literal: true</code>.</span>
<span class="message">Style/FrozenStringLiteralComment: Missing magic "frozen_string_literal" comment.</span>
</div>

<pre><code><span class="highlight convention">c</span>lass BooksController &lt; ApplicationController</code></pre>
Expand Down Expand Up @@ -625,7 +625,7 @@ <h1 class="title">RuboCop Inspection Report</h1>
<div class="meta">
<span class="location">Line #1</span>
<span class="severity convention">convention:</span>
<span class="message">Style/FrozenStringLiteralComment: Missing magic comment <code># frozen_string_literal: true</code>.</span>
<span class="message">Style/FrozenStringLiteralComment: Missing magic "frozen_string_literal" comment.</span>
</div>

<pre><code><span class="highlight convention">c</span>lass Book &lt; ActiveRecord::Base</code></pre>
Expand Down
16 changes: 8 additions & 8 deletions spec/rubocop/cli/cli_auto_gen_config_spec.rb
Expand Up @@ -178,7 +178,7 @@ def f
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, never
# SupportedStyles: always, always_true, never
Style/FrozenStringLiteralComment:
Exclude:
- 'example.rb'
Expand Down Expand Up @@ -226,7 +226,7 @@ def f
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, never
# SupportedStyles: always, always_true, never
Style/FrozenStringLiteralComment:
Exclude:
- 'example.rb'
Expand Down Expand Up @@ -276,7 +276,7 @@ def f
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, never
# SupportedStyles: always, always_true, never
Style/FrozenStringLiteralComment:
Exclude:
- 'example.rb'
Expand Down Expand Up @@ -369,7 +369,7 @@ def f
'# Offense count: 1',
'# Cop supports --auto-correct.',
'# Configuration parameters: EnforcedStyle.',
'# SupportedStyles: always, never',
'# SupportedStyles: always, always_true, never',
'Style/FrozenStringLiteralComment:',
' Exclude:',
" - 'example1.rb'",
Expand Down Expand Up @@ -411,7 +411,7 @@ def f
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, never
# SupportedStyles: always, always_true, never
Style/FrozenStringLiteralComment:
Exclude:
- 'example1.rb'
Expand Down Expand Up @@ -459,7 +459,7 @@ def f
# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, never
# SupportedStyles: always, always_true, never
Style/FrozenStringLiteralComment:
Exclude:
- 'example1.rb'
Expand Down Expand Up @@ -1006,7 +1006,7 @@ def function(arg1, arg2, arg3, arg4, arg5, arg6, arg7)
# Offense count: 3
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, never
# SupportedStyles: always, always_true, never
Style/FrozenStringLiteralComment:
Enabled: false
Expand All @@ -1027,7 +1027,7 @@ def function(arg1, arg2, arg3, arg4, arg5, arg6, arg7)
# Offense count: 4
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle.
# SupportedStyles: always, never
# SupportedStyles: always, always_true, never
Style/FrozenStringLiteralComment:
Exclude:
- 'example1.rb'
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cli/cli_autocorrect_spec.rb
Expand Up @@ -765,7 +765,7 @@ def func
expect(cli.run(%w[--auto-correct --format simple])).to eq(1)
expect($stdout.string).to eq(<<~RESULT)
== example.rb ==
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic "frozen_string_literal" comment.
C: 2: 1: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
C: 3: 1: Style/Documentation: Missing top-level class documentation comment.
W: 4: 3: [Corrected] Lint/RedundantCopDisableDirective: Unnecessary disabling of Metrics/MethodLength.
Expand Down
10 changes: 5 additions & 5 deletions spec/rubocop/cli/cli_disable_uncorrectable_spec.rb
Expand Up @@ -16,7 +16,7 @@
expect($stderr.string).to eq('')
expect($stdout.string).to eq(<<~OUTPUT)
== example.rb ==
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic "frozen_string_literal" comment.
C: 1: 7: [Corrected] Layout/SpaceAroundOperators: Surrounding space missing for operator ==.
C: 2: 1: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
Expand All @@ -40,7 +40,7 @@ def is_example
expect($stderr.string).to eq('')
expect($stdout.string).to eq(<<~OUTPUT)
== example.rb ==
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic "frozen_string_literal" comment.
C: 1: 5: [Todo] Naming/PredicateName: Rename is_example to example?.
C: 2: 1: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
Expand Down Expand Up @@ -68,7 +68,7 @@ def is_example # rubocop:todo Naming/PredicateName
expect($stderr.string).to eq('')
expect($stdout.string).to eq(<<~OUTPUT)
== example.rb ==
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic "frozen_string_literal" comment.
C: 1: 4: [Todo] Style/IpAddresses: Do not hardcode IP addresses.
C: 1: 15: [Todo] Style/IpAddresses: Do not hardcode IP addresses.
C: 2: 1: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
Expand Down Expand Up @@ -111,7 +111,7 @@ def choose_move(who_to_move)
expect($stderr.string).to eq('')
expect($stdout.string).to eq(<<~OUTPUT)
== example.rb ==
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic "frozen_string_literal" comment.
C: 2: 1: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
C: 3: 3: [Todo] Metrics/AbcSize: Assignment Branch Condition size for choose_move is too high. [<8, 12, 6> 15.62/15]
C: 3: 3: [Todo] Metrics/CyclomaticComplexity: Cyclomatic complexity for choose_move is too high. [7/6]
Expand Down Expand Up @@ -170,7 +170,7 @@ def long_method_name(_taking, _a_few, _parameters, _resulting_in_a_long_line)
expect($stdout.string).to eq(<<~OUTPUT)
== example.rb ==
C: 1: 1: [Todo] Metrics/MethodLength: Method has too many lines. [2/1]
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
C: 1: 1: [Corrected] Style/FrozenStringLiteralComment: Missing magic "frozen_string_literal" comment.
C: 3: 1: [Corrected] Layout/EmptyLineAfterMagicComment: Add an empty line after magic comments.
1 file inspected, 3 offenses detected, 3 offenses corrected
Expand Down

0 comments on commit b586912

Please sign in to comment.