Merge pull request #17705 from Homebrew/rubocop-avoid-fileutils-rmrf

rubocop: Discourage the use of `rm_f` and `rm_rf` in formulae and casks

Library/.rubocop.yml

@@ -60,6 +60,11 @@ Homebrew/CompactBlank:
  # `blank?` is not necessarily available here:
  - "Homebrew/extend/enumerable.rb"
 
+Homebrew/NoFileutilsRmrf:
+ Include:
+ - "/**/{Formula,Casks}/**/*.rb"
+ - "**/{Formula,Casks}/**/*.rb"
+
 # only used internally
 Homebrew/MoveToExtendOS:
  Enabled: false

Library/Homebrew/rubocops/all.rb

@@ -8,6 +8,7 @@
 require_relative "io_read"
 require_relative "move_to_extend_os"
 require_relative "negate_include"
+require_relative "no_fileutils_rmrf"
 require_relative "presence"
 require_relative "present"
 require_relative "safe_navigation_with_blank"

Library/Homebrew/rubocops/no_fileutils_rmrf.rb

@@ -0,0 +1,61 @@
+# typed: true
+# frozen_string_literal: true
+
+module RuboCop
+ module Cop
+ module Homebrew
+ # 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
+
+ MSG = "Use `rm` or `rm_r` instead of `rm_rf`, `rm_f`, or `rmtree`."
+
+ def_node_matcher :any_receiver_rm_r_f?, <<~PATTERN
+ (send
+ {(const {nil? cbase} :FileUtils) (self)}
+ {:rm_rf :rm_f}
+ ...)
+ PATTERN
+
+ def_node_matcher :no_receiver_rm_r_f?, <<~PATTERN
+ (send nil? {:rm_rf :rm_f} ...)
+ PATTERN
+
+ def_node_matcher :no_receiver_rmtree?, <<~PATTERN
+ (send nil? :rmtree ...)
+ PATTERN
+
+ 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 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 any_receiver_rmtree?(node)
+ node.receiver&.source || node.arguments.first&.source
+ else
+ node.arguments.first.source
+ end
+ args = "(#{args})" unless args.start_with?("(")
+ corrector.replace(node.loc.expression, "#{class_name}#{new_method}#{args}")
+ end
+ end
+
+ def neither_rm_rf_nor_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
+ end
+end

Library/Homebrew/test/rubocops/no_fileutils_rmrf_spec.rb

@@ -0,0 +1,96 @@
+# frozen_string_literal: true
+
+require "rubocops/no_fileutils_rmrf"
+
+RSpec.describe RuboCop::Cop::Homebrew::NoFileutilsRmrf do
+ subject(:cop) { described_class.new }
+
+ describe "rm_rf" do
+ it "registers an offense" do
+ expect_offense(<<~RUBY)
+ rm_rf("path/to/directory")
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^ Homebrew/NoFileutilsRmrf: #{RuboCop::Cop::Homebrew::NoFileutilsRmrf::MSG}
+ FileUtils.rm_rf("path/to/directory")
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Homebrew/NoFileutilsRmrf: #{RuboCop::Cop::Homebrew::NoFileutilsRmrf::MSG}
+ RUBY
+ end
+
+ it "autocorrects" do
+ corrected = autocorrect_source(<<~RUBY)
+ rm_rf("path/to/directory")
+ FileUtils.rm_rf("path/to/other/directory")
+ RUBY
+
+ expect(corrected).to eq(<<~RUBY)
+ rm_r("path/to/directory")
+ FileUtils.rm_r("path/to/other/directory")
+ RUBY
+ end
+ end
+
+ describe "rm_f" do
+ it "registers an offense" do
+ expect_offense(<<~RUBY)
+ rm_f("path/to/directory")
+ ^^^^^^^^^^^^^^^^^^^^^^^^^ Homebrew/NoFileutilsRmrf: #{RuboCop::Cop::Homebrew::NoFileutilsRmrf::MSG}
+ FileUtils.rm_f("path/to/other/directory")
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Homebrew/NoFileutilsRmrf: #{RuboCop::Cop::Homebrew::NoFileutilsRmrf::MSG}
+ RUBY
+ end
+
+ it "autocorrects" do
+ corrected = autocorrect_source(<<~RUBY)
+ rm_f("path/to/directory")
+ FileUtils.rm_f("path/to/other/directory")
+ RUBY
+
+ expect(corrected).to eq(<<~RUBY)
+ rm("path/to/directory")
+ FileUtils.rm("path/to/other/directory")
+ RUBY
+ end
+ end
+
+ describe "rmtree" do
+ it "registers an offense" do
+ expect_offense(<<~RUBY)
+ rmtree("path/to/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}
+ def buildpath
+ Pathname("path/to/yet/another/directory")
+ end
+ buildpath.rmtree
+ ^^^^^^^^^^^^^^^^ Homebrew/NoFileutilsRmrf: #{RuboCop::Cop::Homebrew::NoFileutilsRmrf::MSG}
+ (path/"here").rmtree
+ ^^^^^^^^^^^^^^^^^^^^ Homebrew/NoFileutilsRmrf: #{RuboCop::Cop::Homebrew::NoFileutilsRmrf::MSG}
+ RUBY
+ end
+
+ it "autocorrects" do
+ corrected = autocorrect_source(<<~RUBY)
+ 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
+ (path/"here").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)
+ FileUtils.rm_r(path/"here")
+ RUBY
+ end
+ end
+end