Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #8576] Fix Style/IfUnlessModifier to ignore cop disable comment directives when considering conversion to modifier form #8577

Merged
merged 1 commit into from Sep 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -43,6 +43,7 @@
* [#8481](https://github.com/rubocop-hq/rubocop/pull/8481): Fix autocorrect for elements with newlines in `Style/SymbolArray` and `Style/WordArray`. ([@biinari][])
* [#8475](https://github.com/rubocop-hq/rubocop/issues/8475): Fix a false positive for `Style/HashAsLastArrayItem` when there are duplicate hashes in the array. ([@wcmonty][])
* [#8497](https://github.com/rubocop-hq/rubocop/issues/8497): Fix `Style/IfUnlessModifier` to add parentheses when converting if-end condition inside a parenthesized method argument list. ([@dsavochkin][])
* [#8576](https://github.com/rubocop-hq/rubocop/issues/8576): Fix `Style/IfUnlessModifier` to ignore cop disable comment directives when considering conversion to the modifier form. ([@dsavochkin][])

### Changes

Expand Down
12 changes: 9 additions & 3 deletions lib/rubocop/cop/mixin/statement_modifier.rb
Expand Up @@ -57,10 +57,11 @@ def to_modifier_form(node)
end

def first_line_comment(node)
comment =
processed_source.find_comment { |c| c.loc.line == node.loc.line }
comment = processed_source.find_comment { |c| c.loc.line == node.loc.line }
return unless comment

comment ? comment.loc.expression.source : nil
comment_source = comment.loc.expression.source
comment_source unless comment_disables_cop?(comment_source)
end

def parenthesize?(node)
Expand All @@ -80,6 +81,11 @@ def max_line_length

config.for_cop('Layout/LineLength')['Max']
end

def comment_disables_cop?(comment)
regexp_pattern = "# rubocop : (disable|todo) ([^,],)* (all|#{cop_name})"
Regexp.new(regexp_pattern.gsub(' ', '\s*')).match?(comment)
end
end
end
end
4 changes: 0 additions & 4 deletions lib/rubocop/cop/style/if_unless_modifier.rb
Expand Up @@ -157,10 +157,6 @@ def to_normal_form(node)
#{indentation}end
RUBY
end

def first_line_comment(node)
processed_source.comment_at_line(node.loc.line)&.text
end
end
end
end
Expand Down
21 changes: 21 additions & 0 deletions spec/rubocop/cli/cli_autocorrect_spec.rb
Expand Up @@ -1607,4 +1607,25 @@ def self.some_method(foo, bar: 1)
RUBY
expect(source_file.read).to eq(corrected)
end

it 'does not correct Style/IfUnlessModifier offense disabled by a comment directive and ' \
'does not fire Lint/RedundantCopDisableDirective offense even though that directive ' \
'would make the modifier form too long' do
create_file('.rubocop.yml', <<~YAML)
Style/FrozenStringLiteralComment:
Enabled: false
YAML

source_file = Pathname('example.rb')
source = <<~RUBY
if i > 1 # rubocop:disable Style/IfUnlessModifier
raise '_______________________________________________________________________'
end
RUBY
create_file(source_file, source)

status = cli.run(['--auto-correct-all'])
expect(status).to eq(0)
expect(source_file.read).to eq(source)
end
end