Replace throw/catch control flow with block return values

Ruby's throw/catch is a non-local control flow mechanism (a form of
goto). Replace all three uses with idiomatic Ruby patterns:

- Dependency: remove catch(:action) from `action`, use the block's
  return value directly. Callers signal actions via `next` with
  named constants (PRUNE, SKIP, KEEP_BUT_PRUNE_RECURSIVE_DEPS).
- Requirement: remove catch(:prune) from `prune?`, check the block's
  return value against the PRUNE constant.
- Livecheck: extract nested loop into `find_version_update_revision`
  helper and use `return` for early exit.

Remove the now-unnecessary `prune`, `skip`, and
`keep_but_prune_recursive_deps` class methods and update all callers
and type signatures.
This commit is contained in:
Douglas Eichelberger
2026-04-13 20:46:20 -07:00
parent 18c76d9762
commit 63d80a98a7
17 changed files with 104 additions and 104 deletions
+4 -3
View File
@@ -59,9 +59,10 @@ class Build
sig { returns(Requirements) }
def expand_reqs
formula.recursive_requirements do |dependent, req|
dependent = T.cast(dependent, Formula)
build = effective_build_options_for(dependent)
if req.prune_from_option?(build) || req.prune_if_build_and_not_dependent?(dependent, formula) || req.test?
Requirement.prune
next Requirement::PRUNE
end
end
end
@@ -73,9 +74,9 @@ class Build
if dep.prune_from_option?(build) ||
dep.prune_if_build_and_not_dependent?(T.cast(dependent, Formula), formula) ||
(dep.test? && !dep.build?) || dep.implicit?
Dependency.prune
next Dependency::PRUNE
elsif dep.build?
Dependency.keep_but_prune_recursive_deps
next Dependency::KEEP_BUT_PRUNE_RECURSIVE_DEPS
end
end
end
+4 -3
View File
@@ -82,7 +82,8 @@ class CaskDependent
sig {
params(
block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent, SoftwareSpec), arg1: ::Dependency).void),
block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent, SoftwareSpec),
arg1: ::Dependency).returns(T.nilable(Symbol))),
).returns(T::Array[::Dependency])
}
def recursive_dependencies(&block)
@@ -91,8 +92,8 @@ class CaskDependent
sig {
params(
block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent, SoftwareSpec), arg1: ::Requirement)
.returns(CaskDependent::Requirement)),
block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent, SoftwareSpec),
arg1: ::Requirement).returns(T.nilable(Symbol))),
).returns(Requirements)
}
def recursive_requirements(&block)
+3 -3
View File
@@ -51,8 +51,8 @@ module DependenciesHelpers
cache_key = "recursive_includes_#{includes}_#{ignores}"
klass.expand(root_dependent, cache_key:) do |dependent, dep|
klass.prune if ignores.any? { |ignore| dep.public_send(ignore) }
klass.prune if includes.none? do |include|
next Dependency::PRUNE if ignores.any? { |ignore| dep.public_send(ignore) }
next Dependency::PRUNE if includes.none? do |include|
# Ignore indirect test dependencies
next if include == :test? && dependent != root_dependent
@@ -61,7 +61,7 @@ module DependenciesHelpers
# If a tap isn't installed, we can't find the dependencies of one of
# its formulae and an exception will be thrown if we try.
Dependency.keep_but_prune_recursive_deps if klass == Dependency && (tap = dep.tap) && !tap.installed?
next Dependency::KEEP_BUT_PRUNE_RECURSIVE_DEPS if klass == Dependency && (tap = dep.tap) && !tap.installed?
end
end
+22 -32
View File
@@ -10,6 +10,16 @@ require "utils"
class Dependency
include Dependable
# Return from an {expand} block to keep a dependency in the result list
# but stop recursing into its own dependencies.
KEEP_BUT_PRUNE_RECURSIVE_DEPS = :keep_but_prune_recursive_deps
# Return from an {expand} block to remove a dependency and all of its
# recursive dependencies from the result list.
PRUNE = :prune
# Return from an {expand} block to omit a dependency from the result list
# but continue expanding its children.
SKIP = :skip
sig { returns(String) }
attr_reader :name
@@ -177,7 +187,8 @@ class Dependency
deps: T::Array[Dependency],
cache_key: T.nilable(String),
cache_timestamp: T.nilable(Time),
block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent), arg1: Dependency).void),
block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent),
arg1: Dependency).returns(T.nilable(Symbol))),
).returns(T::Array[Dependency])
}
def expand(dependent, deps = dependent.deps, cache_key: nil, cache_timestamp: nil, &block)
@@ -200,13 +211,13 @@ class Dependency
next if dependent.name == dep.name
case action(dependent, dep, &block)
when :prune
when PRUNE
next
when :skip
when SKIP
next if @expand_stack.include? dep.name
expanded_deps.concat(expand(dep.to_formula, cache_key:, &block))
when :keep_but_prune_recursive_deps
when KEEP_BUT_PRUNE_RECURSIVE_DEPS
expanded_deps << dep
else
next if @expand_stack.include? dep.name
@@ -233,39 +244,18 @@ class Dependency
params(
dependent: T.any(Formula, CaskDependent),
dep: Dependency,
block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent), arg1: Dependency).void),
).returns(T.nilable(T.any(Integer, Symbol)))
block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent),
arg1: Dependency).returns(T.nilable(Symbol))),
).returns(T.nilable(Symbol))
}
def action(dependent, dep, &block)
catch(:action) do
if block
yield dependent, dep
elsif dep.optional? || dep.recommended?
prune unless T.cast(dependent, Formula).build.with?(dep)
end
if block
yield dependent, dep
elsif dep.optional? || dep.recommended?
PRUNE unless T.cast(dependent, Formula).build.with?(dep)
end
end
# Prune a dependency and its dependencies recursively.
sig { void }
def prune
throw(:action, :prune)
end
# Prune a single dependency but do not prune its dependencies.
sig { void }
def skip
throw(:action, :skip)
end
# Keep a dependency, but prune its dependencies.
#
# @api internal
sig { void }
def keep_but_prune_recursive_deps
throw(:action, :keep_but_prune_recursive_deps)
end
sig { params(all: T::Array[Dependency]).returns(T::Array[Dependency]) }
def merge_repeats(all)
grouped = all.group_by(&:name)
+2 -2
View File
@@ -58,10 +58,10 @@ module Homebrew
# Don't test formulae missing test dependencies
missing_test_deps = f.recursive_dependencies do |dependent, dependency|
Dependency.prune if dependency.installed?
next Dependency::PRUNE if dependency.installed?
next if dependency.test? && dependent == f
Dependency.prune unless dependency.required?
next Dependency::PRUNE unless dependency.required?
end.map(&:to_s)
unless missing_test_deps.empty?
ofail "#{f.full_name} is missing test dependencies: #{missing_test_deps.join(" ")}"
+1 -1
View File
@@ -185,7 +185,7 @@ module Homebrew
all_formulae.each do |f|
deps = Dependency.expand(f, cache_key: "unbottled") do |_, dep|
Dependency.prune if dep.optional?
next Dependency::PRUNE if dep.optional?
end.map(&:to_formula)
deps_hash[f.name] = deps
@@ -20,7 +20,7 @@ module Homebrew
formula,
cache_key: "update-report",
) do |_, dependency|
Dependency.prune if dependency.build? || dependency.test?
next Dependency::PRUNE if dependency.build? || dependency.test?
end
next unless recursive_runtime_dependencies.map(&:name).include? "gcc"
+9 -5
View File
@@ -2628,7 +2628,8 @@ class Formula
# @api internal
T::Sig::WithoutRuntime.sig {
# CaskDependent may not be initialized yet, so we don't use a runtime sig
params(block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent), arg1: Dependency).void)).returns(T::Array[Dependency])
params(block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent),
arg1: Dependency).returns(T.nilable(Symbol)))).returns(T::Array[Dependency])
}
def recursive_dependencies(&block)
cache_key = "Formula#recursive_dependencies"
@@ -2644,7 +2645,10 @@ class Formula
# The full set of {Requirements} for this formula's dependency tree.
#
# @api internal
sig { params(block: T.untyped).returns(Requirements) }
sig {
params(block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent, SoftwareSpec),
arg1: Requirement).returns(T.nilable(Symbol)))).returns(Requirements)
}
def recursive_requirements(&block)
cache_key = "Formula#recursive_requirements" unless block
Requirement.expand(self, cache_key:, &block)
@@ -3260,13 +3264,13 @@ class Formula
cache_timestamp = Time.now
end
Dependency.expand(self, cache_key:, cache_timestamp:) do |_, dependency|
Dependency.prune if dependency.build?
next Dependency::PRUNE if dependency.build?
next if dependency.required?
if build.any_args_or_options?
Dependency.prune if build.without?(dependency)
next Dependency::PRUNE if build.without?(dependency)
elsif !dependency.recommended?
Dependency.prune
next Dependency::PRUNE
end
end
ensure
+5 -4
View File
@@ -704,7 +704,7 @@ on_request: installed_on_request?, options:)
def runtime_requirements(formula)
runtime_deps = formula.runtime_formula_dependencies(undeclared: false)
recursive_requirements = formula.recursive_requirements do |dependent, _|
Requirement.prune unless runtime_deps.include?(dependent)
next Requirement::PRUNE unless runtime_deps.include?(dependent)
end
(recursive_requirements.to_a + formula.requirements.to_a).reject(&:build?).uniq
end
@@ -719,6 +719,7 @@ on_request: installed_on_request?, options:)
while (f = formulae.pop)
runtime_requirements = runtime_requirements(f)
f.recursive_requirements do |dependent, req|
dependent = T.cast(dependent, Formula)
build = effective_build_options_for(dependent)
install_bottle_for_dependent = install_bottle_for?(dependent, build)
@@ -732,7 +733,7 @@ on_request: installed_on_request?, options:)
((req.build? || req.test?) && !keep_build_test) ||
formula_deps_map[dependent.name]&.build? ||
(only_deps? && f == dependent)
Requirement.prune
next Requirement::PRUNE
else
unsatisfied_reqs[dependent] << req
end
@@ -761,9 +762,9 @@ on_request: installed_on_request?, options:)
bottle_os_version = @bottle_built_os_version
if dep.prune_from_option?(build) || ((dep.build? || dep.test?) && !keep_build_test)
Dependency.prune
next Dependency::PRUNE
elsif dep.satisfied?(minimum_version:, minimum_revision:, bottle_os_version:)
Dependency.skip
next Dependency::SKIP
end
end
end
+7 -5
View File
@@ -195,19 +195,21 @@ module Language
# Find any Python bindings provided by recursive dependencies
pth_contents = []
formula.recursive_dependencies do |dependent, dep|
Dependency.prune if dep.build? || dep.test?
next Dependency::PRUNE if dep.build? || dep.test?
# Apply default filter
Dependency.prune if (dep.optional? || dep.recommended?) && !T.cast(dependent, Formula).build.with?(dep)
next Dependency::PRUNE if (dep.optional? || dep.recommended?) && !T.cast(dependent,
Formula).build.with?(dep)
# Do not add the main site-package provided by the brewed
# Python formula, to keep the virtual-env's site-package pristine
Dependency.prune if python_names.include? dep.name
next Dependency::PRUNE if python_names.include? dep.name
# Skip uses_from_macos dependencies as these imply no Python bindings
Dependency.prune if dep.uses_from_macos?
next Dependency::PRUNE if dep.uses_from_macos?
dep_site_packages = dep.to_formula.opt_prefix/Language::Python.site_packages(python)
Dependency.prune unless dep_site_packages.exist?
next Dependency::PRUNE unless dep_site_packages.exist?
pth_contents << "import site; site.addsitedir('#{dep_site_packages}')\n"
nil
end
(venv.site_packages/"homebrew_deps.pth").write pth_contents.join unless pth_contents.empty?
+20 -16
View File
@@ -1131,31 +1131,35 @@ module Homebrew
stable = formula.stable
return if stable.blank?
current_version = stable.version
version_update_revision = find_version_update_revision(formula, stable.version)
return if version_update_revision.nil?
timestamp_for_revision(tap.path, version_update_revision)
end
sig { params(formula: Formula, current_version: Version).returns(T.nilable(String)) }
private_class_method def self.find_version_update_revision(formula, current_version)
version_update_revision = T.let(nil, T.nilable(String))
found_current_version = T.let(false, T::Boolean)
formula_versions = FormulaVersions.new(formula)
catch(:version_update_revision_found) do
formula_versions.rev_list("HEAD") do |revision, path|
formula_versions.formula_at_revision(revision, path) do |historical_formula|
historical_stable = historical_formula.stable
next if historical_stable.blank?
formula_versions.rev_list("HEAD") do |revision, path|
formula_versions.formula_at_revision(revision, path) do |historical_formula|
historical_stable = historical_formula.stable
next if historical_stable.blank?
if historical_stable.version == current_version
found_current_version = true
version_update_revision = revision
elsif found_current_version
throw :version_update_revision_found
end
if historical_stable.version == current_version
found_current_version = true
version_update_revision = revision
elsif found_current_version
return version_update_revision
end
rescue MacOSVersion::Error, LegacyDSLError
break
end
rescue MacOSVersion::Error, LegacyDSLError
break
end
return if version_update_revision.nil?
timestamp_for_revision(tap.path, version_update_revision)
version_update_revision
end
sig {
+12 -15
View File
@@ -16,6 +16,9 @@ class Requirement
extend Cachable
extend T::Helpers
# Return from an {expand} block to exclude a requirement from the result set.
PRUNE = :prune
# This base class enforces no constraints on its own.
# Individual subclasses use the `satisfy` DSL to define those constraints.
abstract!
@@ -319,7 +322,7 @@ class Requirement
dependent: T.any(Formula, CaskDependent, SoftwareSpec),
cache_key: T.nilable(String),
block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent, SoftwareSpec),
arg1: Requirement).void),
arg1: Requirement).returns(T.nilable(Symbol))),
).returns(Requirements)
}
def expand(dependent, cache_key: nil, &block)
@@ -357,25 +360,19 @@ class Requirement
dependent: T.any(Formula, CaskDependent, SoftwareSpec),
req: Requirement,
block: T.nilable(T.proc.params(arg0: T.any(Formula, CaskDependent, SoftwareSpec),
arg1: Requirement).void),
).returns(T.nilable(T::Boolean))
arg1: Requirement).returns(T.nilable(Symbol))),
).returns(T::Boolean)
}
def prune?(dependent, req, &block)
catch(:prune) do
if block
yield dependent, req
elsif req.optional? || req.recommended?
prune unless T.cast(dependent, Formula).build.with?(req)
end
if block
yield(dependent, req) == PRUNE
elsif req.optional? || req.recommended?
!T.cast(dependent, Formula).build.with?(req)
else
false
end
end
# Used to prune requirements when calling expand with a block.
sig { void }
def prune
throw(:prune, true)
end
private
sig { params(dependent: T.untyped).returns(String) }
@@ -32,13 +32,13 @@ RSpec.describe Dependency do
expect(described_class.expand(formula)).to eq(deps)
end
it "prunes all when given a block with ::prune" do
expect(described_class.expand(formula) { described_class.prune }).to be_empty
it "prunes all when given a block with PRUNE" do
expect(described_class.expand(formula) { next described_class::PRUNE }).to be_empty
end
it "can prune selectively" do
deps = described_class.expand(formula) do |_, dep|
described_class.prune if dep.name == "foo"
next described_class::PRUNE if dep.name == "foo"
end
expect(deps).to eq([bar, baz, qux])
@@ -82,7 +82,7 @@ RSpec.describe Dependency do
expect(described_class.expand(formula).first.tags).to eq(%w[option])
end
it "skips parent but yields children with ::skip" do
it "skips parent but yields children with SKIP" do
f = instance_double(
Formula,
name: "f",
@@ -93,19 +93,19 @@ RSpec.describe Dependency do
)
deps = described_class.expand(f) do |_dependent, dep|
described_class.skip if %w[foo qux].include? dep.name
next described_class::SKIP if %w[foo qux].include? dep.name
end
expect(deps).to eq([bar, baz])
end
it "keeps dependency but prunes recursive dependencies with ::keep_but_prune_recursive_deps" do
it "keeps dependency but prunes recursive dependencies with KEEP_BUT_PRUNE_RECURSIVE_DEPS" do
foo = build_dep(:foo, [:test], bar)
baz = build_dep(:baz, [:test])
f = instance_double(Formula, name: "f", deps: [foo, baz])
deps = described_class.expand(f) do |_dependent, dep|
described_class.keep_but_prune_recursive_deps if dep.test?
next described_class::KEEP_BUT_PRUNE_RECURSIVE_DEPS if dep.test?
end
expect(deps).to eq([foo, baz])
+1 -1
View File
@@ -1317,7 +1317,7 @@ RSpec.describe Formula do
).to eq(Set[xcode])
requirements = f2.recursive_requirements do |_dependent, requirement|
Requirement.prune if requirement.is_a?(XcodeRequirement)
next Requirement::PRUNE if requirement.is_a?(XcodeRequirement)
end
expect(Set.new(requirements)).to eq(Set[])
+2 -2
View File
@@ -672,11 +672,11 @@ module Homebrew
bottled_dep_allowlist = /\A(?:glibc|linux-headers)@/
deps = Dependency.expand(Formula[formula_name],
cache_key: "portable-package-#{formula_name}") do |_dependent, dep|
Dependency.prune if dep.test? || dep.optional?
next Dependency::PRUNE if dep.test? || dep.optional?
next unless bottled_dep_allowlist.match?(dep.name)
Dependency.keep_but_prune_recursive_deps
next Dependency::KEEP_BUT_PRUNE_RECURSIVE_DEPS
end.map(&:name)
bottled_deps, deps = deps.partition { |dep| bottled_dep_allowlist.match?(dep) }
@@ -192,8 +192,8 @@ module Homebrew
else
begin
Dependency.expand(f, cache_key: "test-bot-dependents") do |_, dependency|
Dependency.skip if dependency.implicit?
Dependency.keep_but_prune_recursive_deps if dependency.build? || dependency.test?
next Dependency::SKIP if dependency.implicit?
next Dependency::KEEP_BUT_PRUNE_RECURSIVE_DEPS if dependency.build? || dependency.test?
end
rescue TapFormulaUnavailableError => e
raise if e.tap.installed?
@@ -300,7 +300,7 @@ module Homebrew
!dependency.optional? &&
testable_dependents.include?(dependent)
Dependency.prune
next Dependency::PRUNE
end
unless dependent_was_previously_installed
+1 -1
View File
@@ -250,7 +250,7 @@ module Homebrew
formula.recursive_dependencies
else
formula.recursive_dependencies do |_, dep|
Dependency.prune if dep.build? || dep.test?
next Dependency::PRUNE if dep.build? || dep.test?
end
end