Merge pull request #18033 from bevanjkay/revert-18020-installer-safety

Library/Homebrew/cask/artifact/installer.rb

@@ -13,14 +13,22 @@ class Installer < AbstractArtifact
         :script,
       ]).freeze
 
-      def install_phase(command: nil, **_)
-        if manual_install
+      # Extension module for manual installers.
+      module ManualInstaller
+        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
-        else
-          ohai "Running #{self.class.dsl_key} script '#{path}'"
+        end
+      end
+
+      # 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)
 
@@ -61,33 +69,32 @@ def self.from_args(cask, **args)
 
       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 = []
-          @manual_install = true
-        else
-          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)
-          @manual_install = false
+          extend(ManualInstaller)
+          return
         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)
       end
 
-      sig { override.returns(String) }
-      def summarize = path.to_s
+      def summarize
+        path.to_s
+      end
 
       def to_h
         { path: }.tap do |h|
-          h[:args] = args unless manual_install
+          h[:args] = args unless is_a?(ManualInstaller)
         end
       end
     end

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/upgrade.rb

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

Library/Homebrew/test/cask/dsl_spec.rb

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