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
This commit is contained in:
apainintheneck 2023-05-21 16:19:25 -07:00
parent 0c3104a8f2
commit cd430c4d34
2 changed files with 20 additions and 23 deletions

View File

@ -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}.

View File

@ -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")