From 83e21fba1196518b5fae4ee2f05415f479d6ef47 Mon Sep 17 00:00:00 2001 From: Issy Long Date: Sun, 14 Jul 2024 22:34:34 -0400 Subject: [PATCH] rubocop/no_fileutils_rmrf: Handle `rmtree` as an instance method --- Library/Homebrew/rubocops/no_fileutils_rmrf.rb | 18 ++++++++++++------ .../cop/homebrew/no_fileutils_rmrf.rbi | 2 +- .../test/rubocops/no_fileutils_rmrf_spec.rb | 11 +++++++---- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Library/Homebrew/rubocops/no_fileutils_rmrf.rb b/Library/Homebrew/rubocops/no_fileutils_rmrf.rb index cafeda7acc..981f5e916e 100644 --- a/Library/Homebrew/rubocops/no_fileutils_rmrf.rb +++ b/Library/Homebrew/rubocops/no_fileutils_rmrf.rb @@ -4,7 +4,7 @@ module RuboCop module Cop module Homebrew - # This cop checks for the use of `FileUtils.rm_f`, `FileUtils.rm_rf`, or `{FileUtils,Pathname}.rmtree` + # This cop checks for the use of `FileUtils.rm_f`, `FileUtils.rm_rf`, or `{FileUtils,instance}.rmtree` # and recommends the safer versions. class NoFileutilsRmrf < Base extend AutoCorrector @@ -15,8 +15,8 @@ module RuboCop (send (const {nil? cbase} :FileUtils) {:rm_rf :rm_f :rmtree} ...) PATTERN - def_node_matcher :pathname_rm_r_f_tree?, <<~PATTERN - (send (const {nil? cbase} :Pathname) :rmtree ...) + def_node_matcher :instance_rmtree?, <<~PATTERN + (send (lvar ...) :rmtree ...) PATTERN def_node_matcher :self_rm_r_f_tree?, <<~PATTERN @@ -31,20 +31,26 @@ module RuboCop return if neither_rm_rf_nor_rmtree?(node) add_offense(node) do |corrector| - class_name = "FileUtils." if fileutils_rm_r_f_tree?(node) || pathname_rm_r_f_tree?(node) + class_name = "FileUtils." if fileutils_rm_r_f_tree?(node) || instance_rmtree?(node) new_method = if node.method?(:rm_rf) || node.method?(:rmtree) "rm_r" else "rm" end - corrector.replace(node.loc.expression, "#{class_name}#{new_method}(#{node.arguments.first.source})") + args = if instance_rmtree?(node) + node.receiver.source + else + node.arguments.first.source + end + + corrector.replace(node.loc.expression, "#{class_name}#{new_method}(#{args})") end end def neither_rm_rf_nor_rmtree?(node) !self_rm_r_f_tree?(node) && !plain_rm_r_f_tree?(node) && - !fileutils_rm_r_f_tree?(node) && !pathname_rm_r_f_tree?(node) + !fileutils_rm_r_f_tree?(node) && !instance_rmtree?(node) end end end diff --git a/Library/Homebrew/sorbet/rbi/dsl/rubo_cop/cop/homebrew/no_fileutils_rmrf.rbi b/Library/Homebrew/sorbet/rbi/dsl/rubo_cop/cop/homebrew/no_fileutils_rmrf.rbi index 6c6f3c1db7..a95a8dc793 100644 --- a/Library/Homebrew/sorbet/rbi/dsl/rubo_cop/cop/homebrew/no_fileutils_rmrf.rbi +++ b/Library/Homebrew/sorbet/rbi/dsl/rubo_cop/cop/homebrew/no_fileutils_rmrf.rbi @@ -10,7 +10,7 @@ class RuboCop::Cop::Homebrew::NoFileutilsRmrf def fileutils_rm_r_f_tree?(node, **kwargs, &block); end sig { params(node: RuboCop::AST::Node, kwargs: T.untyped, block: T.untyped).returns(T.untyped) } - def pathname_rm_r_f_tree?(node, **kwargs, &block); end + def instance_rmtree?(node, **kwargs, &block); end sig { params(node: RuboCop::AST::Node, kwargs: T.untyped, block: T.untyped).returns(T.untyped) } def plain_rm_r_f_tree?(node, **kwargs, &block); end diff --git a/Library/Homebrew/test/rubocops/no_fileutils_rmrf_spec.rb b/Library/Homebrew/test/rubocops/no_fileutils_rmrf_spec.rb index 93d2f2be78..2774696b9e 100644 --- a/Library/Homebrew/test/rubocops/no_fileutils_rmrf_spec.rb +++ b/Library/Homebrew/test/rubocops/no_fileutils_rmrf_spec.rb @@ -56,20 +56,23 @@ RSpec.describe RuboCop::Cop::Homebrew::NoFileutilsRmrf do expect_offense(<<~RUBY) rmtree("path/to/directory") ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Homebrew/NoFileutilsRmrf: #{RuboCop::Cop::Homebrew::NoFileutilsRmrf::MSG} - Pathname.rmtree("path/to/other/directory") - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Homebrew/NoFileutilsRmrf: #{RuboCop::Cop::Homebrew::NoFileutilsRmrf::MSG} + other_dir = Pathname("path/to/other/directory") + other_dir.rmtree + ^^^^^^^^^^^^^^^^ Homebrew/NoFileutilsRmrf: #{RuboCop::Cop::Homebrew::NoFileutilsRmrf::MSG} RUBY end it "autocorrects" do corrected = autocorrect_source(<<~RUBY) rmtree("path/to/directory") - Pathname.rmtree("path/to/other/directory") + other_dir = Pathname("path/to/other/directory") + other_dir.rmtree RUBY expect(corrected).to eq(<<~RUBY) rm_r("path/to/directory") - FileUtils.rm_r("path/to/other/directory") + other_dir = Pathname("path/to/other/directory") + FileUtils.rm_r(other_dir) RUBY end end