Add types for SystemCommand.

Library/Homebrew/context.rb

@@ -34,15 +34,15 @@ module Context
     end
 
     def debug?
-      @debug
+      @debug == true
     end
 
     def quiet?
-      @quiet
+      @quiet == true
     end
 
     def verbose?
-      @verbose
+      @verbose == true
     end
   end
 

Library/Homebrew/sorbet/plugins/using.rb

@@ -3,7 +3,8 @@
 
 source = ARGV[5]
 
-/\busing +Magic\b/.match(source) do |_|
+case source[/\Ausing\s+(.*)\Z/, 1]
+when "Magic"
   puts <<-RUBY
     # typed: strict
 
@@ -18,4 +19,13 @@ source = ARGV[5]
       def zipinfo; end
     end
   RUBY
+when "HashValidator"
+  puts <<-RUBY
+    # typed: strict
+
+    class ::Hash
+      sig { params(valid_keys: T.untyped).void }
+      def assert_valid_keys!(*valid_keys); end
+    end
+  RUBY
 end

Library/Homebrew/sorbet/rbi/hidden-definitions/hidden.rbi

@@ -28559,6 +28559,7 @@ class Socket
   IPV6_PATHMTU = ::T.let(nil, ::T.untyped)
   IPV6_RECVPATHMTU = ::T.let(nil, ::T.untyped)
   IPV6_USE_MIN_MTU = ::T.let(nil, ::T.untyped)
+  IP_DONTFRAG = ::T.let(nil, ::T.untyped)
   IP_PORTRANGE = ::T.let(nil, ::T.untyped)
   IP_RECVDSTADDR = ::T.let(nil, ::T.untyped)
   IP_RECVIF = ::T.let(nil, ::T.untyped)
@@ -28650,6 +28651,7 @@ module Socket::Constants
   IPV6_PATHMTU = ::T.let(nil, ::T.untyped)
   IPV6_RECVPATHMTU = ::T.let(nil, ::T.untyped)
   IPV6_USE_MIN_MTU = ::T.let(nil, ::T.untyped)
+  IP_DONTFRAG = ::T.let(nil, ::T.untyped)
   IP_PORTRANGE = ::T.let(nil, ::T.untyped)
   IP_RECVDSTADDR = ::T.let(nil, ::T.untyped)
   IP_RECVIF = ::T.let(nil, ::T.untyped)
@@ -29210,6 +29212,11 @@ end
 class SynchronizedDelegator
 end
 
+class SystemCommand::Result
+  extend ::T::Private::Methods::MethodHooks
+  extend ::T::Private::Methods::SingletonMethodHooks
+end
+
 class SystemCommand
   extend ::T::Private::Methods::MethodHooks
   extend ::T::Private::Methods::SingletonMethodHooks

Library/Homebrew/sorbet/triggers.yml

@@ -2,6 +2,6 @@ ruby_extra_args:
   - --disable-gems
 
 triggers:
-  using: sorbet/plugins/unpack_strategy_magic.rb
+  using: sorbet/plugins/using.rb
   attr_predicate: sorbet/plugins/attr_predicate.rb
   delegate: sorbet/plugins/delegate.rb

Library/Homebrew/system_command.rb

@@ -9,7 +9,6 @@ require "shellwords"
 require "extend/io"
 require "extend/predicable"
 require "extend/hash_validator"
-using HashValidator
 
 # Class for running sub-processes and capturing their output and exit status.
 #
@@ -17,14 +16,16 @@ using HashValidator
 class SystemCommand
   extend T::Sig
 
+  using HashValidator
+
   # Helper functions for calling {SystemCommand.run}.
   module Mixin
     def system_command(*args)
-      SystemCommand.run(*args)
+      T.unsafe(SystemCommand).run(*args)
     end
 
     def system_command!(*args)
-      SystemCommand.run!(*args)
+      T.unsafe(SystemCommand).run!(*args)
     end
   end
 
@@ -34,11 +35,11 @@ class SystemCommand
   attr_reader :pid
 
   def self.run(executable, **options)
-    new(executable, **options).run!
+    T.unsafe(self).new(executable, **options).run!
   end
 
   def self.run!(command, **options)
-    run(command, **options, must_succeed: true)
+    T.unsafe(self).run(command, **options, must_succeed: true)
   end
 
   sig { returns(SystemCommand::Result) }
@@ -63,45 +64,61 @@ class SystemCommand
     result
   end
 
+  sig do
+    params(
+      executable:   T.any(String, Pathname),
+      args:         T::Array[T.any(String, Integer, Float, URI::Generic)],
+      sudo:         T::Boolean,
+      env:          T::Hash[String, String],
+      input:        T.any(String, T::Array[String]),
+      must_succeed: T::Boolean,
+      print_stdout: T::Boolean,
+      print_stderr: T::Boolean,
+      verbose:      T::Boolean,
+      secrets:      T::Array[String],
+      chdir:        T.any(String, Pathname),
+    ).void
+  end
   def initialize(executable, args: [], sudo: false, env: {}, input: [], must_succeed: false,
-                 print_stdout: false, print_stderr: true, verbose: false, secrets: [], **options)
+                 print_stdout: false, print_stderr: true, verbose: false, secrets: [], chdir: T.unsafe(nil))
     require "extend/ENV"
     @executable = executable
     @args = args
     @sudo = sudo
-    @input = Array(input)
+    env.each_key do |name|
-    @print_stdout = print_stdout
-    @print_stderr = print_stderr
-    @verbose = verbose
-    @secrets = (Array(secrets) + ENV.sensitive_environment.values).uniq
-    @must_succeed = must_succeed
-    options.assert_valid_keys!(:chdir)
-    @options = options
-    @env = env
-
-    @env.each_key do |name|
       next if /^[\w&&\D]\w*$/.match?(name)
 
       raise ArgumentError, "Invalid variable name: '#{name}'"
     end
+    @env = env
+    @input = Array(input)
+    @must_succeed = must_succeed
+    @print_stdout = print_stdout
+    @print_stderr = print_stderr
+    @verbose = verbose
+    @secrets = (Array(secrets) + ENV.sensitive_environment.values).uniq
+    @chdir = chdir
   end
 
+  sig { returns(T::Array[String]) }
   def command
     [*sudo_prefix, *env_args, executable.to_s, *expanded_args]
   end
 
   private
 
-  attr_reader :executable, :args, :input, :options, :env
+  attr_reader :executable, :args, :input, :chdir, :env
 
   attr_predicate :sudo?, :print_stdout?, :print_stderr?, :must_succeed?
 
+  sig { returns(T::Boolean) }
   def verbose?
     return super if @verbose.nil?
 
     @verbose
   end
 
+  sig { returns(T::Array[String]) }
   def env_args
     set_variables = env.compact.map do |name, value|
       sanitized_name = Shellwords.escape(name)
@@ -114,6 +131,7 @@ class SystemCommand
     ["/usr/bin/env", *set_variables]
   end
 
+  sig { returns(T::Array[String]) }
   def sudo_prefix
     return [] unless sudo?
 
@@ -121,11 +139,12 @@ class SystemCommand
     ["/usr/bin/sudo", *askpass_flags, "-E", "--"]
   end
 
+  sig { returns(T::Array[String]) }
   def expanded_args
     @expanded_args ||= args.map do |arg|
       if arg.respond_to?(:to_path)
         File.absolute_path(arg)
-      elsif arg.is_a?(Integer) || arg.is_a?(Float) || arg.is_a?(URI)
+      elsif arg.is_a?(Integer) || arg.is_a?(Float) || arg.is_a?(URI::Generic)
         arg.to_s
       else
         arg.to_str
@@ -137,7 +156,7 @@ class SystemCommand
     executable, *args = command
 
     raw_stdin, raw_stdout, raw_stderr, raw_wait_thr =
-      Open3.popen3(env, [executable, executable], *args, **options)
+      T.unsafe(Open3).popen3(env, [executable, executable], *args, **{ chdir: chdir }.compact)
     @pid = raw_wait_thr.pid
 
     write_input_to(raw_stdin)
@@ -158,7 +177,7 @@ class SystemCommand
     loop do
       readable_sources, = IO.select(sources)
 
-      readable_sources = readable_sources.reject(&:eof?)
+      readable_sources = T.must(readable_sources).reject(&:eof?)
 
       break if readable_sources.empty?
 
@@ -176,10 +195,20 @@ class SystemCommand
 
   # Result containing the output and exit status of a finished sub-process.
   class Result
+    extend T::Sig
+
     include Context
 
     attr_accessor :command, :status, :exit_status
 
+    sig do
+      params(
+        command: T::Array[String],
+        output:  T::Array[[Symbol, String]],
+        status:  Process::Status,
+        secrets: T::Array[String],
+      ).void
+    end
     def initialize(command, output, status, secrets:)
       @command       = command
       @output        = output
@@ -188,57 +217,65 @@ class SystemCommand
       @secrets       = secrets
     end
 
+    sig { void }
     def assert_success!
       return if @status.success?
 
       raise ErrorDuringExecution.new(command, status: @status, output: @output, secrets: @secrets)
     end
 
+    sig { returns(String) }
     def stdout
       @stdout ||= @output.select { |type,| type == :stdout }
                          .map { |_, line| line }
                          .join
     end
 
+    sig { returns(String) }
     def stderr
       @stderr ||= @output.select { |type,| type == :stderr }
                          .map { |_, line| line }
                          .join
     end
 
+    sig { returns(String) }
     def merged_output
       @merged_output ||= @output.map { |_, line| line }
                                 .join
     end
 
+    sig { returns(T::Boolean) }
     def success?
       return false if @exit_status.nil?
 
       @exit_status.zero?
     end
 
+    sig { returns([String, String, Process::Status]) }
     def to_ary
       [stdout, stderr, status]
     end
 
+    sig { returns(T.nilable(T.any(Array, Hash))) }
     def plist
       @plist ||= begin
         output = stdout
 
-        if /\A(?<garbage>.*?)<\?\s*xml/m =~ output
+        output = output.sub(/\A(.*?)(\s*<\?\s*xml)/m) do
-          output = output.sub(/\A#{Regexp.escape(garbage)}/m, "")
+          warn_plist_garbage(T.must(Regexp.last_match(1)))
-          warn_plist_garbage(garbage)
+          Regexp.last_match(2)
         end
 
-        if %r{<\s*/\s*plist\s*>(?<garbage>.*?)\Z}m =~ output
+        output = output.sub(%r{(<\s*/\s*plist\s*>\s*)(.*?)\Z}m) do
-          output = output.sub(/#{Regexp.escape(garbage)}\Z/, "")
+          warn_plist_garbage(T.must(Regexp.last_match(2)))
-          warn_plist_garbage(garbage)
+          Regexp.last_match(1)
         end
 
         Plist.parse_xml(output)
       end
     end
 
+    sig { params(garbage: String).void }
     def warn_plist_garbage(garbage)
       return unless verbose?
       return unless garbage.match?(/\S/)

Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb

@@ -8,7 +8,7 @@ shared_examples "#uninstall_phase or #zap_phase" do
 
   let(:artifact_dsl_key) { described_class.dsl_key }
   let(:artifact) { cask.artifacts.find { |a| a.is_a?(described_class) } }
-  let(:fake_system_command) { FakeSystemCommand }
+  let(:fake_system_command) { class_double(SystemCommand) }
 
   context "using :launchctl" do
     let(:cask) { Cask::CaskLoader.load(cask_path("with-#{artifact_dsl_key}-launchctl")) }
@@ -31,41 +31,37 @@ shared_examples "#uninstall_phase or #zap_phase" do
     end
 
     it "works when job is owned by user" do
-      FakeSystemCommand.stubs_command(
+      allow(fake_system_command).to receive(:run)
-        launchctl_list_cmd,
+        .with("/bin/launchctl", args: ["list", "my.fancy.package.service"], print_stderr: false, sudo: false)
-        service_info,
+        .and_return(instance_double(SystemCommand::Result, stdout: service_info))
-      )
+      allow(fake_system_command).to receive(:run)
+        .with("/bin/launchctl", args: ["list", "my.fancy.package.service"], print_stderr: false, sudo: true)
+        .and_return(instance_double(SystemCommand::Result, stdout: unknown_response))
 
-      FakeSystemCommand.stubs_command(
+      expect(fake_system_command).to receive(:run!)
-        sudo(launchctl_list_cmd),
+        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service"], sudo: false)
-        unknown_response,
+        .and_return(instance_double(SystemCommand::Result))
-      )
-
-      FakeSystemCommand.expects_command(launchctl_remove_cmd)
 
       subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
     end
 
     it "works when job is owned by system" do
-      FakeSystemCommand.stubs_command(
+      allow(fake_system_command).to receive(:run)
-        launchctl_list_cmd,
+        .with("/bin/launchctl", args: ["list", "my.fancy.package.service"], print_stderr: false, sudo: false)
-        unknown_response,
+        .and_return(instance_double(SystemCommand::Result, stdout: unknown_response))
-      )
+      allow(fake_system_command).to receive(:run)
+        .with("/bin/launchctl", args: ["list", "my.fancy.package.service"], print_stderr: false, sudo: true)
+        .and_return(instance_double(SystemCommand::Result, stdout: service_info))
 
-      FakeSystemCommand.stubs_command(
+      expect(fake_system_command).to receive(:run!)
-        sudo(launchctl_list_cmd),
+        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service"], sudo: true)
-        service_info,
+        .and_return(instance_double(SystemCommand::Result))
-      )
-
-      FakeSystemCommand.expects_command(sudo(launchctl_remove_cmd))
 
       subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
     end
   end
 
   context "using :pkgutil" do
-    let(:fake_system_command) { class_double(SystemCommand) }
-
     let(:cask) { Cask::CaskLoader.load(cask_path("with-#{artifact_dsl_key}-pkgutil")) }
 
     let(:main_pkg_id) { "my.fancy.package.main" }

Library/Homebrew/test/cask/dsl/postflight_spec.rb

@@ -6,7 +6,8 @@ require "test/cask/dsl/shared_examples/staged"
 
 describe Cask::DSL::Postflight, :cask do
   let(:cask) { Cask::CaskLoader.load(cask_path("basic-cask")) }
-  let(:dsl) { described_class.new(cask, FakeSystemCommand) }
+  let(:fake_system_command) { class_double(SystemCommand) }
+  let(:dsl) { described_class.new(cask, fake_system_command) }
 
   it_behaves_like Cask::DSL::Base
 

Library/Homebrew/test/cask/dsl/preflight_spec.rb

@@ -6,7 +6,8 @@ require "test/cask/dsl/shared_examples/staged"
 
 describe Cask::DSL::Preflight, :cask do
   let(:cask) { Cask::CaskLoader.load(cask_path("basic-cask")) }
-  let(:dsl) { described_class.new(cask, FakeSystemCommand) }
+  let(:fake_system_command) { class_double(SystemCommand) }
+  let(:dsl) { described_class.new(cask, fake_system_command) }
 
   it_behaves_like Cask::DSL::Base
 

Library/Homebrew/test/cask/dsl/shared_examples/staged.rb

@@ -4,8 +4,8 @@
 require "cask/staged"
 
 shared_examples Cask::Staged do
-  let(:existing_path) { Pathname.new("/path/to/file/that/exists") }
+  let(:existing_path) { Pathname("/path/to/file/that/exists") }
-  let(:non_existent_path) { Pathname.new("/path/to/file/that/does/not/exist") }
+  let(:non_existent_path) { Pathname("/path/to/file/that/does/not/exist") }
 
   before do
     allow(existing_path).to receive(:exist?).and_return(true)
@@ -17,9 +17,8 @@ shared_examples Cask::Staged do
   end
 
   it "can run system commands with list-form arguments" do
-    FakeSystemCommand.expects_command(
+    expect(fake_system_command).to receive(:run!)
-      ["echo", "homebrew-cask", "rocks!"],
+      .with("echo", args: ["homebrew-cask", "rocks!"])
-    )
 
     staged.system_command("echo", args: ["homebrew-cask", "rocks!"])
   end
@@ -28,9 +27,8 @@ shared_examples Cask::Staged do
     fake_pathname = existing_path
     allow(staged).to receive(:Pathname).and_return(fake_pathname)
 
-    FakeSystemCommand.expects_command(
+    expect(fake_system_command).to receive(:run!)
-      ["/bin/chmod", "-R", "--", "777", fake_pathname],
+      .with("/bin/chmod", args: ["-R", "--", "777", fake_pathname], sudo: false)
-    )
 
     staged.set_permissions(fake_pathname.to_s, "777")
   end
@@ -39,9 +37,8 @@ shared_examples Cask::Staged do
     fake_pathname = existing_path
     allow(staged).to receive(:Pathname).and_return(fake_pathname)
 
-    FakeSystemCommand.expects_command(
+    expect(fake_system_command).to receive(:run!)
-      ["/bin/chmod", "-R", "--", "777", fake_pathname, fake_pathname],
+      .with("/bin/chmod", args: ["-R", "--", "777", fake_pathname, fake_pathname], sudo: false)
-    )
 
     staged.set_permissions([fake_pathname.to_s, fake_pathname.to_s], "777")
   end
@@ -58,9 +55,8 @@ shared_examples Cask::Staged do
     allow(User).to receive(:current).and_return(User.new("fake_user"))
     allow(staged).to receive(:Pathname).and_return(fake_pathname)
 
-    FakeSystemCommand.expects_command(
+    expect(fake_system_command).to receive(:run!)
-      sudo("/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname),
+      .with("/usr/sbin/chown", args: ["-R", "--", "fake_user:staff", fake_pathname], sudo: true)
-    )
 
     staged.set_ownership(fake_pathname.to_s)
   end
@@ -71,9 +67,8 @@ shared_examples Cask::Staged do
     allow(User).to receive(:current).and_return(User.new("fake_user"))
     allow(staged).to receive(:Pathname).and_return(fake_pathname)
 
-    FakeSystemCommand.expects_command(
+    expect(fake_system_command).to receive(:run!)
-      sudo("/usr/sbin/chown", "-R", "--", "fake_user:staff", fake_pathname, fake_pathname),
+      .with("/usr/sbin/chown", args: ["-R", "--", "fake_user:staff", fake_pathname, fake_pathname], sudo: true)
-    )
 
     staged.set_ownership([fake_pathname.to_s, fake_pathname.to_s])
   end
@@ -83,9 +78,8 @@ shared_examples Cask::Staged do
 
     allow(staged).to receive(:Pathname).and_return(fake_pathname)
 
-    FakeSystemCommand.expects_command(
+    expect(fake_system_command).to receive(:run!)
-      sudo("/usr/sbin/chown", "-R", "--", "other_user:other_group", fake_pathname),
+      .with("/usr/sbin/chown", args: ["-R", "--", "other_user:other_group", fake_pathname], sudo: true)
-    )
 
     staged.set_ownership(fake_pathname.to_s, user: "other_user", group: "other_group")
   end

Library/Homebrew/test/cask/dsl/uninstall_postflight_spec.rb

@@ -5,7 +5,7 @@ require "test/cask/dsl/shared_examples/base"
 
 describe Cask::DSL::UninstallPostflight, :cask do
   let(:cask) { Cask::CaskLoader.load(cask_path("basic-cask")) }
-  let(:dsl) { described_class.new(cask, FakeSystemCommand) }
+  let(:dsl) { described_class.new(cask, class_double(SystemCommand)) }
 
   it_behaves_like Cask::DSL::Base
 end

Library/Homebrew/test/cask/dsl/uninstall_preflight_spec.rb

@@ -6,7 +6,8 @@ require "test/cask/dsl/shared_examples/staged"
 
 describe Cask::DSL::UninstallPreflight, :cask do
   let(:cask) { Cask::CaskLoader.load(cask_path("basic-cask")) }
-  let(:dsl) { described_class.new(cask, FakeSystemCommand) }
+  let(:fake_system_command) { class_double(SystemCommand) }
+  let(:dsl) { described_class.new(cask, fake_system_command) }
 
   it_behaves_like Cask::DSL::Base
 

Library/Homebrew/test/cask/pkg_spec.rb

@@ -153,8 +153,12 @@ describe Cask::Pkg, :cask do
         "/usr/sbin/pkgutil",
         args: ["--pkg-info-plist", pkg_id],
       ).and_return(
-        SystemCommand::Result.new(nil, [[:stdout, pkg_info_plist]], instance_double(Process::Status, exitstatus: 0),
+        SystemCommand::Result.new(
-                                  secrets: []),
+          ["/usr/sbin/pkgutil", "--pkg-info-plist", pkg_id],
+          [[:stdout, pkg_info_plist]],
+          instance_double(Process::Status, exitstatus: 0),
+          secrets: [],
+        ),
       )
 
       info = pkg.info

Library/Homebrew/test/support/helper/cask/fake_system_command.rb

@@ -1,74 +0,0 @@
-# typed: true
-# frozen_string_literal: true
-
-def sudo(*args)
-  ["/usr/bin/sudo", "-E", "--"] + args.flatten
-end
-
-class FakeSystemCommand
-  def self.responses
-    @responses ||= {}
-  end
-
-  def self.expectations
-    @expectations ||= {}
-  end
-
-  def self.system_calls
-    @system_calls ||= Hash.new(0)
-  end
-
-  def self.clear
-    @responses    = nil
-    @expectations = nil
-    @system_calls = nil
-  end
-
-  def self.stubs_command(command, response = "")
-    command = command.map(&:to_s)
-    responses[command] = response
-  end
-
-  def self.expects_command(command, response = "", times = 1)
-    command = command.map(&:to_s)
-    stubs_command(command, response)
-    expectations[command] = times
-  end
-
-  def self.verify_expectations!
-    expectations.each do |command, times|
-      unless system_calls[command] == times
-        raise("expected #{command.inspect} to be run #{times} times, but got #{system_calls[command]}")
-      end
-    end
-  end
-
-  def self.run(command_string, options = {})
-    command = SystemCommand.new(command_string, options).command
-    puts command
-    unless responses.key?(command)
-      raise("no response faked for #{command.inspect}, faked responses are: #{responses.inspect}")
-    end
-
-    system_calls[command] += 1
-
-    response = responses[command]
-    if response.respond_to?(:call)
-      response.call(command_string, options)
-    else
-      SystemCommand::Result.new(command, [[:stdout, response]], OpenStruct.new(exitstatus: 0), secrets: [])
-    end
-  end
-
-  def self.run!(command, options = {})
-    run(command, options.merge(must_succeed: true))
-  end
-end
-
-RSpec.configure do |config|
-  config.after do
-    FakeSystemCommand.verify_expectations!
-  ensure
-    FakeSystemCommand.clear
-  end
-end

Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb

@@ -4,7 +4,6 @@
 require "cask/config"
 require "cask/cache"
 
-require "test/support/helper/cask/fake_system_command"
 require "test/support/helper/cask/install_helper"
 require "test/support/helper/cask/never_sudo_system_command"