From e1fdd2eda4c06e80fde0c2e000115a67c61fce30 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Tue, 3 Dec 2024 17:43:22 -0800 Subject: [PATCH] Enable strict typing in NamedArgs --- Library/Homebrew/cli/named_args.rb | 141 ++++++++++++------ Library/Homebrew/cmd/info.rb | 6 - Library/Homebrew/cmd/reinstall.rb | 5 +- Library/Homebrew/cmd/tab.rb | 2 +- Library/Homebrew/cmd/tap.rb | 2 +- Library/Homebrew/dev-cmd/bump-cask-pr.rb | 2 +- Library/Homebrew/dev-cmd/bump-formula-pr.rb | 19 +-- Library/Homebrew/dev-cmd/create.rb | 4 +- .../dev-cmd/determine-test-runners.rb | 7 +- Library/Homebrew/dev-cmd/extract.rb | 4 +- Library/Homebrew/dev-cmd/pr-pull.rb | 2 +- 11 files changed, 122 insertions(+), 72 deletions(-) diff --git a/Library/Homebrew/cli/named_args.rb b/Library/Homebrew/cli/named_args.rb index 5990e875f3..6a05f78ae8 100644 --- a/Library/Homebrew/cli/named_args.rb +++ b/Library/Homebrew/cli/named_args.rb @@ -1,4 +1,4 @@ -# typed: true # rubocop:todo Sorbet/StrictSigil +# typed: strict # frozen_string_literal: true require "delegate" @@ -8,6 +8,10 @@ module Homebrew module CLI # Helper class for loading formulae/casks from named arguments. class NamedArgs < Array + extend T::Generic + + Elem = type_member(:out) { { fixed: String } } + sig { params( args: String, @@ -39,14 +43,23 @@ module Homebrew super(@args) end + sig { returns(Args) } attr_reader :parent + sig { returns(T::Array[Cask::Cask]) } def to_casks - @to_casks ||= to_formulae_and_casks(only: :cask).freeze + @to_casks ||= T.let( + to_formulae_and_casks(only: :cask).freeze, T.nilable(T::Array[T.any(Formula, Keg, Cask::Cask)]) + ) + T.cast(@to_casks, T::Array[Cask::Cask]) end + sig { returns(T::Array[Formula]) } def to_formulae - @to_formulae ||= to_formulae_and_casks(only: :formula).freeze + @to_formulae ||= T.let( + to_formulae_and_casks(only: :formula).freeze, T.nilable(T::Array[T.any(Formula, Keg, Cask::Cask)]) + ) + T.cast(@to_formulae, T::Array[Formula]) end # Convert named arguments to {Formula} or {Cask} objects. @@ -62,13 +75,15 @@ module Homebrew ).returns(T::Array[T.any(Formula, Keg, Cask::Cask)]) } def to_formulae_and_casks( - only: parent&.only_formula_or_cask, + only: parent.only_formula_or_cask, ignore_unavailable: false, method: T.unsafe(nil), uniq: true, warn: T.unsafe(nil) ) - @to_formulae_and_casks ||= {} + @to_formulae_and_casks ||= T.let( + {}, T.nilable(T::Hash[T.nilable(Symbol), T::Array[T.any(Formula, Keg, 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) @@ -83,20 +98,31 @@ module Homebrew end.freeze if uniq - @to_formulae_and_casks[only].uniq.freeze + @to_formulae_and_casks.fetch(only).uniq.freeze else - @to_formulae_and_casks[only] + @to_formulae_and_casks.fetch(only) end end - def to_formulae_to_casks(only: parent&.only_formula_or_cask, method: nil) - @to_formulae_to_casks ||= {} - @to_formulae_to_casks[[method, only]] = to_formulae_and_casks(only:, method:) - .partition { |o| o.is_a?(Formula) || o.is_a?(Keg) } - .map(&:freeze).freeze + sig { + params(only: T.nilable(Symbol), method: T.nilable(Symbol)) + .returns([T::Array[T.any(Formula, Keg)], T::Array[Cask::Cask]]) + } + def to_formulae_to_casks(only: parent.only_formula_or_cask, method: nil) + @to_formulae_to_casks ||= T.let( + {}, T.nilable(T::Hash[[T.nilable(Symbol), T.nilable(Symbol)], + [T::Array[T.any(Formula, Keg)], T::Array[Cask::Cask]]]) + ) + @to_formulae_to_casks[[method, only]] = + T.cast( + to_formulae_and_casks(only:, method:).partition { |o| o.is_a?(Formula) || o.is_a?(Keg) } + .map(&:freeze).freeze, + [T::Array[T.any(Formula, Keg)], T::Array[Cask::Cask]], + ) end # Returns formulae and casks after validating that a tap is present for each of them. + sig { returns(T::Array[T.any(Formula, Keg, Cask::Cask)]) } 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| @@ -118,8 +144,18 @@ module Homebrew ERROR end - def to_formulae_and_casks_and_unavailable(only: parent&.only_formula_or_cask, method: nil) - @to_formulae_casks_unknowns ||= {} + sig { + params(only: T.nilable(Symbol), method: T.nilable(Symbol)) + .returns(T::Array[T.any(Formula, Keg, Cask::Cask, T::Array[Keg], FormulaOrCaskUnavailableError)]) + } + def to_formulae_and_casks_and_unavailable(only: parent.only_formula_or_cask, method: nil) + @to_formulae_casks_unknowns ||= T.let( + {}, + T.nilable( + T::Hash[T.nilable(Symbol), + T::Array[T.any(Formula, Keg, Cask::Cask, T::Array[Keg], FormulaOrCaskUnavailableError)]], + ), + ) @to_formulae_casks_unknowns[method] = downcased_unique_named.map do |name| load_formula_or_cask(name, only:, method:) rescue FormulaOrCaskUnavailableError => e @@ -127,6 +163,10 @@ module Homebrew end.uniq.freeze end + sig { + params(name: String, only: T.nilable(Symbol), method: T.nilable(Symbol), warn: T.nilable(T::Boolean)) + .returns(T.any(Formula, Keg, Cask::Cask, T::Array[Keg])) + } def load_formula_or_cask(name, only: nil, method: nil, warn: nil) Homebrew.with_no_api_env_if_needed(@without_api) do unreadable_error = nil @@ -246,17 +286,16 @@ module Homebrew user, repo, short_name = name.downcase.split("/", 3) if repo.present? && short_name.present? - tap = Tap.fetch(user, repo) + tap = Tap.fetch(T.must(user), repo) raise TapFormulaOrCaskUnavailableError.new(tap, short_name) end raise NoSuchKegError, name if resolve_formula(name) - - raise FormulaOrCaskUnavailableError, name end end private :load_formula_or_cask + sig { params(name: String).returns(Formula) } def resolve_formula(name) Formulary.resolve(name, **{ spec: @override_spec, force_bottle: @force_bottle, flags: @flags }.compact) end @@ -264,12 +303,16 @@ module Homebrew sig { params(uniq: T::Boolean).returns(T::Array[Formula]) } def to_resolved_formulae(uniq: true) - @to_resolved_formulae ||= to_formulae_and_casks(only: :formula, method: :resolve, uniq:) - .freeze + @to_resolved_formulae ||= T.let( + to_formulae_and_casks(only: :formula, method: :resolve, uniq:).freeze, + T.nilable(T::Array[T.any(Formula, Keg, Cask::Cask)]), + ) + T.cast(@to_resolved_formulae, T::Array[Formula]) end - def to_resolved_formulae_to_casks(only: parent&.only_formula_or_cask) - to_formulae_to_casks(only:, method: :resolve) + sig { params(only: T.nilable(Symbol)).returns([T::Array[Formula], T::Array[Cask::Cask]]) } + def to_resolved_formulae_to_casks(only: parent.only_formula_or_cask) + T.cast(to_formulae_to_casks(only:, method: :resolve), [T::Array[Formula], T::Array[Cask::Cask]]) end LOCAL_PATH_REGEX = %r{^/|[.]|/$} @@ -280,8 +323,8 @@ module Homebrew # If a cask and formula with the same name exist, includes both their paths # unless `only` is specified. sig { params(only: T.nilable(Symbol), recurse_tap: T::Boolean).returns(T::Array[Pathname]) } - def to_paths(only: parent&.only_formula_or_cask, recurse_tap: false) - @to_paths ||= {} + def to_paths(only: parent.only_formula_or_cask, recurse_tap: false) + @to_paths ||= T.let({}, T.nilable(T::Hash[T.nilable(Symbol), T::Array[Pathname]])) @to_paths[only] ||= Homebrew.with_no_api_env_if_needed(@without_api) do downcased_unique_named.flat_map do |name| path = Pathname(name).expand_path @@ -328,67 +371,70 @@ module Homebrew def to_default_kegs require "missing_formula" - @to_default_kegs ||= begin + @to_default_kegs ||= T.let(begin to_formulae_and_casks(only: :formula, method: :default_kegs).freeze rescue NoSuchKegError => e if (reason = MissingFormula.suggest_command(e.name, "uninstall")) $stderr.puts reason end raise e - end + end, T.nilable(T::Array[T.any(Formula, Keg, Cask::Cask)])) + T.cast(@to_default_kegs, T::Array[Keg]) end sig { returns(T::Array[Keg]) } def to_latest_kegs require "missing_formula" - @to_latest_kegs ||= begin + @to_latest_kegs ||= T.let(begin to_formulae_and_casks(only: :formula, method: :latest_kegs).freeze rescue NoSuchKegError => e if (reason = MissingFormula.suggest_command(e.name, "uninstall")) $stderr.puts reason end raise e - end + end, T.nilable(T::Array[T.any(Formula, Keg, Cask::Cask)])) + T.cast(@to_latest_kegs, T::Array[Keg]) end sig { returns(T::Array[Keg]) } def to_kegs require "missing_formula" - @to_kegs ||= begin + @to_kegs ||= T.let(begin to_formulae_and_casks(only: :formula, method: :kegs).freeze rescue NoSuchKegError => e if (reason = MissingFormula.suggest_command(e.name, "uninstall")) $stderr.puts reason end raise e - end + end, T.nilable(T::Array[T.any(Formula, Keg, Cask::Cask)])) + T.cast(@to_kegs, T::Array[Keg]) end sig { params(only: T.nilable(Symbol), ignore_unavailable: T::Boolean, all_kegs: T.nilable(T::Boolean)) .returns([T::Array[Keg], T::Array[Cask::Cask]]) } - def to_kegs_to_casks(only: parent&.only_formula_or_cask, ignore_unavailable: false, all_kegs: nil) + def to_kegs_to_casks(only: parent.only_formula_or_cask, ignore_unavailable: false, all_kegs: nil) method = all_kegs ? :kegs : :default_kegs - @to_kegs_to_casks ||= {} + @to_kegs_to_casks ||= T.let({}, T.nilable(T::Hash[T.nilable(Symbol), [T::Array[Keg], T::Array[Cask::Cask]]])) @to_kegs_to_casks[method] ||= - to_formulae_and_casks(only:, ignore_unavailable:, method:) + T.cast(to_formulae_and_casks(only:, ignore_unavailable:, method:) .partition { |o| o.is_a?(Keg) } - .map(&:freeze).freeze + .map(&:freeze).freeze, [T::Array[Keg], T::Array[Cask::Cask]]) end sig { returns(T::Array[Tap]) } def to_taps - @to_taps ||= downcased_unique_named.map { |name| Tap.fetch name }.uniq.freeze + @to_taps ||= T.let(downcased_unique_named.map { |name| Tap.fetch name }.uniq.freeze, T.nilable(T::Array[Tap])) end sig { returns(T::Array[Tap]) } def to_installed_taps - @to_installed_taps ||= to_taps.each do |tap| + @to_installed_taps ||= T.let(to_taps.each do |tap| raise TapUnavailableError, tap.name unless tap.installed? - end.uniq.freeze + end.uniq.freeze, T.nilable(T::Array[Tap])) end sig { returns(T::Array[String]) } @@ -410,6 +456,7 @@ module Homebrew end.uniq end + sig { params(name: String).returns([Pathname, T::Array[Keg]]) } def resolve_kegs(name) raise UsageError if name.blank? @@ -433,23 +480,26 @@ module Homebrew [rack, kegs] end + sig { params(name: String).returns(Keg) } def resolve_latest_keg(name) _, kegs = resolve_kegs(name) # Return keg if it is the only installed keg - return kegs if kegs.length == 1 + return kegs.fetch(0) if kegs.length == 1 stable_kegs = kegs.reject { |keg| keg.version.head? } - if stable_kegs.blank? - return kegs.max_by do |keg| + latest_keg = if stable_kegs.empty? + kegs.max_by do |keg| [keg.tab.source_modified_time, keg.version.revision] end + else + stable_kegs.max_by(&:scheme_and_version) end - - stable_kegs.max_by(&:scheme_and_version) + T.must(latest_keg) end + sig { params(name: String).returns(Keg) } def resolve_default_keg(name) rack, kegs = resolve_kegs(name) @@ -459,7 +509,7 @@ module Homebrew begin return Keg.new(opt_prefix.resolved_path) if opt_prefix.symlink? && opt_prefix.directory? return Keg.new(linked_keg_ref.resolved_path) if linked_keg_ref.symlink? && linked_keg_ref.directory? - return kegs.first if kegs.length == 1 + return kegs.fetch(0) if kegs.length == 1 f = if name.include?("/") || File.exist?(name) Formulary.factory(name) @@ -484,6 +534,12 @@ module Homebrew end end + sig { + params( + ref: String, loaded_type: String, + package: T.any(T::Array[T.any(Formula, Keg)], Cask::Cask, Formula, Keg, NilClass) + ).returns(String) + } def package_conflicts_message(ref, loaded_type, package) message = "Treating #{ref} as a #{loaded_type}." case package @@ -503,6 +559,7 @@ module Homebrew message.freeze end + sig { params(ref: String, loaded_type: String).void } def warn_if_cask_conflicts(ref, loaded_type) available = true cask = begin diff --git a/Library/Homebrew/cmd/info.rb b/Library/Homebrew/cmd/info.rb index 9b0424e65e..32fae43a1d 100644 --- a/Library/Homebrew/cmd/info.rb +++ b/Library/Homebrew/cmd/info.rb @@ -162,12 +162,6 @@ module Homebrew info_formula(obj) when Cask::Cask info_cask(obj) - when FormulaUnreadableError, FormulaClassUnavailableError, - TapFormulaUnreadableError, TapFormulaClassUnavailableError, - Cask::CaskUnreadableError - # We found the formula/cask, but failed to read it - $stderr.puts obj.backtrace if Homebrew::EnvConfig.developer? - ofail obj.message when FormulaOrCaskUnavailableError # The formula/cask could not be found ofail obj.message diff --git a/Library/Homebrew/cmd/reinstall.rb b/Library/Homebrew/cmd/reinstall.rb index 6c66056007..b31fd2204c 100644 --- a/Library/Homebrew/cmd/reinstall.rb +++ b/Library/Homebrew/cmd/reinstall.rb @@ -108,10 +108,7 @@ module Homebrew sig { override.void } def run - formulae, casks = T.cast( - args.named.to_resolved_formulae_to_casks, - [T::Array[Formula], T::Array[Cask::Cask]], - ) + formulae, casks = args.named.to_resolved_formulae_to_casks if args.build_from_source? unless DevelopmentTools.installed? diff --git a/Library/Homebrew/cmd/tab.rb b/Library/Homebrew/cmd/tab.rb index 48c891c74a..5b911422cd 100644 --- a/Library/Homebrew/cmd/tab.rb +++ b/Library/Homebrew/cmd/tab.rb @@ -42,7 +42,7 @@ module Homebrew end raise UsageError, "No marking option specified." if installed_on_request.nil? - formulae, casks = args.named.to_formulae_to_casks + formulae, casks = T.cast(args.named.to_formulae_to_casks, [T::Array[Formula], T::Array[Cask::Cask]]) formulae_not_installed = formulae.reject(&:any_version_installed?) casks_not_installed = casks.reject(&:installed?) if formulae_not_installed.any? || casks_not_installed.any? diff --git a/Library/Homebrew/cmd/tap.rb b/Library/Homebrew/cmd/tap.rb index b902510452..1c2d90021c 100644 --- a/Library/Homebrew/cmd/tap.rb +++ b/Library/Homebrew/cmd/tap.rb @@ -58,7 +58,7 @@ module Homebrew elsif args.no_named? puts Tap.installed.sort_by(&:name) else - tap = Tap.fetch(args.named.first) + tap = Tap.fetch(args.named.fetch(0)) begin tap.install clone_target: args.named.second, custom_remote: args.custom_remote?, diff --git a/Library/Homebrew/dev-cmd/bump-cask-pr.rb b/Library/Homebrew/dev-cmd/bump-cask-pr.rb index 953501321c..579e9db915 100644 --- a/Library/Homebrew/dev-cmd/bump-cask-pr.rb +++ b/Library/Homebrew/dev-cmd/bump-cask-pr.rb @@ -72,7 +72,7 @@ module Homebrew # Use the user's browser, too. ENV["BROWSER"] = EnvConfig.browser - cask = args.named.to_casks.first + cask = args.named.to_casks.fetch(0) odie "This cask is not in a tap!" if cask.tap.blank? odie "This cask's tap is not a Git repository!" unless cask.tap.git? diff --git a/Library/Homebrew/dev-cmd/bump-formula-pr.rb b/Library/Homebrew/dev-cmd/bump-formula-pr.rb index a924c07d01..df97ced7fb 100644 --- a/Library/Homebrew/dev-cmd/bump-formula-pr.rb +++ b/Library/Homebrew/dev-cmd/bump-formula-pr.rb @@ -109,18 +109,19 @@ module Homebrew odie "This formula is disabled!" if formula.disabled? odie "This formula is deprecated and does not build!" if formula.deprecation_reason == :does_not_build - odie "This formula is not in a tap!" if formula.tap.blank? - odie "This formula's tap is not a Git repository!" unless formula.tap.git? + tap = formula.tap + odie "This formula is not in a tap!" if tap.blank? + odie "This formula's tap is not a Git repository!" unless tap.git? - odie <<~EOS unless formula.tap.allow_bump?(formula.name) + odie <<~EOS unless tap.allow_bump?(formula.name) Whoops, the #{formula.name} formula has its version update pull requests automatically opened by BrewTestBot every ~3 hours! We'd still love your contributions, though, so try another one that's not in the autobump list: - #{Formatter.url("#{formula.tap.remote}/blob/master/.github/autobump.txt")} + #{Formatter.url("#{tap.remote}/blob/master/.github/autobump.txt")} EOS - odie "You have too many PRs open: close or merge some first!" if GitHub.too_many_open_prs?(formula.tap) + odie "You have too many PRs open: close or merge some first!" if GitHub.too_many_open_prs?(tap) formula_spec = formula.stable odie "#{formula}: no stable specification found!" if formula_spec.blank? @@ -129,9 +130,9 @@ module Homebrew # spamming during normal output. Homebrew.install_bundler_gems!(groups: ["audit", "style"]) unless args.no_audit? - tap_remote_repo = formula.tap.remote_repository + tap_remote_repo = T.must(tap.remote_repository) remote = "origin" - remote_branch = formula.tap.git_repository.origin_branch_name + remote_branch = tap.git_repository.origin_branch_name previous_branch = "-" check_pull_requests(formula, tap_remote_repo, state: "open") @@ -333,7 +334,7 @@ module Homebrew alias_rename = alias_update_pair(formula, new_formula_version) if alias_rename.present? ohai "Renaming alias #{alias_rename.first} to #{alias_rename.last}" - alias_rename.map! { |a| formula.tap.alias_dir/a } + alias_rename.map! { |a| tap.alias_dir/a } end unless args.dry_run? @@ -389,7 +390,7 @@ module Homebrew branch_name: "bump-#{formula.name}-#{new_formula_version}", commit_message: "#{formula.name} #{new_formula_version}", previous_branch:, - tap: formula.tap, + tap: tap, tap_remote_repo:, pr_message:, } diff --git a/Library/Homebrew/dev-cmd/create.rb b/Library/Homebrew/dev-cmd/create.rb index 13917b8439..7b1f87ca6b 100644 --- a/Library/Homebrew/dev-cmd/create.rb +++ b/Library/Homebrew/dev-cmd/create.rb @@ -81,7 +81,7 @@ module Homebrew sig { returns(Pathname) } def create_cask - url = args.named.first + url = args.named.fetch(0) name = if args.set_name.blank? stem = Pathname.new(url).stem.rpartition("=").last print "Cask name [#{stem}]: " @@ -179,7 +179,7 @@ module Homebrew args.set_name, args.set_version, tap: args.tap, - url: args.named.first, + url: args.named.fetch(0), mode:, license: args.set_license, fetch: !args.no_fetch?, diff --git a/Library/Homebrew/dev-cmd/determine-test-runners.rb b/Library/Homebrew/dev-cmd/determine-test-runners.rb index cba08ded8e..b5b6466691 100644 --- a/Library/Homebrew/dev-cmd/determine-test-runners.rb +++ b/Library/Homebrew/dev-cmd/determine-test-runners.rb @@ -39,9 +39,10 @@ module Homebrew raise UsageError, "`--all-supported` is mutually exclusive to other arguments." end - testing_formulae = args.named.first&.split(",").to_a - testing_formulae.map! { |name| TestRunnerFormula.new(Formulary.factory(name), eval_all: args.eval_all?) } - .freeze + testing_formulae = args.named.first&.split(",").to_a.map do |name| + TestRunnerFormula.new(Formulary.factory(name), eval_all: args.eval_all?) + end + .freeze deleted_formulae = args.named.second&.split(",").to_a.freeze runner_matrix = GitHubRunnerMatrix.new(testing_formulae, deleted_formulae, all_supported: args.all_supported?, diff --git a/Library/Homebrew/dev-cmd/extract.rb b/Library/Homebrew/dev-cmd/extract.rb index d186d4f0d1..6063f9314f 100644 --- a/Library/Homebrew/dev-cmd/extract.rb +++ b/Library/Homebrew/dev-cmd/extract.rb @@ -37,12 +37,12 @@ module Homebrew if (tap_with_name = args.named.first&.then { Tap.with_formula_name(_1) }) source_tap, name = tap_with_name else - name = args.named.first.downcase + name = args.named.fetch(0).downcase source_tap = CoreTap.instance end raise TapFormulaUnavailableError.new(source_tap, name) unless source_tap.installed? - destination_tap = Tap.fetch(args.named.second) + destination_tap = Tap.fetch(args.named.fetch(1)) unless Homebrew::EnvConfig.developer? odie "Cannot extract formula to homebrew/core!" if destination_tap.core_tap? odie "Cannot extract formula to homebrew/cask!" if destination_tap.core_cask_tap? diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index d3f8225109..116fc736c4 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -93,7 +93,7 @@ module Homebrew arg = "#{tap.default_remote}/pull/#{arg}" if arg.to_i.positive? url_match = arg.match HOMEBREW_PULL_OR_COMMIT_URL_REGEX _, user, repo, pr = *url_match - odie "Not a GitHub pull request: #{arg}" unless pr + odie "Not a GitHub pull request: #{arg}" if !user || !repo || !pr git_repo = tap.git_repository if !git_repo.default_origin_branch? && !args.branch_okay? && !args.no_commit? && !args.no_cherry_pick?