Merge pull request #18020 from Homebrew/installer-safety

Make Cask::Artifact::Installer typesafe

Library/Homebrew/cask/artifact/installer.rb

@@ -13,22 +13,14 @@ module Cask
         :script,
       ]).freeze
 
-      # Extension module for manual installers.
+      def install_phase(command: nil, **_)
-      module ManualInstaller
+        if manual_install
-        def install_phase(**)
           puts <<~EOS
             Cask #{cask} only provides a manual installer. To run it and complete the installation:
               open #{cask.staged_path.join(path).to_s.shellescape}
           EOS
-        end
+        else
-      end
+          ohai "Running #{self.class.dsl_key} script '#{path}'"
-
-      # Extension module for script installers.
-      module ScriptInstaller
-        def install_phase(command: nil, **_)
-          # 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)
 
@@ -69,32 +61,33 @@ module Cask
 
       attr_reader :path, :args
 
+      sig { returns(T::Boolean) }
+      attr_reader :manual_install
+
       def initialize(cask, **args)
         super
 
         if args.key?(:manual)
           @path = Pathname(args[:manual])
           @args = []
-          extend(ManualInstaller)
+          @manual_install = true
-          return
+        else
-        end
-
           path, @args = self.class.read_script_arguments(
             args[:script], self.class.dsl_key.to_s, { must_succeed: true, sudo: false }, print_stdout: true
           )
           raise CaskInvalidError.new(cask, "#{self.class.dsl_key} missing executable") if path.nil?
 
           @path = Pathname(path)
-        extend(ScriptInstaller)
+          @manual_install = false
+        end
       end
 
-      def summarize
+      sig { override.returns(String) }
-        path.to_s
+      def summarize = path.to_s
-      end
 
       def to_h
         { path: }.tap do |h|
-          h[:args] = args unless is_a?(ManualInstaller)
+          h[:args] = args unless manual_install
         end
       end
     end

Library/Homebrew/cask/artifact/installer.rbi

@@ -1,11 +0,0 @@
-# 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/upgrade.rb

@@ -65,7 +65,9 @@ module Cask
       end
 
       manual_installer_casks = outdated_casks.select do |cask|
-        cask.artifacts.any?(Artifact::Installer::ManualInstaller)
+        cask.artifacts.any? do |artifact|
+          artifact.is_a?(Artifact::Installer) && artifact.manual_install
+        end
       end
 
       if manual_installer_casks.present?

Library/Homebrew/test/cask/dsl_spec.rb

@@ -501,7 +501,7 @@ RSpec.describe Cask::DSL, :cask do
 
       it "allows installer manual to be specified" do
         installer = cask.artifacts.first
-        expect(installer).to be_a(Cask::Artifact::Installer::ManualInstaller)
+        expect(installer.instance_variable_get(:@manual_install)).to be true
         expect(installer.path).to eq(Pathname("Caffeine.app"))
       end
     end