From 0037b1f6262cd95aa29f516f6a9959a6e67e0e33 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Sat, 15 Feb 2025 23:04:04 -0800 Subject: [PATCH] Enable strict typing in DependenciesHelpers --- Library/Homebrew/cask/macos.rb | 21 +++++---- Library/Homebrew/cli/named_args.rb | 33 +++++++------- Library/Homebrew/cmd/--cache.rb | 2 +- Library/Homebrew/cmd/deps.rb | 4 +- Library/Homebrew/cmd/desc.rb | 2 +- Library/Homebrew/cmd/home.rb | 3 +- Library/Homebrew/cmd/list.rb | 4 +- Library/Homebrew/cmd/uses.rb | 22 +++++---- Library/Homebrew/dependencies.rb | 5 --- Library/Homebrew/dependencies.rbi | 11 +++++ Library/Homebrew/dependencies_helpers.rb | 45 +++++++++++++++---- Library/Homebrew/dependencies_helpers.rbi | 12 +++++ .../test/dependencies_helpers_spec.rb | 2 +- 13 files changed, 108 insertions(+), 58 deletions(-) create mode 100644 Library/Homebrew/dependencies_helpers.rbi diff --git a/Library/Homebrew/cask/macos.rb b/Library/Homebrew/cask/macos.rb index ac1a2349b9..5b402d4089 100644 --- a/Library/Homebrew/cask/macos.rb +++ b/Library/Homebrew/cask/macos.rb @@ -1,11 +1,9 @@ -# typed: true # rubocop:todo Sorbet/StrictSigil +# typed: strict # frozen_string_literal: true module OS module Mac - module_function - - SYSTEM_DIRS = [ + SYSTEM_DIRS = T.let([ "/", "/Applications", "/Applications/Utilities", @@ -234,14 +232,13 @@ module OS "/var/spool/mail", "/var/tmp", ] - .map(&method(:Pathname)) - .to_set - .freeze + .to_set { Pathname(_1) } + .freeze, T::Set[Pathname]) private_constant :SYSTEM_DIRS # TODO: There should be a way to specify a containing # directory under which nothing can be deleted. - UNDELETABLE_PATHS = [ + UNDELETABLE_PATHS = T.let([ "~/", "~/Applications", "~/Applications/.localized", @@ -378,14 +375,16 @@ module OS ] .to_set { |path| Pathname(path.sub(%r{^~(?=(/|$))}, Dir.home)).expand_path } .union(SYSTEM_DIRS) - .freeze + .freeze, T::Set[Pathname]) private_constant :UNDELETABLE_PATHS - def system_dir?(dir) + sig { params(dir: T.any(Pathname, String)).returns(T::Boolean) } + def self.system_dir?(dir) SYSTEM_DIRS.include?(Pathname.new(dir).expand_path) end - def undeletable?(path) + sig { params(path: T.any(Pathname, String)).returns(T::Boolean) } + def self.undeletable?(path) UNDELETABLE_PATHS.include?(Pathname.new(path).expand_path) end end diff --git a/Library/Homebrew/cli/named_args.rb b/Library/Homebrew/cli/named_args.rb index f74719babb..2620ec1ccb 100644 --- a/Library/Homebrew/cli/named_args.rb +++ b/Library/Homebrew/cli/named_args.rb @@ -70,26 +70,29 @@ module Homebrew method: T.nilable(Symbol), uniq: T::Boolean, warn: T::Boolean, - ).returns(T::Array[T.any(Formula, Keg, Cask::Cask)]) + ).returns(T::Array[T.any(Formula, Cask::Cask)]) } def to_formulae_and_casks( only: parent.only_formula_or_cask, ignore_unavailable: false, method: nil, uniq: true, warn: false ) @to_formulae_and_casks ||= T.let( - {}, T.nilable(T::Hash[T.nilable(Symbol), T::Array[T.any(Formula, Keg, Cask::Cask)]]) + {}, T.nilable(T::Hash[T.nilable(Symbol), T::Array[T.any(Formula, Cask::Cask)]]) + ) + @to_formulae_and_casks[only] ||= T.cast( + downcased_unique_named.flat_map do |name| + options = { warn: }.compact + load_formula_or_cask(name, only:, method:, **options) + rescue FormulaUnreadableError, FormulaClassUnavailableError, + TapFormulaUnreadableError, TapFormulaClassUnavailableError, + Cask::CaskUnreadableError + # Need to rescue before `*UnavailableError` (superclass of this) + # The formula/cask was found, but there's a problem with its implementation + raise + rescue NoSuchKegError, FormulaUnavailableError, Cask::CaskUnavailableError, FormulaOrCaskUnavailableError + ignore_unavailable ? [] : raise + end.freeze, + T::Array[T.any(Formula, Cask::Cask)], ) - @to_formulae_and_casks[only] ||= downcased_unique_named.flat_map do |name| - options = { warn: }.compact - load_formula_or_cask(name, only:, method:, **options) - rescue FormulaUnreadableError, FormulaClassUnavailableError, - TapFormulaUnreadableError, TapFormulaClassUnavailableError, - Cask::CaskUnreadableError - # Need to rescue before `*UnavailableError` (superclass of this) - # The formula/cask was found, but there's a problem with its implementation - raise - rescue NoSuchKegError, FormulaUnavailableError, Cask::CaskUnavailableError, FormulaOrCaskUnavailableError - ignore_unavailable ? [] : raise - end.freeze if uniq @to_formulae_and_casks.fetch(only).uniq.freeze @@ -120,7 +123,7 @@ module Homebrew def to_formulae_and_casks_with_taps formulae_and_casks_with_taps, formulae_and_casks_without_taps = to_formulae_and_casks.partition do |formula_or_cask| - T.cast(formula_or_cask, T.any(Formula, Cask::Cask)).tap&.installed? + formula_or_cask.tap&.installed? end return formulae_and_casks_with_taps if formulae_and_casks_without_taps.empty? diff --git a/Library/Homebrew/cmd/--cache.rb b/Library/Homebrew/cmd/--cache.rb index 63939f8816..7cdbab67b7 100644 --- a/Library/Homebrew/cmd/--cache.rb +++ b/Library/Homebrew/cmd/--cache.rb @@ -81,7 +81,7 @@ module Homebrew end end else - raise "Invalid type: #{formula_or_cask.class}" + T.absurd(formula_or_cask) end end end diff --git a/Library/Homebrew/cmd/deps.rb b/Library/Homebrew/cmd/deps.rb index 583e7621f1..5aeb277dbc 100644 --- a/Library/Homebrew/cmd/deps.rb +++ b/Library/Homebrew/cmd/deps.rb @@ -221,8 +221,8 @@ module Homebrew deps = dependency.runtime_dependencies if @use_runtime_dependencies if recursive - deps ||= recursive_includes(Dependency, dependency, includes, ignores) - reqs = recursive_includes(Requirement, dependency, includes, ignores) + deps ||= recursive_dep_includes(dependency, includes, ignores) + reqs = recursive_req_includes(dependency, includes, ignores) else deps ||= select_includes(dependency.deps, ignores, includes) reqs = select_includes(dependency.requirements, ignores, includes) diff --git a/Library/Homebrew/cmd/desc.rb b/Library/Homebrew/cmd/desc.rb index 9b87d7b6aa..76632b0c1a 100644 --- a/Library/Homebrew/cmd/desc.rb +++ b/Library/Homebrew/cmd/desc.rb @@ -65,7 +65,7 @@ module Homebrew description = formula_or_cask.desc.presence || Formatter.warning("[no description]") desc[formula_or_cask.full_name] = "(#{formula_or_cask.name.join(", ")}) #{description}" else - raise TypeError, "Unsupported formula_or_cask type: #{formula_or_cask.class}" + T.absurd(formula_or_cask) end end Descriptions.new(desc).print diff --git a/Library/Homebrew/cmd/home.rb b/Library/Homebrew/cmd/home.rb index 59ac2b6c5c..128213caeb 100644 --- a/Library/Homebrew/cmd/home.rb +++ b/Library/Homebrew/cmd/home.rb @@ -29,8 +29,7 @@ module Homebrew return end - # to_formulae_and_casks is typed to possibly return Kegs (but won't without explicitly asking) - formulae_or_casks = T.cast(args.named.to_formulae_and_casks, T::Array[T.any(Formula, Cask::Cask)]) + formulae_or_casks = args.named.to_formulae_and_casks homepages = formulae_or_casks.map do |formula_or_cask| puts "Opening homepage for #{name_of(formula_or_cask)}" formula_or_cask.homepage diff --git a/Library/Homebrew/cmd/list.rb b/Library/Homebrew/cmd/list.rb index 538baed2c1..695718585d 100644 --- a/Library/Homebrew/cmd/list.rb +++ b/Library/Homebrew/cmd/list.rb @@ -99,9 +99,7 @@ module Homebrew else args.named.to_formulae_and_casks(only: :cask, method: :resolve) end - # The cast is because `Keg`` does not define `full_name` - full_cask_names = T.cast(cask_names, T::Array[T.any(Formula, Cask::Cask)]) - .map(&:full_name).sort(&tap_and_name_comparison) + full_cask_names = cask_names.map(&:full_name).sort(&tap_and_name_comparison) full_cask_names = Formatter.columns(full_cask_names) unless args.public_send(:"1?") puts full_cask_names if full_cask_names.present? end diff --git a/Library/Homebrew/cmd/uses.rb b/Library/Homebrew/cmd/uses.rb index 10d5328d45..b2334025af 100644 --- a/Library/Homebrew/cmd/uses.rb +++ b/Library/Homebrew/cmd/uses.rb @@ -94,7 +94,7 @@ module Homebrew sig { params(use_runtime_dependents: T::Boolean, used_formulae: T::Array[T.any(Formula, UnavailableFormula)]) - .returns(T::Array[Formula]) + .returns(T::Array[T.any(Formula, CaskDependent)]) } def intersection_of_dependents(use_runtime_dependents, used_formulae) recursive = args.recursive? @@ -150,26 +150,30 @@ module Homebrew sig { params( - dependents: T::Array[Formula], used_formulae: T::Array[T.any(Formula, UnavailableFormula)], - recursive: T::Boolean, includes: T::Array[Symbol], ignores: T::Array[Symbol] - ).returns( - T::Array[Formula], - ) + dependents: T::Array[T.any(Formula, CaskDependent)], + used_formulae: T::Array[T.any(Formula, UnavailableFormula)], + recursive: T::Boolean, + includes: T::Array[Symbol], + ignores: T::Array[Symbol], + ).returns(T::Array[T.any(Formula, CaskDependent)]) } def select_used_dependents(dependents, used_formulae, recursive, includes, ignores) dependents.select do |d| deps = if recursive - recursive_includes(Dependency, d, includes, ignores) + recursive_dep_includes(d, includes, ignores) else select_includes(d.deps, ignores, includes) end used_formulae.all? do |ff| deps.any? do |dep| - match = begin + match = case dep + when Dependency dep.to_formula.full_name == ff.full_name if dep.name.include?("/") - rescue + when CaskDependent, Formula nil + else + T.absurd(dep) end next match unless match.nil? diff --git a/Library/Homebrew/dependencies.rb b/Library/Homebrew/dependencies.rb index 354b12745e..3e8bd125e8 100644 --- a/Library/Homebrew/dependencies.rb +++ b/Library/Homebrew/dependencies.rb @@ -39,11 +39,6 @@ class Dependencies < SimpleDelegator def inspect "#<#{self.class.name}: #{__getobj__}>" end - - sig { returns(T::Array[Dependency]) } - def to_a - __getobj__.to_a - end end # A collection of requirements. diff --git a/Library/Homebrew/dependencies.rbi b/Library/Homebrew/dependencies.rbi index ea20b01d7e..c1327a6086 100644 --- a/Library/Homebrew/dependencies.rbi +++ b/Library/Homebrew/dependencies.rbi @@ -1,13 +1,24 @@ # typed: strict class Dependencies < SimpleDelegator + include Enumerable include Kernel + # This is a workaround to enable `alias eql? ==` # @see https://github.com/sorbet/sorbet/issues/2378#issuecomment-569474238 sig { params(other: BasicObject).returns(T::Boolean) } def ==(other); end + + sig { params(blk: T.proc.params(arg0: Dependency).void).returns(T.self_type) } + sig { returns(T::Enumerator[Dependency]) } + def each(&blk); end end class Requirements < SimpleDelegator + include Enumerable include Kernel + + sig { params(blk: T.proc.params(arg0: Requirement).void).returns(T.self_type) } + sig { returns(T::Enumerator[Requirement]) } + def each(&blk); end end diff --git a/Library/Homebrew/dependencies_helpers.rb b/Library/Homebrew/dependencies_helpers.rb index 0c4276f707..8c14980623 100644 --- a/Library/Homebrew/dependencies_helpers.rb +++ b/Library/Homebrew/dependencies_helpers.rb @@ -1,14 +1,10 @@ -# typed: true # rubocop:todo Sorbet/StrictSigil +# typed: strict # frozen_string_literal: true require "cask_dependent" # Helper functions for dependencies. module DependenciesHelpers - extend T::Helpers - - requires_ancestor { Kernel } - def args_includes_ignores(args) includes = [:required?, :recommended?] # included by default includes << :implicit? if args.include_implicit? @@ -23,9 +19,35 @@ module DependenciesHelpers [includes, ignores] end - def recursive_includes(klass, root_dependent, includes, ignores) - raise ArgumentError, "Invalid class argument: #{klass}" if klass != Dependency && klass != Requirement + sig { + params(root_dependent: T.any(Formula, CaskDependent), includes: T::Array[Symbol], ignores: T::Array[Symbol]) + .returns(T::Array[Dependency]) + } + def recursive_dep_includes(root_dependent, includes, ignores) + # The use of T.unsafe is recommended by the Sorbet docs: + # https://sorbet.org/docs/overloads#multiple-methods-but-sharing-a-common-implementation + T.unsafe(recursive_includes(Dependency, root_dependent, includes, ignores)) + end + sig { + params(root_dependent: T.any(Formula, CaskDependent), includes: T::Array[Symbol], ignores: T::Array[Symbol]) + .returns(Requirements) + } + def recursive_req_includes(root_dependent, includes, ignores) + # The use of T.unsafe is recommended by the Sorbet docs: + # https://sorbet.org/docs/overloads#multiple-methods-but-sharing-a-common-implementation + T.unsafe(recursive_includes(Requirement, root_dependent, includes, ignores)) + end + + sig { + params( + klass: T.any(T.class_of(Dependency), T.class_of(Requirement)), + root_dependent: T.any(Formula, CaskDependent), + includes: T::Array[Symbol], + ignores: T::Array[Symbol], + ).returns(T.any(T::Array[Dependency], Requirements)) + } + def recursive_includes(klass, root_dependent, includes, ignores) cache_key = "recursive_includes_#{includes}_#{ignores}" klass.expand(root_dependent, cache_key:) do |dependent, dep| @@ -43,6 +65,11 @@ module DependenciesHelpers end end + sig { + params(dependables: T::Array[T.any(Formula, CaskDependent)], ignores: T::Array[Symbol], + includes: T::Array[Symbol]) + .returns(T::Array[T.any(Formula, CaskDependent)]) + } def select_includes(dependables, ignores, includes) dependables.select do |dep| next false if ignores.any? { |ignore| dep.public_send(ignore) } @@ -51,6 +78,9 @@ module DependenciesHelpers end end + sig { + params(formulae_or_casks: T::Array[T.any(Formula, Cask::Cask)]).returns(T::Array[T.any(Formula, CaskDependent)]) + } def dependents(formulae_or_casks) formulae_or_casks.map do |formula_or_cask| if formula_or_cask.is_a?(Formula) @@ -60,5 +90,4 @@ module DependenciesHelpers end end end - module_function :dependents end diff --git a/Library/Homebrew/dependencies_helpers.rbi b/Library/Homebrew/dependencies_helpers.rbi new file mode 100644 index 0000000000..8ee3283fab --- /dev/null +++ b/Library/Homebrew/dependencies_helpers.rbi @@ -0,0 +1,12 @@ +# typed: strict + +module DependenciesHelpers + include Kernel + + # This sig is in an RBI to avoid both circular dependencies and unnecessary requires + sig { + params(args: T.any(Homebrew::Cmd::Deps::Args, Homebrew::Cmd::Uses::Args)) + .returns([T::Array[Symbol], T::Array[Symbol]]) + } + def args_includes_ignores(args); end +end diff --git a/Library/Homebrew/test/dependencies_helpers_spec.rb b/Library/Homebrew/test/dependencies_helpers_spec.rb index bae4998fe1..75dc834bd4 100644 --- a/Library/Homebrew/test/dependencies_helpers_spec.rb +++ b/Library/Homebrew/test/dependencies_helpers_spec.rb @@ -35,7 +35,7 @@ RSpec.describe DependenciesHelpers do :any_version_installed?, ] - dependents = described_class.dependents([foo, foo_cask, bar, bar_cask]) + dependents = Class.new.extend(described_class).dependents([foo, foo_cask, bar, bar_cask]) dependents.each do |dependent| methods.each do |method|