From 9a2b386a52768a8440eaea53c62ab8760565d4c1 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 6 Dec 2024 11:06:27 -0800 Subject: [PATCH 1/2] Remove unsafe references from CLI and Formulary code --- Library/Homebrew/cli/args.rb | 2 +- Library/Homebrew/cli/named_args.rb | 20 ++- Library/Homebrew/cli/parser.rb | 10 +- Library/Homebrew/formulary.rb | 204 +++++++++++------------------ 4 files changed, 90 insertions(+), 146 deletions(-) diff --git a/Library/Homebrew/cli/args.rb b/Library/Homebrew/cli/args.rb index 462d4eef1e5ef..1a6b0aa618c70 100644 --- a/Library/Homebrew/cli/args.rb +++ b/Library/Homebrew/cli/args.rb @@ -73,7 +73,7 @@ def named end sig { returns(T::Boolean) } - def no_named? = named.blank? + def no_named? = named.empty? sig { returns(T::Array[String]) } def build_from_source_formulae diff --git a/Library/Homebrew/cli/named_args.rb b/Library/Homebrew/cli/named_args.rb index 378f5133f82cd..f74719babba1e 100644 --- a/Library/Homebrew/cli/named_args.rb +++ b/Library/Homebrew/cli/named_args.rb @@ -18,7 +18,7 @@ class NamedArgs < Array params( args: String, parent: Args, - override_spec: Symbol, + override_spec: T.nilable(Symbol), force_bottle: T::Boolean, flags: T::Array[String], cask_options: T::Boolean, @@ -28,9 +28,9 @@ class NamedArgs < Array def initialize( *args, parent: Args.new, - override_spec: T.unsafe(nil), - force_bottle: T.unsafe(nil), - flags: T.unsafe(nil), + override_spec: nil, + force_bottle: false, + flags: [], cask_options: false, without_api: false ) @@ -73,11 +73,7 @@ def to_formulae ).returns(T::Array[T.any(Formula, Keg, Cask::Cask)]) } def to_formulae_and_casks( - only: parent.only_formula_or_cask, - ignore_unavailable: false, - method: T.unsafe(nil), - uniq: true, - warn: T.unsafe(nil) + 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)]]) @@ -127,7 +123,7 @@ def to_formulae_and_casks_with_taps T.cast(formula_or_cask, T.any(Formula, Cask::Cask)).tap&.installed? end - return formulae_and_casks_with_taps if formulae_and_casks_without_taps.blank? + return formulae_and_casks_with_taps if formulae_and_casks_without_taps.empty? types = [] types << "formulae" if formulae_and_casks_without_taps.any?(Formula) @@ -369,7 +365,7 @@ def load_formula_or_cask(name, only: nil, method: nil, warn: nil) options = { warn: }.compact candidate_cask = Cask::CaskLoader.load(name, config:, **options) - if unreadable_error.present? + if unreadable_error onoe <<~EOS Failed to load formula: #{name} #{unreadable_error} @@ -435,7 +431,7 @@ def load_formula_or_cask(name, only: nil, method: nil, warn: nil) end end - raise unreadable_error if unreadable_error.present? + raise unreadable_error if unreadable_error user, repo, short_name = name.downcase.split("/", 3) if repo.present? && short_name.present? diff --git a/Library/Homebrew/cli/parser.rb b/Library/Homebrew/cli/parser.rb index 3d9e6553e1ff6..c486fb5258884 100644 --- a/Library/Homebrew/cli/parser.rb +++ b/Library/Homebrew/cli/parser.rb @@ -458,11 +458,9 @@ def formula_options ).void } def named_args(type = nil, number: nil, min: nil, max: nil, without_api: false) - if number.present? && (min.present? || max.present?) - raise ArgumentError, "Do not specify both `number` and `min` or `max`" - end + raise ArgumentError, "Do not specify both `number` and `min` or `max`" if number && (min || max) - if type == :none && (number.present? || min.present? || max.present?) + if type == :none && (number || min || max) raise ArgumentError, "Do not specify both `number`, `min` or `max` with `named_args :none`" end @@ -527,9 +525,9 @@ def generate_usage_banner "<#{@named_args_type}>" end - named_args = if @min_named_args.blank? && @max_named_args == 1 + named_args = if @min_named_args.nil? && @max_named_args == 1 " [#{arg_type}]" - elsif @min_named_args.blank? + elsif @min_named_args.nil? " [#{arg_type} ...]" elsif @min_named_args == 1 && @max_named_args == 1 " #{arg_type}" diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index b6af0296bb079..ea0d0dc65b768 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -472,20 +472,17 @@ def ruby_source_checksum platform_cache[:api][name] = klass end - sig { params(name: String, spec: Symbol, force_bottle: T::Boolean, flags: T::Array[String]).returns(Formula) } + sig { + params(name: String, spec: T.nilable(Symbol), force_bottle: T::Boolean, flags: T::Array[String]).returns(Formula) + } def self.resolve( name, - spec: T.unsafe(nil), - force_bottle: T.unsafe(nil), - flags: T.unsafe(nil) + spec: nil, + force_bottle: false, + flags: [] ) - options = { - force_bottle:, - flags:, - }.compact - if name.include?("/") || File.exist?(name) - f = factory(name, *spec, **options) + f = factory(name, *spec, force_bottle:, flags:) if f.any_version_installed? tab = Tab.for_formula(f) resolved_spec = spec || tab.spec @@ -498,10 +495,8 @@ def self.resolve( end else rack = to_rack(name) - if (alias_path = factory(name, **options).alias_path) - options[:alias_path] = alias_path - end - f = from_rack(rack, *spec, **options) + alias_path = factory(name, force_bottle:, flags:).alias_path + f = from_rack(rack, *spec, alias_path:, force_bottle:, flags:) end # If this formula was installed with an alias that has since changed, @@ -556,8 +551,8 @@ class FormulaLoader sig { returns(T.nilable(Tap)) } attr_reader :tap - sig { params(name: String, path: Pathname, alias_path: Pathname, tap: Tap).void } - def initialize(name, path, alias_path: T.unsafe(nil), tap: T.unsafe(nil)) + sig { params(name: String, path: Pathname, alias_path: T.nilable(Pathname), tap: T.nilable(Tap)).void } + def initialize(name, path, alias_path: nil, tap: nil) @name = name @path = path @alias_path = alias_path @@ -590,10 +585,10 @@ def load_file(flags:, ignore_errors:) # Loads a formula from a bottle. class FromBottleLoader < FormulaLoader sig { - params(ref: T.any(String, Pathname, URI::Generic), from: Symbol, warn: T::Boolean) + params(ref: T.any(String, Pathname, URI::Generic), from: T.nilable(Symbol), warn: T::Boolean) .returns(T.nilable(T.attached_class)) } - def self.try_new(ref, from: T.unsafe(nil), warn: false) + def self.try_new(ref, from: nil, warn: false) return if Homebrew::EnvConfig.forbid_packages_from_paths? ref = ref.to_s @@ -633,10 +628,10 @@ def get_formula(spec, force_bottle: false, flags: [], ignore_errors: false, **) # Loads formulae from disk using a path. class FromPathLoader < FormulaLoader sig { - params(ref: T.any(String, Pathname, URI::Generic), from: Symbol, warn: T::Boolean) + params(ref: T.any(String, Pathname, URI::Generic), from: T.nilable(Symbol), warn: T::Boolean) .returns(T.nilable(T.attached_class)) } - def self.try_new(ref, from: T.unsafe(nil), warn: false) + def self.try_new(ref, from: nil, warn: false) path = case ref when String Pathname(ref) @@ -651,58 +646,42 @@ def self.try_new(ref, from: T.unsafe(nil), warn: false) return if Homebrew::EnvConfig.forbid_packages_from_paths? && !path.realpath.to_s.start_with?("#{HOMEBREW_CELLAR}/", "#{HOMEBREW_LIBRARY}/Taps/") - options = if (tap = Tap.from_path(path)) + if (tap = Tap.from_path(path)) # Only treat symlinks in taps as aliases. if path.symlink? alias_path = path path = alias_path.resolved_path - - { - alias_path:, - tap:, - } - else - { - tap:, - } end - elsif (tap = Homebrew::API.tap_from_source_download(path)) - # Don't treat cache symlinks as aliases. - { - tap:, - } else - {} + # Don't treat cache symlinks as aliases. + tap = Homebrew::API.tap_from_source_download(path) end return if path.extname != ".rb" - new(path, **options) + new(path, alias_path:, tap:) end - sig { params(path: T.any(Pathname, String), alias_path: Pathname, tap: Tap).void } - def initialize(path, alias_path: T.unsafe(nil), tap: T.unsafe(nil)) + sig { params(path: T.any(Pathname, String), alias_path: T.nilable(Pathname), tap: T.nilable(Tap)).void } + def initialize(path, alias_path: nil, tap: nil) path = Pathname(path).expand_path name = path.basename(".rb").to_s alias_path = alias_path&.expand_path alias_dir = alias_path&.dirname - options = { - alias_path: (alias_path if alias_dir == tap&.alias_dir), - tap:, - }.compact + alias_path = nil if alias_dir != tap&.alias_dir - super(name, path, **options) + super(name, path, alias_path:, tap: tap) end end # Loads formula from a URI. class FromURILoader < FormulaLoader sig { - params(ref: T.any(String, Pathname, URI::Generic), from: Symbol, warn: T::Boolean) + params(ref: T.any(String, Pathname, URI::Generic), from: T.nilable(Symbol), warn: T::Boolean) .returns(T.nilable(T.attached_class)) } - def self.try_new(ref, from: T.unsafe(nil), warn: false) + def self.try_new(ref, from: nil, warn: false) return if Homebrew::EnvConfig.forbid_packages_from_paths? # Cache compiled regex @@ -763,10 +742,10 @@ class FromTapLoader < FormulaLoader attr_reader :path sig { - params(ref: T.any(String, Pathname, URI::Generic), from: Symbol, warn: T::Boolean) + params(ref: T.any(String, Pathname, URI::Generic), from: T.nilable(Symbol), warn: T::Boolean) .returns(T.nilable(FormulaLoader)) } - def self.try_new(ref, from: T.unsafe(nil), warn: false) + def self.try_new(ref, from: nil, warn: false) ref = ref.to_s return unless (name_tap_type = Formulary.tap_formula_name_type(ref, warn:)) @@ -774,28 +753,24 @@ def self.try_new(ref, from: T.unsafe(nil), warn: false) name, tap, type = name_tap_type path = Formulary.find_formula_in_tap(name, tap) - options = if type == :alias + if type == :alias # TODO: Simplify this by making `tap_formula_name_type` return the alias name. - { alias_name: T.must(ref[HOMEBREW_TAP_FORMULA_REGEX, :name]).downcase } - else - {} + alias_name = T.must(ref[HOMEBREW_TAP_FORMULA_REGEX, :name]).downcase end if type == :migration && tap.core_tap? && (loader = FromAPILoader.try_new(name)) loader else - new(name, path, tap:, **options) + new(name, path, tap:, alias_name:) end end - sig { params(name: String, path: Pathname, tap: Tap, alias_name: String).void } - def initialize(name, path, tap:, alias_name: T.unsafe(nil)) - options = { - alias_path: (tap.alias_dir/alias_name if alias_name), - tap:, - }.compact + sig { params(name: String, path: Pathname, tap: Tap, alias_name: T.nilable(String)).void } + def initialize(name, path, tap:, alias_name: nil) + alias_path = tap.alias_dir/alias_name if alias_name - super(name, path, **options) + super(name, path, alias_path:, tap:) + @tap = tap end def get_formula(spec, alias_path: nil, force_bottle: false, flags: [], ignore_errors: false) @@ -819,10 +794,10 @@ def load_file(flags:, ignore_errors:) # Loads a formula from a name, as long as it exists only in a single tap. class FromNameLoader < FromTapLoader sig { - params(ref: T.any(String, Pathname, URI::Generic), from: Symbol, warn: T::Boolean) + params(ref: T.any(String, Pathname, URI::Generic), from: T.nilable(Symbol), warn: T::Boolean) .returns(T.nilable(FormulaLoader)) } - def self.try_new(ref, from: T.unsafe(nil), warn: false) + def self.try_new(ref, from: nil, warn: false) return unless ref.is_a?(String) return unless ref.match?(/\A#{HOMEBREW_TAP_FORMULA_NAME_REGEX}\Z/o) @@ -851,10 +826,10 @@ def self.try_new(ref, from: T.unsafe(nil), warn: false) # Loads a formula from a formula file in a keg. class FromKegLoader < FormulaLoader sig { - params(ref: T.any(String, Pathname, URI::Generic), from: Symbol, warn: T::Boolean) + params(ref: T.any(String, Pathname, URI::Generic), from: T.nilable(Symbol), warn: T::Boolean) .returns(T.nilable(T.attached_class)) } - def self.try_new(ref, from: T.unsafe(nil), warn: false) + def self.try_new(ref, from: nil, warn: false) ref = ref.to_s return unless (keg_formula = HOMEBREW_PREFIX/"opt/#{ref}/.brew/#{ref}.rb").file? @@ -866,10 +841,10 @@ def self.try_new(ref, from: T.unsafe(nil), warn: false) # Loads a formula from a cached formula file. class FromCacheLoader < FormulaLoader sig { - params(ref: T.any(String, Pathname, URI::Generic), from: Symbol, warn: T::Boolean) + params(ref: T.any(String, Pathname, URI::Generic), from: T.nilable(Symbol), warn: T::Boolean) .returns(T.nilable(T.attached_class)) } - def self.try_new(ref, from: T.unsafe(nil), warn: false) + def self.try_new(ref, from: nil, warn: false) ref = ref.to_s return unless (cached_formula = HOMEBREW_CACHE_FORMULA/"#{ref}.rb").file? @@ -881,10 +856,10 @@ def self.try_new(ref, from: T.unsafe(nil), warn: false) # Pseudo-loader which will raise a {FormulaUnavailableError} when trying to load the corresponding formula. class NullLoader < FormulaLoader sig { - params(ref: T.any(String, Pathname, URI::Generic), from: Symbol, warn: T::Boolean) + params(ref: T.any(String, Pathname, URI::Generic), from: T.nilable(Symbol), warn: T::Boolean) .returns(T.nilable(T.attached_class)) } - def self.try_new(ref, from: T.unsafe(nil), warn: false) + def self.try_new(ref, from: nil, warn: false) return if ref.is_a?(URI::Generic) new(ref) @@ -920,10 +895,10 @@ def klass(flags:, ignore_errors:) # Load a formula from the API. class FromAPILoader < FormulaLoader sig { - params(ref: T.any(String, Pathname, URI::Generic), from: Symbol, warn: T::Boolean) + params(ref: T.any(String, Pathname, URI::Generic), from: T.nilable(Symbol), warn: T::Boolean) .returns(T.nilable(T.attached_class)) } - def self.try_new(ref, from: T.unsafe(nil), warn: false) + def self.try_new(ref, from: nil, warn: false) return if Homebrew::EnvConfig.no_install_from_api? return unless ref.is_a?(String) return unless (name = ref[HOMEBREW_DEFAULT_TAP_FORMULA_REGEX, :name]) @@ -941,23 +916,16 @@ def self.try_new(ref, from: T.unsafe(nil), warn: false) name, tap, type = name_tap_type - options = if type == :alias - { alias_name: alias_name.downcase } - else - {} - end + alias_name = alias_name.downcase if type == :alias - new(name, tap:, **options) + new(name, tap:, alias_name:) end - sig { params(name: String, tap: Tap, alias_name: String).void } - def initialize(name, tap: T.unsafe(nil), alias_name: T.unsafe(nil)) - options = { - alias_path: (CoreTap.instance.alias_dir/alias_name if alias_name), - tap:, - }.compact + sig { params(name: String, tap: T.nilable(Tap), alias_name: T.nilable(String)).void } + def initialize(name, tap: nil, alias_name: nil) + alias_path = CoreTap.instance.alias_dir/alias_name if alias_name - super(name, Formulary.core_path(name), **options) + super(name, Formulary.core_path(name), alias_path:, tap:) end def klass(flags:, ignore_errors:) @@ -986,7 +954,7 @@ def load_from_api(flags:) ref: T.any(Pathname, String), spec: Symbol, alias_path: T.any(NilClass, Pathname, String), - from: Symbol, + from: T.nilable(Symbol), warn: T::Boolean, force_bottle: T::Boolean, flags: T::Array[String], @@ -997,25 +965,19 @@ def self.factory( ref, spec = :stable, alias_path: nil, - from: T.unsafe(nil), - warn: T.unsafe(nil), - force_bottle: T.unsafe(nil), - flags: T.unsafe(nil), - ignore_errors: T.unsafe(nil) + from: nil, + warn: false, + force_bottle: false, + flags: [], + ignore_errors: false ) cache_key = "#{ref}-#{spec}-#{alias_path}-#{from}" if factory_cached? && platform_cache[:formulary_factory]&.key?(cache_key) return platform_cache[:formulary_factory][cache_key] end - loader_options = { from:, warn: }.compact - formula_options = { alias_path:, - force_bottle:, - flags:, - ignore_errors: }.compact - - formula = loader_for(ref, **loader_options) - .get_formula(spec, **formula_options) + formula = loader_for(ref, from:, warn:) + .get_formula(spec, alias_path:, force_bottle:, flags:, ignore_errors:) if factory_cached? platform_cache[:formulary_factory] ||= {} @@ -1035,18 +997,13 @@ def self.factory( params( rack: Pathname, # Automatically resolves the formula's spec if not specified. - spec: Symbol, - alias_path: T.any(Pathname, String), + spec: T.nilable(Symbol), + alias_path: T.any(NilClass, Pathname, String), force_bottle: T::Boolean, flags: T::Array[String], ).returns(Formula) } - def self.from_rack( - rack, spec = T.unsafe(nil), - alias_path: T.unsafe(nil), - force_bottle: T.unsafe(nil), - flags: T.unsafe(nil) - ) + def self.from_rack(rack, spec = nil, alias_path: nil, force_bottle: false, flags: []) kegs = rack.directory? ? rack.subdirs.map { |d| Keg.new(d) } : [] keg = kegs.find(&:linked?) || kegs.find(&:optlinked?) || kegs.max_by(&:scheme_and_version) @@ -1075,18 +1032,18 @@ def self.keg_only?(rack) params( keg: Keg, # Automatically resolves the formula's spec if not specified. - spec: Symbol, - alias_path: T.any(Pathname, String), + spec: T.nilable(Symbol), + alias_path: T.any(NilClass, Pathname, String), force_bottle: T::Boolean, flags: T::Array[String], ).returns(Formula) } def self.from_keg( keg, - spec = T.unsafe(nil), - alias_path: T.unsafe(nil), - force_bottle: T.unsafe(nil), - flags: T.unsafe(nil) + spec = nil, + alias_path: nil, + force_bottle: false, + flags: [] ) tab = keg.tab tap = tab.tap @@ -1125,7 +1082,7 @@ def self.from_keg( path: Pathname, contents: String, spec: Symbol, - alias_path: Pathname, + alias_path: T.nilable(Pathname), force_bottle: T::Boolean, flags: T::Array[String], ignore_errors: T::Boolean, @@ -1136,18 +1093,13 @@ def self.from_contents( path, contents, spec = :stable, - alias_path: T.unsafe(nil), - force_bottle: T.unsafe(nil), - flags: T.unsafe(nil), - ignore_errors: T.unsafe(nil) + alias_path: nil, + force_bottle: false, + flags: [], + ignore_errors: false ) - options = { - alias_path:, - force_bottle:, - flags:, - ignore_errors:, - }.compact - FormulaContentsLoader.new(name, path, contents).get_formula(spec, **options) + FormulaContentsLoader.new(name, path, contents) + .get_formula(spec, alias_path:, force_bottle:, flags:, ignore_errors:) end def self.to_rack(ref) @@ -1219,9 +1171,7 @@ def self.tap_formula_name_type(tapped_name, warn:) [name, tap, type] end - def self.loader_for(ref, from: T.unsafe(nil), warn: true) - options = { from:, warn: }.compact - + def self.loader_for(ref, from: nil, warn: true) [ FromBottleLoader, FromURILoader, @@ -1233,7 +1183,7 @@ def self.loader_for(ref, from: T.unsafe(nil), warn: true) FromCacheLoader, NullLoader, ].each do |loader_class| - if (loader = loader_class.try_new(ref, **options)) + if (loader = loader_class.try_new(ref, from:, warn:)) $stderr.puts "#{$PROGRAM_NAME} (#{loader_class}): loading #{ref}" if debug? return loader end From e2d10337f36241a5439d48333a7e289ba11f5088 Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Fri, 6 Dec 2024 11:25:51 -0800 Subject: [PATCH 2/2] Fix alias_name logic --- Library/Homebrew/formulary.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index ea0d0dc65b768..823a51f4ddf9f 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -671,7 +671,7 @@ def initialize(path, alias_path: nil, tap: nil) alias_path = nil if alias_dir != tap&.alias_dir - super(name, path, alias_path:, tap: tap) + super(name, path, alias_path:, tap:) end end @@ -916,7 +916,7 @@ def self.try_new(ref, from: nil, warn: false) name, tap, type = name_tap_type - alias_name = alias_name.downcase if type == :alias + alias_name = (type == :alias) ? alias_name.downcase : nil new(name, tap:, alias_name:) end