Merge pull request #20344 from Homebrew/extend_sorbet_typing_pathname_cleanup

Cleanup some `extend/*` types/locations

Library/Homebrew/extend/kernel.rb

@@ -2,7 +2,8 @@
 # frozen_string_literal: true
 
 # Contains shorthand Homebrew utility methods like `ohai`, `opoo`, `odisabled`.
-# TODO: move these out of `Kernel`.
+# TODO: move these out of `Kernel` into `Homebrew::GlobalMethods` and add
+# necessary Sorbet and global Kernel inclusions.
 
 module Kernel
   sig { params(env: T.nilable(String)).returns(T::Boolean) }
@@ -13,6 +14,7 @@ module Kernel
   end
   private :superenv?
 
+  sig { params(path: T.nilable(T.any(String, Pathname))).returns(T::Boolean) }
   def require?(path)
     return false if path.nil?
 
@@ -20,16 +22,17 @@ module Kernel
       # Work around require warning when done repeatedly:
       # https://bugs.ruby-lang.org/issues/21091
       Warnings.ignore(/already initialized constant/, /previous definition of/) do
-        require path
+        require path.to_s
       end
     else
-      require path
+      require path.to_s
     end
     true
   rescue LoadError
     false
   end
 
+  sig { params(title: String).returns(String) }
   def ohai_title(title)
     verbose = if respond_to?(:verbose?)
       T.unsafe(self).verbose?
@@ -42,7 +45,7 @@ module Kernel
   end
 
   def ohai(title, *sput)
-    puts ohai_title(title)
+    puts ohai_title(title.to_s)
     puts sput
   end
 
@@ -55,10 +58,11 @@ module Kernel
 
     return if !debug && !always_display
 
-    $stderr.puts Formatter.headline(title, color: :magenta)
+    $stderr.puts Formatter.headline(title.to_s, color: :magenta)
     $stderr.puts sput unless sput.empty?
   end
 
+  sig { params(title: String, truncate: T.any(Symbol, T::Boolean)).returns(String) }
   def oh1_title(title, truncate: :auto)
     verbose = if respond_to?(:verbose?)
       T.unsafe(self).verbose?
@@ -70,6 +74,7 @@ module Kernel
     Formatter.headline(title, color: :green)
   end
 
+  sig { params(title: String, truncate: T.any(Symbol, T::Boolean)).void }
   def oh1(title, truncate: :auto)
     puts oh1_title(title, truncate:)
   end
@@ -134,6 +139,10 @@ module Kernel
   end
 
   # Output a deprecation warning/error message.
+  sig {
+    params(method: String, replacement: T.nilable(T.any(String, Symbol)), disable: T::Boolean,
+           disable_on: T.nilable(Time), disable_for_developers: T::Boolean, caller: T::Array[String]).void
+  }
   def odeprecated(method, replacement = nil,
                   disable:                false,
                   disable_on:             nil,
@@ -213,12 +222,20 @@ module Kernel
     end
   end
 
-  def odisabled(method, replacement = nil, **options)
+  sig {
-    options = { disable: true, caller: }.merge(options)
+    params(method: String, replacement: T.nilable(T.any(String, Symbol)), disable: T::Boolean,
+           disable_on: T.nilable(Time), disable_for_developers: T::Boolean, caller: T::Array[String]).void
+  }
+  def odisabled(method, replacement = nil,
+                disable:                false,
+                disable_on:             nil,
+                disable_for_developers: true,
+                caller:                 send(:caller))
     # This odeprecated should stick around indefinitely.
-    odeprecated(method, replacement, **options)
+    odeprecated(method, replacement, disable:, disable_on:, disable_for_developers:, caller:)
   end
 
+  sig { params(formula: T.any(String, Formula)).returns(String) }
   def pretty_installed(formula)
     if !$stdout.tty?
       formula.to_s
@@ -229,6 +246,7 @@ module Kernel
     end
   end
 
+  sig { params(formula: T.any(String, Formula)).returns(String) }
   def pretty_outdated(formula)
     if !$stdout.tty?
       formula.to_s
@@ -239,6 +257,7 @@ module Kernel
     end
   end
 
+  sig { params(formula: T.any(String, Formula)).returns(String) }
   def pretty_uninstalled(formula)
     if !$stdout.tty?
       formula.to_s
@@ -249,6 +268,7 @@ module Kernel
     end
   end
 
+  sig { params(seconds: T.nilable(T.any(Integer, Float))).returns(String) }
   def pretty_duration(seconds)
     seconds = seconds.to_i
     res = +""
@@ -266,9 +286,10 @@ module Kernel
     res.freeze
   end
 
+  sig { params(formula: T.nilable(Formula)).void }
   def interactive_shell(formula = nil)
     unless formula.nil?
-      ENV["HOMEBREW_DEBUG_PREFIX"] = formula.prefix
+      ENV["HOMEBREW_DEBUG_PREFIX"] = formula.prefix.to_s
       ENV["HOMEBREW_DEBUG_INSTALL"] = formula.full_name
     end
 
@@ -295,6 +316,7 @@ module Kernel
 
   # Kernel.system but with exceptions.
   def safe_system(cmd, *args, **options)
+    # TODO: migrate to utils.rb Homebrew.safe_system
     require "utils"
 
     return if Homebrew.system(cmd, *args, **options)
@@ -306,6 +328,7 @@ module Kernel
   #
   # @api internal
   def quiet_system(cmd, *args)
+    # TODO: migrate to utils.rb Homebrew.quiet_system
     require "utils"
 
     Homebrew._system(cmd, *args) do
@@ -367,11 +390,13 @@ module Kernel
     editor
   end
 
-  def exec_editor(*args)
+  sig { params(filename: T.any(String, Pathname)).void }
-    puts "Editing #{args.join "\n"}"
+  def exec_editor(filename)
-    with_homebrew_path { safe_system(*which_editor.shellsplit, *args) }
+    puts "Editing #{filename}"
+    with_homebrew_path { safe_system(*which_editor.shellsplit, filename) }
   end
 
+  sig { params(args: T.any(String, Pathname)).void }
   def exec_browser(*args)
     browser = Homebrew::EnvConfig.browser
     browser ||= OS::PATH_OPEN if defined?(OS::PATH_OPEN)
@@ -384,7 +409,7 @@ module Kernel
     end
   end
 
-  IGNORE_INTERRUPTS_MUTEX = Thread::Mutex.new.freeze
+  IGNORE_INTERRUPTS_MUTEX = T.let(Thread::Mutex.new.freeze, Thread::Mutex)
 
   def ignore_interrupts
     IGNORE_INTERRUPTS_MUTEX.synchronize do
@@ -417,6 +442,10 @@ module Kernel
 
   # Ensure the given formula is installed
   # This is useful for installing a utility formula (e.g. `shellcheck` for `brew style`)
+  sig {
+    params(formula_or_name: T.any(String, Formula), reason: String, latest: T::Boolean, output_to_stderr: T::Boolean,
+           quiet: T::Boolean).returns(Formula)
+  }
   def ensure_formula_installed!(formula_or_name, reason: "", latest: false,
                                 output_to_stderr: true, quiet: false)
     if output_to_stderr || quiet
@@ -456,6 +485,7 @@ module Kernel
   end
 
   # Ensure the given executable is exist otherwise install the brewed version
+  sig { params(name: String, formula_name: T.nilable(String), reason: String, latest: T::Boolean).returns(T.nilable(Pathname)) }
   def ensure_executable!(name, formula_name = nil, reason: "", latest: false)
     formula_name ||= name
 
@@ -472,10 +502,12 @@ module Kernel
     ensure_formula_installed!(formula_name, reason:, latest:).opt_bin/name
   end
 
+  sig { returns(T::Array[Pathname]) }
   def paths
-    @paths ||= ORIGINAL_PATHS.uniq.map(&:to_s)
+    @paths ||= T.let(ORIGINAL_PATHS.uniq.map(&:to_s), T.nilable(T::Array[Pathname]))
   end
 
+  sig { params(size_in_bytes: T.any(Integer, Float)).returns(String) }
   def disk_usage_readable(size_in_bytes)
     if size_in_bytes.abs >= 1_073_741_824
       size = size_in_bytes.to_f / 1_073_741_824
@@ -509,6 +541,7 @@ module Kernel
   # preserving character encoding validity. The returned string will
   # be not much longer than the specified max_bytes, though the exact
   # shortfall or overrun may vary.
+  sig { params(str: String, max_bytes: Integer, options: T::Hash[Symbol, T.untyped]).returns(String) }
   def truncate_text_to_approximate_size(str, max_bytes, options = {})
     front_weight = options.fetch(:front_weight, 0.5)
     raise "opts[:front_weight] must be between 0.0 and 1.0" if front_weight < 0.0 || front_weight > 1.0
@@ -530,7 +563,7 @@ module Kernel
       front = bytes[0..(n_front_bytes - 1)]
       back = bytes[-n_back_bytes..]
     end
-    out = front + glue_bytes + back
+    out = T.must(front) + glue_bytes + T.must(back)
     out.force_encoding("UTF-8")
     out.encode!("UTF-16", invalid: :replace)
     out.encode!("UTF-8")
@@ -568,6 +601,7 @@ module Kernel
     end
   end
 
+  sig { returns(T.proc.params(a: String, b: String).returns(Integer)) }
   def tap_and_name_comparison
     proc do |a, b|
       if a.include?("/") && b.exclude?("/")
@@ -580,6 +614,7 @@ module Kernel
     end
   end
 
+  sig { params(input: String, secrets: T::Array[String]).returns(String) }
   def redact_secrets(input, secrets)
     secrets.compact
            .reduce(input) { |str, secret| str.gsub secret, "******" }

Library/Homebrew/extend/pathname.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 require "system_command"
@@ -45,30 +45,6 @@ class Pathname
     end
   end
 
-  sig { params(src: T.any(String, Pathname), new_basename: String).void }
-  def install_p(src, new_basename)
-    src = Pathname(src)
-    raise Errno::ENOENT, src.to_s if !src.symlink? && !src.exist?
-
-    dst = join(new_basename)
-    dst = yield(src, dst) if block_given?
-    return unless dst
-
-    mkpath
-
-    # Use `FileUtils.mv` over `File.rename` to handle filesystem boundaries. If `src`
-    # is a symlink and its target is moved first, `FileUtils.mv` will fail
-    # (https://bugs.ruby-lang.org/issues/7707).
-    #
-    # In that case, use the system `mv` command.
-    if src.symlink?
-      raise unless Kernel.system "mv", src.to_s, dst
-    else
-      FileUtils.mv src, dst
-    end
-  end
-  private :install_p
-
   # Creates symlinks to sources in this folder.
   #
   # @api public
@@ -90,15 +66,6 @@ class Pathname
     end
   end
 
-  def install_symlink_p(src, new_basename)
-    mkpath
-    dstdir = realpath
-    src = Pathname(src).expand_path(dstdir)
-    src = src.dirname.realpath/src.basename if src.dirname.exist?
-    FileUtils.ln_sf(src.relative_path_from(dstdir), dstdir/new_basename)
-  end
-  private :install_symlink_p
-
   # Only appends to a file that is already created.
   #
   # @api public
@@ -146,9 +113,15 @@ class Pathname
     end
   end
 
-  def cp_path_sub(pattern, replacement)
+  sig {
+    params(pattern: T.any(Pathname, String, Regexp), replacement: T.any(Pathname, String),
+           _block: T.nilable(T.proc.params(src: Pathname, dst: Pathname).returns(Pathname))).void
+  }
+  def cp_path_sub(pattern, replacement, &_block)
     raise "#{self} does not exist" unless exist?
 
+    pattern = pattern.to_s if pattern.is_a?(Pathname)
+    replacement = replacement.to_s if replacement.is_a?(Pathname)
     dst = sub(pattern, replacement)
 
     raise "#{self} is the same file as #{dst}" if self == dst
@@ -269,12 +242,14 @@ class Pathname
     dirname.join(link).exist?
   end
 
+  sig { params(src: Pathname).void }
   def make_relative_symlink(src)
     dirname.mkpath
     File.symlink(src.relative_path_from(dirname), self)
   end
 
-  def ensure_writable
+  sig { params(_block: T.proc.void).void }
+  def ensure_writable(&_block)
     saved_perms = nil
     unless writable?
       saved_perms = stat.mode
@@ -285,24 +260,18 @@ class Pathname
     chmod saved_perms if saved_perms
   end
 
-  def which_install_info
+  sig { void }
-    @which_install_info ||=
-      if File.executable?("/usr/bin/install-info")
-        "/usr/bin/install-info"
-      elsif Formula["texinfo"].any_version_installed?
-        Formula["texinfo"].opt_bin/"install-info"
-      end
-  end
-
   def install_info
     quiet_system(which_install_info, "--quiet", to_s, "#{dirname}/dir")
   end
 
+  sig { void }
   def uninstall_info
     quiet_system(which_install_info, "--delete", "--quiet", to_s, "#{dirname}/dir")
   end
 
   # Writes an exec script in this folder for each target pathname.
+  sig { params(targets: T::Array[Pathname]).void }
   def write_exec_script(*targets)
     targets.flatten!
     if targets.empty?
@@ -320,6 +289,7 @@ class Pathname
   end
 
   # Writes an exec script that sets environment variables.
+  sig { params(target: Pathname, args: T.any(T::Array[String], T::Hash[String, String]), env: T.nilable(T::Hash[String, String])).void }
   def write_env_script(target, args, env = nil)
     unless env
       env = args
@@ -335,13 +305,14 @@ class Pathname
   end
 
   # Writes a wrapper env script and moves all files to the dst.
+  sig { params(dst: Pathname, env: T::Hash[String, String]).void }
   def env_script_all_files(dst, env)
     dst.mkpath
     Pathname.glob("#{self}/*") do |file|
       next if file.directory?
 
       new_file = dst.join(file.basename)
-      raise Errno::EEXIST, new_file if new_file.exist?
+      raise Errno::EEXIST, new_file.to_s if new_file.exist?
 
       dst.install(file)
       file.write_env_script(new_file, env)
@@ -366,6 +337,7 @@ class Pathname
     EOS
   end
 
+  sig { params(from: Pathname).void }
   def install_metafiles(from = Pathname.pwd)
     require "metafiles"
 
@@ -417,6 +389,7 @@ class Pathname
 
   sig { returns(String) }
   def magic_number
+    @magic_number ||= T.let(nil, T.nilable(String))
     @magic_number ||= if directory?
       ""
     else
@@ -428,16 +401,66 @@ class Pathname
 
   sig { returns(String) }
   def file_type
+    @file_type ||= T.let(nil, T.nilable(String))
     @file_type ||= system_command("file", args: ["-b", self], print_stderr: false)
                    .stdout.chomp
   end
 
   sig { returns(T::Array[String]) }
   def zipinfo
+    @zipinfo ||= T.let(nil, T.nilable(String))
     @zipinfo ||= system_command("zipinfo", args: ["-1", self], print_stderr: false)
                  .stdout
                  .encode(Encoding::UTF_8, invalid: :replace)
                  .split("\n")
   end
+
+  private
+
+  sig {
+    params(src: T.any(String, Pathname), new_basename: String,
+           _block: T.nilable(T.proc.params(src: Pathname, dst: Pathname).returns(T.nilable(Pathname)))).void
+  }
+  def install_p(src, new_basename, &_block)
+    src = Pathname(src)
+    raise Errno::ENOENT, src.to_s if !src.symlink? && !src.exist?
+
+    dst = join(new_basename)
+    dst = yield(src, dst) if block_given?
+    return unless dst
+
+    mkpath
+
+    # Use `FileUtils.mv` over `File.rename` to handle filesystem boundaries. If `src`
+    # is a symlink and its target is moved first, `FileUtils.mv` will fail
+    # (https://bugs.ruby-lang.org/issues/7707).
+    #
+    # In that case, use the system `mv` command.
+    if src.symlink?
+      raise unless Kernel.system "mv", src.to_s, dst.to_s
+    else
+      FileUtils.mv src, dst
+    end
+  end
+
+  sig { params(src: T.any(String, Pathname), new_basename: String).void }
+  def install_symlink_p(src, new_basename)
+    mkpath
+    dstdir = realpath
+    src = Pathname(src).expand_path(dstdir)
+    src = src.dirname.realpath/src.basename if src.dirname.exist?
+    FileUtils.ln_sf(src.relative_path_from(dstdir), dstdir/new_basename)
+  end
+
+  sig { returns(T.nilable(String)) }
+  def which_install_info
+    @which_install_info ||= T.let(nil, T.nilable(String))
+    @which_install_info ||=
+      if File.executable?("/usr/bin/install-info")
+        "/usr/bin/install-info"
+      elsif Formula["texinfo"].any_version_installed?
+        (Formula["texinfo"].opt_bin/"install-info").to_s
+      end
+  end
 end
 require "extend/os/pathname"

Library/Homebrew/extend/pathname/disk_usage_extension.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 module DiskUsageExtension
@@ -8,24 +8,26 @@ module DiskUsageExtension
 
   sig { returns(Integer) }
   def disk_usage
-    return @disk_usage if defined?(@disk_usage)
+    @disk_usage ||= T.let(nil, T.nilable(Integer))
+    return @disk_usage unless @disk_usage.nil?
 
-    compute_disk_usage
+    @file_count, @disk_usage = compute_disk_usage
     @disk_usage
   end
 
   sig { returns(Integer) }
   def file_count
-    return @file_count if defined?(@file_count)
+    @file_count ||= T.let(nil, T.nilable(Integer))
+    return @file_count unless @file_count.nil?
 
-    compute_disk_usage
+    @file_count, @disk_usage = compute_disk_usage
     @file_count
   end
 
   sig { returns(String) }
   def abv
     out = +""
-    compute_disk_usage
+    @file_count, @disk_usage = compute_disk_usage
     out << "#{number_readable(@file_count)} files, " if @file_count > 1
     out << disk_usage_readable(@disk_usage).to_s
     out.freeze
@@ -33,12 +35,12 @@ module DiskUsageExtension
 
   private
 
-  sig { void }
+  sig { returns([Integer, Integer]) }
   def compute_disk_usage
     if symlink? && !exist?
-      @file_count = 1
+      file_count = 1
-      @disk_usage = 0
+      disk_usage = 0
-      return
+      return [file_count, disk_usage]
     end
 
     path = if symlink?
@@ -49,26 +51,28 @@ module DiskUsageExtension
 
     if path.directory?
       scanned_files = Set.new
-      @file_count = 0
+      file_count = 0
-      @disk_usage = 0
+      disk_usage = 0
       path.find do |f|
         if f.directory?
-          @disk_usage += f.lstat.size
+          disk_usage += f.lstat.size
         else
-          @file_count += 1 if f.basename.to_s != ".DS_Store"
+          file_count += 1 if f.basename.to_s != ".DS_Store"
           # use Pathname#lstat instead of Pathname#stat to get info of symlink itself.
           stat = f.lstat
           file_id = [stat.dev, stat.ino]
           # count hardlinks only once.
           unless scanned_files.include?(file_id)
-            @disk_usage += stat.size
+            disk_usage += stat.size
             scanned_files.add(file_id)
           end
         end
       end
     else
-      @file_count = 1
+      file_count = 1
-      @disk_usage = path.lstat.size
+      disk_usage = path.lstat.size
     end
+
+    [file_count, disk_usage]
   end
 end

Library/Homebrew/formula.rb

@@ -1320,6 +1320,7 @@ class Formula
       path = Pathname.new(path)
       path.extend(InstallRenamed)
       path.cp_path_sub(bottle_prefix, HOMEBREW_PREFIX)
+      path
     end
   end
 

Library/Homebrew/install.rb

@@ -445,9 +445,11 @@ module Homebrew
 
         puts "#{::Utils.pluralize("Formula", formulae.count, plural: "e")} \
 (#{formulae.count}): #{formulae.join(", ")}\n\n"
-        puts "Download Size: #{disk_usage_readable(sizes[:download])}"
+        puts "Download Size: #{disk_usage_readable(sizes.fetch(:download))}"
-        puts "Install Size:  #{disk_usage_readable(sizes[:installed])}"
+        puts "Install Size:  #{disk_usage_readable(sizes.fetch(:installed))}"
-        puts "Net Install Size: #{disk_usage_readable(sizes[:net])}" if sizes[:net] != 0
+        if (net_install_size = sizes[:net]) && net_install_size != 0
+          puts "Net Install Size: #{disk_usage_readable(net_install_size)}"
+        end
 
         ask_input
       end

Library/Homebrew/install_renamed.rb

@@ -1,9 +1,13 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 # Helper module for installing default files.
 module InstallRenamed
-  def install_p(_, new_basename)
+  sig {
+    params(src: T.any(String, Pathname), new_basename: String,
+           _block: T.nilable(T.proc.params(src: Pathname, dst: Pathname).returns(T.nilable(Pathname)))).void
+  }
+  def install_p(src, new_basename, &_block)
     super do |src, dst|
       if src.directory?
         dst.install(src.children)
@@ -14,22 +18,29 @@ module InstallRenamed
     end
   end
 
-  def cp_path_sub(pattern, replacement)
+  sig {
+    params(pattern: T.any(Pathname, String, Regexp), replacement: T.any(Pathname, String),
+           _block: T.nilable(T.proc.params(src: Pathname, dst: Pathname).returns(Pathname))).void
+  }
+  def cp_path_sub(pattern, replacement, &_block)
     super do |src, dst|
       append_default_if_different(src, dst)
     end
   end
 
+  sig { params(other: T.any(String, Pathname)).returns(Pathname) }
   def +(other)
     super.extend(InstallRenamed)
   end
 
+  sig { params(other: T.any(String, Pathname)).returns(Pathname) }
   def /(other)
     super.extend(InstallRenamed)
   end
 
   private
 
+  sig { params(src: Pathname, dst: Pathname).returns(Pathname) }
   def append_default_if_different(src, dst)
     if dst.file? && !FileUtils.identical?(src, dst)
       Pathname.new("#{dst}.default")

Library/Homebrew/utils.rb

@@ -19,9 +19,11 @@ module Homebrew
       end
       exit! 1 # never gets here unless exec failed
     end
-    Process.wait(T.must(pid))
+    Process.wait(pid)
     $CHILD_STATUS.success?
   end
+  # TODO: make private_class_method when possible
+  # private_class_method :_system
   # rubocop:enable Naming/PredicateMethod
 
   def self.system(cmd, *args, **options)
@@ -37,9 +39,9 @@ module Homebrew
   # rubocop:disable Style/GlobalVars
   sig { params(the_module: Module, pattern: Regexp).void }
   def self.inject_dump_stats!(the_module, pattern)
-    @injected_dump_stat_modules ||= {}
+    @injected_dump_stat_modules ||= T.let({}, T.nilable(T::Hash[Module, T::Array[String]]))
     @injected_dump_stat_modules[the_module] ||= []
-    injected_methods = @injected_dump_stat_modules[the_module]
+    injected_methods = @injected_dump_stat_modules.fetch(the_module)
     the_module.module_eval do
       instance_methods.grep(pattern).each do |name|
         next if injected_methods.include? name