From 7404735654cecb5f4139ecc7f78ce86bd355b326 Mon Sep 17 00:00:00 2001 From: Issy Long Date: Mon, 15 Jul 2024 14:05:36 -0400 Subject: [PATCH] rubocops/no_fileutils_rmrf: Fix `rmtree` on a method returning Pathname - Tidy up the node matchers. Either `FileUtils.rm_rf` or `rm_rf` on a `Pathname` instance or `self`. --- .../Homebrew/rubocops/no_fileutils_rmrf.rb | 29 ++++++++++--------- .../cop/homebrew/no_fileutils_rmrf.rbi | 8 ++--- .../test/rubocops/no_fileutils_rmrf_spec.rb | 13 +++++++++ 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/Library/Homebrew/rubocops/no_fileutils_rmrf.rb b/Library/Homebrew/rubocops/no_fileutils_rmrf.rb index 981f5e916e..9828d12c30 100644 --- a/Library/Homebrew/rubocops/no_fileutils_rmrf.rb +++ b/Library/Homebrew/rubocops/no_fileutils_rmrf.rb @@ -11,35 +11,38 @@ module RuboCop MSG = "Use `rm` or `rm_r` instead of `rm_rf`, `rm_f`, or `rmtree`." - def_node_matcher :fileutils_rm_r_f_tree?, <<~PATTERN - (send (const {nil? cbase} :FileUtils) {:rm_rf :rm_f :rmtree} ...) + def_node_matcher :any_receiver_rm_r_f?, <<~PATTERN + (send + {(const {nil? cbase} :FileUtils) (self)} + {:rm_rf :rm_f} + ...) PATTERN - def_node_matcher :instance_rmtree?, <<~PATTERN - (send (lvar ...) :rmtree ...) + def_node_matcher :no_receiver_rm_r_f?, <<~PATTERN + (send nil? {:rm_rf :rm_f} ...) PATTERN - def_node_matcher :self_rm_r_f_tree?, <<~PATTERN - (send (self) {:rm_rf :rm_f :rmtree} ...) + def_node_matcher :no_receiver_rmtree?, <<~PATTERN + (send nil? :rmtree ...) PATTERN - def_node_matcher :plain_rm_r_f_tree?, <<~PATTERN - (send nil? {:rm_rf :rm_f :rmtree} ...) + def_node_matcher :any_receiver_rmtree?, <<~PATTERN + (send !nil? :rmtree ...) PATTERN def on_send(node) return if neither_rm_rf_nor_rmtree?(node) add_offense(node) do |corrector| - class_name = "FileUtils." if fileutils_rm_r_f_tree?(node) || instance_rmtree?(node) + class_name = "FileUtils." if any_receiver_rm_r_f?(node) || any_receiver_rmtree?(node) new_method = if node.method?(:rm_rf) || node.method?(:rmtree) "rm_r" else "rm" end - args = if instance_rmtree?(node) - node.receiver.source + args = if any_receiver_rmtree?(node) + node.receiver&.source || node.arguments.first&.source else node.arguments.first.source end @@ -49,8 +52,8 @@ module RuboCop 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) && !instance_rmtree?(node) + !any_receiver_rm_r_f?(node) && !no_receiver_rm_r_f?(node) && + !any_receiver_rmtree?(node) && !no_receiver_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 a95a8dc793..b6d9c6a094 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 @@ -7,14 +7,14 @@ class RuboCop::Cop::Homebrew::NoFileutilsRmrf sig { params(node: RuboCop::AST::Node, kwargs: T.untyped, block: T.untyped).returns(T.untyped) } - def fileutils_rm_r_f_tree?(node, **kwargs, &block); end + def any_receiver_rm_r_f?(node, **kwargs, &block); end sig { params(node: RuboCop::AST::Node, kwargs: T.untyped, block: T.untyped).returns(T.untyped) } - def instance_rmtree?(node, **kwargs, &block); end + def any_receiver_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 + def no_receiver_rm_r_f?(node, **kwargs, &block); end sig { params(node: RuboCop::AST::Node, kwargs: T.untyped, block: T.untyped).returns(T.untyped) } - def self_rm_r_f_tree?(node, **kwargs, &block); end + def no_receiver_rmtree?(node, **kwargs, &block); end end diff --git a/Library/Homebrew/test/rubocops/no_fileutils_rmrf_spec.rb b/Library/Homebrew/test/rubocops/no_fileutils_rmrf_spec.rb index 2774696b9e..128d3bd3e5 100644 --- a/Library/Homebrew/test/rubocops/no_fileutils_rmrf_spec.rb +++ b/Library/Homebrew/test/rubocops/no_fileutils_rmrf_spec.rb @@ -59,6 +59,11 @@ RSpec.describe RuboCop::Cop::Homebrew::NoFileutilsRmrf do other_dir = Pathname("path/to/other/directory") other_dir.rmtree ^^^^^^^^^^^^^^^^ Homebrew/NoFileutilsRmrf: #{RuboCop::Cop::Homebrew::NoFileutilsRmrf::MSG} + def buildpath + Pathname("path/to/yet/another/directory") + end + buildpath.rmtree + ^^^^^^^^^^^^^^^^ Homebrew/NoFileutilsRmrf: #{RuboCop::Cop::Homebrew::NoFileutilsRmrf::MSG} RUBY end @@ -67,12 +72,20 @@ RSpec.describe RuboCop::Cop::Homebrew::NoFileutilsRmrf do rmtree("path/to/directory") other_dir = Pathname("path/to/other/directory") other_dir.rmtree + def buildpath + Pathname("path/to/yet/another/directory") + end + buildpath.rmtree RUBY expect(corrected).to eq(<<~RUBY) rm_r("path/to/directory") other_dir = Pathname("path/to/other/directory") FileUtils.rm_r(other_dir) + def buildpath + Pathname("path/to/yet/another/directory") + end + FileUtils.rm_r(buildpath) RUBY end end