From ceb2291be1139d5f9675c0d9e97efdd333fedaca Mon Sep 17 00:00:00 2001 From: Douglas Eichelberger Date: Sat, 23 Aug 2025 18:44:36 -0700 Subject: [PATCH] Enable strict typing in Kernel extensions + utils.rb --- Library/Homebrew/PATH.rb | 1 - Library/Homebrew/bundle.rb | 2 +- Library/Homebrew/dev-cmd/typecheck.rb | 2 +- Library/Homebrew/extend/ENV/shared.rbi | 2 +- Library/Homebrew/extend/kernel.rb | 57 ++++++++++++++++---- Library/Homebrew/extend/os/mac/diagnostic.rb | 5 +- Library/Homebrew/git_repository.rb | 2 +- Library/Homebrew/utils.rb | 27 ++++++++-- Library/Homebrew/utils/service.rb | 2 + 9 files changed, 78 insertions(+), 22 deletions(-) diff --git a/Library/Homebrew/PATH.rb b/Library/Homebrew/PATH.rb index 099613ac95..0aea775d98 100644 --- a/Library/Homebrew/PATH.rb +++ b/Library/Homebrew/PATH.rb @@ -15,7 +15,6 @@ class PATH Element = T.type_alias { T.nilable(T.any(Pathname, String, PATH)) } private_constant :Element Elements = T.type_alias { T.any(Element, T::Array[Element]) } - private_constant :Elements sig { params(paths: Elements).void } def initialize(*paths) @paths = T.let(parse(paths), T::Array[String]) diff --git a/Library/Homebrew/bundle.rb b/Library/Homebrew/bundle.rb index 5fe4d93c63..e87dac8885 100644 --- a/Library/Homebrew/bundle.rb +++ b/Library/Homebrew/bundle.rb @@ -137,7 +137,7 @@ module Homebrew def reset! @mas_installed = T.let(nil, T.nilable(T::Boolean)) @vscode_installed = T.let(nil, T.nilable(T::Boolean)) - @which_vscode = T.let(nil, T.nilable(String)) + @which_vscode = T.let(nil, T.nilable(Pathname)) @whalebrew_installed = T.let(nil, T.nilable(T::Boolean)) @cask_installed = T.let(nil, T.nilable(T::Boolean)) @formula_versions_from_env = T.let(nil, T.nilable(T::Hash[String, String])) diff --git a/Library/Homebrew/dev-cmd/typecheck.rb b/Library/Homebrew/dev-cmd/typecheck.rb index fc3d5d614b..7b71911492 100644 --- a/Library/Homebrew/dev-cmd/typecheck.rb +++ b/Library/Homebrew/dev-cmd/typecheck.rb @@ -97,7 +97,7 @@ module Homebrew if args.lsp? srb_exec << "--lsp" if (watchman = which("watchman", ORIGINAL_PATHS)) - srb_exec << "--watchman-path" << watchman + srb_exec << "--watchman-path" << watchman.to_s else srb_exec << "--disable-watchman" end diff --git a/Library/Homebrew/extend/ENV/shared.rbi b/Library/Homebrew/extend/ENV/shared.rbi index 971159b7e4..306770cb1a 100644 --- a/Library/Homebrew/extend/ENV/shared.rbi +++ b/Library/Homebrew/extend/ENV/shared.rbi @@ -5,7 +5,7 @@ module SharedEnvExtension sig { type_parameters(:U).params( key: String, - value: T.all(T.type_parameter(:U), T.nilable(T.any(String, PATH))), + value: T.all(T.type_parameter(:U), T.nilable(T.any(String, Pathname, PATH))), ).returns(T.type_parameter(:U)) } def []=(key, value); end diff --git a/Library/Homebrew/extend/kernel.rb b/Library/Homebrew/extend/kernel.rb index 848cca8161..a38121b094 100644 --- a/Library/Homebrew/extend/kernel.rb +++ b/Library/Homebrew/extend/kernel.rb @@ -1,4 +1,4 @@ -# typed: true # rubocop:todo Sorbet/StrictSigil +# typed: strict # frozen_string_literal: true require "utils/output" @@ -32,32 +32,53 @@ module Kernel raise $CHILD_STATUS.inspect end + sig { type_parameters(:U).params(block: T.proc.returns(T.type_parameter(:U))).returns(T.type_parameter(:U)) } def with_homebrew_path(&block) with_env(PATH: PATH.new(ORIGINAL_PATHS), &block) end + sig { + type_parameters(:U) + .params(locale: String, block: T.proc.returns(T.type_parameter(:U))) + .returns(T.type_parameter(:U)) + } def with_custom_locale(locale, &block) with_env(LC_ALL: locale, &block) end # Kernel.system but with exceptions. - def safe_system(cmd, *args, **options) + sig { + params( + cmd: T.any(NilClass, Pathname, String, [String, String], T::Hash[String, T.nilable(String)]), + argv0: T.any(NilClass, Pathname, String, [String, String]), + args: T.any(NilClass, Pathname, String), + options: T.untyped, + ).void + } + def safe_system(cmd, argv0 = nil, *args, **options) # TODO: migrate to utils.rb Homebrew.safe_system require "utils" - return if Homebrew.system(cmd, *args, **options) + return if Homebrew.system(cmd, argv0, *args, **options) - raise ErrorDuringExecution.new([cmd, *args], status: $CHILD_STATUS) + raise ErrorDuringExecution.new([cmd, argv0, *args], status: $CHILD_STATUS) end # Run a system command without any output. # # @api internal - def quiet_system(cmd, *args) + sig { + params( + cmd: T.any(NilClass, Pathname, String, [String, String], T::Hash[String, T.nilable(String)]), + argv0: T.any(NilClass, String, [String, String]), + args: T.any(Pathname, String), + ).returns(T::Boolean) + } + def quiet_system(cmd, argv0 = nil, *args) # TODO: migrate to utils.rb Homebrew.quiet_system require "utils" - Homebrew._system(cmd, *args) do + Homebrew._system(cmd, argv0, *args) do # Redirect output streams to `/dev/null` instead of closing as some programs # will fail to execute if they can't write to an open stream. $stdout.reopen(File::NULL) @@ -68,6 +89,7 @@ module Kernel # Find a command. # # @api public + sig { params(cmd: String, path: PATH::Elements).returns(T.nilable(Pathname)) } def which(cmd, path = ENV.fetch("PATH")) PATH.new(path).each do |p| begin @@ -82,6 +104,7 @@ module Kernel nil end + sig { params(silent: T::Boolean).returns(String) } def which_editor(silent: false) editor = Homebrew::EnvConfig.editor return editor if editor @@ -124,7 +147,8 @@ module Kernel IGNORE_INTERRUPTS_MUTEX = T.let(Thread::Mutex.new.freeze, Thread::Mutex) - def ignore_interrupts + sig { type_parameters(:U).params(_block: T.proc.returns(T.type_parameter(:U))).returns(T.type_parameter(:U)) } + def ignore_interrupts(&_block) IGNORE_INTERRUPTS_MUTEX.synchronize do interrupted = T.let(false, T::Boolean) old_sigint_handler = trap(:INT) do @@ -144,7 +168,12 @@ module Kernel end end - def redirect_stdout(file) + sig { + type_parameters(:U) + .params(file: T.any(IO, Pathname, String), _block: T.proc.returns(T.type_parameter(:U))) + .returns(T.type_parameter(:U)) + } + def redirect_stdout(file, &_block) out = $stdout.dup $stdout.reopen(file) yield @@ -196,9 +225,10 @@ module Kernel end end + sig { params(number: Integer).returns(String) } def number_readable(number) numstr = number.to_i.to_s - (numstr.size - 3).step(1, -3) { |i| numstr.insert(i, ",") } + (numstr.size - 3).step(1, -3) { |i| numstr.insert(i.to_i, ",") } numstr end @@ -251,7 +281,12 @@ module Kernel # ``` # # @api public - def with_env(hash) + sig { + type_parameters(:U) + .params(hash: T::Hash[Object, String], _block: T.proc.returns(T.type_parameter(:U))) + .returns(T.type_parameter(:U)) + } + def with_env(hash, &_block) old_values = {} begin hash.each do |key, value| @@ -260,7 +295,7 @@ module Kernel ENV[key] = value end - yield if block_given? + yield ensure ENV.update(old_values) end diff --git a/Library/Homebrew/extend/os/mac/diagnostic.rb b/Library/Homebrew/extend/os/mac/diagnostic.rb index ced2ce3a8f..6288204a6b 100644 --- a/Library/Homebrew/extend/os/mac/diagnostic.rb +++ b/Library/Homebrew/extend/os/mac/diagnostic.rb @@ -10,7 +10,10 @@ module OS @volumes = T.let(get_mounts, T::Array[String]) end - sig { params(path: T.nilable(Pathname)).returns(Integer) } + # This is a pre-existing violation that should be refactored + # rubocop:todo Sorbet/AllowIncompatibleOverride + sig { override(allow_incompatible: true).params(path: T.nilable(Pathname)).returns(Integer) } + # rubocop:enable Sorbet/AllowIncompatibleOverride def which(path) vols = get_mounts path diff --git a/Library/Homebrew/git_repository.rb b/Library/Homebrew/git_repository.rb index 792f3c001d..22d77cb81a 100644 --- a/Library/Homebrew/git_repository.rb +++ b/Library/Homebrew/git_repository.rb @@ -27,7 +27,7 @@ class GitRepository end # Sets the URL of the Git origin remote. - sig { params(origin: String).returns(T.nilable(T::Boolean)) } + sig { params(origin: String).void } def origin_url=(origin) return if !git_repository? || !Utils::Git.available? diff --git a/Library/Homebrew/utils.rb b/Library/Homebrew/utils.rb index 4ced10e762..c969bfeb8a 100644 --- a/Library/Homebrew/utils.rb +++ b/Library/Homebrew/utils.rb @@ -1,4 +1,4 @@ -# typed: true # rubocop:todo Sorbet/StrictSigil +# typed: strict # frozen_string_literal: true require "context" @@ -26,12 +26,21 @@ module Homebrew # Need to keep this naming as-is for backwards compatibility. # rubocop:disable Naming/PredicateMethod - def self._system(cmd, *args, **options) + sig { + params( + cmd: T.any(NilClass, Pathname, String, [String, String], T::Hash[String, T.nilable(String)]), + argv0: T.any(NilClass, Pathname, String, [String, String]), + args: T.any(Pathname, String), + options: T.untyped, + _block: T.nilable(T.proc.void), + ).returns(T::Boolean) + } + def self._system(cmd, argv0 = nil, *args, **options, &_block) pid = fork do yield if block_given? args.map!(&:to_s) begin - exec(cmd, *args, **options) + exec(cmd, argv0, *args, **options) rescue nil end @@ -44,13 +53,21 @@ module Homebrew # private_class_method :_system # rubocop:enable Naming/PredicateMethod - def self.system(cmd, *args, **options) + sig { + params( + cmd: T.any(Pathname, String, [String, String], T::Hash[String, T.nilable(String)]), + argv0: T.any(NilClass, Pathname, String, [String, String]), + args: T.any(Pathname, String), + options: T.untyped, + ).returns(T::Boolean) + } + def self.system(cmd, argv0 = nil, *args, **options) if verbose? out = (options[:out] == :err) ? $stderr : $stdout out.puts "#{cmd} #{args * " "}".gsub(RUBY_PATH, "ruby") .gsub($LOAD_PATH.join(File::PATH_SEPARATOR).to_s, "$LOAD_PATH") end - _system(cmd, *args, **options) + _system(cmd, argv0, *args, **options) end # `Module` and `Regexp` are global variables used as types here so they don't need to be imported diff --git a/Library/Homebrew/utils/service.rb b/Library/Homebrew/utils/service.rb index f682b19f21..710d2c0f33 100644 --- a/Library/Homebrew/utils/service.rb +++ b/Library/Homebrew/utils/service.rb @@ -11,6 +11,8 @@ module Utils quiet_system(launchctl, "list", formula.plist_name) elsif systemctl? quiet_system(systemctl, "is-active", "--quiet", formula.service_name) + else + false end end