diff --git a/README.md b/README.md index 5060ad2..cdd4657 100644 --- a/README.md +++ b/README.md @@ -182,24 +182,6 @@ reorder('field(id, ?)', a) reorder(Arel.sql('field(id, ?)'), a) ``` -### Sevencop/RedundantExistenceCheck - -Identifies redundant existent check before file operation. - -```ruby -# bad -FileUtils.mkdir(a) unless FileTest.exist?(a) - -# good -FileUtils.mkdir_p(a) - -# bad -FileUtils.rm(a) if FileTest.exist?(a) - -# good -FileUtils.rm_f(a) -``` - ### Sevencop/UniquenessValidatorExplicitCaseSensitivity Identifies use of UniquenessValidator without :case_sensitive option. diff --git a/config/default.yml b/config/default.yml index a5074d3..52c1724 100644 --- a/config/default.yml +++ b/config/default.yml @@ -42,13 +42,6 @@ Sevencop/OrderField: Safe: false VersionAdded: '0.4' -Sevencop/RedundantExistenceCheck: - Description: | - Avoid redundant existent check before file operation. - Enabled: false - Safe: false - VersionAdded: '0.1' - Sevencop/ToSWithArgument: Description: | Identifies passing any argument to `#to_s`. diff --git a/lib/rubocop/cop/sevencop/redundant_existence_check.rb b/lib/rubocop/cop/sevencop/redundant_existence_check.rb deleted file mode 100644 index b758013..0000000 --- a/lib/rubocop/cop/sevencop/redundant_existence_check.rb +++ /dev/null @@ -1,188 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Cop - module Sevencop - # Identifies redundant existent check before file operation. - # - # @safety - # This cop is unsafe because it can register a false positive where the check is truly needed. - # - # @example - # - # # bad - # FileUtils.mkdir(a) unless FileTest.exist?(a) - # - # # good - # FileUtils.mkdir_p(a) - # - # # bad - # FileUtils.rm(a) if File.exist?(a) - # - # # good - # FileUtils.rm_f(a) - # - class RedundantExistenceCheck < Base - extend AutoCorrector - - CLASS_NAMES_FOR_EXIST = ::Set[ - :File, - :FileTest, - ] - - CLASS_NAMES_FOR_OPERATION = ::Set[ - :File, - :FileUtils, - ] - - METHOD_NAMES_FOR_MAKE = ::Set[ - :makedirs, - :mkdir, - :mkdir_p, - :mkpath, - :touch, - ] - - METHOD_NAMES_FOR_REMOVE = ::Set[ - :delete, - :remove, - :remove_dir, - :remove_entry, - :remove_entry_secure, - :remove_file, - :rm, - :rm_f, - :rm_r, - :rm_rf, - :rmdir, - :rmtree, - :safe_unlink, - :unlink, - ] - - METHOD_NAMES_FOR_EXIST = ::Set[ - :exist?, - :exists?, - ] - - METHOD_NAMES_FOR_FORCE_OPERATION = ::Set[ - :makedirs, - :mkdir_p, - :mkpath, - :rm_f, - :rm_rf, - :rm_tree, - :safe_unlink, - :touch, - ] - - METHOD_MAPPING_FOR_FORCE_REPLACEMENT = { - 'FileUtils.mkdir' => 'FileUtils.mkdir_p', - 'File.delete' => 'FileUtils.rm_f', - 'File.unlink' => 'FileUtils.rm_f' - }.freeze - - MSG = 'Avoid redundant existent check before file operation.' - - # @!method make_unless_exist?(node) - def_node_matcher :make_unless_exist?, <<~PATTERN - (if - (send (const nil? CLASS_NAMES_FOR_EXIST) METHOD_NAMES_FOR_EXIST _) - nil? - (send (const nil? CLASS_NAMES_FOR_OPERATION) METHOD_NAMES_FOR_MAKE ...) - ) - PATTERN - - # @!method remove_if_exist?(node) - def_node_matcher :remove_if_exist?, <<~PATTERN - (if - (send (const nil? CLASS_NAMES_FOR_EXIST) METHOD_NAMES_FOR_EXIST _) - (send (const nil? CLASS_NAMES_FOR_OPERATION) METHOD_NAMES_FOR_REMOVE ...) - nil? - ) - PATTERN - - def on_if(node) - return unless redundant_on_if(node) || redundant_on_unless(node) - - add_offense(node) do |corrector| - corrector.replace( - node.location.expression, - enforce(node) - ) - end - end - - private - - def enforce(node) - if force_operation?(node) - node.if_branch.source - elsif force_replaceable_method?(node) - enforce_by_replacement(node) - else - enforce_by_force_option(node) - end - end - - def enforce_by_force_option(node) - arguments = node.if_branch.arguments.map(&:source) - arguments << 'force: true' unless force_operation?(node) - format( - '%s.%s(%s)', - arguments: arguments.join(', '), - method_name: node.if_branch.method_name, - receiver: node.if_branch.receiver.source - ) - end - - def enforce_by_replacement(node) - format( - '%s(%s)', - arguments: node.if_branch.arguments.map(&:source).join(', '), - signature: METHOD_MAPPING_FOR_FORCE_REPLACEMENT[operation_method_signature(node)] - ) - end - - def force_operation?(node) - force_operation_method_name?(node) || force_operation_argument?(node) - end - - def force_operation_argument?(node) - node.if_branch.last_argument.hash_type? && - node.if_branch.last_argument.pairs.any? do |pair| - pair.key.value == :force && pair.value.true_type? - end - end - - def force_operation_method_name?(node) - METHOD_NAMES_FOR_FORCE_OPERATION.include?(node.if_branch.method_name) - end - - def redundant_on_if(node) - remove_if_exist?(node) && same_argument?(node) - end - - def redundant_on_unless(node) - make_unless_exist?(node) && same_argument?(node) - end - - def force_replaceable_method?(node) - METHOD_MAPPING_FOR_FORCE_REPLACEMENT.key?(operation_method_signature(node)) - end - - def operation_method_signature(node) - format( - '%s.%s', - method_name: node.if_branch.method_name, - receiver: node.if_branch.receiver.source - ) - end - - def same_argument?(node) - node.condition.first_argument == node.if_branch.first_argument - end - end - end - end -end diff --git a/lib/sevencop.rb b/lib/sevencop.rb index 210642f..553d0a3 100644 --- a/lib/sevencop.rb +++ b/lib/sevencop.rb @@ -10,7 +10,6 @@ require_relative 'rubocop/cop/sevencop/method_definition_keyword_arguments_ordered' require_relative 'rubocop/cop/sevencop/method_definition_multiline_arguments' require_relative 'rubocop/cop/sevencop/order_field' -require_relative 'rubocop/cop/sevencop/redundant_existence_check' require_relative 'rubocop/cop/sevencop/to_s_with_argument' require_relative 'rubocop/cop/sevencop/uniqueness_validator_explicit_case_sensitivity' require_relative 'rubocop/cop/sevencop/where_not' diff --git a/spec/rubocop/cop/sevencop/redundant_existence_check_spec.rb b/spec/rubocop/cop/sevencop/redundant_existence_check_spec.rb deleted file mode 100644 index d7bb5ca..0000000 --- a/spec/rubocop/cop/sevencop/redundant_existence_check_spec.rb +++ /dev/null @@ -1,115 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe RuboCop::Cop::Sevencop::RedundantExistenceCheck, :config do - context 'with FileUtils.rm(a)' do - it 'autocorrects offense' do - expect_offense(<<~TEXT) - FileUtils.rm(a) if FileTest.exist?(a) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid redundant existent check before file operation. - TEXT - - expect_correction(<<~RUBY) - FileUtils.rm(a, force: true) - RUBY - end - end - - context 'with FileUtils.rm(a, force: true)' do - it 'autocorrects offense' do - expect_offense(<<~TEXT) - FileUtils.rm(a, force: true) if FileTest.exist?(a) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid redundant existent check before file operation. - TEXT - - expect_correction(<<~RUBY) - FileUtils.rm(a, force: true) - RUBY - end - end - - context 'with FileUtils.rm_f(a)' do - it 'autocorrects offense' do - expect_offense(<<~TEXT) - FileUtils.rm_f(a) if FileTest.exist?(a) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid redundant existent check before file operation. - TEXT - - expect_correction(<<~RUBY) - FileUtils.rm_f(a) - RUBY - end - end - - context 'with File.delete(a)' do - it 'autocorrects offense' do - expect_offense(<<~TEXT) - File.delete(a) if FileTest.exist?(a) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid redundant existent check before file operation. - TEXT - - expect_correction(<<~RUBY) - FileUtils.rm_f(a) - RUBY - end - end - - context 'with FileUtils.mkdir(a)' do - it 'autocorrects offense' do - expect_offense(<<~TEXT) - FileUtils.mkdir(a) unless FileTest.exist?(a) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid redundant existent check before file operation. - TEXT - - expect_correction(<<~RUBY) - FileUtils.mkdir_p(a) - RUBY - end - end - - context 'with FileUtils.mkdir_p(a, verbose: true)' do - it 'autocorrects offense' do - expect_offense(<<~TEXT) - FileUtils.mkdir_p(a, verbose: true) unless FileTest.exist?(a) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid redundant existent check before file operation. - TEXT - - expect_correction(<<~RUBY) - FileUtils.mkdir_p(a, verbose: true) - RUBY - end - end - - context 'with FileUtils.mkdir(a, verbose: true)' do - it 'autocorrects offense' do - expect_offense(<<~TEXT) - FileUtils.mkdir(a, verbose: true) unless FileTest.exist?(a) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid redundant existent check before file operation. - TEXT - - expect_correction(<<~RUBY) - FileUtils.mkdir_p(a, verbose: true) - RUBY - end - end - - context 'with FileUtils.rm(a, force: true, verbose: true)' do - it 'autocorrects offense' do - expect_offense(<<~TEXT) - FileUtils.rm(a, force: true, verbose: true) if File.exist?(a) - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid redundant existent check before file operation. - TEXT - - expect_correction(<<~RUBY) - FileUtils.rm(a, force: true, verbose: true) - RUBY - end - end - - context 'with different 1st argument' do - it 'registers no offense' do - expect_no_offenses(<<~TEXT) - FileUtils.mkdir(a) unless FileTest.exist?(b) - TEXT - end - end -end