From cd430c4d345f2419090aac08c69055dbfc05e30a Mon Sep 17 00:00:00 2001
From: apainintheneck <apainintheneck@gmail.com>
Date: Sun, 21 May 2023 16:19:25 -0700
Subject: [PATCH] cmd/update-report: improve formula file validation

Currently, ruby files that are not in the Cask directory are
considered to be formulae if a Formula or HomebrewFormula directory
doesn't exist  which doesn't make sense. We know that these should
only be in a few directories so we can check for that explicitly.

Beyond that the `Tap#cask_file?` and `Tap.formula_file?` methods
were only used inside update-report so it doesn't make sense to
turn them into pathnames and expand things when we know that
each string will be a relative path from a tap that we can just
check with a regex.

This change will stop other tap changes like new commands or
changes to other directories like lib/ from showing up as new formulae.

I tried opening a PR for this a long time ago but I got busy
with other things and it got closed by the stale bot.

- https://github.com/Homebrew/brew/pull/15489
---
 Library/Homebrew/tap.rb           | 42 +++++++++++++++----------------
 Library/Homebrew/test/tap_spec.rb |  1 -
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb
index 0fa277417d..c8696f8cf4 100644
--- a/Library/Homebrew/tap.rb
+++ b/Library/Homebrew/tap.rb
@@ -753,34 +753,32 @@ class Tap
     end
   end
 
-  # Check whether the file has a Ruby extension.
-  sig { params(file: Pathname).returns(T::Boolean) }
-  def ruby_file?(file)
-    file.extname == ".rb"
+  sig { returns(Regexp) }
+  def formula_file_regex
+    @formula_file_regex ||= case formula_dir.basename.to_s
+    when "Formula"
+      %r{^Formula(/[^/]+)+\.rb$}
+    when "HomebrewFormula"
+      %r{^HomebrewFormula(/[^/]+)+\.rb$}
+    else
+      %r{^[^/]+\.rb$}
+    end
   end
-  private :ruby_file?
+  private :formula_file_regex
 
-  # Check whether the given path would present a {Formula} file in this {Tap}.
-  # Accepts either an absolute path or a path relative to this {Tap}'s path.
-  sig { params(file: T.any(String, Pathname)).returns(T::Boolean) }
+  # accepts the relative path of a file from {Tap}'s path
+  sig { params(file: String).returns(T::Boolean) }
   def formula_file?(file)
-    file = Pathname.new(file) unless file.is_a? Pathname
-    file = file.expand_path(path)
-    return false unless ruby_file?(file)
-    return false if cask_file?(file)
-
-    file.to_s.start_with?("#{formula_dir}/")
+    file.match?(formula_file_regex)
   end
 
-  # Check whether the given path would present a {Cask} file in this {Tap}.
-  # Accepts either an absolute path or a path relative to this {Tap}'s path.
-  sig { params(file: T.any(String, Pathname)).returns(T::Boolean) }
-  def cask_file?(file)
-    file = Pathname.new(file) unless file.is_a? Pathname
-    file = file.expand_path(path)
-    return false unless ruby_file?(file)
+  CASK_FILE_REGEX = %r{^Casks(/[^/]+)+\.rb$}
+  private_constant :CASK_FILE_REGEX
 
-    file.to_s.start_with?("#{cask_dir}/")
+  # accepts the relative path of a file from {Tap}'s path
+  sig { params(file: String).returns(T::Boolean) }
+  def cask_file?(file)
+    file.match?(CASK_FILE_REGEX)
   end
 
   # An array of all {Formula} names of this {Tap}.
diff --git a/Library/Homebrew/test/tap_spec.rb b/Library/Homebrew/test/tap_spec.rb
index 25932e6eb7..adb60d9c38 100644
--- a/Library/Homebrew/test/tap_spec.rb
+++ b/Library/Homebrew/test/tap_spec.rb
@@ -201,7 +201,6 @@ RSpec.describe Tap do
     expect(homebrew_foo_tap.tap_migrations).to eq("removed-formula" => "homebrew/foo")
     expect(homebrew_foo_tap.command_files).to eq([cmd_file])
     expect(homebrew_foo_tap.to_hash).to be_a(Hash)
-    expect(homebrew_foo_tap).to have_formula_file(formula_file)
     expect(homebrew_foo_tap).to have_formula_file("Formula/foo.rb")
     expect(homebrew_foo_tap).not_to have_formula_file("bar.rb")
     expect(homebrew_foo_tap).not_to have_formula_file("Formula/baz.sh")