Merge pull request #14769 from dduugg/enable-types

Enable typing in Cask::Artifact

Library/Homebrew/cask/artifact/abstract_artifact.rb

@@ -1,4 +1,4 @@
-# typed: false
+# typed: true
 # frozen_string_literal: true
 
 require "active_support/core_ext/object/deep_dup"
@@ -10,12 +10,14 @@ module Cask
     # @api private
     class AbstractArtifact
       extend T::Sig
+      extend T::Helpers
+      abstract!
 
       include Comparable
       extend Predicable
 
       def self.english_name
-        @english_name ||= name.sub(/^.*:/, "").gsub(/(.)([A-Z])/, '\1 \2')
+        @english_name ||= T.must(name).sub(/^.*:/, "").gsub(/(.)([A-Z])/, '\1 \2')
       end
 
       def self.english_article
@@ -23,13 +25,16 @@ module Cask
       end
 
       def self.dsl_key
-        @dsl_key ||= name.sub(/^.*:/, "").gsub(/(.)([A-Z])/, '\1_\2').downcase.to_sym
+        @dsl_key ||= T.must(name).sub(/^.*:/, "").gsub(/(.)([A-Z])/, '\1_\2').downcase.to_sym
       end
 
       def self.dirmethod
         @dirmethod ||= "#{dsl_key}dir".to_sym
       end
 
+      sig { abstract.returns(String) }
+      def summarize; end
+
       def staged_path_join_executable(path)
         path = Pathname(path)
         path = path.expand_path if path.to_s.start_with?("~")

Library/Homebrew/cask/artifact/abstract_uninstall.rb

@@ -1,8 +1,9 @@
-# typed: false
+# typed: true
 # frozen_string_literal: true
 
 require "timeout"
 
+require "utils/splat"
 require "utils/user"
 require "cask/artifact/abstract_artifact"
 require "cask/pkg"
@@ -47,6 +48,7 @@ module Cask
         return unless directives.key?(:kext)
 
         cask.caveats do
+          T.bind(self, ::Cask::DSL::Caveats)
           kext
         end
       end
@@ -55,7 +57,7 @@ module Cask
         directives.to_h
       end
 
-      sig { returns(String) }
+      sig { override.returns(String) }
       def summarize
         to_h.flat_map { |key, val| Array(val).map { |v| "#{key.inspect} => #{v.inspect}" } }.join(", ")
       end
@@ -73,7 +75,7 @@ module Cask
 
         args = directives[directive_sym]
 
-        send("uninstall_#{directive_sym}", *(args.is_a?(Hash) ? [args] : args), **options)
+        send("uninstall_#{directive_sym}", (args.is_a?(Enumerable) ? args : [args]), **options)
       end
 
       def stanza
@@ -88,7 +90,7 @@ module Cask
       end
 
       # :launchctl must come before :quit/:signal for cases where app would instantly re-launch
-      def uninstall_launchctl(*services, command: nil, **_)
+      def uninstall_launchctl(services, command: nil, **_)
         booleans = [false, true]
 
         all_services = []
@@ -165,11 +167,11 @@ module Cask
       end
 
       # :quit/:signal must come before :kext so the kext will not be in use by a running process
-      def uninstall_quit(*bundle_ids, command: nil, **_)
+      def uninstall_quit(bundle_ids, command: nil, **_)
         bundle_ids.each do |bundle_id|
           next unless running?(bundle_id)
 
-          unless User.current.gui?
+          unless T.must(User.current).gui?
             opoo "Not logged into a GUI; skipping quitting application ID '#{bundle_id}'."
             next
           end
@@ -242,7 +244,7 @@ module Cask
       private :quit
 
       # :signal should come after :quit so it can be used as a backup when :quit fails
-      def uninstall_signal(*signals, command: nil, **_)
+      def uninstall_signal(signals, command: nil, **_)
         signals.each do |pair|
           raise CaskInvalidError.new(cask, "Each #{stanza} :signal must consist of 2 elements.") unless pair.size == 2
 
@@ -258,12 +260,12 @@ module Cask
           # learned the pid from AppleScript is already some degree of protection,
           # though indirect.
           odebug "Unix ids are #{pids.inspect} for processes with bundle identifier #{bundle_id}"
-          Process.kill(signal, *pids)
+          ::Utils::Splat.process_kill(signal, pids)
           sleep 3
         end
       end
 
-      def uninstall_login_item(*login_items, command: nil, upgrade: false, **_)
+      def uninstall_login_item(login_items, command: nil, upgrade: false, **_)
         return if upgrade
 
         apps = cask.artifacts.select { |a| a.class.dsl_key == :app }
@@ -293,7 +295,7 @@ module Cask
       end
 
       # :kext should be unloaded before attempting to delete the relevant file
-      def uninstall_kext(*kexts, command: nil, **_)
+      def uninstall_kext(kexts, command: nil, **_)
         kexts.each do |kext|
           ohai "Unloading kernel extension #{kext}"
           is_loaded = system_command!("/usr/sbin/kextstat", args: ["-l", "-b", kext], sudo: true).stdout
@@ -335,7 +337,7 @@ module Cask
         sleep 1
       end
 
-      def uninstall_pkgutil(*pkgs, command: nil, **_)
+      def uninstall_pkgutil(pkgs, command: nil, **_)
         ohai "Uninstalling packages; your password may be necessary:"
         pkgs.each do |regex|
           ::Cask::Pkg.all_matching(regex, command).each do |pkg|
@@ -374,7 +376,7 @@ module Cask
         end
       end
 
-      def uninstall_delete(*paths, command: nil, **_)
+      def uninstall_delete(paths, command: nil, **_)
         return if paths.empty?
 
         ohai "Removing files:"
@@ -389,16 +391,16 @@ module Cask
         end
       end
 
-      def uninstall_trash(*paths, **options)
+      def uninstall_trash(paths, **options)
         return if paths.empty?
 
         resolved_paths = each_resolved_path(:trash, paths).to_a
 
         ohai "Trashing files:", resolved_paths.map(&:first)
-        trash_paths(*resolved_paths.flat_map(&:last), **options)
+        trash_paths(resolved_paths.flat_map(&:last), **options)
       end
 
-      def trash_paths(*paths, command: nil, **_)
+      def trash_paths(paths, command: nil, **_)
         return if paths.empty?
 
         stdout, stderr, = system_command HOMEBREW_LIBRARY_PATH/"cask/utils/trash.swift",
@@ -433,7 +435,7 @@ module Cask
       end
 
       def recursive_rmdir(*directories, command: nil, **_)
-        success = true
+        success = T.let(true, T::Boolean)
         each_resolved_path(:rmdir, directories) do |_path, resolved_paths|
           resolved_paths.select(&method(:all_dirs?)).each do |resolved_path|
             puts resolved_path.sub(Dir.home, "~")
@@ -454,7 +456,7 @@ module Cask
         success
       end
 
-      def uninstall_rmdir(*args, **kwargs)
+      def uninstall_rmdir(args, **kwargs)
         return if args.empty?
 
         ohai "Removing directories if empty:"

Library/Homebrew/cask/artifact/installer.rb

@@ -1,4 +1,4 @@
-# typed: false
+# typed: true
 # frozen_string_literal: true
 
 require "cask/artifact/abstract_artifact"
@@ -31,7 +31,9 @@ module Cask
       # Extension module for script installers.
       module ScriptInstaller
         def install_phase(command: nil, **_)
-          ohai "Running #{self.class.dsl_key} script '#{path}'"
+          # TODO: The `T.unsafe` is a false positive that is unnecessary in newer releasese of Sorbet
+          # (confirmend with sorbet v0.5.10672)
+          ohai "Running #{T.unsafe(self.class).dsl_key} script '#{path}'"
 
           executable_path = staged_path_join_executable(path)
 

Library/Homebrew/cask/artifact/installer.rbi

@@ -0,0 +1,11 @@
+# typed: strict
+
+module Cask::Artifact::Installer::ManualInstaller
+  include Kernel
+  requires_ancestor { Cask::Artifact::Installer }
+end
+
+module Cask::Artifact::Installer::ScriptInstaller
+  requires_ancestor { Cask::Artifact::Installer }
+  requires_ancestor { Cask::Artifact::AbstractArtifact }
+end

Library/Homebrew/cask/artifact/relocated.rb

@@ -69,7 +69,7 @@ module Cask
         end
       end
 
-      sig { returns(String) }
+      sig { override.returns(String) }
       def summarize
         target_string = @target_string.empty? ? "" : " -> #{@target_string}"
         "#{@source_string}#{target_string}"

Library/Homebrew/cask/artifact/stage_only.rb

@@ -24,7 +24,7 @@ module Cask
         [true]
       end
 
-      sig { returns(String) }
+      sig { override.returns(String) }
       def summarize
         "true"
       end

Library/Homebrew/cask/artifact_set.rb

@@ -1,4 +1,4 @@
-# typed: false
+# typed: true
 # frozen_string_literal: true
 
 require "set"
@@ -9,7 +9,10 @@ module Cask
   # @api private
   class ArtifactSet < ::Set
     def each(&block)
-      return enum_for(__method__) { size } unless block
+      # TODO: This is a false positive: https://github.com/rubocop/rubocop/issues/11591
+      # rubocop:disable Lint/ToEnumArguments
+      return enum_for(T.must(__method__)) { size } unless block
+      # rubocop:enable Lint/ToEnumArguments
 
       to_a.each(&block)
       self

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

@@ -221,7 +221,7 @@ shared_examples "#uninstall_phase or #zap_phase" do
                                                    .and_return(unix_pids.map { |pid| [pid, 0, bundle_id] })
 
       signals.each do |signal|
-        expect(Process).to receive(:kill).with(signal, *unix_pids)
+        expect(Process).to receive(:kill).with(signal, *unix_pids).and_return(1)
       end
 
       subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)

Library/Homebrew/utils/splat.rb

@@ -0,0 +1,20 @@
+# typed: false
+# frozen_string_literal: true
+
+module Utils
+  # Wrappers for Ruby core methods that accept splat arguments. This file is `typed: false` by design, but allows
+  # other files to enable typing while making use of the wrapped methods.
+  #
+  # @api private
+  module Splat
+    extend T::Sig
+
+    # Wrapper around `Process.kill` that accepts an array of pids.
+    # @see https://ruby-doc.org/3.2.1/Process.html#method-c-kill Process.kill
+    # @see https://github.com/sorbet/sorbet/blob/eaebdcd/rbi/core/process.rbi#L793-L800 Sorbet RBI for `Process.kill`
+    sig { params(signal: T.any(Integer, Symbol, String), pids: T::Array[Integer]).returns(Integer) }
+    def self.process_kill(signal, pids)
+      Process.kill(signal, *pids)
+    end
+  end
+end