Skip to content

Commit

Permalink
[Fix rubocop#126] Fix an incorrect auto-correct for `Rails/SafeNaviga…
Browse files Browse the repository at this point in the history
…tion`

Fixes rubocop#126.

This PR fixes an incorrect auto-correct for `Rails/SafeNavigation` with
`Style/RedndantSelf`.
  • Loading branch information
koic committed May 3, 2021
1 parent f0e8893 commit fb75399
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
* [#147](https://github.com/rubocop/rubocop-rails/issues/147): Fix a false positive for `Rails/HasManyOrHasOneDependent` when specifying default `dependent: nil` strategy. ([@koic][])
* [#137](https://github.com/rubocop/rubocop-rails/issues/137): Make `Rails/HasManyOrHasOneDependent` aware of `readonly?` is `true`. ([@koic][])
* [#474](https://github.com/rubocop/rubocop-rails/pull/474): Fix a false negative for `Rails/SafeNavigation` when using `try!` without receiver. ([@koic][])
* [#126](https://github.com/rubocop/rubocop-rails/issues/126): Fix an incorrect auto-correct for `Rails/SafeNavigation` with `Style/RedndantSelf`. ([@koic][])

### Changes

Expand Down
13 changes: 13 additions & 0 deletions lib/rubocop/cop/rails/safe_navigation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ class SafeNavigation < Base
(send _ ${:try :try!} $_ ...)
PATTERN

# Monkey patching for `Style/RedundantSelf` of RuboCop core.
# rubocop:disable Style/ClassAndModuleChildren
class Style::RedundantSelf
def self.autocorrect_incompatible_with
[Rails::SafeNavigation]
end
end
# rubocop:enable Style/ClassAndModuleChildren

def self.autocorrect_incompatible_with
[Style::RedundantSelf]
end

def on_send(node)
try_call(node) do |try_method, dispatch|
return if try_method == :try && !cop_config['ConvertTry']
Expand Down
27 changes: 27 additions & 0 deletions spec/rubocop/cli/autocorrect_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# frozen_string_literal: true

RSpec.describe 'RuboCop::CLI --autocorrect', :isolated_environment do # rubocop:disable RSpec/DescribeClass
subject(:cli) { RuboCop::CLI.new }

include_context 'cli spec behavior'

before do
RuboCop::ConfigLoader.default_configuration.for_all_cops['SuggestExtensions'] = false
end

it 'corrects `Rails/SafeNavigation` with `Style/RedndantSelf`' do
create_file('.rubocop.yml', <<~YAML)
Rails/SafeNavigation:
ConvertTry: true
YAML

create_file('example.rb', <<~RUBY)
self.try(:bar).try(:baz)
RUBY

expect(cli.run(['-a', '--only', 'Rails/SafeNavigation,Style/RedundantSelf'])).to eq(0)
expect(File.read('example.rb')).to eq(<<~RUBY)
self&.bar&.baz
RUBY
end
end
4 changes: 4 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
require_relative 'support/file_helper'
require_relative 'support/shared_contexts'

# Requires supporting files with custom matchers and macros, etc,
# in ./support/ and its subdirectories.
Dir["#{File.dirname(__FILE__)}/support/**/*.rb"].sort.each { |f| require f }

if ENV['COVERAGE'] == 'true'
require 'simplecov'
SimpleCov.start
Expand Down
28 changes: 28 additions & 0 deletions spec/support/cli_spec_behavior.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# frozen_string_literal: true

RSpec.shared_context 'cli spec behavior' do
include_context 'mock console output'

include FileHelper

def abs(path)
File.expand_path(path)
end

before do
RuboCop::ConfigLoader.debug = false

# OPTIMIZE: Makes these specs faster. Work directory (the parent of
# .rubocop_cache) is removed afterwards anyway.
RuboCop::ResultCache.inhibit_cleanup = true
end

# Wrap all cli specs in `aggregate_failures` so that the expected and
# actual results of every expectation per example are shown. This is
# helpful because it shows information like expected and actual
# $stdout messages while not using `aggregate_failures` will only
# show information about expected and actual exit code
around { |example| aggregate_failures(&example) }

after { RuboCop::ResultCache.inhibit_cleanup = false }
end

0 comments on commit fb75399

Please sign in to comment.