From b771b052a5c3c062c5e6ff4734119d4d7abe4819 Mon Sep 17 00:00:00 2001
From: Branch Vincent <branchevincent@gmail.com>
Date: Fri, 9 Aug 2024 19:38:31 -0700
Subject: [PATCH] rubocops/lines: require a comment for `skip_clean`

---
 Library/Homebrew/rubocops/lines.rb            | 22 ++++++
 .../text/skip_clean_commented_spec.rb         | 69 +++++++++++++++++++
 2 files changed, 91 insertions(+)
 create mode 100644 Library/Homebrew/test/rubocops/text/skip_clean_commented_spec.rb

diff --git a/Library/Homebrew/rubocops/lines.rb b/Library/Homebrew/rubocops/lines.rb
index 2c1ef8103c605..29fed37fdb9ae 100644
--- a/Library/Homebrew/rubocops/lines.rb
+++ b/Library/Homebrew/rubocops/lines.rb
@@ -84,6 +84,28 @@ def audit_formula(_formula_nodes)
         end
       end
 
+      # Ensure every `skip_clean` is documented with a comment.
+      class SkipCleanCommented < FormulaCop
+        sig { override.params(formula_nodes: FormulaNodes).void }
+        def audit_formula(formula_nodes)
+          return if formula_tap != "homebrew-core"
+          return if (body_node = formula_nodes.body_node).nil?
+
+          # Some formulae call `skip_clean` multiple times, only require a
+          # comment on the first
+          method = find_every_method_call_by_name(body_node, :skip_clean).first
+          return if method.nil?
+
+          method_comment = processed_source.comments.find do |comment|
+            (method.loc.line - comment.loc.line).abs <= 1
+          end
+          return unless method_comment.nil?
+
+          offending_node(method)
+          problem "Formulae in homebrew/core should document why `skip_clean` is needed with a comment."
+        end
+      end
+
       # This cop makes sure that idiomatic `assert_*` statements are used.
       class AssertStatements < FormulaCop
         sig { override.params(formula_nodes: FormulaNodes).void }
diff --git a/Library/Homebrew/test/rubocops/text/skip_clean_commented_spec.rb b/Library/Homebrew/test/rubocops/text/skip_clean_commented_spec.rb
new file mode 100644
index 0000000000000..fe18f48dd7c05
--- /dev/null
+++ b/Library/Homebrew/test/rubocops/text/skip_clean_commented_spec.rb
@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+require "rubocops/lines"
+
+RSpec.describe RuboCop::Cop::FormulaAudit::SkipCleanCommented do
+  subject(:cop) { described_class.new }
+
+  context "when auditing formulae in homebrew-core" do
+    it "reports an offense when skip_clean does not have a comment" do
+      expect_offense(<<~RUBY, "/homebrew-core/")
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+
+          skip_clean "bin"
+          ^^^^^^^^^^^^^^^^ FormulaAudit/SkipCleanCommented: Formulae in homebrew/core should document why `skip_clean` is needed with a comment.
+        end
+      RUBY
+    end
+
+    it "reports a single offense for multiple skip_clean lines" do
+      expect_offense(<<~RUBY, "/homebrew-core/")
+        class Foo < Formula
+          url 'https://brew.sh/foo-1.0.tgz'
+
+          skip_clean "bin"
+          ^^^^^^^^^^^^^^^^ FormulaAudit/SkipCleanCommented: Formulae in homebrew/core should document why `skip_clean` is needed with a comment.
+          skip_clean "libexec/bin"
+        end
+      RUBY
+    end
+
+    it "does not report an offense for a comment" do
+      expect_no_offenses(<<~RUBY, "/homebrew-core/")
+        class Foo < Formula
+          # some reason
+          skip_clean "bin"
+        end
+      RUBY
+    end
+
+    it "does not report an offense for an inline comment" do
+      expect_no_offenses(<<~RUBY, "/homebrew-core/")
+        class Foo < Formula
+          skip_clean "bin" # some reason
+        end
+      RUBY
+    end
+
+    it "does not report an offense for a comment above multiple skip_clean lines" do
+      expect_no_offenses(<<~RUBY, "/homebrew-core/")
+        class Foo < Formula
+          # some reason
+          skip_clean "bin"
+          skip_clean "libexec/bin"
+        end
+      RUBY
+    end
+  end
+
+  context "when auditing formulae not in homebrew-core" do
+    it "does not report an offense" do
+      expect_no_offenses(<<~RUBY)
+        class Foo < Formula
+          skip_clean "bin"
+        end
+      RUBY
+    end
+  end
+end