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`.
This commit is contained in:
Issy Long 2024-07-15 14:05:36 -04:00
parent 83e21fba11
commit 7404735654
No known key found for this signature in database
3 changed files with 33 additions and 17 deletions

View File

@ -11,35 +11,38 @@ module RuboCop
MSG = "Use `rm` or `rm_r` instead of `rm_rf`, `rm_f`, or `rmtree`." MSG = "Use `rm` or `rm_r` instead of `rm_rf`, `rm_f`, or `rmtree`."
def_node_matcher :fileutils_rm_r_f_tree?, <<~PATTERN def_node_matcher :any_receiver_rm_r_f?, <<~PATTERN
(send (const {nil? cbase} :FileUtils) {:rm_rf :rm_f :rmtree} ...) (send
{(const {nil? cbase} :FileUtils) (self)}
{:rm_rf :rm_f}
...)
PATTERN PATTERN
def_node_matcher :instance_rmtree?, <<~PATTERN def_node_matcher :no_receiver_rm_r_f?, <<~PATTERN
(send (lvar ...) :rmtree ...) (send nil? {:rm_rf :rm_f} ...)
PATTERN PATTERN
def_node_matcher :self_rm_r_f_tree?, <<~PATTERN def_node_matcher :no_receiver_rmtree?, <<~PATTERN
(send (self) {:rm_rf :rm_f :rmtree} ...) (send nil? :rmtree ...)
PATTERN PATTERN
def_node_matcher :plain_rm_r_f_tree?, <<~PATTERN def_node_matcher :any_receiver_rmtree?, <<~PATTERN
(send nil? {:rm_rf :rm_f :rmtree} ...) (send !nil? :rmtree ...)
PATTERN PATTERN
def on_send(node) def on_send(node)
return if neither_rm_rf_nor_rmtree?(node) return if neither_rm_rf_nor_rmtree?(node)
add_offense(node) do |corrector| 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) new_method = if node.method?(:rm_rf) || node.method?(:rmtree)
"rm_r" "rm_r"
else else
"rm" "rm"
end end
args = if instance_rmtree?(node) args = if any_receiver_rmtree?(node)
node.receiver.source node.receiver&.source || node.arguments.first&.source
else else
node.arguments.first.source node.arguments.first.source
end end
@ -49,8 +52,8 @@ module RuboCop
end end
def neither_rm_rf_nor_rmtree?(node) def neither_rm_rf_nor_rmtree?(node)
!self_rm_r_f_tree?(node) && !plain_rm_r_f_tree?(node) && !any_receiver_rm_r_f?(node) && !no_receiver_rm_r_f?(node) &&
!fileutils_rm_r_f_tree?(node) && !instance_rmtree?(node) !any_receiver_rmtree?(node) && !no_receiver_rmtree?(node)
end end
end end
end end

View File

@ -7,14 +7,14 @@
class RuboCop::Cop::Homebrew::NoFileutilsRmrf class RuboCop::Cop::Homebrew::NoFileutilsRmrf
sig { params(node: RuboCop::AST::Node, kwargs: T.untyped, block: T.untyped).returns(T.untyped) } 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) } 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) } 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) } 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 end

View File

@ -59,6 +59,11 @@ RSpec.describe RuboCop::Cop::Homebrew::NoFileutilsRmrf do
other_dir = Pathname("path/to/other/directory") other_dir = Pathname("path/to/other/directory")
other_dir.rmtree other_dir.rmtree
^^^^^^^^^^^^^^^^ Homebrew/NoFileutilsRmrf: #{RuboCop::Cop::Homebrew::NoFileutilsRmrf::MSG} ^^^^^^^^^^^^^^^^ 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 RUBY
end end
@ -67,12 +72,20 @@ RSpec.describe RuboCop::Cop::Homebrew::NoFileutilsRmrf do
rmtree("path/to/directory") rmtree("path/to/directory")
other_dir = Pathname("path/to/other/directory") other_dir = Pathname("path/to/other/directory")
other_dir.rmtree other_dir.rmtree
def buildpath
Pathname("path/to/yet/another/directory")
end
buildpath.rmtree
RUBY RUBY
expect(corrected).to eq(<<~RUBY) expect(corrected).to eq(<<~RUBY)
rm_r("path/to/directory") rm_r("path/to/directory")
other_dir = Pathname("path/to/other/directory") other_dir = Pathname("path/to/other/directory")
FileUtils.rm_r(other_dir) FileUtils.rm_r(other_dir)
def buildpath
Pathname("path/to/yet/another/directory")
end
FileUtils.rm_r(buildpath)
RUBY RUBY
end end
end end