From fb7539977b32a39abdeca42ef8c6f27f52c16e64 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 4 May 2021 01:23:23 +0900 Subject: [PATCH] [Fix #126] Fix an incorrect auto-correct for `Rails/SafeNavigation` Fixes #126. This PR fixes an incorrect auto-correct for `Rails/SafeNavigation` with `Style/RedndantSelf`. --- CHANGELOG.md | 1 + lib/rubocop/cop/rails/safe_navigation.rb | 13 +++++++++++ spec/rubocop/cli/autocorrect_spec.rb | 27 +++++++++++++++++++++++ spec/spec_helper.rb | 4 ++++ spec/support/cli_spec_behavior.rb | 28 ++++++++++++++++++++++++ 5 files changed, 73 insertions(+) create mode 100644 spec/rubocop/cli/autocorrect_spec.rb create mode 100644 spec/support/cli_spec_behavior.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f8cbb6067..6fe0501ab2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/rubocop/cop/rails/safe_navigation.rb b/lib/rubocop/cop/rails/safe_navigation.rb index 1e82928c48..b5e67e4d74 100644 --- a/lib/rubocop/cop/rails/safe_navigation.rb +++ b/lib/rubocop/cop/rails/safe_navigation.rb @@ -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'] diff --git a/spec/rubocop/cli/autocorrect_spec.rb b/spec/rubocop/cli/autocorrect_spec.rb new file mode 100644 index 0000000000..833a700a4b --- /dev/null +++ b/spec/rubocop/cli/autocorrect_spec.rb @@ -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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b9bce6146c..f86679c1bf 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -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 diff --git a/spec/support/cli_spec_behavior.rb b/spec/support/cli_spec_behavior.rb new file mode 100644 index 0000000000..4c1d018d69 --- /dev/null +++ b/spec/support/cli_spec_behavior.rb @@ -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